SearchUtil#getFilteredBuiltinSearchParameters uses List#contains #1743
Assignees
Labels
Comments
punktilious
added
bug
|
See #1625 for some related thoughts on improving performance of getting the filtered set of search parameters. |
lmsurpre
added a commit
that referenced
this issue
Dec 23, 2020
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 <[email protected]>
lmsurpre
added a commit
that referenced
this issue
Jan 4, 2021
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 <[email protected]>
JohnTimm
added a commit
that referenced
this issue
Jan 7, 2021
* ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <[email protected]> * ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <[email protected]> * ci: add integration tests for fhir-audit feature and fix one bug with use of/from Signed-off-by: Paul Bastide <[email protected]> * fix: pseudo tty Signed-off-by: Paul Bastide <[email protected]> * fix: small change to the timeout length to 120s Signed-off-by: Paul Bastide <[email protected]> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <[email protected]> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <[email protected]> * fix: add get_results.sh creation of the directory Signed-off-by: Paul Bastide <[email protected]> * fix: update Signed-off-by: Paul Bastide <[email protected]> * fix: update Signed-off-by: Paul Bastide <[email protected]> * fix: for privileged execution Signed-off-by: Paul Bastide <[email protected]> * fix: difference running ci local and remote Signed-off-by: Paul Bastide <[email protected]> * fix: audit Signed-off-by: Paul Bastide <[email protected]> * fix: audit with docker copy Signed-off-by: Paul Bastide <[email protected]> * fix: adding tty support and stdin support Signed-off-by: Paul Bastide <[email protected]> * fix: work around tty issue Signed-off-by: Paul Bastide <[email protected]> * removing the tty references -it Signed-off-by: Paul Bastide <[email protected]> * changed the execution pattern Signed-off-by: Paul Bastide <[email protected]> * changed the execution pattern Signed-off-by: Paul Bastide <[email protected]> * fix: update to predefine output file Signed-off-by: Paul Bastide <[email protected]> * fix: update to predefine output file Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * 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 <[email protected]> * ci: work around issue with tty Signed-off-by: Paul Bastide <[email protected]> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <[email protected]> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <[email protected]> * ci: work around issue with tty Signed-off-by: Paul Bastide <[email protected]> * remove hardcoded year from SearchLastUpdatedIdTest Signed-off-by: Lee Surprenant <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: last two tests to update with dynamic year Signed-off-by: Paul Bastide <[email protected]> * 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 <[email protected]> * Apply suggestions from code review Signed-off-by: Lee Surprenant <[email protected]> * Update build/audit/README.md Signed-off-by: Paul Bastide <[email protected]> Co-authored-by: Lee Surprenant <[email protected]> * add info on accessing the bulk operation job logs Signed-off-by: Lee Surprenant <[email protected]> * 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 <[email protected]> * Issue #1849 - AuthZ interceptor validate/convert search requests Signed-off-by: Mike Schroeder <[email protected]> * Issue #1849 - address review comments Signed-off-by: Mike Schroeder <[email protected]> Co-authored-by: Paul Bastide <[email protected]> Co-authored-by: Lee Surprenant <[email protected]> Co-authored-by: Mike Schroeder <[email protected]> Co-authored-by: Michael W Schroeder <[email protected]>
JohnTimm
added a commit
that referenced
this issue
Jan 21, 2021
* ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <[email protected]> * ci: introduce integration tests for fhir-audit Signed-off-by: Paul Bastide <[email protected]> * ci: add integration tests for fhir-audit feature and fix one bug with use of/from Signed-off-by: Paul Bastide <[email protected]> * fix: pseudo tty Signed-off-by: Paul Bastide <[email protected]> * fix: small change to the timeout length to 120s Signed-off-by: Paul Bastide <[email protected]> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <[email protected]> * fix: alternative method for getting the results from the kafka-1 container Signed-off-by: Paul Bastide <[email protected]> * fix: add get_results.sh creation of the directory Signed-off-by: Paul Bastide <[email protected]> * fix: update Signed-off-by: Paul Bastide <[email protected]> * fix: update Signed-off-by: Paul Bastide <[email protected]> * fix: for privileged execution Signed-off-by: Paul Bastide <[email protected]> * fix: difference running ci local and remote Signed-off-by: Paul Bastide <[email protected]> * fix: audit Signed-off-by: Paul Bastide <[email protected]> * fix: audit with docker copy Signed-off-by: Paul Bastide <[email protected]> * fix: adding tty support and stdin support Signed-off-by: Paul Bastide <[email protected]> * fix: work around tty issue Signed-off-by: Paul Bastide <[email protected]> * removing the tty references -it Signed-off-by: Paul Bastide <[email protected]> * changed the execution pattern Signed-off-by: Paul Bastide <[email protected]> * changed the execution pattern Signed-off-by: Paul Bastide <[email protected]> * fix: update to predefine output file Signed-off-by: Paul Bastide <[email protected]> * fix: update to predefine output file Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * fix permissions Signed-off-by: Paul Bastide <[email protected]> * 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 <[email protected]> * ci: work around issue with tty Signed-off-by: Paul Bastide <[email protected]> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <[email protected]> * fix: test that doesn't account for year shifts Signed-off-by: Paul Bastide <[email protected]> * ci: work around issue with tty Signed-off-by: Paul Bastide <[email protected]> * remove hardcoded year from SearchLastUpdatedIdTest Signed-off-by: Lee Surprenant <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: change the integration pattern slightly for tty Signed-off-by: Paul Bastide <[email protected]> * fix: last two tests to update with dynamic year Signed-off-by: Paul Bastide <[email protected]> * 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 <[email protected]> * Apply suggestions from code review Signed-off-by: Lee Surprenant <[email protected]> * Update build/audit/README.md Signed-off-by: Paul Bastide <[email protected]> Co-authored-by: Lee Surprenant <[email protected]> * add info on accessing the bulk operation job logs Signed-off-by: Lee Surprenant <[email protected]> * 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 <[email protected]> * Issue #1849 - AuthZ interceptor validate/convert search requests Signed-off-by: Mike Schroeder <[email protected]> * Issue #1849 - address review comments Signed-off-by: Mike Schroeder <[email protected]> * Duplicate Job Parameters fhir.dataSourcesInfo are created #1855 - Removed the duplicate serialization of the fhir.dataSourcesInfo Signed-off-by: Paul Bastide <[email protected]> * 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 <[email protected]> * Confusing error when request is targetted for an invalid tenant id #1792 1 - added layer at the rest level Signed-off-by: Paul Bastide <[email protected]> * Issue #1615 - Enforce configured interactions in REST layer Signed-off-by: Mike Schroeder <[email protected]> * 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 <[email protected]> * Issue #1615 - address review comments Signed-off-by: Mike Schroeder <[email protected]> * Issue #1615 - fix whitespace Signed-off-by: Mike Schroeder <[email protected]> * Issue #1615 - add enum for interaction types Signed-off-by: Mike Schroeder <[email protected]> * 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 <[email protected]> * Issue #1494 - add Bundle.entry.search to search results Signed-off-by: Mike Schroeder <[email protected]> * Issue #1494 - add documentation Signed-off-by: Mike Schroeder <[email protected]> * Issue #1494 - address review comments Signed-off-by: Mike Schroeder <[email protected]> Co-authored-by: Paul Bastide <[email protected]> Co-authored-by: Lee Surprenant <[email protected]> Co-authored-by: Mike Schroeder <[email protected]> Co-authored-by: Michael W Schroeder <[email protected]>
|
LGTM. Verified through inspection. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
List#contains performs a linear search, which could have performs implications if the list is large and called repeatedly.
To Reproduce
See code
Expected behavior
A performant implementation.
Additional context
Review usage to see if a HashSet would be more appropriate in this context.
The text was updated successfully, but these errors were encountered: