← Back to team overview

dhis2-devs team mailing list archive

[Branch ~dhis2-devs-core/dhis2/trunk] Rev 2028: Improved peformance in validation rule analysis

 

------------------------------------------------------------
revno: 2028
committer: Lars <larshelg@larshelg-laptop>
branch nick: trunk
timestamp: Wed 2010-09-01 08:14:47 +0200
message:
  Improved peformance in validation rule analysis
modified:
  dhis-2/dhis-api/src/main/java/org/hisp/dhis/source/Source.java
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/validation/DefaultValidationRuleService.java
  dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/validation/ValidationRuleServiceTest.java
  dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/Counter.java
  dhis-2/dhis-web/dhis-web-validationrule/src/main/java/org/hisp/dhis/validationrule/action/RunValidationAction.java
  dhis-2/dhis-web/dhis-web-validationrule/src/main/webapp/dhis-web-validationrule/runValidationForm.vm


--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk

Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/source/Source.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/source/Source.java	2010-04-12 21:23:33 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/source/Source.java	2010-09-01 06:14:47 +0000
@@ -32,6 +32,7 @@
 import java.util.Set;
 
 import org.hisp.dhis.common.IdentifiableObject;
+import org.hisp.dhis.dataelement.DataElement;
 import org.hisp.dhis.dataset.DataSet;
 import org.hisp.dhis.dimension.Dimension;
 import org.hisp.dhis.dimension.DimensionOption;
@@ -51,6 +52,22 @@
     // Dimension
     // -------------------------------------------------------------------------
 
+    public Set<DataElement> getDataElementsInDataSets()
+    {
+        Set<DataElement> dataElements = new HashSet<DataElement>();
+        
+        for ( DataSet dataSet : dataSets )
+        {
+            dataElements.addAll( dataSet.getDataElements() );
+        }
+        
+        return dataElements;
+    }
+    
+    // -------------------------------------------------------------------------
+    // Dimension
+    // -------------------------------------------------------------------------
+
     public static final Dimension DIMENSION = new SourceDimension();
     
     public static class SourceDimension

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/validation/DefaultValidationRuleService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/validation/DefaultValidationRuleService.java	2010-08-31 11:40:23 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/validation/DefaultValidationRuleService.java	2010-09-01 06:14:47 +0000
@@ -31,9 +31,8 @@
 
 import java.util.Collection;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Map;
+import java.util.Set;
 
 import org.hisp.dhis.common.GenericIdentifiableObjectStore;
 import org.hisp.dhis.dataelement.DataElement;
