Added service loader pattern for deserialization #587

vitorpamplona · 2022-06-28T22:22:54Z

Merge after: #581

This PR splits the engine in Jackson and JAXB flavors. It copies the structure from cqframework/clinical_quality_language#760

engine.jackson: ElmLibraryReader implementations using Jackson
engine.jaxb: ElmLibraryReader implementations using JAXB

Both Jackson and JAXB modules can deserialize XML and JSON into Executable ELM.

Since the current engine requires reloading a compiled ELM into executable ELM through JSON, all CQL tests were moved to the Jackson module until a better solution is found for https://github.com/DBCG/cql_engine/issues/586

# Conflicts: # pom.xml

…ically create new tests based on the number of files in the directory.

…sults

# Conflicts: # engine.fhir/pom.xml # engine.jaxb/pom.xml # engine/pom.xml # pom.xml

# Conflicts: # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java

# Conflicts: # engine.fhir/pom.xml # engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/Dstu2FhirModelResolver.java # engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/Dstu3FhirModelResolver.java # engine.fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/R4FhirModelResolver.java # engine.fhir/src/test/java/org/hl7/fhirpath/CQLOperationsDstu3Test.java # engine.fhir/src/test/java/org/hl7/fhirpath/CQLOperationsR4Test.java # engine.fhir/src/test/java/org/hl7/fhirpath/TestLibraryLoader.java # engine.fhir/src/test/java/org/hl7/fhirpath/TranslatorHelper.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFhirDataProviderDstu3.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/retrieve/TestDstu3FhirQueryGenerator.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/retrieve/TestR4FhirQueryGenerator.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/retrieve/TestRestFhirRetrieveProvider.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlEngineTests.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlExecutionTestBase.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlInternalTypeRepresentationSuiteTest.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TestLibraryLoader.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TranslatingTestBase.java # engine.jaxb/pom.xml # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ANCFHIRTests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/CMS53Tests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ElmRegressionTests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ElmTests.java # engine/pom.xml # engine/src/main/java/org/opencds/cqf/cql/engine/execution/JsonCqlLibraryReader.java # engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlInternalTypeRepresentationSuiteTest.java # engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlTestSuite.java # pom.xml

# Conflicts: # engine.fhir/pom.xml # engine.fhir/src/test/java/org/hl7/fhirpath/TestLibraryLoader.java # engine.fhir/src/test/java/org/hl7/fhirpath/TranslatorHelper.java # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlEngineTests.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlExecutionTestBase.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlInternalTypeRepresentationSuiteTest.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TestLibraryLoader.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TranslatingTestBase.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ANCFHIRTests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/CMS53Tests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ElmRegressionTests.java # engine.jaxb/src/test/java/org/opencds/cqf/cql/engine/execution/ElmTests.java # engine/pom.xml # engine/src/main/java/org/opencds/cqf/cql/engine/execution/JsonCqlLibraryReader.java # engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlInternalTypeRepresentationSuiteTest.java # engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlTestSuite.java

engine.jackson/src/main/resources/log4j.properties

...n/src/test/java/org/opencds/cqf/cql/engine/execution/CqlErrorsAndMessagingOperatorsTest.java

engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListOperatorsTest.java

JPercival

Got about half-way through the review, but there's a ton of noise due to different import ordering. Can you take a look at that and see if you can match the current pattern?

# Conflicts: # engine.fhir/src/test/java/org/hl7/fhirpath/TestLibraryLoader.java # engine.jackson/src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/CqlToElmBaseMixIn.java # engine.jackson/src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/ElementMixin.java # engine.jackson/src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/ExpressionDefMixin.java # engine.jackson/src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/ExpressionMixin.java # engine.jackson/src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/TypeSpecifierMixin.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TestLibraryLoader.java # engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TranslatingTestBase.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/ElementMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/Executable.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/ExpressionMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/TypeSpecifierMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/serialization/ElementMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/serialization/ExpressionMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/elm/serialization/TypeSpecifierMixin.java # engine/src/main/java/org/opencds/cqf/cql/engine/execution/JsonCqlLibraryReader.java # engine/src/main/java/org/opencds/cqf/cql/engine/serializing/LibraryWrapper.java

vitorpamplona · 2022-08-15T21:47:02Z

@JPercival ready for you again.

.../src/main/java/org/opencds/cqf/cql/engine/serializing/jackson/mixins/ExpressionDefMixin.java

JPercival · 2022-08-16T16:54:47Z

engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/TranslatingTestBase.java

+
+    public String convertToJson(org.hl7.elm.r1.Library library) throws IOException {
+        StringWriter writer = new StringWriter();
+        ElmLibraryWriterFactory.getWriter("application/elm+json").write(library, writer);


There's an enum for this now: LibraryContentType.XML.mimeType()

Only available in the 2.1.0 release :)

...e.jackson/src/test/java/org/opencds/cqf/cql/engine/serializing/JsonCqlLibraryReaderTest.java

JPercival · 2022-08-16T17:00:30Z

engine/pom.xml

                                        <counter>LINE</counter>
                                        <value>COVEREDRATIO</value>
-                                        <minimum>0.50</minimum>
+                                        <minimum>0.02</minimum>


This does raise the question of how to best get coverage in the engine itself. There's not much in the way of unit tests for individual Evaluators.

Because most of the tests uses jackson to convert to json back and forth, I moved them to the jackson branch, which reduces the coverage ratio of the main engine module. It would be nice for organizational purposes if we had the evaluator's mapper in the engine's test cases.

vitorpamplona added 30 commits June 20, 2022 16:03

Upgrading to CQ 2.0.0 library, Java 11 and HAPI 6

80fed50

Updating Schema definitions to match CQ2.0.0

d4be965

Merge remote-tracking branch 'origin/HEAD' into cq2.0.0-upgrade

ddf2d44

# Conflicts: # pom.xml

Rolling back tabs to spaces.

9a7241b

Updating Java11 Deprecations

3b201d4

Fixing all JSONs generated by Jackson for the tests.

0fd72ab

Updating dependencies to the latest versions

c1a4446

keeping test dependencies on test scopes

5fc7b6a

Updating Maven profile to java11

38a4b37

Ignoring new test output directory.

a651936

Refactoring JAXB test cases using @factory and @dataProvider to dynam…

c127172

…ically create new tests based on the number of files in the directory.

Merging new version

e37cf07

Refactoring FHIR Tests.

dc409b4

Adding logging on tests to facilitate the understanding of failing re…

ee3ddaa

…sults

Restructuring compare results to serve R4 and DSTU3

89dcef5

Reverting comparable to full types

2245373

Using R4's test structure for DSTU3

c97b1ae

Better formatting

e363928

Reusing cached Contexts.

9d2c4c9

Reducing memory use

abc0f52

Marking down individual tests to skip on Engine.FHIR

4ab5dc8

Adding a new test to skip.

4a69d41

Fix unused imports

df54591

Removing full package names from ObjectFactory.

564776c

Moving moxy to test only

27b7476

Removing that are already declared on the Engine or via Moxy.

6e2fb7a

removing unnecessary dependencies at the engine level

4752f3d

Adding Glassfish JAXB to the dependencies

dae80d2

no need for glassfish.

e61c542

Fixing Path for SchemaLocation

d18252c

vitorpamplona added 8 commits July 18, 2022 18:37

Adding serialVersionUID to CqlException subclass

539c0f1

Changing the interfaces of the Model Resolver to protected.

2b45eb2

Changing tests to subclasses of the ModelResolvers with cached contexts

fd27941

Enabling Quick on the Test environment for the Engine.FHIR

9918cb1

Merge remote-tracking branch 'origin/HEAD' into cq2.0.0-upgrade

a366eae

# Conflicts: # engine.fhir/pom.xml # engine.jaxb/pom.xml # engine/pom.xml # pom.xml

Merge remote-tracking branch 'origin/HEAD' into cq2.0.0-upgrade

b33959e

# Conflicts: # engine.fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/model/TestDstu2ModelResolver.java

vitorpamplona marked this pull request as ready for review August 8, 2022 19:06

JPercival reviewed Aug 8, 2022

View reviewed changes

engine.jackson/src/main/resources/log4j.properties Outdated Show resolved Hide resolved

JPercival reviewed Aug 8, 2022

View reviewed changes

...n/src/test/java/org/opencds/cqf/cql/engine/execution/CqlErrorsAndMessagingOperatorsTest.java Outdated Show resolved Hide resolved

JPercival reviewed Aug 8, 2022

View reviewed changes

engine.jackson/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListOperatorsTest.java Outdated Show resolved Hide resolved

JPercival reviewed Aug 8, 2022

View reviewed changes

vitorpamplona added 7 commits August 8, 2022 19:10

Fixing imports

ec5b268

Fixing imports

39af611

Fixing imports

94fa968

Fixing imports 3

c724e8a

Fixing imports 4

83b8eb5

Deleting log4j properties in the jackson module.

3438bd8

JPercival reviewed Aug 16, 2022

View reviewed changes

vitorpamplona added 4 commits August 16, 2022 17:09

Fixing redundant File creation

2fd444c

No need for a class to define Mixins.

74cb3eb

Using resource as stream instead of File

a1eadd6

Moves hapi-fhir-client to test scope.

2e19d94

JPercival approved these changes Aug 23, 2022

View reviewed changes

JPercival merged commit b31601c into cqframework:master Aug 23, 2022

vitorpamplona deleted the jackson-module branch August 23, 2022 15:27

vitorpamplona mentioned this pull request Sep 19, 2022

CQL combo 2.0.0 Upgrade - Full PD$apply implementation google/android-fhir#1603

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added service loader pattern for deserialization #587

Added service loader pattern for deserialization #587

Uh oh!

vitorpamplona commented Jun 28, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JPercival left a comment

Uh oh!

vitorpamplona commented Aug 15, 2022

Uh oh!

Uh oh!

JPercival Aug 16, 2022

Uh oh!

vitorpamplona Aug 16, 2022

Uh oh!

Uh oh!

JPercival Aug 16, 2022

Uh oh!

vitorpamplona Aug 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Added service loader pattern for deserialization #587

Added service loader pattern for deserialization #587

Uh oh!

Conversation

vitorpamplona commented Jun 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JPercival left a comment

Choose a reason for hiding this comment

Uh oh!

vitorpamplona commented Aug 15, 2022

Uh oh!

Uh oh!

JPercival Aug 16, 2022

Choose a reason for hiding this comment

Uh oh!

vitorpamplona Aug 16, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JPercival Aug 16, 2022

Choose a reason for hiding this comment

Uh oh!

vitorpamplona Aug 16, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vitorpamplona commented Jun 28, 2022 •

edited

Loading