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

Constrain Search based on SearchParameters and CapabilityStatements #1351

Closed
prb112 opened this issue Jul 22, 2020 · 12 comments
Closed

Constrain Search based on SearchParameters and CapabilityStatements #1351

prb112 opened this issue Jul 22, 2020 · 12 comments
Assignees
Labels
cms-interop This issue is associated with the CMS interoperability rule profile_carin-bb profiling search
Milestone

Comments

@prb112
Copy link
Contributor

prb112 commented Jul 22, 2020

Is your feature request related to a problem? Please describe.
In the FHIR Search specification, there are complicating conditions for each SearchParameter where multipleOr, multipleAnd, accepted comparators and chains.

Also, one must consider profile specific constraints which limit the usage of particular codes and combinations of codes https://build.fhir.org/ig/HL7/carin-bb/search.html

Describe the solution you'd like

  1. Update SearchUtil.java to enforce the following restrictions of a registered (or in extension-search-parameter.json) SearchParameter:
  • multipleOr
  • multipleAnd
  • comparator
  • modifier
  1. Update SearchUtil.java to enforce the following restrictions of fhir-server-config.json:
  • supported searches
  • supported _includes
  • supported _revincludes

Describe alternatives you've considered
Controlling these restrictions from fhir-server-config.json. That can be done as follow-on work to this, in another GitHub issue, to amend the search restrictions enforced by this issue.

Additional context
None

@prb112 prb112 added the search label Jul 22, 2020
@JohnTimm JohnTimm added cms-interop This issue is associated with the CMS interoperability rule profile_carin-bb profiling labels Aug 25, 2020
@lmsurpre lmsurpre modified the milestone: sprint17 Aug 25, 2020
@JohnTimm JohnTimm added this to the Sprint 18 milestone Sep 14, 2020
@kmbarton423
Copy link
Contributor

Today had an Inferno search fail which was using "identifier" vs "id". There is no enforcement of unique identifiers so in this situation, data for multiple patients was being retrieved by the search. Search fails as unauthorized. Not sure if there is anything programmatic that can be done here but this could be a very confusing. Just something to consider when finalizing any interceptor behaviors.

@lmsurpre lmsurpre modified the milestones: Sprint 18, Sprint 19 Oct 6, 2020
@prb112
Copy link
Contributor Author

prb112 commented Oct 12, 2020

Today had an Inferno search fail which was using "identifier" vs "id". There is no enforcement of unique identifiers so in this situation, data for multiple patients was being retrieved by the search. Search fails as unauthorized. Not sure if there is anything programmatic that can be done here but this could be a very confusing. Just something to consider when finalizing any interceptor behaviors.

HI Karen, I think that's a data quality issue. Is there anything you need Troy or I to look at with regards to data retrieval and constraining the retrieval? Thanks, Paul

@tbieste tbieste self-assigned this Oct 12, 2020
@tbieste
Copy link
Contributor

tbieste commented Oct 12, 2020

@prb112
After reading through it again, trying to confirm my understanding if this is one requirement, or multiple things in one. If I'm off-base, I could use another call to talk through things about it.

In the FHIR Search specification, there are complicating conditions for each SearchParameter where multipleOr, multipleAnd, accepted comparators and chains.

Based on the description, am I correct that the idea is to have the Search interceptor (which gets registered by being put in the userlib directory?) that would read the registered search parameters and check the multipleOr, multipleAnd, accepted comparators and chains? Shouldn't that be something handled by the actual search when the SearchParameter is looked up? Or is the idea for this to be represented in fhir-server-config.json?

Also, one must consider profile specific constraints which limit the usage of particular codes and combinations of codes https://build.fhir.org/ig/HL7/carin-bb/search.html

For this part, we talked about adding configuration info to the fhir-server-config.json to support having a restricted list of Supported Searches, Supported _includes, and Supported _revincludes. This configuration would be read by the Search interceptor to enforce the restricted searches.

@prb112
Copy link
Contributor Author

prb112 commented Oct 12, 2020

@prb112
After reading through it again, trying to confirm my understanding if this is one requirement, or multiple things in one. If I'm off-base, I could use another call to talk through things about it.

In the FHIR Search specification, there are complicating conditions for each SearchParameter where multipleOr, multipleAnd, accepted comparators and chains.

Based on the description, am I correct that the idea is to have the Search interceptor (which gets registered by being put in the userlib directory?) that would read the registered search parameters and check the multipleOr, multipleAnd, accepted comparators and chains? Shouldn't that be something handled by the actual search when the SearchParameter is looked up? Or is the idea for this to be represented in fhir-server-config.json?

I think we can hook on without a full interceptor or registered Service Loader.

Exactly, when the SearchParameter is looked up, and backed by either the fhir-server-config.json or the CapabilityStatement to verify.

Also, one must consider profile specific constraints which limit the usage of particular codes and combinations of codes https://build.fhir.org/ig/HL7/carin-bb/search.html

For this part, we talked about adding configuration info to the fhir-server-config.json to support having a restricted list of Supported Searches, Supported _includes, and Supported _revincludes. This configuration would be read by the Search interceptor to enforce the restricted searches.

Right.

@tbieste
Copy link
Contributor

tbieste commented Oct 12, 2020

Exactly, when the SearchParameter is looked up, and backed by either the fhir-server-config.json or the CapabilityStatement to verify.

What do you mean by this part?

@prb112
Copy link
Contributor Author

prb112 commented Oct 12, 2020

Shouldn't that be something handled by the actual search when the SearchParameter is looked up? Or is the idea for this to be represented in fhir-server-config.json?
Exactly, when the SearchParameter is looked up, and backed by either the fhir-server-config.json or the CapabilityStatement to verify.

If I understood correctly, you are suggesting at the time of looking up the SearchParameter confirming the combination of search parameters from the fhir-server-config.json or CapabilityStatement. Which makes sense to me.

@tbieste
Copy link
Contributor

tbieste commented Oct 12, 2020

If I understood correctly, you are suggesting at the time of looking up the SearchParameter confirming the combination of search parameters from the fhir-server-config.json or CapabilityStatement. Which makes sense to me.

Yeah, I think that sounded like a good approach.

In the FHIR Search specification, there are complicating conditions for each SearchParameter where multipleOr, multipleAnd, accepted comparators and chains.

Is adding support to enforce the values of specifically the 4 fields of: "multipleOr", "multipleAnd", accepted "comparators" and "chains" what this part of the requirement is really saying? I wasn't clear if this was a requirement or just related info.

@prb112
Copy link
Contributor Author

prb112 commented Oct 12, 2020

Oh - sorry - I didn't cover this... yes - we should look at each SearchParameter too. and see if there are conditions for use for multipleOr, multipleAnd, and prefixes supported. This is pretty detailed.

@tbieste
Copy link
Contributor

tbieste commented Oct 12, 2020

ok, we should probably meet again tomorrow morning, then. Sounds like there's more to this one. Some parts having to do with a Search interceptor (if any), some not, etc. We'll get it ironed out.

@prb112
Copy link
Contributor Author

prb112 commented Oct 12, 2020

Sure - let's meet right after DSU

@tbieste tbieste changed the title Constrained Search Interceptor Constrain Search based on SearchParameters and CapabilityStatements Oct 13, 2020
tbieste added a commit that referenced this issue Oct 13, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 13, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 13, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 13, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 14, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
@tbieste
Copy link
Contributor

tbieste commented Oct 15, 2020

After further discussion, instead of discovering the valid list of search parameters, includes, and revIncludes from the CapabilityStatement, they with be discovered by looking at the FHIR configuration (fhir-server-config.json). Issue #1449 is also in need of changes to the fhir-server-config.json, so a "resources" section will be added to the fhir-server-config to support both of these issues.

Here's an example:

