-
Notifications
You must be signed in to change notification settings - Fork 157
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.
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
Use search parameter filtering for search parameter disambiguation #1839
Comments
1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
* ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: add integration tests for fhir-audit feature and fix one bug with use of/from Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: pseudo tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: small change to the timeout length to 120s Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: add get_results.sh creation of the directory Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: for privileged execution Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: difference running ci local and remote Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: audit with docker copy Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: adding tty support and stdin support Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: work around tty issue Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * removing the tty references -it Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * changed the execution pattern Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * changed the execution pattern Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update to predefine output file Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update to predefine output file Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * issues #1839 and #1743 - support search parameter disambiguation 1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * ci: work around issue with tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: work around issue with tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * remove hardcoded year from SearchLastUpdatedIdTest Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: last two tests to update with dynamic year Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * issues #1839 and #1743 - support search parameter disambiguation 1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Apply suggestions from code review Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Update build/audit/README.md Signed-off-by: Paul Bastide <pbastide@us.ibm.com> Co-authored-by: Lee Surprenant <lmsurpre@us.ibm.com> * add info on accessing the bulk operation job logs Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Add unit tests for the ParametersMap Also made a minor change to insertAll so it gets the code from the existing ParametersMap instead of from the SearchParameters in the map. Usually these are the same, but they can differ. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Issue #1849 - AuthZ interceptor validate/convert search requests Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1849 - address review comments Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> Co-authored-by: Paul Bastide <pbastide@us.ibm.com> Co-authored-by: Lee Surprenant <lmsurpre@us.ibm.com> Co-authored-by: Mike Schroeder <mschroed@us.ibm.com> Co-authored-by: Michael W Schroeder <66479070+michaelwschroeder@users.noreply.github.com>
* ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: add integration tests for fhir-audit feature and fix one bug with use of/from Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: pseudo tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: small change to the timeout length to 120s Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: add get_results.sh creation of the directory Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: for privileged execution Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: difference running ci local and remote Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: audit Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: audit with docker copy Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: adding tty support and stdin support Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: work around tty issue Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * removing the tty references -it Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * changed the execution pattern Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * changed the execution pattern Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update to predefine output file Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: update to predefine output file Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix permissions Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * issues #1839 and #1743 - support search parameter disambiguation 1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * ci: work around issue with tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * ci: work around issue with tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * remove hardcoded year from SearchLastUpdatedIdTest Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * fix: last two tests to update with dynamic year Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * issues #1839 and #1743 - support search parameter disambiguation 1. Update ParametersMap to support storing multiple search parameters with the same code 2. Address #1743 by collecting to a map instead of a list 3. Update SearchUtil.getSearchParameter to lookup the search parameter by URI from the config if possible (instead of applying the filter to the full set of built-in parameters). 4. Update the docs to reflect that search parameter filtering now applies to tenant-specific search parameters as well. This should help us move toward #1596 Also fixed a bad trace message and did some minor formatting / javadoc. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Apply suggestions from code review Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Update build/audit/README.md Signed-off-by: Paul Bastide <pbastide@us.ibm.com> Co-authored-by: Lee Surprenant <lmsurpre@us.ibm.com> * add info on accessing the bulk operation job logs Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Add unit tests for the ParametersMap Also made a minor change to insertAll so it gets the code from the existing ParametersMap instead of from the SearchParameters in the map. Usually these are the same, but they can differ. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Issue #1849 - AuthZ interceptor validate/convert search requests Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1849 - address review comments Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Duplicate Job Parameters fhir.dataSourcesInfo are created #1855 - Removed the duplicate serialization of the fhir.dataSourcesInfo Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * Confusing error when request is targetted for an invalid tenant id #1792 - Disambiguates the Error Messages that are bubbled up through the Persistence Layer - Add Integration Test Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * Confusing error when request is targetted for an invalid tenant id #1792 1 - added layer at the rest level Signed-off-by: Paul Bastide <pbastide@us.ibm.com> * Issue #1615 - Enforce configured interactions in REST layer Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Update FHIRValidationGuide.md 1. update the version references for the packaged implementation guides 2. add a section at the top to describe where to get the validation module Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Issue #1615 - address review comments Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1615 - fix whitespace Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1615 - add enum for interaction types Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Modify davinci-pdex CapabilityStatement-pdex-server.json updated the searchRevInclude value for the Coverage resource in pdex-server to work around https://jira.hl7.org/browse/FHIR-30338 Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com> * Issue #1494 - add Bundle.entry.search to search results Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1494 - add documentation Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> * Issue #1494 - address review comments Signed-off-by: Mike Schroeder <mschroed@us.ibm.com> Co-authored-by: Paul Bastide <pbastide@us.ibm.com> Co-authored-by: Lee Surprenant <lmsurpre@us.ibm.com> Co-authored-by: Mike Schroeder <mschroed@us.ibm.com> Co-authored-by: Michael W Schroeder <66479070+michaelwschroeder@users.noreply.github.com>
Started the server and noted these messages in the log file: Added the following to the fhir-server-config.json restarted the server, and those messages were no longer issued. |
Is your feature request related to a problem? Please describe.
In #1272 we introduced url-based search parameter configuration in
fhir-server-config.json
.However, when we build the built-in ParametersMap object, this is done outside of the context of any tenant and so we cannot really consult the tenant-specific search filter.
Further, the current ParametersMap only supports having a single search parameter per code (per resourceType).
There is some logic to warn about cases where there are multiple search parameters with the same code but differing expressions, but I thought that having multiple search parameters with the same code and equivalent expressions would be fine.
However, for our 4.5.3 release, we added some logic to
SearchUtil.filterSearchParameters
where, if the search parameter chosen during the initialization of the ParametersMap has a URL that differs from the one used in the tenant configuration, then the system skips over this parameter altogether.Describe the solution you'd like
fhir-server-config.json
For number 2, we used to do just this. However, since we had no mechanism for disambiguating, the logs were very frustrating because we were packaging multiple IGs with conflicting parameters. Now that we don't prepackage any IGs, I think it is reasonable to expect operators to do the disambiguation in the config when they package IGs that conflict with base spec search params.
Describe alternatives you've considered
#1625 outlines an alternative idea I had which was to build a per-tenant ParametersMap object. If we did that, then we probably wouldn't need to have ParametersMap support having multiple search parameters with the same code. However, that change seems slightly larger.
Additional context
The workaround for this issue is to avoid packaging conflicting search parameters.
The text was updated successfully, but these errors were encountered: