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

Patient Export Results in duplicate data included in the Write. #2058

Closed
prb112 opened this issue Mar 11, 2021 · 2 comments
Closed

Patient Export Results in duplicate data included in the Write. #2058

prb112 opened this issue Mar 11, 2021 · 2 comments
Assignees
Labels
bug Something isn't working bulk-data

Comments

@prb112
Copy link
Contributor

prb112 commented Mar 11, 2021

Describe the bug
Patient Export Results in duplicate data included in the Write.

To Reproduce
Steps to reproduce the behavior:

  1. Execute integration test for ExportOperationTest
  2. Note the duplicate Patient data the top.

Expected behavior
From https://hl7.org/fhir/uv/bulkdata/export/index.html#response---complete-status:
Each file SHALL contain resources of only one type, but a server MAY create more than one file for each resource type returned.

Our intent is to include only the Resources in the file. Except Attachments and DocumentReferences.

Additional context
n/a

@prb112 prb112 added bug Something isn't working bulk-data labels Mar 11, 2021
@prb112 prb112 self-assigned this Mar 11, 2021
@prb112 prb112 added this to the Sprint 2021-04 milestone Mar 11, 2021
prb112 added a commit that referenced this issue Mar 11, 2021
- Empty DTOs cause the write to skip
- PatientResourceHandler has confusing logic, and skips the resource
write.

References in Patient Export are not honored #2059

- We extract IDs, and should use the relative reference when searching.

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
prb112 added a commit that referenced this issue Mar 11, 2021
Added Integration Test to verify that only a single resource type is
written to a file.

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@lmsurpre
Copy link
Member

lmsurpre commented Mar 15, 2021

I wonder if its worth opening a separate issue for generating a better query for this one.
Specifically, we now have a special internal "compartment" search parameter that we should be able to leverage so that we don't need to aggregate all the different compartment inclusion criteria parameters separately.

For example, we might be able to get rid of this code in PatientResourceHandler.fillChunkDataBuffer(List<String> patientIds, ReadResultDTO dto):

            List<String> compartmentSearchCriterias = CompartmentUtil.getCompartmentResourceTypeInclusionCriteria("Patient", resourceType.getSimpleName());
            if (compartmentSearchCriterias.size() > 1) {
                isDoDuplicationCheck = true;
            }
            for (String compartmentSearchCriteria : compartmentSearchCriterias) {
                HashMap<String, List<String>> queryTmpParameters = new HashMap<>();
                queryTmpParameters.putAll(queryParameters);
                queryTmpParameters.put(compartmentSearchCriteria, Arrays.asList(String.join(",", patientIds)));
                searchContext = SearchUtil.parseQueryParameters(resourceType, queryTmpParameters);

One thing that might make this difficult is that, under normal operation, a patient compartment search is only ever scoped to a single patient (whereas here we have a list of patients). A couple ideas:
A. Perform a different search for each specific patient included in the export. It would definitely be a lot more searches (i.e. more round trips to the DB), but maybe these simple searches will perform better than one giant OR query like we have now?
B. Add support (if we don't already support it) for having a logical OR on the internal compartment search parameter.

If we're able to do B, this would also help us to support user scoped SMART App Launch scopes in our AuthzPolicyEnforcementPersistenceInterceptor, which are for cases where the user has access to a set of patient compartment resources and not just their own.

@d0roppe
Copy link
Collaborator

d0roppe commented Mar 19, 2021

verified this is now working correctly on the export.

@d0roppe d0roppe closed this as completed Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bulk-data
Projects
None yet
Development

No branches or pull requests

3 participants