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

Preserve contained resource meta when encoding to dao #1311

Merged

Conversation

Projects
None yet
3 participants
@basecade
Copy link
Contributor

commented May 16, 2019

The contained resource meta data is not persisted when saving parent resource to database. When persisting resource to database the following setting is used on the database:
parser.setDontEncodeElements(ResourceMetaParams.EXCLUDE_ELEMENTS_IN_ENCODED);

This is okay for the parent resource beause the meta data is saved somewhere else, but for the contained resource the meta is just lost when not persisting.

Test to reproduce problem: https://groups.google.com/forum/#!topic/hapi-fhir/uxVxxldqsBI

Preserve contained resource meta when encoding with: parser.setDontEn…
…codeElements(ResourceMetaParams.EXCLUDE_ELEMENTS_IN_ENCODED)
@jamesagnew

This comment has been minimized.

Copy link
Owner

commented May 16, 2019

Hi @basecade ,

Thanks for the PR! This is neat.

Can I ask you to include unit tests that demonstrate and test the functionality being added?

@patrick-werner

This comment has been minimized.

Copy link
Collaborator

commented May 16, 2019

Hi @basecade,

did you pay respect to these constrains of the FHIR spec?

Contained resources SHALL NOT contain meta.versionId, meta.lastUpdated, or meta.security.

@basecade

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Hi James,

I have actually written a test but was not sure where to place it:

public void encodeResourceToString_withEXCLUDE_ELEMENTS_IN_ENCODED_metaKeptInContainedResource() {
// Arrange
Organization containedOrganization = new Organization();
containedOrganization.getMeta().addProfile("http://organization.custom");

    Patient patient = new Patient();
    patient.setId(UUID.randomUUID().toString());
    patient.getMeta().addProfile("http://hl7.org/fhir/StructureDefinition/Patient");
    patient.setGeneralPractitioner(Arrays.asList(new Reference(containedOrganization)));

    IParser parser = ResourceEncodingEnum.JSONC.newParser(new DummyFhirContext());
    parser.setDontEncodeElements(ResourceMetaParams.EXCLUDE_ELEMENTS_IN_ENCODED);

    // Act
    String encodedPatient = parser.encodeResourceToString(patient);

    // Assert
    assertThat(encodedPatient).contains("\"meta\":{\"profile\":[\"http://organization.custom\"]}");
    assertThat(StringUtils.countMatches(encodedPatient, "\"meta\"")).isEqualTo(1);
}

Did you have another test in mind?

Best Regards,
Anders Havn

@basecade

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Contained resources SHALL NOT contain meta.versionId, meta.lastUpdated, or meta.security

Hi Patrick,

I did not know that. Is it the Hapi framework that should enforce this or the user of the framework?

Best Regards,
Anders Havn

@jamesagnew

This comment has been minimized.

Copy link
Owner

commented May 16, 2019

@basecade One of the JsonParserXXXTest or XmlParserXXXTest classes would be a good home for tests for this new functionality.

And to @patrick-werner 's point, it would be good to have tests that confirm that the FHIR rules he mentioned are still being respected.

@basecade

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

I have now tried to update the code and added a test.

@jamesagnew

This comment has been minimized.

Copy link
Owner

commented May 22, 2019

Nice work, this looks like it fits the bill. 👍

@jamesagnew jamesagnew merged commit 3bb3c3a into jamesagnew:master May 22, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

jamesagnew added a commit that referenced this pull request May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.