chore: prepare rule engine to work fine with new tracker importer (#48)

dhis2 · Jul 13, 2020 · ddab487 · ddab487
1 parent e7b0475
commit ddab487
Show file tree

Hide file tree

Showing 16 changed files with 11 additions and 120 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -45,4 +45,4 @@ jobs:
           nexus_username: ${{ secrets.nexus_username }}
           nexus_password: ${{ secrets.nexus_password }}
         env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/pom.xml b/pom.xml
@@ -6,7 +6,7 @@
 
     <groupId>org.hisp.dhis.rules</groupId>
     <artifactId>rule-engine</artifactId>
-    <version>2.0.4-SNAPSHOT</version>
+    <version>2.0.5-SNAPSHOT</version>
     <packaging>jar</packaging>
     <name>rule-engine</name>
 
@@ -246,7 +246,7 @@
         <dependency>
             <groupId>org.hisp.dhis.parser</groupId>
             <artifactId>dhis-antlr-expression-parser</artifactId>
-            <version>1.0.7-SNAPSHOT</version>
+            <version>1.0.7</version>
         </dependency>
     </dependencies>
 

diff --git a/src/main/java/org/hisp/dhis/rules/RuleEngine.java b/src/main/java/org/hisp/dhis/rules/RuleEngine.java
@@ -83,7 +83,6 @@ public Callable<List<RuleEffect>> evaluate( @Nonnull RuleEvent ruleEvent, @Nonnu
             .ruleEnrollment( ruleEnrollment )
             .triggerEnvironment( triggerEnvironment )
             .ruleEvents( ruleEvents )
-            .calculatedValueMap( ruleEngineContext.calculatedValueMap() )
             .constantValueMap( ruleEngineContext.constantsValues() )
             .build();
 
@@ -103,7 +102,6 @@ public Callable<List<RuleEffect>> evaluate( @Nonnull RuleEnrollment ruleEnrollme
             .ruleVariables( ruleEngineContext.ruleVariables() )
             .triggerEnvironment( triggerEnvironment )
             .ruleEvents( ruleEvents )
-            .calculatedValueMap( ruleEngineContext.calculatedValueMap() )
             .constantValueMap( ruleEngineContext.constantsValues() )
             .build();
 

diff --git a/src/main/java/org/hisp/dhis/rules/RuleEngineContext.java b/src/main/java/org/hisp/dhis/rules/RuleEngineContext.java
@@ -22,20 +22,15 @@ public final class RuleEngineContext
     @Nonnull
     private final Map<String, List<String>> supplementaryData;
 
-    @Nonnull
-    private final Map<String, Map<String, String>> calculatedValueMap;
-
     @Nonnull
     private final Map<String, String> constantsValues;
 
     RuleEngineContext( @Nonnull List<Rule> rules, @Nonnull List<RuleVariable> ruleVariables,
-        Map<String, List<String>> supplementaryData,
-        Map<String, Map<String, String>> calculatedValueMap, Map<String, String> constantsValues )
+        Map<String, List<String>> supplementaryData, Map<String, String> constantsValues )
     {
         this.rules = rules;
         this.ruleVariables = ruleVariables;
         this.supplementaryData = supplementaryData;
-        this.calculatedValueMap = calculatedValueMap;
         this.constantsValues = constantsValues;
     }
 
@@ -70,12 +65,6 @@ public Map<String, List<String>> supplementaryData()
         return supplementaryData;
     }
 
-    @Nonnull
-    public Map<String, Map<String, String>> calculatedValueMap()
-    {
-        return calculatedValueMap;
-    }
-
     @Nonnull
     public Map<String, String> constantsValues()
     {
@@ -100,9 +89,6 @@ public static class Builder
         @Nullable
         private Map<String, List<String>> supplementaryData;
 
-        @Nullable
-        private Map<String, Map<String, String>> calculatedValueMap;
-
         @Nullable
         private Map<String, String> constantsValues;
 
@@ -149,14 +135,9 @@ public Builder supplementaryData( Map<String, List<String>> supplementaryData )
             return this;
         }
 
-        @Nonnull
+        @Deprecated
         public Builder calculatedValueMap( Map<String, Map<String, String>> calculatedValueMap )
         {
-            if ( calculatedValueMap == null )
-            {
-                throw new IllegalArgumentException( "calculatedValueMap == null" );
-            }
-            this.calculatedValueMap = calculatedValueMap;
             return this;
         }
 
@@ -184,8 +165,7 @@ public RuleEngineContext build()
                 ruleVariables = unmodifiableList( new ArrayList<RuleVariable>() );
             }
 
