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

Unify handling of _type parameter across system-search, system-history, $everything and $export #3319

Closed
lmsurpre opened this issue Feb 9, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Feb 9, 2022

Is your feature request related to a problem? Please describe.
Currently, our system-history implementations accepts multiple instances of the _type parameter and combine them into a single list.

Conversely, our system-search implementation takes just the first one.

In a recent version of the bulkdata spec, they relaxed the cardinality and so multiple instances of this parameter should be supported there as well. We have a separate issue, #3081, for that one.

Describe the solution you'd like
across these 4 endpoints, we should have common behavior for this parameter

Describe alternatives you've considered

Acceptance Criteria

  1. WHEN a search like GET [base]?_type=Patient&_type=Condition
    THEN the values of the _type parameters are combined
    AND the result matches GET [base]?_type=Patient,Condition

And verify this same behavior when:

  1. system-history is invoked with multiple _type query params
  2. $everything is invoked with multiple _type query params
  3. $export is invoked via GET with multiple _type query params
  4. $export is invoked via POST with multiple _type params in the Parameters body

And finally, verify that when $export is invoked via POST, none of the query parameters are used (only the Parameters from the body).

Additional context
We should probably document the behavior in Conformance.md as well, since this differs from how we handle other "return parameters" (#3318 )

@lmsurpre lmsurpre added the enhancement New feature or request label Feb 9, 2022
@lmsurpre lmsurpre changed the title Unify handling of _type parameter across system-search, system-history, and $export Unify handling of _type parameter across system-search, system-history, $everything and $export Feb 9, 2022
@lmsurpre lmsurpre self-assigned this Mar 15, 2022
@lmsurpre
Copy link
Member Author

The $export operation has some special handling for the _types parameter where it checks in both the request url AND the Parameters object. I'd like to try normalizing the two so that downstream code can look in a single spot for request parameters. I think we already do something like that for extended operations that are invoked via GET.

lmsurpre added a commit that referenced this issue Mar 15, 2022
* Introduce the ResourceTypeName enum in fhir-core and ResourcesConfigAdapter in fhir-config
* Use ResourcesConfigAdapter from whole-system search, whole-system history, $everything, and $export

I also started on #3319 - system-search now supports multiple instances of the `_type` parameter

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 16, 2022
* Introduce the ResourceTypeName enum in fhir-core and ResourcesConfigAdapter in fhir-config
* Use ResourcesConfigAdapter from whole-system search, whole-system history, $everything, and $export

I also started on #3319 - system-search now supports multiple instances of the `_type` parameter

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 16, 2022
* Introduce the ResourceTypeName enum in fhir-core and ResourcesConfigAdapter in fhir-config
* Use ResourcesConfigAdapter from whole-system search, whole-system history, $everything, and $export

I also started on #3319 - system-search now supports multiple instances of the `_type` parameter

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 16, 2022
* Introduce the ResourceTypeName enum in fhir-core and ResourcesConfigAdapter in fhir-config
* Use ResourcesConfigAdapter from whole-system search, whole-system history, $everything, and $export

I also started on #3319 - system-search now supports multiple instances of the `_type` parameter

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 18, 2022
Previously, I updated whole-system search to support combining multiple
instances of the `_type` parameter.  Now we will handle multiple '_type'
parameters in the extended operations $everything and $export as well.

I updated the search section in Conformance.md to indicate this change
and added a note in the extended operations section to indicate that
query parameters will be ignored for operations invoked via POST.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 18, 2022
Previously, I updated whole-system search to support combining multiple
instances of the `_type` parameter.  Now we will handle multiple '_type'
parameters in the extended operations $everything and $export as well.

I updated the search section in Conformance.md to indicate this change
and added a note in the extended operations section to indicate that
query parameters will be ignored for operations invoked via POST.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 18, 2022
Previously, I updated whole-system search to support combining multiple
instances of the `_type` parameter.  Now we will handle multiple '_type'
parameters in the extended operations $everything and $export as well.

I updated the search section in Conformance.md to indicate this change
and added a note in the extended operations section to indicate that
query parameters will be ignored for operations invoked via POST.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Mar 18, 2022
Previously, I updated whole-system search to support combining multiple
instances of the `_type` parameter.  Now we will handle multiple '_type'
parameters in the extended operations $everything and $export as well.

I updated the search section in Conformance.md to indicate this change
and added a note in the extended operations section to indicate that
query parameters will be ignored for operations invoked via POST.

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

lmsurpre commented Apr 1, 2022

Based on the last comment, I added this line to the acceptance criteria:

And finally, verify that when $export is invoked via POST, none of the query parameters are used (only the Parameters from the body).

That was done so that its consistent with our other operation implementations (and so that it matches the fhir-smart logic for controlling access). That can be revisited under #3480

@d0roppe
Copy link
Collaborator

d0roppe commented Apr 3, 2022

Verified that specifying _type=, or _type=&_type= are handled the same for Search, History, $everything, and Bulk Data Export as requested in the Acceptance Criteria.

@d0roppe d0roppe closed this as completed Apr 3, 2022
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

2 participants