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.

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

token search for |<code> should only match codes with no systems #2550

Closed
lmsurpre opened this issue Jun 24, 2021 · 1 comment
Closed

token search for |<code> should only match codes with no systems #2550

lmsurpre opened this issue Jun 24, 2021 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@lmsurpre
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently our SearchUtil.parseQueryParameterValuesString has logic like this:

                } else if (parts.length == 1 && v.endsWith("|") && v.indexOf("|") == v.length()-1) {
                    // Only a system was specified (uri followed by a single '|')
                    parameterValue.setValueSystem(unescapeSearchParm(parts[0]));
                } else {
                    // Treat as a single code.
                    // Optimization for search parameters that always reference the same system, added under #1929
                    if (!Modifier.MISSING.equals(modifier)) {
                        try {
                            String implicitSystem = searchParameter.getExtension().stream()
                                    .filter(e -> SearchConstants.IMPLICIT_SYSTEM_EXT_URL.equals(e.getUrl()) && e.getValue() != null)
                                    .findFirst()
                                    .map(e -> e.getValue().as(Uri.class).getValue())
                                    .orElse(null);
                            if (implicitSystem != null) {
                                parameterValue.setValueSystem(implicitSystem);
                            }
                        } catch (ClassCastException e) {
                            log.log(Level.INFO, "Found " + SearchConstants.IMPLICIT_SYSTEM_EXT_URL + " extension with unexpected value type", e);
                        }
                    }
                    parameterValue.setValueCode(unescapeSearchParm(v));
                }

That means its handling |<code> just like <code>. This is less efficient (see the performance guide) and also not quite right

Describe the solution you'd like
In our JDBC persistence layer we have an optimization whereby we store code with no system with a placeholder system.
The REST layer needs to somehow pass a placeholder like this to the Persistence layer so that we can distinguish between a search for no system vs a search for any system.

This can be the same placeholder or it could be a different one thats part of the contract. Either way, it should probably go into the javadocs somewhere.

Describe alternatives you've considered

Acceptance Criteria

  1. GIVEN a target resource with a searchable Coding that has a system and a code
    WHEN we get a search like param=|<code>
    THEN it does NOT retrieve this resource

Additional context

@prb112 prb112 added the enhancement New feature or request label Jun 24, 2021
@lmsurpre lmsurpre self-assigned this Jun 24, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-09 milestone Jun 24, 2021
lmsurpre added a commit that referenced this issue Jun 24, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 24, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 24, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 24, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 24, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 25, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 25, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 25, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Jun 25, 2021
When we store a token value without a system, we end up using a special
placeholder system instead of saving NULL.

With this PR, we will now use that system when we search for system-less
codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
tbieste added a commit that referenced this issue Jun 30, 2021
issue #2550 - add default system for `|<code>` searches
@michaelwschroeder
Copy link
Contributor

I verified this fix in my local development environment running the latest code from main. I ran the following searches:

  1. searched on a token field using the <token-search-parm>=<code> form and verified that resources returned included those which specified a system value and those that did not.
  2. searched on the same token field using the <token-search-parm>=|<code> form and verified that resources returned included only those which did not specify a system value.

I also inspected the generated SQL for the <token-search-parm>=|<code> search and verified that the common_token_value_id specified in the query was for a token that had a code_system_id of the default code system value, as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants