Merge pull request #756 from GSA/staging_bug_get_phone

optimize how we look up phone numbers

app/aws/s3.py

@@ -11,7 +11,8 @@ from app.clients import AWS_CLIENT_CONFIG
 FILE_LOCATION_STRUCTURE = "service-{}-notify/{}.csv"
 
 
-JOBS = ExpiringDict(max_len=100, max_age_seconds=3600 * 4)
+JOBS = ExpiringDict(max_len=1000, max_age_seconds=3600 * 4)
+
 
 JOBS_CACHE_HITS = "JOBS_CACHE_HITS"
 JOBS_CACHE_MISSES = "JOBS_CACHE_MISSES"
@@ -93,7 +94,33 @@ def incr_jobs_cache_hits():
         redis_store.incr(JOBS_CACHE_HITS)
     hits = redis_store.get(JOBS_CACHE_HITS).decode("utf-8")
     misses = redis_store.get(JOBS_CACHE_MISSES).decode("utf-8")
-    current_app.logger.info(f"JOBS CACHE MISS hits {hits} misses {misses}")
+    current_app.logger.info(f"JOBS CACHE HIT hits {hits} misses {misses}")
+
+
+def extract_phones(job):
+    job = job.split("\r\n")
+    first_row = job[0]
+    job.pop(0)
+    first_row = first_row.split(",")
+    phone_index = 0
+    for item in first_row:
+        if item.lower() == "phone number":
+            break
+        phone_index = phone_index + 1
+    phones = {}
+    job_row = 0
+    for row in job:
+        row = row.split(",")
+        phone_index = 0
+        for item in first_row:
+            if item.lower() == "phone number":
+                break
+            phone_index = phone_index + 1
+        my_phone = row[phone_index]
+        my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone)
+        phones[job_row] = my_phone
+        job_row = job_row + 1
+    return phones
 
 
 def get_phone_number_from_s3(service_id, job_id, job_row_number):
@@ -108,25 +135,37 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number):
     else:
         incr_jobs_cache_hits()
 
-    job = job.split("\r\n")
-    first_row = job[0]
-    job.pop(0)
-    first_row = first_row.split(",")
-    phone_index = 0
-    for item in first_row:
-        if item.lower() == "phone number":
-            break
-        phone_index = phone_index + 1
-
-    correct_row = job[job_row_number]
-    correct_row = correct_row.split(",")
-
-    # This could happen if an old job cannot be retrieved from s3
-    if len(correct_row) <= phone_index:
+    # If the job is None after our attempt to retrieve it from s3, it
+    # probably means the job is old and has been deleted from s3, in
+    # which case there is nothing we can do.  It's unlikely to run into
+    # this, but it could theoretically happen, especially if we ever
+    # change the task schedules
+    if job is None:
+        current_app.logger.warning(
+            "Couldnt find phone for job_id {job_id} row number {job_row_number} because job is missing"
+        )
+        return "Unknown Phone"
+
+    # If we look in the JOBS cache for the quick lookup dictionary of phones for a given job
+    # and that dictionary is not there, create it
+    if JOBS.get(f"{job_id}_phones") is None:
+        JOBS[f"{job_id}_phones"] = extract_phones(job)
+
+    # If we can find the quick dictionary, use it
+    if JOBS.get(f"{job_id}_phones") is not None:
+        phone_to_return = JOBS.get(f"{job_id}_phones").get(job_row_number)
+        if phone_to_return:
+            return phone_to_return
+        else:
+            current_app.logger.warning(
+                f"Was unable to retrieve phone number from lookup dictionary for job {job_id}"
+            )
+            return "Unknown Phone"
+    else:
+        current_app.logger.error(
+            f"Was unable to construct lookup dictionary for job {job_id}"
+        )
         return "Unknown Phone"
-    my_phone = correct_row[phone_index]
-    my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone)
-    return my_phone
 
 
 def get_job_metadata_from_s3(service_id, job_id):

app/service/rest.py

@@ -435,6 +435,9 @@ def get_all_notifications_for_service(service_id):
             )
             notification.to = recipient
             notification.normalised_to = recipient
+        else:
+            notification.to = "1"
+            notification.normalised_to = "1"
 
     kwargs = request.args.to_dict()
     kwargs["service_id"] = service_id
@@ -474,7 +477,7 @@ def get_all_notifications_for_service(service_id):
                 page,
                 len(next_page_of_pagination.items),
                 ".get_all_notifications_for_service",
-                **kwargs
+                **kwargs,
             )
             if count_pages
             else {},