@@ -155,25 +154,17 @@
     {
         Collection<ValidationResult> validationViolations = new HashSet<ValidationResult>();
 
-        Map<DataSet, Collection<ValidationRule>> relevantValidationRulesMap = getRelevantValidationRulesMap( sources );
-                
         Collection<Period> relevantPeriods = periodService.getPeriodsBetweenDates( startDate, endDate );
 
         for ( Source source : sources )
         {
-            for ( DataSet dataSet : source.getDataSets() )
-            {
-                Collection<ValidationRule> relevantRules = relevantValidationRulesMap.get( dataSet );
+            Collection<ValidationRule> relevantRules = getRelevantValidationRules( source.getDataElementsInDataSets() );
                 
-                if ( relevantRules != null && relevantRules.size() > 0 )
-                {                    
-                    for ( Period period : relevantPeriods )
-                    {
-                        if ( dataSet.getPeriodType().equals( period.getPeriodType() ) )
-                        {
-                            validationViolations.addAll( validate( period, source, relevantRules, false ) );
-                        }
-                    }
+            if ( relevantRules != null && relevantRules.size() > 0 )
+            {                    
+                for ( Period period : relevantPeriods )
+                {
+                    validationViolations.addAll( validate( period, source, relevantRules, false ) );
                 }
             }
         }
@@ -186,26 +177,18 @@
     {
         Collection<ValidationResult> validationViolations = new HashSet<ValidationResult>();
 
-        Map<DataSet, Collection<ValidationRule>> relevantValidationRulesMap = getRelevantValidationRulesMap( sources );
-                
         Collection<Period> relevantPeriods = periodService.getPeriodsBetweenDates( startDate, endDate );
 
         for ( Source source : sources )
         {
-            for ( DataSet dataSet : source.getDataSets() )
+            Collection<ValidationRule> relevantRules = getRelevantValidationRules( source.getDataElementsInDataSets() );
+            relevantRules.retainAll( group.getMembers() );
+            
+            if ( relevantRules != null && relevantRules.size() > 0 )
             {
-                Collection<ValidationRule> relevantRules = relevantValidationRulesMap.get( dataSet );
-                relevantRules.retainAll( group.getMembers() );
-                
-                if ( relevantRules != null && relevantRules.size() > 0 )
+                for ( Period period : relevantPeriods )
                 {
-                    for ( Period period : relevantPeriods )
-                    {
-                        if ( dataSet.getPeriodType().equals( period.getPeriodType() ) )
-                        {
-                            validationViolations.addAll( validate( period, source, relevantRules, false ) );
-                        }
-                    }
+                    validationViolations.addAll( validate( period, source, relevantRules, false ) );
                 }
             }
         }
@@ -217,32 +200,21 @@
     {
         Collection<ValidationResult> validationViolations = new HashSet<ValidationResult>();
 
-        Map<DataSet, Collection<ValidationRule>> relevantValidationRulesMap = getRelevantValidationRulesMap( source );
+        Collection<ValidationRule> relevantRules = getRelevantValidationRules( source.getDataElementsInDataSets() );
                 
         Collection<Period> relevantPeriods = periodService.getPeriodsBetweenDates( startDate, endDate );
 
-        for ( DataSet dataSet : source.getDataSets() )
+        for ( Period period : relevantPeriods )
         {
-            Collection<ValidationRule> relevantRules = relevantValidationRulesMap.get( dataSet ); //TODO incorrect, fix
-            
-            if ( relevantRules != null && relevantRules.size() > 0 )
-            {
-                for ( Period period : relevantPeriods )
-                {
-                    if ( dataSet.getPeriodType().equals( period.getPeriodType() ) )
-                    {
-                        validationViolations.addAll( validate( period, source, relevantRules, false ) );
-                    }
-                }
-            }
+            validationViolations.addAll( validate( period, source, relevantRules, false ) );
         }
-
+        
         return validationViolations;
     }
 
     public Collection<ValidationResult> validate( DataSet dataSet, Period period, Source source )
     {
-        return validate( period, source, getRelevantValidationRules( dataSet ), false );
+        return validate( period, source, getRelevantValidationRules( dataSet.getDataElements() ), false );
     }
 
     // -------------------------------------------------------------------------
@@ -269,16 +241,19 @@
 
         for ( final ValidationRule validationRule : validationRules )
         {
-            leftSide = expressionService.getExpressionValue( validationRule.getLeftSide(), period, source, true, aggregate );
-            rightSide = expressionService.getExpressionValue( validationRule.getRightSide(), period, source, true, aggregate );
-
-            if ( leftSide != null && rightSide != null )
+            if ( validationRule.getPeriodType() != null && validationRule.getPeriodType().equals( period.getPeriodType() ) )
             {
-                violation = !expressionIsTrue( leftSide, validationRule.getMathematicalOperator(), rightSide );
-
-                if ( violation )
+                leftSide = expressionService.getExpressionValue( validationRule.getLeftSide(), period, source, true, aggregate );
+                rightSide = expressionService.getExpressionValue( validationRule.getRightSide(), period, source, true, aggregate );
+    
+                if ( leftSide != null && rightSide != null )
                 {
-                    validationResults.add( new ValidationResult( period, source, validationRule, leftSide, rightSide ) );
+                    violation = !expressionIsTrue( leftSide, validationRule.getMathematicalOperator(), rightSide );
+    
+                    if ( violation )
+                    {
+                        validationResults.add( new ValidationResult( period, source, validationRule, leftSide, rightSide ) );
+                    }
                 }
             }
         }
@@ -287,53 +262,6 @@
     }
 
     /**
-     * Creates a mapping between data set and its relevant validation rules for
-     * the given source.
-     * 
-     * @param source the source.
-     * @return a map.
-     */
-    private Map<DataSet, Collection<ValidationRule>> getRelevantValidationRulesMap( Source source )
-    {
-        Map<DataSet, Collection<ValidationRule>> map = new HashMap<DataSet, Collection<ValidationRule>>();
-        
-        for ( DataSet dataSet : source.getDataSets() )
-        {
-            if ( !map.keySet().contains( dataSet ) )
-            {
-                map.put( dataSet, getRelevantValidationRules( dataSet ) );
-            }
-        }
-        
-        return map;
-    }
-    
-    /**
-     * Creates a mapping between data set and its relevant validation rules for
-     * the given collection of sources.
-     * 
-     * @param sources the collection of sources.
-     * @return a map.
-     */
-    private Map<DataSet, Collection<ValidationRule>> getRelevantValidationRulesMap( Collection<? extends Source> sources )
-    {
-        Map<DataSet, Collection<ValidationRule>> map = new HashMap<DataSet, Collection<ValidationRule>>();
-        
-        for ( Source source : sources )
-        {
-            for ( DataSet dataSet : source.getDataSets() )
-            {
-                if ( !map.keySet().contains( dataSet ) )
-                {
-                    map.put( dataSet, getRelevantValidationRules( dataSet ) );
-                }
-            }
-        }
-        
-        return map;
-    }
-    
-    /**
      * Returns all validation rules which have data elements assigned to it
      * which are members of the given data set.
      * 
@@ -341,15 +269,15 @@
      * @return all validation rules which have data elements assigned to it
      *         which are members of the given data set.
      */
-    private Collection<ValidationRule> getRelevantValidationRules( final DataSet dataSet )
+    private Collection<ValidationRule> getRelevantValidationRules( Collection<DataElement> dataElements )
     {
-        final Collection<ValidationRule> relevantValidationRules = new HashSet<ValidationRule>();
+        final Set<ValidationRule> relevantValidationRules = new HashSet<ValidationRule>();
 
         for ( ValidationRule validationRule : getAllValidationRules() )
         {
-            if ( validationRule.getPeriodType() == dataSet.getPeriodType() )
+            for ( DataElement dataElement : dataElements )
             {
-                for ( DataElement dataElement : dataSet.getDataElements() )
+                if ( validationRule.getPeriodType() != null && dataElement.getPeriodType() != null && validationRule.getPeriodType().equals( dataElement.getPeriodType() ) )
                 {
                     if ( validationRule.getLeftSide().getDataElementsInExpression().contains( dataElement )
                         || validationRule.getRightSide().getDataElementsInExpression().contains( dataElement ) )

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/validation/ValidationRuleServiceTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/validation/ValidationRuleServiceTest.java	2010-05-28 19:17:07 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/validation/ValidationRuleServiceTest.java	2010-09-01 06:14:47 +0000
@@ -50,6 +50,7 @@
 import org.hisp.dhis.expression.Expression;
 import org.hisp.dhis.expression.ExpressionService;
 import org.hisp.dhis.mock.MockSource;
+import org.hisp.dhis.period.MonthlyPeriodType;
 import org.hisp.dhis.period.Period;
 import org.hisp.dhis.period.PeriodService;
 import org.hisp.dhis.period.PeriodType;
@@ -149,7 +150,7 @@
 
         periodService = (PeriodService) getBean( PeriodService.ID );
 
-        periodType = PeriodType.getAvailablePeriodTypes().iterator().next();
+        periodType = new MonthlyPeriodType();
 
         dataElementA = createDataElement( 'A' );
         dataElementB = createDataElement( 'B' );
@@ -209,8 +210,18 @@
         dataSet.getSources().add( sourceA );
         dataSet.getSources().add( sourceB );
 
+        dataElementA.getDataSets().add( dataSet );
+        dataElementB.getDataSets().add( dataSet );
+        dataElementC.getDataSets().add( dataSet );
+        dataElementD.getDataSets().add( dataSet );
+        
         dataSetService.addDataSet( dataSet );
-
+        
+        dataElementService.updateDataElement( dataElementA );
+        dataElementService.updateDataElement( dataElementB );
+        dataElementService.updateDataElement( dataElementC );
+        dataElementService.updateDataElement( dataElementD );
+        
         validationRuleA = createValidationRule( 'A', ValidationRule.OPERATOR_EQUAL, expressionA, expressionB,
             periodType );
         validationRuleB = createValidationRule( 'B', ValidationRule.OPERATOR_GREATER, expressionB, expressionC,
@@ -233,6 +244,10 @@
     // Business logic tests
     // ----------------------------------------------------------------------
 
+    public void testValidateAggregatedDateDateSources()
+    {   
+    }
+    
     @Test
     public void testValidateDateDateSources()
     {

=== modified file 'dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/Counter.java'
--- dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/Counter.java	2010-04-12 21:23:33 +0000
+++ dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/Counter.java	2010-09-01 06:14:47 +0000
@@ -33,16 +33,16 @@
 /**
  * @author Lars Helge Overland
  */
-public class Counter
+public class Counter<T>
 {
-    private Map<Integer, Integer> map;
+    private Map<T, Integer> map;
     
     public Counter()
     {
-        map = new HashMap<Integer, Integer>();
+        map = new HashMap<T, Integer>();
     }
     
-    public int count( int key )
+    public int count( T key )
     {
         if ( !map.containsKey( key ) )
         {
@@ -55,4 +55,14 @@
         
         return count;
     }
+    
+    public Map<T, Integer> getCount()
+    {
+        return map;
+    }
+    
+    public Integer getCount( T key )
+    {
+        return map != null ? map.get( key ) : null;
+    }
 }

=== modified file 'dhis-2/dhis-web/dhis-web-validationrule/src/main/java/org/hisp/dhis/validationrule/action/RunValidationAction.java'
--- dhis-2/dhis-web/dhis-web-validationrule/src/main/java/org/hisp/dhis/validationrule/action/RunValidationAction.java	2010-08-31 11:40:23 +0000
+++ dhis-2/dhis-web/dhis-web-validationrule/src/main/java/org/hisp/dhis/validationrule/action/RunValidationAction.java	2010-09-01 06:14:47 +0000
@@ -134,6 +134,11 @@
     
     private boolean aggregate;
 
+    public boolean isAggregate()
+    {
+        return aggregate;
+    }
+
     public void setAggregate( boolean aggregate )
     {
         this.aggregate = aggregate;

=== modified file 'dhis-2/dhis-web/dhis-web-validationrule/src/main/webapp/dhis-web-validationrule/runValidationForm.vm'
--- dhis-2/dhis-web/dhis-web-validationrule/src/main/webapp/dhis-web-validationrule/runValidationForm.vm	2010-08-31 11:40:23 +0000
+++ dhis-2/dhis-web/dhis-web-validationrule/src/main/webapp/dhis-web-validationrule/runValidationForm.vm	2010-09-01 06:14:47 +0000
@@ -45,7 +45,7 @@
     	</td>
     </tr>
     <tr>
-    	<td colspan="2"><span id="info">$i18n.getString( "aggregate_data_info" )</span></td>
+    	<td colspan="2"><span id="info">$i18n.getString( "captured_data_info" )</span></td>
     </tr>
     <tr>
         <td colspan="2">