Skip to content

Commit 20c3a7f

Browse files
JamesW1-NHSJamesW1-NHS
committed
review cleanup
1 parent 4ffea44 commit 20c3a7f

File tree

2 files changed

+110
-75
lines changed

2 files changed

+110
-75
lines changed

‎backend/src/log_structure.py‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,22 @@
1717

1818
def _log_data_from_body(event) -> dict:
1919
log_data = {}
20-
if event.get("body"):
21-
try:
22-
imms = json.loads(event["body"])
23-
except json.decoder.JSONDecodeError:
24-
# it can't be parsed
25-
return log_data
26-
try:
27-
vaccine_type = get_vaccine_type(imms)
28-
log_data["vaccine_type"] = vaccine_type
29-
except Exception:
30-
pass
31-
try:
32-
local_id = imms["identifier"][0]["value"] + "^" + imms["identifier"][0]["system"]
33-
log_data["local_id"] = local_id
34-
except Exception:
35-
pass
20+
if event.get("body") is None:
21+
return log_data
22+
try:
23+
imms = json.loads(event["body"])
24+
except json.decoder.JSONDecodeError:
25+
return log_data
26+
try:
27+
vaccine_type = get_vaccine_type(imms)
28+
log_data["vaccine_type"] = vaccine_type
29+
except Exception:
30+
pass
31+
try:
32+
local_id = imms["identifier"][0]["value"] + "^" + imms["identifier"][0]["system"]
33+
log_data["local_id"] = local_id
34+
except Exception:
35+
pass
3636
return log_data
3737

3838

‎backend/tests/test_log_structure_wrapper.py‎

Lines changed: 94 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@
99
class TestFunctionInfoWrapper(unittest.TestCase):
1010

1111
def setUp(self):
12-
super().setUp()
1312
self.redis_patcher = patch("models.utils.validation_utils.redis_client")
1413
self.mock_redis_client = self.redis_patcher.start()
1514

1615
def tearDown(self):
17-
self.redis_patcher.stop()
18-
super().tearDown()
16+
patch.stopall()
1917

2018
@staticmethod
2119
def mock_success_function(_event, _context):
@@ -27,16 +25,22 @@ def mock_function_raises(_event, _context):
2725

2826
def test_successful_execution(self, mock_logger, mock_firehose_logger):
2927
# Arrange
28+
test_correlation = "test_correlation"
29+
test_request = "test_request"
30+
test_supplier = "test_supplier"
31+
test_actual_path = "/test"
32+
test_resource_path = "/test"
33+
3034
self.mock_redis_client.hget.return_value = "FLU"
3135
wrapped_function = function_info(self.mock_success_function)
3236
event = {
3337
'headers': {
34-
'X-Correlation-ID': 'test_correlation',
35-
'X-Request-ID': 'test_request',
36-
'SupplierSystem': 'test_supplier'
38+
'X-Correlation-ID': test_correlation,
39+
'X-Request-ID': test_request,
40+
'SupplierSystem': test_supplier
3741
},
38-
'path': '/test',
39-
'requestContext': {'resourcePath': '/test'},
42+
'path': test_actual_path,
43+
'requestContext': {'resourcePath': test_resource_path},
4044
'body': "{\"identifier\": [{\"system\": \"http://test\", \"value\": \"12345\"}], \"protocolApplied\": [{\"targetDisease\": [{\"coding\": [{\"system\": \"http://snomed.info/sct\", \"code\": \"840539006\", \"display\": \"Disease caused by severe acute respiratory syndrome coronavirus 2\"}]}]}]}"
4145
}
4246

@@ -53,16 +57,22 @@ def test_successful_execution(self, mock_logger, mock_firehose_logger):
5357

5458
self.assertIn('function_name', logged_info)
5559
self.assertIn('time_taken', logged_info)
56-
self.assertEqual(logged_info['X-Correlation-ID'], 'test_correlation')
57-
self.assertEqual(logged_info['X-Request-ID'], 'test_request')
58-
self.assertEqual(logged_info['supplier'], 'test_supplier')
59-
self.assertEqual(logged_info['actual_path'], '/test')
60-
self.assertEqual(logged_info['resource_path'], '/test')
60+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
61+
self.assertEqual(logged_info['X-Request-ID'], test_request)
62+
self.assertEqual(logged_info['supplier'], test_supplier)
63+
self.assertEqual(logged_info['actual_path'], test_actual_path)
64+
self.assertEqual(logged_info['resource_path'], test_resource_path)
6165
self.assertEqual(logged_info['local_id'], '12345^http://test')
6266
self.assertEqual(logged_info['vaccine_type'], 'FLU')
6367

6468
def test_exception_handling(self, mock_logger, mock_firehose_logger):
6569
# Arrange
70+
test_correlation = "failed_test_correlation"
71+
test_request = "failed_test_request"
72+
test_supplier = "failed_test_supplier"
73+
test_actual_path = "/failed_test"
74+
test_resource_path = "/failed_test"
75+
6676
self.mock_redis_client.hget.return_value = "FLU"
6777

6878
#Act
@@ -71,11 +81,11 @@ def test_exception_handling(self, mock_logger, mock_firehose_logger):
7181
with self.assertRaises(ValueError):
7282
#Assert
7383
event = {'headers': {
74-
'X-Correlation-ID': 'failed_test_correlation',
75-
'X-Request-ID': 'failed_test_request',
76-
'SupplierSystem': 'failed_test_supplier'
84+
'X-Correlation-ID': test_correlation,
85+
'X-Request-ID': test_request,
86+
'SupplierSystem': test_supplier
7787
},
78-
'path': '/failed_test', 'requestContext': {'resourcePath': '/failed_test'},
88+
'path': test_actual_path, 'requestContext': {'resourcePath': test_resource_path},
7989
'body': "{\"identifier\": [{\"system\": \"http://test\", \"value\": \"12345\"}], \"protocolApplied\": [{\"targetDisease\": [{\"coding\": [{\"system\": \"http://snomed.info/sct\", \"code\": \"840539006\", \"display\": \"Disease caused by severe acute respiratory syndrome coronavirus 2\"}]}]}]}"}
8090

8191
context = {}
@@ -90,26 +100,32 @@ def test_exception_handling(self, mock_logger, mock_firehose_logger):
90100

91101
self.assertIn('function_name', logged_info)
92102
self.assertIn('time_taken', logged_info)
93-
self.assertEqual(logged_info['X-Correlation-ID'], 'failed_test_correlation')
94-
self.assertEqual(logged_info['X-Request-ID'], 'failed_test_request')
95-
self.assertEqual(logged_info['supplier'], 'failed_test_supplier')
96-
self.assertEqual(logged_info['actual_path'], '/failed_test')
97-
self.assertEqual(logged_info['resource_path'], '/failed_test')
103+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
104+
self.assertEqual(logged_info['X-Request-ID'], test_request)
105+
self.assertEqual(logged_info['supplier'], test_supplier)
106+
self.assertEqual(logged_info['actual_path'], test_actual_path)
107+
self.assertEqual(logged_info['resource_path'], test_resource_path)
98108
self.assertEqual(logged_info['error'], str(ValueError("Test error")))
99109
self.assertEqual(logged_info['local_id'], '12345^http://test')
100110
self.assertEqual(logged_info['vaccine_type'], 'FLU')
101111

102112
def test_body_missing(self, mock_logger, mock_firehose_logger):
103113
# Arrange
114+
test_correlation = "failed_test_correlation_body_missing"
115+
test_request = "failed_test_request_body_missing"
116+
test_supplier = "failed_test_supplier_body_missing"
117+
test_actual_path = "/failed_test_body_missing"
118+
test_resource_path = "/failed_test_body_missing"
119+
104120
wrapped_function = function_info(self.mock_success_function)
105121
event = {
106122
'headers': {
107-
'X-Correlation-ID': 'test_correlation',
108-
'X-Request-ID': 'test_request',
109-
'SupplierSystem': 'test_supplier'
123+
'X-Correlation-ID': test_correlation,
124+
'X-Request-ID': test_request,
125+
'SupplierSystem': test_supplier
110126
},
111-
'path': '/test',
112-
'requestContext': {'resourcePath': '/test'}
127+
'path': test_actual_path,
128+
'requestContext': {'resourcePath': test_resource_path},
113129
}
114130

115131
# Act
@@ -119,26 +135,33 @@ def test_body_missing(self, mock_logger, mock_firehose_logger):
119135
args, kwargs = mock_logger.info.call_args
120136
logged_info = json.loads(args[0])
121137

122-
self.assertEqual(logged_info['X-Correlation-ID'], 'test_correlation')
123-
self.assertEqual(logged_info['X-Request-ID'], 'test_request')
124-
self.assertEqual(logged_info['supplier'], 'test_supplier')
125-
self.assertEqual(logged_info['actual_path'], '/test')
126-
self.assertEqual(logged_info['resource_path'], '/test')
138+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
139+
self.assertEqual(logged_info['X-Request-ID'], test_request)
140+
self.assertEqual(logged_info['supplier'], test_supplier)
141+
self.assertEqual(logged_info['actual_path'], test_actual_path)
142+
self.assertEqual(logged_info['resource_path'], test_resource_path)
127143
self.assertNotIn('local_id', logged_info)
128144
self.assertNotIn('vaccine_type', logged_info)
129145

130146
def test_body_not_json(self, mock_logger, mock_firehose_logger):
147+
# Arrange
148+
test_correlation = "failed_test_correlation_body_not_json"
149+
test_request = "failed_test_request_body_not_json"
150+
test_supplier = "failed_test_supplier_body_not_json"
151+
test_actual_path = "/failed_test_body_not_json"
152+
test_resource_path = "/failed_test_body_not_json"
153+
131154
# Act
132155
decorated_function_raises = function_info(self.mock_function_raises)
133156

134157
with self.assertRaises(ValueError):
135158
#Assert
136159
event = {'headers': {
137-
'X-Correlation-ID': 'failed_test_correlation',
138-
'X-Request-ID': 'failed_test_request',
139-
'SupplierSystem': 'failed_test_supplier'
160+
'X-Correlation-ID': test_correlation,
161+
'X-Request-ID': test_request,
162+
'SupplierSystem': test_supplier
140163
},
141-
'path': '/failed_test', 'requestContext': {'resourcePath': '/failed_test'},
164+
'path': test_actual_path, 'requestContext': {'resourcePath': test_resource_path},
142165
'body': "invalid"}
143166

144167
context = {}
@@ -148,16 +171,22 @@ def test_body_not_json(self, mock_logger, mock_firehose_logger):
148171
args, kwargs = mock_logger.exception.call_args
149172
logged_info = json.loads(args[0])
150173

151-
self.assertEqual(logged_info['X-Correlation-ID'], 'failed_test_correlation')
152-
self.assertEqual(logged_info['X-Request-ID'], 'failed_test_request')
153-
self.assertEqual(logged_info['supplier'], 'failed_test_supplier')
154-
self.assertEqual(logged_info['actual_path'], '/failed_test')
155-
self.assertEqual(logged_info['resource_path'], '/failed_test')
174+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
175+
self.assertEqual(logged_info['X-Request-ID'], test_request)
176+
self.assertEqual(logged_info['supplier'], test_supplier)
177+
self.assertEqual(logged_info['actual_path'], test_actual_path)
178+
self.assertEqual(logged_info['resource_path'], test_resource_path)
156179
self.assertNotIn('local_id', logged_info)
157180
self.assertNotIn('vaccine_type', logged_info)
158181

159182
def test_body_invalid_identifier(self, mock_logger, mock_firehose_logger):
160183
# Arrange
184+
test_correlation = "failed_test_correlation_invalid_identifier"
185+
test_request = "failed_test_request_invalid_identifier"
186+
test_supplier = "failed_test_supplier_invalid_identifier"
187+
test_actual_path = "/failed_test_invalid_identifier"
188+
test_resource_path = "/failed_test_invalid_identifier"
189+
161190
self.mock_redis_client.hget.return_value = "FLU"
162191

163192
# Act
@@ -166,11 +195,11 @@ def test_body_invalid_identifier(self, mock_logger, mock_firehose_logger):
166195
with self.assertRaises(ValueError):
167196
#Assert
168197
event = {'headers': {
169-
'X-Correlation-ID': 'failed_test_correlation',
170-
'X-Request-ID': 'failed_test_request',
171-
'SupplierSystem': 'failed_test_supplier'
198+
'X-Correlation-ID': test_correlation,
199+
'X-Request-ID': test_request,
200+
'SupplierSystem': test_supplier
172201
},
173-
'path': '/failed_test', 'requestContext': {'resourcePath': '/failed_test'},
202+
'path': test_actual_path, 'requestContext': {'resourcePath': test_resource_path},
174203
'body': "{\"identifier\": [], \"protocolApplied\": [{\"targetDisease\": [{\"coding\": [{\"system\": \"http://snomed.info/sct\", \"code\": \"840539006\", \"display\": \"Disease caused by severe acute respiratory syndrome coronavirus 2\"}]}]}]}"}
175204

176205
context = {}
@@ -180,16 +209,22 @@ def test_body_invalid_identifier(self, mock_logger, mock_firehose_logger):
180209
args, kwargs = mock_logger.exception.call_args
181210
logged_info = json.loads(args[0])
182211

183-
self.assertEqual(logged_info['X-Correlation-ID'], 'failed_test_correlation')
184-
self.assertEqual(logged_info['X-Request-ID'], 'failed_test_request')
185-
self.assertEqual(logged_info['supplier'], 'failed_test_supplier')
186-
self.assertEqual(logged_info['actual_path'], '/failed_test')
187-
self.assertEqual(logged_info['resource_path'], '/failed_test')
212+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
213+
self.assertEqual(logged_info['X-Request-ID'], test_request)
214+
self.assertEqual(logged_info['supplier'], test_supplier)
215+
self.assertEqual(logged_info['actual_path'], test_actual_path)
216+
self.assertEqual(logged_info['resource_path'], test_resource_path)
188217
self.assertNotIn('local_id', logged_info)
189218
self.assertEqual(logged_info['vaccine_type'], 'FLU')
190219

191220
def test_body_invalid_protocol_applied(self, mock_logger, mock_firehose_logger):
192221
# Arrange
222+
test_correlation = "failed_test_correlation_invalid_protocol"
223+
test_request = "failed_test_request_invalid_protocol"
224+
test_supplier = "failed_test_supplier_invalid_protocol"
225+
test_actual_path = "/failed_test_invalid_protocol"
226+
test_resource_path = "/failed_test_invalid_protocol"
227+
193228
self.mock_redis_client.hget.return_value = "FLU"
194229

195230
# Act
@@ -198,11 +233,11 @@ def test_body_invalid_protocol_applied(self, mock_logger, mock_firehose_logger):
198233
with self.assertRaises(ValueError):
199234
#Assert
200235
event = {'headers': {
201-
'X-Correlation-ID': 'failed_test_correlation',
202-
'X-Request-ID': 'failed_test_request',
203-
'SupplierSystem': 'failed_test_supplier'
236+
'X-Correlation-ID': test_correlation,
237+
'X-Request-ID': test_request,
238+
'SupplierSystem': test_supplier
204239
},
205-
'path': '/failed_test', 'requestContext': {'resourcePath': '/failed_test'},
240+
'path': test_actual_path, 'requestContext': {'resourcePath': test_resource_path},
206241
'body': "{\"identifier\": [{\"system\": \"http://test\", \"value\": \"12345\"}], \"protocolApplied\": []}"}
207242

208243
context = {}
@@ -212,10 +247,10 @@ def test_body_invalid_protocol_applied(self, mock_logger, mock_firehose_logger):
212247
args, kwargs = mock_logger.exception.call_args
213248
logged_info = json.loads(args[0])
214249

215-
self.assertEqual(logged_info['X-Correlation-ID'], 'failed_test_correlation')
216-
self.assertEqual(logged_info['X-Request-ID'], 'failed_test_request')
217-
self.assertEqual(logged_info['supplier'], 'failed_test_supplier')
218-
self.assertEqual(logged_info['actual_path'], '/failed_test')
219-
self.assertEqual(logged_info['resource_path'], '/failed_test')
250+
self.assertEqual(logged_info['X-Correlation-ID'], test_correlation)
251+
self.assertEqual(logged_info['X-Request-ID'], test_request)
252+
self.assertEqual(logged_info['supplier'], test_supplier)
253+
self.assertEqual(logged_info['actual_path'], test_actual_path)
254+
self.assertEqual(logged_info['resource_path'], test_resource_path)
220255
self.assertEqual(logged_info['local_id'], '12345^http://test')
221256
self.assertNotIn('vaccine_type', logged_info)

0 commit comments

Comments
 (0)