Repository navigation
adds support for federated terminology provider - #905
Conversation
|
Formatting check succeeded! |
applies spotless
There was a problem hiding this comment.
This is what I see implemented:
- Infrastructure for federated routing - The FederatedTerminologyProviderRouter and related interfaces are in place
- Client-level VSAC detection - VsacTerminologyServerClient.isCanonicalMatch() detects VSAC URLs via regex: https*://cts.nlm.nih.gov/fhir.*
- Endpoint prioritization logic - prioritizeEndpoints() can sort endpoints by URL matching
- Single terminologyEndpoint support - Works as before, one endpoint for all terminology
These are the gaps I see:
CRMI Requirement │ Current State
Parse artifactEndpointConfiguration │ Throws NotImplementedOperationException
Route based on artifactRoute │ Not implemented
Support multiple endpoints │ Router methods exist but aren't called
Priority by match length │ prioritizeEndpoints() uses boolean match, not character count
Do you agree with my assessment of what is implemented and the gaps with CRMI?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #905 +/- ##
============================================
- Coverage 73.82% 73.55% -0.28%
Complexity 364 364
============================================
Files 601 606 +5
Lines 29548 29758 +210
Branches 3923 3944 +21
============================================
+ Hits 21815 21888 +73
- Misses 5818 5946 +128
- Partials 1915 1924 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Summary of updates:
ArtifactEndpointConfiguration.java - New class
ITerminologyProviderRouter.java - Added methods:
FederatedTerminologyProviderRouter.java - Implemented:
VisitorHelper.java - Added:
ExpandHelper.java - Added:
PackageVisitor.java - Updated:
|
raleigh-g-thompson
left a comment
raleigh-g-thompson
left a comment
There was a problem hiding this comment.
No changes recommended.
I can't approve though since I'm also one of the authors.
|
The artifactEnpointConfiguration path should be aware of and respect the authoritativeSource extension - if an authoritativeSource is specified, it should be used in the comparison logic for endpoint resolution rather than the artifact's URL/canonical. |
|
|
||
| @Override | ||
| public boolean isCanonicalMatch(String address) { | ||
| return address.matches("https*://cts.nlm.nih.gov/fhir.*"); |
There was a problem hiding this comment.
I don't understand this bit, the client shouldn't be involved in the routing configuration.
- Add buildResourceIdForExpand() to handle VSAC's non-standard URL format where version is appended with a dash (ValueSet/id-version) instead of the standard pipe separator - Add formatParametersForLogging() and formatParameterValue() helpers for structured debug output of expansion parameters - Enhance expansion attempt logging with terminology server base URL, full expand URL, and parameter details - Change failure logging from info to warn level with additional context
…figuration support
FederatedTerminologyProviderRouterTest (16 tests): - List-based expand overloads exercising prioritizeEndpoints and stream filtering - List-based getValueSetResource, getCodeSystemResource, getLatestValueSetResource - Empty endpoint list edge cases - Exception catch-and-fallback paths in expandWithConfigurations, getValueSetResourceWithConfigurations, getCodeSystemResourceWithConfigurations ExpandRunnerTest (2 tests): - Transient server error (500) retry path exercising isTransient() true branch - Part-parameters formatting exercising formatParameterValue hasPart() branch GenericTerminologyServerClientClientTest (3 tests): - Null URL with no url parameter throws UnprocessableEntityException - initializeClientWithAuth with endpoint headers exercises hasHeaders() branch - Single-arg constructor exercises default settings fallback ArtifactEndpointConfigurationTest (8 tests): - Optional getters for artifactRoute, endpointUri, endpoint (present and empty) - toString with non-null endpoint resource
|
Still a WIP, working on tests