dhis2-devs team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 20328
committer: Lars Helge Overland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Wed 2015-09-23 20:53:55 +0200
message:
  Org unit query, simplification and performance improvement using hierarchyLevel property
modified:
  dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnit.java
  dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitQueryParams.java
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/hibernate/HibernateOrganisationUnitStore.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/organisationunit/OrganisationUnit.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnit.java	2015-09-23 18:19:16 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnit.java	2015-09-23 18:53:55 +0000
@@ -850,8 +850,7 @@
             }
             else
             {
-                // we have tests in the system which needs cyclic OU graphs, so we need to short-circuit here if we encounter that
-                currentParent = null;
+                currentParent = null; // Protect against cyclic org unit graphs
             }
         }
 
@@ -876,16 +875,21 @@
      */
     public Integer getHierarchyLevel()
     {
-        int count = 1;
+        Set<String> uids = Sets.newHashSet( uid );
         
         OrganisationUnit current = this;
         
         while ( ( current = current.getParent() ) != null )
         {
-            count++;
+            boolean add = uids.add( current.getUid() );
+            
+            if ( !add )
+            {
+                break; // Protect against cyclic org unit graphs
+            }
         }
         
-        hierarchyLevel = count;
+        hierarchyLevel = uids.size();
         
         return hierarchyLevel;
     }

=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitQueryParams.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitQueryParams.java	2015-09-22 11:29:52 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitQueryParams.java	2015-09-23 18:53:55 +0000
@@ -31,17 +31,13 @@
 import java.util.HashSet;
 import java.util.Set;
 
-import org.hisp.dhis.common.CodeGenerator;
-
 import com.google.common.base.MoreObjects;
 
 /**
  * @author Lars Helge Overland
  */
 public class OrganisationUnitQueryParams
-{
-    public static final int CODE_SEP_LENGTH = CodeGenerator.CODESIZE + 1;
-    
+{    
     /**
      * Query string to match like name and exactly on UID and code.
      */
@@ -104,17 +100,7 @@
     {
         return levels != null && !levels.isEmpty();
     }
-    
-    public int getMaxLevelsPathLength()
-    {
-        return maxLevels != null ? ( CODE_SEP_LENGTH * maxLevels ) : -1;
-    }
-    
-    public int getLevelPathLength( int level )
-    {
-        return CODE_SEP_LENGTH * level;
-    }
-    
+        
     public void setLevel( Integer level )
     {
         if ( level != null )

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/hibernate/HibernateOrganisationUnitStore.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/hibernate/HibernateOrganisationUnitStore.java	2015-09-23 18:19:16 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/hibernate/HibernateOrganisationUnitStore.java	2015-09-23 18:53:55 +0000
@@ -185,19 +185,12 @@
         
         if ( params.hasLevels() )
         {
-            hql += hlp.whereAnd() + " (";
-            
-            for ( Integer level : params.getLevels() )
-            {
-                hql += " length(o.path) = :levelPathLength" + level + " or ";
-            }
-            
-            hql = TextUtils.removeLastOr( hql ) + ") ";
+            hql += hlp.whereAnd() + " o.hierarchyLevel in (:levels) ";
         }
         
         if ( params.getMaxLevels() != null )
         {
-            hql += hlp.whereAnd() + " length(o.path) <= :pathLength ";
+            hql += hlp.whereAnd() + " o.hierarchyLevel <= :maxLevels ";
         }
         
         hql += "order by o.name";
@@ -228,15 +221,12 @@
         
         if ( params.hasLevels() )
         {
-            for ( Integer level : params.getLevels() )
-            {
-                query.setInteger( "levelPathLength" + level, params.getLevelPathLength( level ) );
-            }
+            query.setParameterList( "levels", params.getLevels() );
         }
         
         if ( params.getMaxLevels() != null )
         {
-            query.setInteger( "pathLength", params.getMaxLevelsPathLength() );
+            query.setInteger( "maxLevels", params.getMaxLevels() );
         }
 
         if ( params.getFirst() != null )
@@ -360,17 +350,10 @@
     @Override
     public int getMaxLevel()
     {
-        String hql = "select max(length(ou.path)) from OrganisationUnit ou";
+        String hql = "select max(ou.hierarchyLevel) from OrganisationUnit ou";
 
         Integer maxLength = (Integer) getQuery( hql ).uniqueResult();
         
-        if ( maxLength != null )
-        {        
-            int level = maxLength / OrganisationUnitQueryParams.CODE_SEP_LENGTH;
-        
-            return level;
-        }
-        
-        return 0;
+        return maxLength != null ? maxLength.intValue() : 0;
     }
 }