VED-821 Refactor get by ID journey in line with cleaner design

[dlzhry2nhs]

Summary

• Routine Change

Refactors the Get by ID journey to use the Controller -> Service -> Repository pattern properly. As this is quite a simple endpoint, I have also made quite a number of additional changes to show how I would like us to begin structuring the API code.

Changes:

• Added some Controller layer utilities for handling the parsing and response creation specific to API GW -> Lambda integration.
• Added top level exception wrapper to be used by controller methods. Means we can allow known custom exceptions to bubble up and return an appropriate response while also handling unexpected exceptions gracefully and getting a full stack trace.
• Vastly simplified get by ID journey to use the new utilities, raise errors where appropriate and ensure functionality is at the right level. (Due to the number of changes, it may be easiest to check out the branch and walk through the journey.)
• Create folders for controller, service, repository. Relevant functionality should go in those directories.
• Introduced a new utils directory for generic utilities across the project. I have just added a dictionary util for now. Changed utils in the testing folder to testing_utils to make the intent of this directory clearer, but also so it does not clash!

Reviews Required

• Dev

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

• I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
• If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
• I have ensured the changelog has been updated by the submitter, if necessary.

[mfjarvis]

Minor point but I don't love the "reflective" access of pathParameters (and headers below) as we don't really get the benefits of the type hints. What about:

return dict_utils.get_field(
    event.pathParameters,
    param_name,
    default=""
)

[dlzhry2nhs]

Yeh, that's fair. I'll look into it once I'm free.

[dlzhry2nhs]

Done

[mfjarvis]

I do think tuple is a less misleading return value than the previous dict, but I still had to read the last line to know what that tuple contained. Maybe something like get_immunisation_and_version_by_id would be clearer?

[dlzhry2nhs]

Yup, that sounds good. Had been thinking it might be better to have a single object that contains it, but it seems that the version is separate to the FHIR content and goes out as the header.

Will rename the function as such.

[mfjarvis]

Same deal here

[dlzhry2nhs]

Done for both.

[mfjarvis]

Could provide the exact error to raise in the test case, then we wouldn't need the msg logic:

Suggested change

	test_cases = {
	("Test UnauthorisedError", UnauthorizedError, 403, "forbidden", "Unauthorized request"),
	("Test UnauthorizedVaxError", UnauthorizedVaxError, 403, "forbidden",
	"Unauthorized request for vaccine type"),
	("Test ResourceNotFoundError", ResourceNotFoundError, 404, "not-found", "Immunization resource does not "
	"exist. ID: 123")
	}
	for msg, error_type, expected_status, expected_code, expected_message in test_cases:
	with self.subTest(msg=msg):
	@fhir_api_exception_handler
	def dummy_func():
	if msg == "Test ResourceNotFoundError":
	raise error_type(resource_type="Immunization", resource_id="123")
	raise error_type()
	test_cases = {
	(UnauthorizedError(), 403, "forbidden", "Unauthorized request"),
	(UnauthorizedVaxError(), 403, "forbidden", "Unauthorized request for vaccine type"),
	(
	ResourceNotFoundError(resource_type="Immunization", resource_id="123"),
	404,
	"not-found",
	"Immunization resource does not exist. ID: 123"
	)
	}
	for error, expected_status, expected_code, expected_message in test_cases:
	with self.subTest(msg=f`Test {error.__class__.__name__}`):
	@fhir_api_exception_handler
	def dummy_func():
	raise error

[dlzhry2nhs]

Done - much nicer.

[sonarqubecloud]

Quality Gate passed

Issues
17 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.8% Coverage on New Code
1.5% Duplication on New Code

See analysis details on SonarQube Cloud