Rules engine refactoring #1037

basiliskus · 2024-04-23T23:11:01Z

Rules engine refactoring

Refactoring to make validation rule engine code more generic and reusable for the transformation rule engine

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

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java

Added dummy definitions from design discussion

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java

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

Update tests to work with refactoring changes

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

…class

...src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java

Added test-case for base Rule class exclusive implementation.

sonarqubecloud · 2024-04-30T00:09:47Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

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

jcrichlake · 2024-04-30T19:02:59Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java

@@ -133,7 +133,9 @@ public Map<HttpEndpoint, Function<DomainRequest, DomainResponse>> domainRegistra
                PartnerMetadataConverter.class, HapiPartnerMetadataConverter.getInstance());
        // Validation rules
        ApplicationContext.register(RuleLoader.class, RuleLoader.getInstance());
-        ApplicationContext.register(RuleEngine.class, RuleEngine.getInstance());
+        ApplicationContext.register(
+                ValidationRuleEngine.class,


The first value in register should be an interface and the second should be the class that is defining the behavior for an interface

As commented above, we need to be able to identify two different implementations of the rule engine

Thoughts on doing a parent interface (RuleEngine), and two child interfaces that will extend the parent interface (ValidationRuleEngine, TransformationRuleEngine)? With this setup, you can use the child interface to register the implementation for each (ValidationRuleEngineImp, TransformationRuleEngineImp). The only challenge here is the naming for the parent interface, the child interfaces, and the implementations for each child interface. You register both engines with a unique interface for each.

I don't think the value we register in the ApplicationContext needs to be an interface, a lot of the existing ones are not. Is there a benefit in creating the interface even if it will have only one implementation?

I think we generally wrap 3rd party dependencies in an interface to reduce dependencies

jcrichlake · 2024-04-30T19:09:00Z

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java


    /**
-     * Descriptive message when there's a rule violation Note: When implementing this method, make
-     * sure that no PII or PHI is included in the message!
+     * Do not delete this constructor! It is used for JSON deserialization when loading rules from a


Right, Jackson requires an empty constructor to de-serialize things, don't have to add this but something to think about

etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy

basiliskus added 3 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

luis-pabon-tf reviewed Apr 25, 2024

View reviewed changes

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java Outdated Show resolved Hide resolved

Refactored to make Rule interface more generic

bdd568a

github-advanced-security bot found potential problems Apr 25, 2024

View reviewed changes

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java Fixed Show fixed Hide fixed

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java Fixed Show fixed Hide fixed

basiliskus and others added 6 commits April 25, 2024 11:51

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

github-advanced-security bot found potential problems Apr 25, 2024

View reviewed changes

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java Fixed Show fixed Hide fixed

Converted RuleEngine back into a singleton and added parameters

1de28ef

github-advanced-security bot found potential problems Apr 26, 2024

View reviewed changes

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java Fixed Show fixed Hide fixed

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java Fixed Show fixed Hide fixed

Refactored to have a separate implementation for ValidationRuleEngine

5c8073b

github-advanced-security bot found potential problems Apr 26, 2024

View reviewed changes

basiliskus added 4 commits April 26, 2024 13:36

Refactored to pass TypeReference and fix mapping issue

45a3bca

Added missing override tags

d5490e8

Fixed tests in ValidationRuleEngineIntegrationTest

8e7398c

Fixed more tests

dbd8043

basiliskus changed the title ~~Transformation rules engine implementation~~ Rules engine refactoring Apr 26, 2024

basiliskus and others added 8 commits April 26, 2024 14:56

Removed transformation engine implementation to move into new PR

d399054

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

basiliskus and others added 7 commits April 29, 2024 10:57

Fixed remaining test

34a2bf9

Fixed actual remaining test

e137c1d

Changed naming

0fc78c5

Update ValidationRuleEngineTest.groovy

95d4b2d

Update tests to work with refactoring changes

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

a27a876

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

Added test coverage

c0f0093

Updated javadocs

2a5f7ab

basiliskus marked this pull request as ready for review April 29, 2024 18:35

basiliskus added 3 commits April 29, 2024 12:58

Created trasnformation package and moved transformation engine files

4a5068d

Changed implementation for rules implementation to inherit from Rule …

2bd21c8

…class

Clean up

6ba1943

github-advanced-security bot found potential problems Apr 29, 2024

View reviewed changes

...src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java Fixed Show fixed Hide fixed

basiliskus and others added 2 commits April 29, 2024 16:28

Added missing override tag

a03dd0b

Create RuleTest.groovy

e9290aa

Added test-case for base Rule class exclusive implementation.

jcrichlake reviewed Apr 30, 2024

View reviewed changes

etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java Show resolved Hide resolved

jcrichlake reviewed Apr 30, 2024

View reviewed changes

etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy Show resolved Hide resolved

jcrichlake approved these changes May 3, 2024

View reviewed changes

basiliskus merged commit 32abaaf into main May 3, 2024
16 checks passed

basiliskus deleted the story/1024/transformation-engine branch May 3, 2024 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rules engine refactoring #1037

Rules engine refactoring #1037

basiliskus commented Apr 23, 2024 •

edited

Loading

sonarqubecloud bot commented Apr 30, 2024

jcrichlake Apr 30, 2024

basiliskus Apr 30, 2024

jorg3lopez May 1, 2024 •

edited

Loading

basiliskus May 1, 2024

jcrichlake May 3, 2024

jcrichlake Apr 30, 2024

Rules engine refactoring #1037

Rules engine refactoring #1037

Conversation

basiliskus commented Apr 23, 2024 • edited Loading

Rules engine refactoring

Issue

Checklist

sonarqubecloud bot commented Apr 30, 2024

Quality Gate passed

jcrichlake Apr 30, 2024

Choose a reason for hiding this comment

basiliskus Apr 30, 2024

Choose a reason for hiding this comment

jorg3lopez May 1, 2024 • edited Loading

Choose a reason for hiding this comment

basiliskus May 1, 2024

Choose a reason for hiding this comment

jcrichlake May 3, 2024

Choose a reason for hiding this comment

jcrichlake Apr 30, 2024

Choose a reason for hiding this comment

basiliskus commented Apr 23, 2024 •

edited

Loading

jorg3lopez May 1, 2024 •

edited

Loading