Transformation rule engine implementation #1046

basiliskus · 2024-04-26T21:54:09Z

Transformation rule engine implementation

Note: there is code duplication in ValidationRuleEngine and TransformationRuleEngine that we have decided to refactor in a future PR

Issue

#1024

Checklist

I have added tests to cover my changes
I have added logging where useful (with appropriate log level)
I have added JavaDocs where required
I have updated the documentation accordingly

…gine implementation

Added dummy definitions from design discussion

…formation-engine-implementation

Fix method names

Updated one of the failing tests

Use double checked locking on ensureRulesloaded

Update more test cases

All tests pass

…/CDCgov/trusted-intermediary into story/1024/transformation-engine

...gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/CustomFhirTransformation.java

...in/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java

...gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/TransformationRuleEngine.java

shared/src/main/java/gov/hhs/cdc/trustedintermediary/context/ApplicationContext.java

e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/e2e/DemographicsTest.groovy

.../java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/TransformationRule.java

…per class with static methods

sonarqubecloud · 2024-05-08T18:49:45Z

Quality Gate failed

Failed conditions
7.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

jorg3lopez · 2024-05-08T21:39:12Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderConverter.java

-package gov.hhs.cdc.trustedintermediary.etor.orders;
-
-/** Interface for converting things to orders and things in orders. */
-public interface OrderConverter {


All three methods from this interface belong to the general flow of the code. Are we going to create a section in the Transformation Engine that will be for our default transformations and a section for custom ones? I'm thinking on a way of separating the default behavior with the custom one. I also see that the custom transformations will go under the transformations/custom folder. Maybe these can live in a system default folder: transformation/system-default folder.

This PR replaces the transformation in OrderConverter and ResultConverter. There's no distinction between default or custom behavior. Is there a benefit to have them separated? If we want, we could add a "default" flag in the json file in a future PR. We could discuss this in eng. block

jorg3lopez · 2024-05-08T21:40:14Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/results/ResultConverter.java

-package gov.hhs.cdc.trustedintermediary.etor.results;
-
-/** Interface for converting things to results and things in results. */
-public interface ResultConverter {


Same comment as the one for OrderConverter

jorg3lopez · 2024-05-08T21:54:18Z

...gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/custom/convertToOmlOrder.java

+    @Override
+    public void transform(FhirResource<?> resource, Map<String, String> args) {
+        Bundle bundle = (Bundle) resource.getUnderlyingResource();
+        HapiOrderConverterHelper.convertToOmlOrder(bundle);


I was thinking that the logic was going to be part of the transformation object. But instead, it calls a helper class that has the logic there. What would be the difference if we have the logic in here, so if we need to change the logic, we go toconvertToOmlOrder.java instead of the helper class, and the helper class can be a wrapper that calls all the transformation objects?

We could have the logic here. The idea of having the helper is to have functions that could be used by multiple transformations. I don't think we have that right now but I'd propose to leave this refactoring to a separate PR

jorg3lopez · 2024-05-08T21:55:13Z

.../hhs/cdc/trustedintermediary/etor/ruleengine/transformation/custom/addEtorProcessingTag.java

+    @Override
+    public void transform(FhirResource<?> resource, Map<String, String> args) {
+        Bundle bundle = (Bundle) resource.getUnderlyingResource();
+        HapiMessageConverterHelper.addEtorTagToBundle(bundle);


Same as the transformation for convertToOmlOrder

jorg3lopez · 2024-05-08T21:55:31Z

...edintermediary/etor/ruleengine/transformation/custom/addContactSectionToPatientResource.java

+    @Override
+    public void transform(FhirResource<?> resource, Map<String, String> args) {
+        Bundle bundle = (Bundle) resource.getUnderlyingResource();
+        HapiOrderConverterHelper.addContactSectionToPatientResource(bundle);


Same as the transformation for convertToOmlOrder

jorg3lopez · 2024-05-08T21:57:33Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiOrderConverterHelper.java

+import org.hl7.fhir.r4.model.Patient;
+
+/** Helper class with transformation methods that take an order Bundle and modifies it */
+public class HapiOrderConverterHelper {


Check the comments for the transformation objects. Thoughts on having the transformation logic as part of each object and having this class be a wrapper that couples with all the transformation objects?

Agreed that we need refactoring and move logic between helper and transformations. As mentioned above, I propose to do it in a separate PR

jcrichlake · 2024-05-09T16:15:30Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiOrderConverterHelper.java

+                    new Coding(
+                            "http://terminology.hl7.org/CodeSystem/v3-RoleCode", "MTH", "mother"));
+
+    public static void convertToOmlOrder(Bundle order) {


Could we make these methods more generic? Like ConvertToOrderType

That's a great idea. As I wrote on a response to Jorge, I think we should refactor this helper in a separate PR

luis-pabon-tf · 2024-05-09T17:11:52Z

We're bypassing the SonarCloud duplication notice because we are saving refactoring of Rule Engines for a future PR.

luis-pabon-tf

This is a very thoroughly worked on PR, with good coverage and tested functionality. Code refactoring is needed, but due to the size of the PR it will be better off done separately.

basiliskus and others added 18 commits April 23, 2024 16:10

Refactored validation rule engine in preparation for tranformation en…

f5e94fe

…gine implementation

Fixed instantiation

28505a5

Merge branch 'main' into story/1024/transformation-engine

2c3f69d

Refactored to make Rule interface more generic

bdd568a

Fixed tests

f65cede

Update transformation_definitions.json

0cf5640

Added dummy definitions from design discussion

Merge branch 'main' into story/1024/transformation-engine

a08c687

Added missing override tags

8631334

Fixed arg naming

6214e84

Use generics to deserialize correct type of Rule implementation

4756364

Converted RuleEngine back into a singleton and added parameters

1de28ef

Refactored to have a separate implementation for ValidationRuleEngine

5c8073b

Refactored to pass TypeReference and fix mapping issue

45a3bca

Added missing override tags

d5490e8

Fixed tests in ValidationRuleEngineIntegrationTest

8e7398c

Fixed more tests

dbd8043

Removed transformation engine implementation to move into new PR

d399054

Added TransformationRuleEngine skeleton

24ae3d3

basiliskus changed the base branch from main to story/1024/transformation-engine April 26, 2024 22:01

basiliskus and others added 11 commits April 26, 2024 15:02

Merge branch 'story/1024/transformation-engine' into story/1024/trans…

90698e4

…formation-engine-implementation

Moved transformation engine files from refactoring PR

0eb2682

Added actual transformation rules based on existing transformations

a865999

Update ValidationRuleTest.groovy

76b7d82

Fix method names

Update ValidationRuleEngineTest.groovy

27ea75d

Updated one of the failing tests

Update ValidationRuleEngine.java

0f1b275

Use double checked locking on ensureRulesloaded

Update ValidationRuleEngineTest.groovy

5acab7a

Update more test cases

Update ValidationRuleEngineTest.groovy

28b0d7a

All tests pass

Update Rule loading to handle Path for flexibility

1f57bb1

Merge branch 'story/1024/transformation-engine' of https://github.com…

5d4dd9e

…/CDCgov/trusted-intermediary into story/1024/transformation-engine

Fixed remaining test

34a2bf9

github-advanced-security bot found potential problems May 7, 2024

View reviewed changes

...gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/CustomFhirTransformation.java Dismissed Show dismissed Hide dismissed

basiliskus added 2 commits May 7, 2024 13:35

Fixed tests

ec714d8

Fixed test

8333f55

jorg3lopez reviewed May 7, 2024

View reviewed changes

...in/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java Outdated Show resolved Hide resolved

jorg3lopez reviewed May 7, 2024

View reviewed changes

...gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/TransformationRuleEngine.java Outdated Show resolved Hide resolved

jorg3lopez reviewed May 7, 2024

View reviewed changes

shared/src/main/java/gov/hhs/cdc/trustedintermediary/context/ApplicationContext.java Show resolved Hide resolved

jorg3lopez reviewed May 7, 2024

View reviewed changes

shared/src/main/java/gov/hhs/cdc/trustedintermediary/context/ApplicationContext.java Show resolved Hide resolved

jcrichlake reviewed May 8, 2024

View reviewed changes

e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/e2e/DemographicsTest.groovy Outdated Show resolved Hide resolved

jcrichlake reviewed May 8, 2024

View reviewed changes

.../java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/TransformationRule.java Show resolved Hide resolved

basiliskus added 8 commits May 8, 2024 07:34

Using spock's ignore annotation instead of commenting test

14d3e3d

Added test coverage

95d2f96

Added more test coverage

b70f46d

Added remaining test coverage

7bd0a82

Resolved conflicts with main

f818524

Fixed conflicts from merge

64ac1cc

Removed remaining demographics code

abf4efa

Converted HapiMessageConverter singleton into HapiMessageConverterHel…

14d967e

…per class with static methods

basiliskus marked this pull request as ready for review May 8, 2024 19:14

jorg3lopez reviewed May 8, 2024

View reviewed changes

jcrichlake reviewed May 9, 2024

View reviewed changes

luis-pabon-tf approved these changes May 9, 2024

View reviewed changes

halprin merged commit 04693bf into main May 9, 2024
15 of 16 checks passed

halprin deleted the story/1024/transformation-engine-implementation branch May 9, 2024 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transformation rule engine implementation #1046

Transformation rule engine implementation #1046

basiliskus commented Apr 26, 2024 •

edited

Loading

sonarqubecloud bot commented May 8, 2024

jorg3lopez May 8, 2024 •

edited

Loading

basiliskus May 9, 2024 •

edited

Loading

jorg3lopez May 8, 2024

jorg3lopez May 8, 2024 •

edited

Loading

basiliskus May 9, 2024

jorg3lopez May 8, 2024

jorg3lopez May 8, 2024

jorg3lopez May 8, 2024

basiliskus May 9, 2024

jcrichlake May 9, 2024

basiliskus May 9, 2024

luis-pabon-tf commented May 9, 2024

luis-pabon-tf left a comment

Transformation rule engine implementation #1046

Transformation rule engine implementation #1046

Conversation

basiliskus commented Apr 26, 2024 • edited Loading

Transformation rule engine implementation

Issue

Checklist

sonarqubecloud bot commented May 8, 2024

Quality Gate failed

jorg3lopez May 8, 2024 • edited Loading

Choose a reason for hiding this comment

basiliskus May 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorg3lopez May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luis-pabon-tf commented May 9, 2024

luis-pabon-tf left a comment

Choose a reason for hiding this comment

basiliskus commented Apr 26, 2024 •

edited

Loading

jorg3lopez May 8, 2024 •

edited

Loading

basiliskus May 9, 2024 •

edited

Loading

jorg3lopez May 8, 2024 •

edited

Loading