dhis2-devs team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 21567
committer: Lars Helge Overland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Mon 2016-01-04 10:48:25 +0100
message:
  Expression service. Performance improvement for substitution of expressions.
added:
  dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection/
  dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection/CachingMapTest.java
modified:
  dhis-2/dhis-api/src/main/java/org/hisp/dhis/expression/ExpressionService.java
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/expression/DefaultExpressionService.java
  dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/collection/CachingMap.java


--
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/expression/ExpressionService.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/expression/ExpressionService.java	2016-01-04 02:27:49 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/expression/ExpressionService.java	2016-01-04 09:48:25 +0000
@@ -303,12 +303,6 @@
     String getExpressionDescription( String expression );
 
     /**
-     * Substitutes potential constant and days in the numerator and denominator
-     * on all indicators in the given collection.
-     */
-    void substituteExpressions( Collection<Indicator> indicators, Integer days );
-    
-    /**
      * Populates the explodedExpression property on the Expression object of all
      * validation rules in the given collection. This method uses
      * explodeExpression( String ) internally to generate the exploded expressions.
@@ -327,16 +321,12 @@
      * @return the exploded expression string.
      */
     String explodeExpression( String expression );
-    
+
     /**
-     * Substitutes potential constants and days in the given expression.
-     * 
-     * @param expression the expression to operate on.
-     * @param days the number of days to substitute for potential days in the
-     *        expression, 0 if null
-     * @return the substituted expression.
+     * Substitutes potential constant and days in the numerator and denominator
+     * on all indicators in the given collection.
      */
-    String substituteExpression( String expression, Integer days );
+    void substituteExpressions( Collection<Indicator> indicators, Integer days );
     
     /**
      * Generates an expression where the Operand identifiers, consisting of 

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/expression/DefaultExpressionService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/expression/DefaultExpressionService.java	2016-01-04 02:27:49 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/expression/DefaultExpressionService.java	2016-01-04 09:48:25 +0000
@@ -72,6 +72,7 @@
 import org.hisp.dhis.period.Period;
 import org.hisp.dhis.system.util.DateUtils;
 import org.hisp.dhis.system.util.MathUtils;
+import org.hisp.dhis.commons.collection.CachingMap;
 import org.hisp.dhis.commons.util.TextUtils;
 import org.hisp.dhis.validation.ValidationRule;
 import org.springframework.transaction.annotation.Transactional;
@@ -662,20 +663,6 @@
 
     @Override
     @Transactional
-    public void substituteExpressions( Collection<Indicator> indicators, Integer days )
-    {
-        if ( indicators != null && !indicators.isEmpty() )
-        {
-            for ( Indicator indicator : indicators )
-            {
-                indicator.setExplodedNumerator( substituteExpression( indicator.getNumerator(), days ) );
-                indicator.setExplodedDenominator( substituteExpression( indicator.getDenominator(), days ) );
-            }
-        }                
-    }
-    
-    @Override
-    @Transactional
     public void explodeValidationRuleExpressions( Collection<ValidationRule> validationRules )
     {
         if ( validationRules != null && !validationRules.isEmpty() )
@@ -776,7 +763,25 @@
 
     @Override
     @Transactional
-    public String substituteExpression( String expression, Integer days )
+    public void substituteExpressions( Collection<Indicator> indicators, Integer days )
+    {
+        if ( indicators != null && !indicators.isEmpty() )
+        {
+            Map<String, Constant> constants = new CachingMap<String, Constant>().
+                load( idObjectManager.getAllNoAcl( Constant.class ), c -> c.getUid() );
+            
+            Map<String, OrganisationUnitGroup> orgUnitGroups = new CachingMap<String, OrganisationUnitGroup>().
+                load( idObjectManager.getAllNoAcl( OrganisationUnitGroup.class ), g -> g.getUid() );
+            
+            for ( Indicator indicator : indicators )
+            {
+                indicator.setExplodedNumerator( substituteExpression( indicator.getNumerator(), constants, orgUnitGroups, days ) );
+                indicator.setExplodedDenominator( substituteExpression( indicator.getDenominator(), constants, orgUnitGroups, days ) );
+            }
+        }                
+    }
+    
+    private String substituteExpression( String expression, Map<String, Constant> constants, Map<String, OrganisationUnitGroup> orgUnitGroups, Integer days )
     {
         if ( expression == null || expression.isEmpty() )
         {

=== modified file 'dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/collection/CachingMap.java'
--- dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/collection/CachingMap.java	2016-01-04 02:27:49 +0000
+++ dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/collection/CachingMap.java	2016-01-04 09:48:25 +0000
@@ -1,5 +1,7 @@
 package org.hisp.dhis.commons.collection;
 
+import java.util.Collection;
+
 /*
  * Copyright (c) 2004-2016, University of Oslo
  * All rights reserved.
@@ -30,6 +32,7 @@
 
 import java.util.HashMap;
 import java.util.concurrent.Callable;
+import java.util.function.Function;
 
 /**
  * Map which allows storing a {@link java.util.concurrent.Callable}
@@ -69,4 +72,28 @@
         
         return value;
     }
+    
+    /**
+     * Loads the cache with the given content.
+     * 
+     * @param collection the content collection.
+     * @param keyMapper the function to produce the cache key for each content item.
+     * @return a reference to this caching map.
+     */
+    public CachingMap<K, V> load( Collection<V> collection, Function<V, K> keyMapper )
+    {
+        for ( V item : collection )
+        {
+            K key = keyMapper.apply( item );
+            
+            if ( key == null )
+            {
+                throw new IllegalArgumentException( "Key cannot be null for item: " + item );
+            }
+            
+            super.put( key, item );
+        }
+        
+        return this;
+    }
 }

=== added directory 'dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection'
=== added file 'dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection/CachingMapTest.java'
--- dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection/CachingMapTest.java	1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-support/dhis-support-commons/src/test/java/org/hisp/dhis/commons/collection/CachingMapTest.java	2016-01-04 09:48:25 +0000
@@ -0,0 +1,77 @@
+package org.hisp.dhis.commons.collection;
+
+/*
+ * Copyright (c) 2004-2015, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class CachingMapTest
+{
+    @Test
+    public void testLoad()
+    {
+        Set<Animal> animals = new HashSet<>();
+        animals.add( new Animal( 1, "horse" ) );
+        animals.add( new Animal( 2, "dog" ) );
+        animals.add( new Animal( 3, "cat" ) );
+
+        CachingMap<Integer, Animal> cache = new CachingMap<Integer, Animal>().load( animals, a -> a.getId() );
+        
+        assertEquals( "horse", cache.get( 1 ).getName() );
+        assertEquals( "dog", cache.get( 2 ).getName() );
+        assertEquals( "cat", cache.get( 3 ).getName() );        
+        assertFalse( cache.containsKey( "deer" ) );
+    }
+    
+    private class Animal
+    {
+        private int id;
+        private String name;
+        
+        public Animal( int id, String name )
+        {
+            this.id = id;
+            this.name = name;
+        }
+        
+        public int getId()
+        {
+            return id;
+        }
+
+        public String getName()
+        {
+            return name;
+        }
+    }
+}