Refactor SdsClient and SdsRequestBuilder for correct encapsulation of configuration property - #358
Open
MartinWheelerMT wants to merge 2 commits into
Open
MartinWheelerMT wants to merge 2 commits into
MartinWheelerMT wants to merge 2 commits into
Conversation
* Remove unused method in `SdsClientTest`.
* Moved configuration property `supplierOdsCode` which is read from environment variables from `SdsClient` to `SdsRequestBuilder` * Refactored `SdsRequestBuilder` to directly use the injected configuration `supplierOdsCode` removing and replacing in methods where appropriate. * Removed `buildAsidClientFor` as it is now just a wrapper for `buildAsDeviceAsidRequest`, which in itself has now been renamed to `buildAsidClientFor` to match existing code call. * Removed tests associated with removed the code for checking if the `supplierOdsCode` is empty. This is due to the fact that validation has subsequently been enabled so that the adaptor will not start if the `supplierOdsCode` is not provided or is an empty / whitespace only string. * Added new test to `SSdsConfigurationValidationTest` to test with a whitespace only value for `supplierOdsCode`.
MartinWheelerMT
enabled auto-merge (squash)
September 15, 2026 13:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
SdsClientTest.supplierOdsCodewhich is read from environment variables fromSdsClienttoSdsRequestBuilderSdsRequestBuilderto directly use the injected configurationsupplierOdsCoderemoving and replacing in methods where appropriate.buildAsidClientForas it is now just a wrapper forbuildAsDeviceAsidRequest, which in itself has now been renamed tobuildAsidClientForto match existing code call.supplierOdsCodeis empty. This is due to the fact that validation has subsequently been enabled so that the adaptor will not start if thesupplierOdsCodeis not provided or is an empty / whitespace only string.SSdsConfigurationValidationTestto test with a whitespace only value forsupplierOdsCode.Why
Application configuration validation already takes place, preventing startup on errors. Due to this we can correctly inject
supplierOdsCodewhere it is actually used. There is no longer a required to check if the value is null or whitespace within the method due to this validation. Due to this we have updated several tests and passed the reflection value update toSdsRequestBuilderrather thanSdsClient.An extra test has been introduced to confirm this works as expected, and several now unneeded tests have been removed.
Type of Change
Please check the option(s) that apply.
Checklist