cleanup, add tests

Author: JamesW1-NHS

backend/src/log_structure.py

@@ -15,19 +15,18 @@
 
 firehose_logger = FirehoseLogger()
 
-def _log_data_from_body(event, log_data):
+def _log_data_from_body(event) -> dict:
+    log_data = {}
     if event.get("body"):
         try:
             imms = json.loads(event["body"])
-        except json.decoder.JSONDecodeError as e:
-            return log_data
-
-        if imms.get("protocolApplied"):
             vaccine_type = get_vaccine_type(imms)
             log_data["vaccine_type"] = vaccine_type
-        if imms.get("identifier"):
             local_id = imms["identifier"][0]["value"] + "^" + imms["identifier"][0]["system"]
             log_data["local_id"] = local_id
+        except Exception as e:
+            # if there's no body, or it can't be parsed
+            return {}
     return log_data
 
 
@@ -62,7 +61,7 @@ def wrapper(*args, **kwargs):
             print(f"Result:{result}")
             end = time.time()
             log_data["time_taken"] = f"{round(end - start, 5)}s"
-            log_data = _log_data_from_body(event, log_data)
+            log_data.update(_log_data_from_body(event))
             status = "500"
             status_code = "Exception"
             diagnostics = str()
@@ -98,7 +97,7 @@ def wrapper(*args, **kwargs):
             log_data["error"] = str(e)
             end = time.time()
             log_data["time_taken"] = f"{round(end - start, 5)}s"
-            log_data = _log_data_from_body(event, log_data)
+            log_data.update(_log_data_from_body(event))
             logger.exception(json.dumps(log_data))
             firehose_log["event"] = log_data
             firehose_logger.send_log(firehose_log)

backend/tests/test_log_structure_wrapper.py

@@ -97,4 +97,62 @@ def test_exception_handling(self, mock_logger, mock_firehose_logger):
         self.assertEqual(logged_info['resource_path'], '/failed_test')
         self.assertEqual(logged_info['error'], str(ValueError("Test error")))
         self.assertEqual(logged_info['local_id'], '12345^http://test')
-        self.assertEqual(logged_info['vaccine_type'], 'FLU')
+        self.assertEqual(logged_info['vaccine_type'], 'FLU')
+
+    def test_invalid_body(self, mock_logger, mock_firehose_logger):
+        # Arrange
+        wrapped_function = function_info(self.mock_success_function)
+        event = {
+            'headers': {
+                'X-Correlation-ID': 'test_correlation',
+                'X-Request-ID': 'test_request',
+                'SupplierSystem': 'test_supplier'
+            },
+            'path': '/test',
+            'requestContext': {'resourcePath': '/test'},
+            'body': "{\"identifier\": [{\"system\": \"http://test\", \"value\": \"12345\"}], \"protocolApplied\": []}"
+        }
+
+        # Act
+        result = wrapped_function(event, {})
+
+        # Assert
+        args, kwargs = mock_logger.info.call_args
+        logged_info = json.loads(args[0])
+
+        self.assertEqual(logged_info['X-Correlation-ID'], 'test_correlation')
+        self.assertEqual(logged_info['X-Request-ID'], 'test_request')
+        self.assertEqual(logged_info['supplier'], 'test_supplier')
+        self.assertEqual(logged_info['actual_path'], '/test')
+        self.assertEqual(logged_info['resource_path'], '/test')
+        self.assertNotIn('local_id', logged_info)
+        self.assertNotIn('vaccine_type', logged_info)
+
+    def test_nonexistent_body(self, mock_logger, mock_firehose_logger):
+        # Arrange
+        wrapped_function = function_info(self.mock_success_function)
+        event = {
+            'headers': {
+                'X-Correlation-ID': 'test_correlation',
+                'X-Request-ID': 'test_request',
+                'SupplierSystem': 'test_supplier'
+            },
+            'path': '/test',
+            'requestContext': {'resourcePath': '/test'}
+        }
+
+        # Act
+        result = wrapped_function(event, {})
+
+        # Assert
+        args, kwargs = mock_logger.info.call_args
+        logged_info = json.loads(args[0])
+
+        self.assertEqual(logged_info['X-Correlation-ID'], 'test_correlation')
+        self.assertEqual(logged_info['X-Request-ID'], 'test_request')
+        self.assertEqual(logged_info['supplier'], 'test_supplier')
+        self.assertEqual(logged_info['actual_path'], '/test')
+        self.assertEqual(logged_info['resource_path'], '/test')
+        self.assertNotIn('local_id', logged_info)
+        self.assertNotIn('vaccine_type', logged_info)
+