google / fhir-data-pipes

A collection of tools for extracting FHIR resources and analytics services on top of that data.
https://google.github.io/fhir-data-pipes/
Apache License 2.0
151 stars 84 forks source link

Support for multiple FHIR profile extensions #986

Closed chandrashekar-s closed 5 months ago

chandrashekar-s commented 6 months ago

Description of what I changed

Fixes #980

This is an extension to the PR #924. In this PR we provide support for multiple FHIR profile extension for a given resource type. This is achieved by creating a Master AvroConverter for the given resource type by merging all the individual AvroConverters against the configured extensions profiles including the base profile. The merging includes a deep merge of the nested elements under the AvroConverter.

E2E test

Tested the changes with the default US Core FHIR profile extensions and verified that the extension fields are properly populated in the parquet files.

TESTED:

Please replace this with a description of how you tested your PR beyond the automated e2e/unit tests.

Checklist: I completed these to help reviewers :)

codecov-commenter commented 6 months ago

Codecov Report

Attention: Patch coverage is 59.20578% with 113 lines in your changes are missing coverage. Please review.

Project coverage is 50.52%. Comparing base (b354beb) to head (178366b).

Files Patch % Lines
...r/bunsen/avro/tools/GenerateAggregatedSchemas.java 0.00% 55 Missing :warning:
...unsen/avro/converters/DefinitionToAvroVisitor.java 88.18% 9 Missing and 4 partials :warning:
.../cerner/bunsen/definitions/PrimitiveConverter.java 38.46% 6 Missing and 2 partials :warning:
...m/cerner/bunsen/definitions/HapiConverterUtil.java 12.50% 5 Missing and 2 partials :warning:
...ner/bunsen/definitions/LeafExtensionConverter.java 40.00% 3 Missing and 3 partials :warning:
.../com/cerner/bunsen/avro/tools/GenerateSchemas.java 44.44% 4 Missing and 1 partial :warning:
.../com/google/fhir/analytics/AvroConversionUtil.java 44.44% 4 Missing and 1 partial :warning:
...java/com/cerner/bunsen/ProfileMappingProvider.java 66.66% 3 Missing :warning:
...m/cerner/bunsen/avro/converters/NoOpConverter.java 0.00% 2 Missing :warning:
...a/com/cerner/bunsen/ProfileMapperFhirContexts.java 50.00% 2 Missing :warning:
... and 6 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #986 +/- ## ============================================ + Coverage 50.36% 50.52% +0.16% - Complexity 644 663 +19 ============================================ Files 87 91 +4 Lines 5266 5465 +199 Branches 669 701 +32 ============================================ + Hits 2652 2761 +109 - Misses 2366 2444 +78 - Partials 248 260 +12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

chandrashekar-s commented 6 months ago

@bashir2 The PR is ready for the first round of review, can you please have a look and let me know if the approach taken is good. In the meanwhile, I will add unit tests.

chandrashekar-s commented 6 months ago

@bashir2 Please have a look at the latest commits containing more unit tests and some minor fixes.

chandrashekar-s commented 6 months ago

Thanks @bashir2 for the detailed review. I have addressed most of the comments, there are only 2 comments left which we can discuss in our tomorrow's meeting.