Fix versioned canonical profile resolution#5503
Fix versioned canonical profile resolution#5503rohithkg-code wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@rohithkg-code please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
| public async Task<Resource> ResolveByCanonicalUriAsync(string uri) | ||
| { | ||
| var summary = (await ListSummariesAsync(CancellationToken.None)).ResolveByCanonicalUri(uri); | ||
| var summaries = (await ListSummariesAsync(CancellationToken.None)).ToList(); |
There was a problem hiding this comment.
Thanks for the contribution!
I have a question on this method:
It appears that this fallback only works correctly if at most one version of each StructureDefinition exists on the server.
_summaries is built in GetSummariesAsync using result[artifact.ResourceUri] = artifact — keyed on the base canonical URL with no version. When a server holds both v2.0.0 and v3.0.0 of the same SD, only the last one processed survives in _summaries, so url|2.0.0 will silently fail to resolve if v3.0.0 was stored last.
I think to fully support multi-version scenarios, GetSummariesAsync needs to key the dictionary by url + version (e.g. $"{artifact.ResourceUri}|{ver}"), and LoadBySummary's cache key needs the same treatment (see separate comment).
| string.Equals(summaryVersion?.ToString(), version, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
|
|
||
| return LoadBySummary(summary); |
There was a problem hiding this comment.
I think, if we fix the first comment I made, we will have a new issue:
The _resourcesByUri cache key doesn't include version, so it will return the wrong resource once multi-version support is added.
Steps to reproduce (after fixing the deduplication bug above):
ResolveByCanonicalUriAsync("url|v2") → loads v2 from JSON → stores in cache under key url
ResolveByCanonicalUriAsync("url|v3") → cache hit on url → returns v2 incorrectly
| Assert.Contains("http://example.org/fhir/StructureDefinition/custom-patient", profiles); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
More possible test cases:
Version mismatch returns null — call ResolveByCanonicalUriAsync("url|9.9.9") when only url|3.0.0 exists; assert the result is null. This ensures the fallback doesn't match on URL alone.
Unversioned URI still resolves — regression guard that an unversioned canonical URI resolves correctly after the restructured code path in ResolveByCanonicalUriAsync.
Two versions of the same SD, each resolves correctly — a test with both v2.0.0 and v3.0.0 of the same SD on the server, verifying each versioned URI resolves to the right resource. This will also expose the deduplication/cache bugs noted above.
Description
Fix versioned canonical profile resolution for validation-backed profile lookup.
This change updates
ServerProvideProfileValidation.ResolveByCanonicalUriAsyncso that when a profile is referenced using a versioned canonical URI such ashttp://example.org/fhir/StructureDefinition/custom-patient|3.0.0, the resolver can match it against the storedStructureDefinitionby canonical URL plus version.Without this fallback, versioned profile references could fail resolution even when the corresponding
StructureDefinitionwas present on the server.This PR also adds focused unit tests covering:
StructureDefinitionRelated issues
Addresses #5176.
Testing
Code changes were validated by adding targeted unit tests in
ServerProvideProfileValidationTestsfor:GivenVersionedStructureDefinition_WhenGettingSupportedProfiles_ThenVersionedCanonicalIsReturnedGivenVersionedStructureDefinition_WhenResolvingByCanonicalUriWithVersion_ThenMatchingProfileIsReturnedPlanned test command:
dotnet test src\Microsoft.Health.Fhir.R4.Core.UnitTests\Microsoft.Health.Fhir.R4.Core.UnitTests.csproj --filter ServerProvideProfileValidationTests