Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SearchParameter processing should be version-aware #2843

Closed
lmsurpre opened this issue Oct 8, 2021 · 1 comment
Closed

SearchParameter processing should be version-aware #2843

lmsurpre opened this issue Oct 8, 2021 · 1 comment
Assignees
@lmsurpre
Labels
search
Milestone
Sprint 2021-15

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Oct 8, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.
During startup, we build a ParametersMap from the search parameters in the registry:
https://github.com/IBM/FHIR/blob/main/fhir-search/src/main/java/com/ibm/fhir/search/parameters/ParametersUtil.java#L136-L138

Currently, this map is keyed by "code" and "url"...it does not even look at version.

Then, when we get a request in, we read the relevant searchParameter config and filter the list of all search parameters down to the list of applicable ones.

Once we are packaging multiple versions of an IG (#2551), we might hit a case where there is some subtle change in the registered SearchParameter from one version to the next.

Does FHIRRegistry.getInstance().getSearchParameters(searchParamType.value()) return all the versions, or just the "latest" one for each url?
If it returns multiple versions, we will currently get some warnings as we add the duplicate versions to our ParametersMap (and only the one that "wins" will end up being used).

It would be nice to allow the operator to specify which versions of the search parameters they'd like to use as part of the searchParameter filtering config (fhirServer/resources/[resource]/searchParameters/[code]).

Describe the solution you'd like

  1. ensure that FHIRRegistry.getInstance().getSearchParameters(searchParamType.value()) returns all versions of all search parameters
  2. update ParametersMap to be search parameter version aware
  3. update our search parameter filtering logic to support users that specify a |version at the end of the search parameter urls from our config (fhirServer/resources/[resource]/searchParameters/[code])

Describe alternatives you've considered
Support only the latest version of each search parameter

Acceptance Criteria

  1. GIVEN [a precondition]
    AND [another precondition]
    WHEN [test step]
    AND [test step]
    THEN [verification step]
    AND [verification step]

Additional context
Add any other context or screenshots about the feature request here.
@prb112 prb112 added the search label Oct 14, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-15 milestone Oct 25, 2021
@lmsurpre lmsurpre self-assigned this Oct 29, 2021
lmsurpre added a commit that referenced this issue Oct 29, 2021
@lmsurpre
issue #2843 - store both versioned and unversioned canonicals in
8a9c044 
ParametersMap

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 29, 2021
@lmsurpre
issue #2843 - added test for config-based sp version filtering
a7af948 
This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Nov 1, 2021
@lmsurpre
issue #2843 - added test for config-based sp version filtering
cee6f3a 
This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Nov 1, 2021
@lmsurpre
issue #2843 - added test for config-based sp version filtering
b44d203 
This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Nov 1, 2021
@lmsurpre
issue #2843 - added test for config-based sp version filtering
28a54e9 
This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Nov 3, 2021
@lmsurpre
Issue 2843 - store both versioned and unversioned canonicals in Param…
9492fbe 
…etersMap (#2929)

* issue #2843 - store both versioned and unversioned canonicals in
ParametersMap

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

* issue #2843 - added test for config-based sp version filtering

This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

* bump fhir-examples dependency version

otherwise we run into the following error during validation if us-core
is on the path:
```
SEVERE: validateResource(json/profiles/fhir-ig-us-core/Observation-some-day-smoker.json)
unexpected failure: Input resource failed validation:
generated-us-core-smokingstatus-5: Constraint violation:
effective.where(is(dateTime)).exists() (Observation)
```

I also added back in the fhir-examples.version variable because I think
our release automation still depends on this one (I had maybe
accidentally removed it while removing the other variables for
dependabot).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

* Remove url variants instead of deprecating

per review feedback

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

* Remove url variants instead of deprecating

per review feedback

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112
Copy link
Contributor

prb112 commented Nov 12, 2021

QA Complete
1 - Tested without Version (0..1) https://www.hl7.org/fhir/searchparameter.html
2 - Tested with Version

Worked as expected with the canonical and versions.

@prb112 prb112 closed this as completed Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmsurpre lmsurpre

Labels
search
Projects
None yet
Milestone
Sprint 2021-15
Development

No branches or pull requests
2 participants
@prb112 @lmsurpre