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

…uplicates [DHIS2-11330] (dhis2#8220) * 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
davidhuser · Jun 22, 2021 · c525517 · c525517
1 parent a265313
commit c525517
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 9 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
@@ -29,10 +29,13 @@
 
 import static org.hisp.dhis.dxf2.Constants.PROGRAM_RULE_VARIABLE_NAME_INVALID_KEYWORDS;
 
+import java.util.List;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.IntStream;
 
+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;
@@ -94,18 +97,47 @@ public <T extends IdentifiableObject> void validate( T object, ObjectBundle bund
     private void validateUniqueProgramRuleName( ObjectBundle bundle,
         ProgramRuleVariable programRuleVariable, Consumer<ErrorReport> addReports )
     {
-        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 ) )
+            {
+                failPrvWithSameNAmeAlreadyExists( programRuleVariable, addReports );
+            }
+            return;
+        }
 
-        if ( query.getResultList().size() > allowedCount )
+        // insert
+        if ( CollectionUtils.isNotEmpty( prvWithSameNameAndSameProgram ) )
         {
-            addReports.accept(
-                new ErrorReport( ProgramRuleVariable.class, ErrorCode.E4051, programRuleVariable.getName(),
-                    programRuleVariable.getProgram().getUid() ) );
+            failPrvWithSameNAmeAlreadyExists( programRuleVariable, addReports );
         }
     }
 
+    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 void failPrvWithSameNAmeAlreadyExists( ProgramRuleVariable programRuleVariable,
+        Consumer<ErrorReport> addReports )
+    {
+        addReports.accept(
+            new ErrorReport( ProgramRuleVariable.class, ErrorCode.E4051, programRuleVariable.getName(),
+                programRuleVariable.getProgram().getUid() ) );
+    }
+
     private Query<ProgramRuleVariable> getProgramRuleVariableQuery( ProgramRuleVariable programRuleVariable )
     {
         Session session = sessionFactory.getCurrentSession();

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( E4051 ) ) );
+    }
+
     @Test
     public void shouldFailValidationInvalidCountAndInvalidName()
     {