"resources": {
      "includeOmittedResourceTypes": true,
      "ExplanationOfBenefit": {
        "interactions": ["read",
        "vread",
        "history",
        "search"],
        "searchIncludes": ["ExplanationOfBenefit:patient",
        "ExplanationOfBenefit:provider",
        "ExplanationOfBenefit:care-team",
        "ExplanationOfBenefit:coverage",
        "ExplanationOfBenefit:insurer"],
        "searchRevIncludes": [],
        "searchParameters": {
          "patient": {
            "url": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-patient",
            "required": true
          },
          "type": {
            "url": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-type"
          },
          "identifier": {
            "url": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-identifier"
          },
          "date": {
            "url": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-service-date"
          },
          "reference": {
            "url": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-some-ref"
          }
        }
      }
    }

tbieste added a commit that referenced this issue Oct 15, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 15, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 15, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 19, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 20, 2020
Initially we made each searchParameter its own object so that we could
add additional properties for issue #1351.
However, now we don't think that any additional fields are needed and so
we can simplify the config to be a simple map of SearchParameter  code
-> SearchParameter.url

This format will give us the flexibility of overriding a SearchParameter
code for a given resource type for a given deploy. However, currently
the code from the config is not used...it will only use the url and it
will auto-resolve any conflicting codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 20, 2020
Initially we made each searchParameter its own object so that we could
add additional properties for issue #1351.
However, now we don't think that any additional fields are needed and so
we can simplify the config to be a simple map of SearchParameter  code
-> SearchParameter.url

This format will give us the flexibility of overriding a SearchParameter
code for a given resource type for a given deploy. However, currently
the code from the config is not used...it will only use the url and it
will auto-resolve any conflicting codes.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
@tbieste
Copy link
Contributor

tbieste commented Oct 20, 2020

Updated fhir-server-config.json example:

{
    "__comment": "FHIR Server configuration",
    "fhirServer": {
        "resources": {
            "open": true,
            "CarePlan": {
                "searchParameterCombinations": ["patient+category+status+date",
                "patient+category+status",
                "patient+category",
                "patient+category+date"]
            },
            "ExplanationOfBenefit": {
                "interactions": [
                    "read",
                    "vread",
                    "history",
                    "search"
                ],
                "searchIncludes": [
                    "ExplanationOfBenefit:patient",
                    "ExplanationOfBenefit:provider",
                    "ExplanationOfBenefit:care-team",
                    "ExplanationOfBenefit:coverage",
                    "ExplanationOfBenefit:insurer",
                    "ExplanationOfBenefit:*"
                ],
                "searchRevIncludes": [],
                "searchParameters": {
                    "_id": "http://hl7.org/fhir/SearchParameter/Resource-id",
                    "_lastUpdated": "http://hl7.org/fhir/SearchParameter/Resource-lastUpdated",
                    "patient": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-patient",
                    "type": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-type",
                    "identifier": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-identifier",
                    "service-date": "http://hl7.org/fhir/us/carin-bb/SearchParameter/explanationofbenefit-service-date"
                }
            },
            "Patient": {
                "searchIncludes": ["Patient:general-practitioner"],
                "searchRevIncludes": ["MedicationRequest:intended-performer"],
                "searchParameterCombinations": ["","_id","multiple-birth-count","multiple-birth-count-basic"]
            },
            "RelatedPerson": {
                "searchParameterCombinations": ["*"]
            },
            "Resource": {
                "searchIncludes": ["MedicationRequest:patient"],
                "searchRevIncludes": ["Provenance:target"],
                "searchParameterCombinations": ["","_id"]
            }
        }
    }
}

tbieste added a commit that referenced this issue Oct 20, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 26, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 26, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 26, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
tbieste added a commit that referenced this issue Oct 26, 2020
Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
prb112 added a commit that referenced this issue Oct 27, 2020
issue #1351 - Check _include and _revinclude values are supported
@prb112 prb112 closed this as completed Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cms-interop This issue is associated with the CMS interoperability rule profile_carin-bb profiling search
Projects
None yet
Development

No branches or pull requests

5 participants