-            return new RuleEngineContext( rules, ruleVariables, supplementaryData, calculatedValueMap,
-                constantsValues );
+            return new RuleEngineContext( rules, ruleVariables, supplementaryData, constantsValues );
         }
     }
 }
diff --git a/src/main/java/org/hisp/dhis/rules/RuleEngineExecution.java b/src/main/java/org/hisp/dhis/rules/RuleEngineExecution.java
@@ -160,6 +160,7 @@ private String process( String condition )
         {
             return "";
         }
+
         CommonExpressionVisitor commonExpressionVisitor = CommonExpressionVisitor.newBuilder()
             .withFunctionMap( FUNCTIONS )
             .withVariablesMap( valueMap )

diff --git a/src/main/java/org/hisp/dhis/rules/RuleVariableValueMapBuilder.java b/src/main/java/org/hisp/dhis/rules/RuleVariableValueMapBuilder.java
@@ -54,9 +54,6 @@ public final class RuleVariableValueMapBuilder
     @Nonnull
     public final SimpleDateFormat dateFormat;
 
-    @Nonnull
-    public final Map<String, Map<String, String>> calculatedValueMap;
-
     @Nonnull
     private final Map<String, String> allConstantValues;
 
@@ -82,7 +79,6 @@ private RuleVariableValueMapBuilder()
         // collections used for construction of resulting variable value map
         this.ruleVariables = new ArrayList<>();
         this.ruleEvents = new ArrayList<>();
-        this.calculatedValueMap = new HashMap<>();
         this.allConstantValues = new HashMap<>();
     }
 
@@ -159,13 +155,6 @@ RuleVariableValueMapBuilder ruleEvents( @Nonnull List<RuleEvent> ruleEvents )
         return this;
     }
 
-    @Nonnull
-    RuleVariableValueMapBuilder calculatedValueMap( @Nonnull Map<String, Map<String, String>> calculatedValueMap )
-    {
-        this.calculatedValueMap.putAll( calculatedValueMap );
-        return this;
-    }
-
     @Nonnull
     RuleVariableValueMapBuilder constantValueMap( @Nonnull Map<String, String> constantValues )
     {

diff --git a/src/main/java/org/hisp/dhis/rules/models/RuleActionHideOptionGroup.java b/src/main/java/org/hisp/dhis/rules/models/RuleActionHideOptionGroup.java
@@ -12,7 +12,7 @@ public abstract class RuleActionHideOptionGroup
 
     @Nonnull
     public static RuleActionHideOptionGroup create(
-        @Nullable String content, @Nonnull String optionGroup, @Nonnull String field )
+        @Nullable String content, @Nonnull String optionGroup, @Nonnull String field  )
     {
         return new AutoValue_RuleActionHideOptionGroup( "", content == null ? "" : content, optionGroup, field );
     }

diff --git a/src/main/java/org/hisp/dhis/rules/models/RuleActionMessage.java b/src/main/java/org/hisp/dhis/rules/models/RuleActionMessage.java
@@ -2,7 +2,7 @@
 
 import javax.annotation.Nonnull;
 
-abstract class RuleActionMessage
+public abstract class RuleActionMessage
     extends RuleAction
 {
 

diff --git a/src/main/java/org/hisp/dhis/rules/models/RuleVariableCalculatedValue.java b/src/main/java/org/hisp/dhis/rules/models/RuleVariableCalculatedValue.java
@@ -69,32 +69,8 @@ public Map<String, RuleVariableValue> createValues( RuleVariableValueMapBuilder
         Map<String, RuleDataValue> currentEventValues )
     {
         Map<String, RuleVariableValue> valueMap = Maps.newHashMap();
-        if ( builder.ruleEnrollment == null )
-        {
-            return valueMap;
-        }
 
-        RuleVariableValue variableValue;
-        if ( builder.calculatedValueMap.containsKey( builder.ruleEnrollment.enrollment() ) )
-        {
-            if ( builder.calculatedValueMap.get( builder.ruleEnrollment.enrollment() ).containsKey( this.name() ) )
-            {
-                String value = builder.calculatedValueMap.get( builder.ruleEnrollment.enrollment() ).get( this.name() );
-
-                variableValue = RuleVariableValue.create( value, this.calculatedValueType(),
-                    Arrays.asList( value ), dateFormat.format( new Date() ) );
-            }
-            else
-            {
-                variableValue = RuleVariableValue.create( this.calculatedValueType() );
-            }
-        }
-        else
-        {
-            variableValue = RuleVariableValue.create( this.calculatedValueType() );
-        }
-
-        valueMap.put( this.name(), variableValue );
+        valueMap.put( this.name(), RuleVariableValue.create( this.calculatedValueType() ) );
         return valueMap;
     }
 }
diff --git a/src/test/java/org/hisp/dhis/rules/ConstantsValueTests.java b/src/test/java/org/hisp/dhis/rules/ConstantsValueTests.java
@@ -54,7 +54,6 @@ public void shouldThrowExceptionIfConstantsValueMapIsNull()
             .rules( Arrays.asList( mock( org.hisp.dhis.rules.models.Rule.class ) ) )
             .ruleVariables( Arrays.asList( mock( RuleVariable.class ) ) )
             .supplementaryData( new HashMap<String, List<String>>() )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .constantsValue( null )
             .build();
 
@@ -202,7 +201,6 @@ private RuleEngine.Builder getRuleEngine( List<Rule> rules,
             .builder()
             .rules( rules )
             .ruleVariables( Arrays.<RuleVariable>asList() )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( constantsValueMap )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );

diff --git a/src/test/java/org/hisp/dhis/rules/ProgramRuleVariableTest.java b/src/test/java/org/hisp/dhis/rules/ProgramRuleVariableTest.java
@@ -348,7 +348,6 @@ private RuleEngine.Builder getRuleEngine( List<Rule> rules )
             .builder()
             .rules( rules )
             .ruleVariables( Arrays.<RuleVariable>asList() )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );

diff --git a/src/test/java/org/hisp/dhis/rules/RuleEngineEffectTypesTests.java b/src/test/java/org/hisp/dhis/rules/RuleEngineEffectTypesTests.java
@@ -132,7 +132,6 @@ public void simpleConditionMustResultInHideFieldEffect()
         RuleEngine ruleEngine = RuleEngineContext
             .builder()
             .rules( Arrays.asList( rule, rule2 ) )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )
@@ -331,7 +330,6 @@ private RuleEngine getRuleEngine( Rule rule )
         return RuleEngineContext
             .builder()
             .rules( Arrays.asList( rule ) )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )

diff --git a/src/test/java/org/hisp/dhis/rules/RuleEngineFunctionTests.java b/src/test/java/org/hisp/dhis/rules/RuleEngineFunctionTests.java
@@ -187,7 +187,6 @@ public void evaluateD2InOrgUnitGroup()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( Arrays.asList( ruleVariableOne ) )
             .supplementaryData( supplementaryData )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )
             .build();
@@ -226,7 +225,6 @@ public void evaluateD2InOrgUnitGroupWithStringValue()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( Arrays.asList( ruleVariableOne ) )
             .supplementaryData( supplementaryData )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )
             .build();
@@ -264,7 +262,6 @@ public void evaluateD2HasUserRole()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( Arrays.asList( ruleVariableOne ) )
             .supplementaryData( supplementaryData )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )
             .build();
@@ -302,7 +299,6 @@ public void evaluateD2HasUserRoleWithStringValue()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( Arrays.asList( ruleVariableOne ) )
             .supplementaryData( supplementaryData )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )
             .build();
@@ -1346,7 +1342,6 @@ private RuleEngine.Builder getRuleEngineBuilder( Rule rule, List<RuleVariable> r
             .builder()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( ruleVariables )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );

diff --git a/src/test/java/org/hisp/dhis/rules/RuleEngineValueTypesTests.java b/src/test/java/org/hisp/dhis/rules/RuleEngineValueTypesTests.java
@@ -86,7 +86,6 @@ private RuleEngine getRuleEngine( Rule rule, List<RuleVariable> ruleVariables )
             .builder()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( ruleVariables )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER )

diff --git a/src/test/java/org/hisp/dhis/rules/RuleEngineVariableNameTests.java b/src/test/java/org/hisp/dhis/rules/RuleEngineVariableNameTests.java
@@ -545,7 +545,6 @@ private RuleEngine.Builder getRuleEngineBuilder( Rule rule, List<RuleVariable> r
             .builder()
             .rules( Arrays.asList( rule ) )
             .ruleVariables( ruleVariables )
-            .calculatedValueMap( new HashMap<String, Map<String, String>>() )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );

diff --git a/src/test/java/org/hisp/dhis/rules/models/CalculatedValueTests.java b/src/test/java/org/hisp/dhis/rules/models/CalculatedValueTests.java
@@ -54,17 +54,6 @@
 @RunWith( JUnit4.class )
 public class CalculatedValueTests
 {
-    @Test( expected = IllegalArgumentException.class )
-    public void shouldThrowExceptionIfCalculatedValueMapIsNull()
-    {
-        RuleEngineContext.builder()
-            .ruleVariables( Arrays.asList( mock( RuleVariable.class ) ) )
-            .supplementaryData( new HashMap<String, List<String>>() )
-            .calculatedValueMap( null )
-            .rules( Arrays.asList( mock( org.hisp.dhis.rules.models.Rule.class ) ) )
-            .build();
-    }
-
     @Test
     public void evaluateTenThousandRulesTest()
         throws Exception
@@ -138,23 +127,11 @@ public void sendMessageMustGetValueFromAssignAction()
                 new Date(), "test_program_stage", "test_data_element", "test_value" ) ) )
             .build();
 
-        Map<String, Map<String, String>> calculatedValueMap = new HashMap<>();
-        Map<String, String> valueMap = new HashMap<>();
-        valueMap.put( "test_calculated_value", "4" );
-        calculatedValueMap.put( enrollment.enrollment(), valueMap );
-
-        RuleEngine.Builder ruleEngineBuilder = getRuleEngine( Lists.newArrayList( rule2 ), calculatedValueMap );
-        RuleEngine ruleEngine = ruleEngineBuilder.enrollment( enrollment ).build();
+        RuleEngine ruleEngine = getRuleEngine( Arrays.asList( rule, rule2 ) ).enrollment( enrollment ).build();
         List<RuleEffect> ruleEffects = ruleEngine.evaluate( ruleEvent ).call();
 
         assertThat( ruleEffects.get( 0 ).data() ).isEqualTo( "4.0" );
         assertThat( ruleEffects.get( 0 ).ruleAction() ).isEqualTo( sendMessageAction );
-
-        RuleEngine ruleEngine2 = getRuleEngine( Arrays.asList( rule, rule2 ) ).enrollment( enrollment ).build();
-        List<RuleEffect> ruleEffects2 = ruleEngine2.evaluate( ruleEvent ).call();
-
-        assertThat( ruleEffects2.get( 0 ).data() ).isEqualTo( "4.0" );
-        assertThat( ruleEffects2.get( 0 ).ruleAction() ).isEqualTo( sendMessageAction );
     }
 
     private List<org.hisp.dhis.rules.models.Rule> createRules( int i )
@@ -224,23 +201,6 @@ public void sendMessageMustGetValueFromAssignActionInSingleExecution()
     }
 
     private RuleEngine.Builder getRuleEngine( List<org.hisp.dhis.rules.models.Rule> rules )
-    {
-        RuleVariable ruleVariable = RuleVariableCalculatedValue
-            .create( "test_calculated_value", "", RuleValueType.TEXT );
-
-        Map<String, Map<String, String>> calculatedValueMap = new HashMap<>();
-
-        return RuleEngineContext
-            .builder()
-            .rules( rules )
-            .ruleVariables( Arrays.asList( ruleVariable ) )
-            .calculatedValueMap( calculatedValueMap )
-            .supplementaryData( new HashMap<String, List<String>>() )
-            .constantsValue( new HashMap<String, String>() )
-            .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );
-    }
-
-    private RuleEngine.Builder getRuleEngine( List<org.hisp.dhis.rules.models.Rule> rules, Map<String, Map<String, String>> calculatedValueMap )
     {
         RuleVariable ruleVariable = RuleVariableCalculatedValue
             .create( "test_calculated_value", "", RuleValueType.TEXT );
@@ -249,7 +209,6 @@ private RuleEngine.Builder getRuleEngine( List<org.hisp.dhis.rules.models.Rule>
             .builder()
             .rules( rules )
             .ruleVariables( Arrays.asList( ruleVariable ) )
-            .calculatedValueMap( calculatedValueMap )
             .supplementaryData( new HashMap<String, List<String>>() )
             .constantsValue( new HashMap<String, String>() )
             .build().toEngineBuilder().triggerEnvironment( TriggerEnvironment.SERVER );