fix: program rule variable name check now also check UID to prevent d…

…uplicates [DHIS2-11330] (#8220) (#8366) * fix: program rule variable name check should be now properly implemented * fix: changed check implementation to tolerate bad database data * fix: added test case * style: formatting * fix: compilation issue with JDK 9 * style: formatting (cherry picked from commit c525517)
dhis2 · Jul 12, 2021 · c2d5c77 · c2d5c77
1 parent bb6802f
commit c2d5c77
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 10 deletions.
diff --git a/...a/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHook.java b/...a/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHook.java
@@ -30,13 +30,16 @@
 import static org.hisp.dhis.dxf2.Constants.PROGRAM_RULE_VARIABLE_NAME_INVALID_KEYWORDS;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.hibernate.Session;
 import org.hibernate.query.Query;
 import org.hisp.dhis.common.IdentifiableObject;
@@ -109,18 +112,45 @@ public <T extends IdentifiableObject> List<ErrorReport> validate( T object, Obje
     private List<ErrorReport> validateUniqueProgramRuleName( ObjectBundle bundle,
         ProgramRuleVariable programRuleVariable )
     {
-        Query<ProgramRuleVariable> query = getProgramRuleVariableQuery( programRuleVariable );
+        List<ProgramRuleVariable> prvWithSameNameAndSameProgram = getProgramRuleVariableQuery( programRuleVariable )
+            .getResultList();
 
-        int allowedCount = UPDATE_STRATEGIES.contains( bundle.getImportMode() ) ? 1 : 0;
+        // update
+        if ( UPDATE_STRATEGIES.contains( bundle.getImportMode() ) )
+        {
+            if ( !isLegitUpdate( programRuleVariable, prvWithSameNameAndSameProgram ) )
+            {
+                return prvWithSameNameAlreadyExistsError( programRuleVariable );
+            }
+        }
 
-        if ( query.getResultList().size() > allowedCount )
+        // insert
+        else if ( CollectionUtils.isNotEmpty( prvWithSameNameAndSameProgram ) )
         {
-            return ImmutableList.of(
-                new ErrorReport( ProgramRuleVariable.class, ErrorCode.E4032, programRuleVariable.getName(),
-                    programRuleVariable.getProgram().getUid() ) );
+            return prvWithSameNameAlreadyExistsError( programRuleVariable );
         }
 
-        return new ArrayList<>();
+        return Collections.emptyList();
+    }
+
+    private boolean isLegitUpdate( ProgramRuleVariable programRuleVariable,
+        List<ProgramRuleVariable> existingPrvs )
+    {
+        return existingPrvs.isEmpty() ||
+            existingPrvs.stream()
+                .anyMatch( existingPrv -> hasSameUid( existingPrv, programRuleVariable ) );
+    }
+
+    private boolean hasSameUid( ProgramRuleVariable existingPrv, ProgramRuleVariable programRuleVariable )
+    {
+        return StringUtils.equals( existingPrv.getUid(), programRuleVariable.getUid() );
+    }
+
+    private List<ErrorReport> prvWithSameNameAlreadyExistsError( ProgramRuleVariable programRuleVariable )
+    {
+        return Collections.singletonList(
+            new ErrorReport( ProgramRuleVariable.class, ErrorCode.E4032, programRuleVariable.getName(),
+                programRuleVariable.getProgram().getUid() ) );
     }
 
     private Query<ProgramRuleVariable> getProgramRuleVariableQuery( ProgramRuleVariable programRuleVariable )

diff --git a/...g/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHookTest.java b/...g/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHookTest.java
@@ -57,6 +57,8 @@
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
 
+import com.google.common.collect.ImmutableList;
+
 /**
  * @author Luca Cambi
  */
@@ -108,7 +110,7 @@ public void shouldExitObjectNotInstanceOfProgramRuleVariable()
     }
 
     @Test
-    public void shouldFailValidationInvalidCount()
+    public void shouldFailInsertAlreadyExisting()
     {
         when( programRuleVariable.getProgram() ).thenReturn( program );
         when( objectBundle.getImportMode() ).thenReturn( ImportStrategy.CREATE );
@@ -122,18 +124,74 @@ public void shouldFailValidationInvalidCount()
     }
 
     @Test
-    public void shouldNotFailValidationInvalidCount()
+    public void shouldNotFailUpdateExistingSameUid()
     {
         when( programRuleVariable.getProgram() ).thenReturn( program );
         when( objectBundle.getImportMode() ).thenReturn( ImportStrategy.CREATE_AND_UPDATE );
-        when( query.getResultList() ).thenReturn( Collections.singletonList( new ProgramRuleVariable() ) );
+
+        ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
+        existingProgramRuleVariable.setName( "word" );
+        existingProgramRuleVariable.setUid( "uid1" );
+
+        when( query.getResultList() ).thenReturn( Collections.singletonList( existingProgramRuleVariable ) );
+
+        when( programRuleVariable.getName() ).thenReturn( "word" );
+        when( programRuleVariable.getUid() ).thenReturn( "uid1" );
+
+        List<ErrorReport> errorReports = programRuleVariableObjectBundleHook.validate( programRuleVariable,
+            objectBundle );
+
+        assertEquals( 0, errorReports.size() );
+    }
+
+    @Test
+    public void shouldNotFailUpdateExistingMoreThanOneSameUid()
+    {
+        when( programRuleVariable.getProgram() ).thenReturn( program );
+        when( objectBundle.getImportMode() ).thenReturn( ImportStrategy.CREATE_AND_UPDATE );
+
+        ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
+        existingProgramRuleVariable.setName( "word" );
+        existingProgramRuleVariable.setUid( "uid1" );
+
+        ProgramRuleVariable anotherExistingProgramRuleVariable = new ProgramRuleVariable();
+        anotherExistingProgramRuleVariable.setName( "word" );
+        anotherExistingProgramRuleVariable.setUid( "uid2" );
+
+        when( query.getResultList() )
+            .thenReturn( ImmutableList.of( existingProgramRuleVariable, anotherExistingProgramRuleVariable ) );
 
         when( programRuleVariable.getName() ).thenReturn( "word" );
+        when( programRuleVariable.getUid() ).thenReturn( "uid1" );
+
         List<ErrorReport> errorReports = programRuleVariableObjectBundleHook.validate( programRuleVariable,
             objectBundle );
+
         assertEquals( 0, errorReports.size() );
     }
 
+    @Test
+    public void shouldFailUpdateExistingDifferentUid()
+    {
+        when( programRuleVariable.getProgram() ).thenReturn( program );
+        when( objectBundle.getImportMode() ).thenReturn( ImportStrategy.CREATE_AND_UPDATE );
+
+        ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
+        existingProgramRuleVariable.setName( "word" );
+        existingProgramRuleVariable.setUid( "uid1" );
+
+        when( query.getResultList() ).thenReturn( Collections.singletonList( existingProgramRuleVariable ) );
+
+        when( programRuleVariable.getName() ).thenReturn( "word" );
+        when( programRuleVariable.getUid() ).thenReturn( "uid2" );
+
+        List<ErrorReport> errorReports = programRuleVariableObjectBundleHook.validate( programRuleVariable,
+            objectBundle );
+
+        assertEquals( 1, errorReports.size() );
+        assertTrue( errorReports.stream().anyMatch( e -> e.getErrorCode().equals( E4032 ) ) );
+    }
+
     @Test
     public void shouldFailValidationInvalidCountAndInvalidName()
     {