SSDM-10292 : Internal vocabulary terms created by 'system' user should replace...

SSDM-10292 : Internal vocabulary terms created by 'system' user should replace existing terms - fix integration tests where old master data registration was failing due to changed behaviour of CommonServer.addVocabularyTerms method (before the changes it was silently ignoring existing terms and after the change it was throwing an exception to be consistent with v3 api behaviour).

openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/vocabulary/CreateVocabularyTermExecutor.java

@@ -20,9 +20,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.annotation.Resource;
 
@@ -166,8 +168,22 @@ public class CreateVocabularyTermExecutor implements ICreateVocabularyTermExecut
         IVocabularyBO vocabularyBO = businessObjectFactory.createVocabularyBO(context.getSession());
         vocabularyBO.loadDataByTechId(new TechId(vocabulary.getId()));
 
+        Set<String> existingTermCodes = new HashSet<String>();
+        for (VocabularyTermPE existingTerm : vocabulary.getTerms())
+        {
+            existingTermCodes.add(existingTerm.getCode());
+        }
+
         for (VocabularyTermCreation creation : creations)
         {
+            if (existingTermCodes.contains(creation.getCode()))
+            {
+                throw new UserFailureException("Vocabulary term " + creation.getCode() + " (" + vocabulary.getCode() + ") already exists.");
+            } else
+            {
+                existingTermCodes.add(creation.getCode());
+            }
+
             Long previousTermOrdinal = getPreviousTermOrdinal(context, vocabulary, creation);
 
             if (creation.isOfficial())

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/VocabularyBO.java

@@ -198,28 +198,38 @@ public class VocabularyBO extends AbstractBusinessObject implements IVocabularyB
     {
         final VocabularyTermPE existingTermPE = vocabularyPE.tryGetVocabularyTerm(code);
 
-        if (existingTermPE == null)
+        if (existingTermPE != null)
         {
-            final VocabularyTermPE vocabularyTermPE = new VocabularyTermPE();
-            vocabularyTermPE.setCode(code);
-            vocabularyTermPE.setDescription(description);
-            if (label != null && label.length() > 0)
-            {
-                vocabularyTermPE.setLabel(label);
-            }
-            vocabularyTermPE.setRegistrator(findPerson());
-            vocabularyTermPE.setOrdinal(ordinal);
-            vocabularyTermPE.setOfficial(isOfficial);
+            PersonPE user = session.tryGetPerson();
 
-            new InternalVocabularyAuthorization().canCreateTerm(session, vocabularyPE, vocabularyTermPE);
+            if (user != null && user.isSystemUser() && vocabularyPE.isManagedInternally())
+            {
+                existingTermPE.setDescription(description);
+                existingTermPE.setLabel(label);
+                existingTermPE.setOrdinal(ordinal);
+                existingTermPE.setOfficial(isOfficial);
+                getVocabularyTermDAO().updateRegistrator(existingTermPE, user);
 
-            vocabularyPE.addTerm(vocabularyTermPE);
+                return existingTermPE;
+            }
+        }
 
-            return vocabularyTermPE;
-        } else
+        final VocabularyTermPE vocabularyTermPE = new VocabularyTermPE();
+        vocabularyTermPE.setCode(code);
+        vocabularyTermPE.setDescription(description);
+        if (label != null && label.length() > 0)
         {
-            throw new UserFailureException("Vocabulary term " + code + " (" + vocabularyPE.getCode() + ") already exists.");
+            vocabularyTermPE.setLabel(label);
         }
+        vocabularyTermPE.setRegistrator(findPerson());
+        vocabularyTermPE.setOrdinal(ordinal);
+        vocabularyTermPE.setOfficial(isOfficial);
+
+        new InternalVocabularyAuthorization().canCreateTerm(session, vocabularyPE, vocabularyTermPE);
+
+        vocabularyPE.addTerm(vocabularyTermPE);
+
+        return vocabularyTermPE;
     }
 
     private VocabularyTermPE addTerm(VocabularyTerm term, Long ordinal, Boolean isOfficial)

openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/CommonServerTest.java

@@ -844,6 +844,143 @@ public class CommonServerTest extends SystemTestCase
             }, expectedError);
     }
 
+    @DataProvider
+    private Object[][] providerTestCreateAndTakeOverExistingVocabularyTerm()
+    {
+        return new Object[][] {
+                { "ORGANISM", SYSTEM_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "ORGANISM", SYSTEM_USER, TEST_USER, SYSTEM_USER, null },
+
+                { "ORGANISM", TEST_USER, SYSTEM_USER, TEST_USER, null },
+                { "ORGANISM", TEST_USER, TEST_USER, TEST_USER, null },
+
+                { "$PLATE_GEOMETRY", SYSTEM_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "$PLATE_GEOMETRY", SYSTEM_USER, TEST_USER, SYSTEM_USER, null },
+
+                { "$PLATE_GEOMETRY", TEST_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "$PLATE_GEOMETRY", TEST_USER, TEST_USER, TEST_USER, null },
+        };
+    }
+
+    @Test(dataProvider = "providerTestCreateAndTakeOverExistingVocabularyTerm")
+    public void testCreateAndTakeOverExistingVocabularyTerm(String vocabularyCode, String originalTermRegistrator, String duplicatedTermRegistrator,
+            String expectedTermRegistratorAfterCreation, String expectedError)
+    {
+        SessionContextDTO originalTermRegistratorSession =
+                originalTermRegistrator.equals(SYSTEM_USER) ? commonServer.tryToAuthenticateAsSystem()
+                        : commonServer.tryAuthenticate(originalTermRegistrator, PASSWORD);
+        SessionContextDTO duplicatedTermRegistratorSession =
+                duplicatedTermRegistrator.equals(SYSTEM_USER) ? commonServer.tryToAuthenticateAsSystem()
+                        : commonServer.tryAuthenticate(duplicatedTermRegistrator, PASSWORD);
+
+        VocabularyPE vocabularyPE = daoFactory.getVocabularyDAO().tryFindVocabularyByCode(vocabularyCode);
+
+        VocabularyTerm originalTerm = new VocabularyTerm();
+        originalTerm.setCode("TERM-TO-TAKE-OVER");
+        originalTerm.setLabel("Test Label");
+        originalTerm.setDescription("Test Description");
+        commonServer.addVocabularyTerms(originalTermRegistratorSession.getSessionToken(), new TechId(vocabularyPE.getId()),
+                Arrays.asList(originalTerm),
+                null);
+
+        VocabularyTerm duplicatedTerm = new VocabularyTerm();
+        duplicatedTerm.setCode("TERM-TO-TAKE-OVER");
+        duplicatedTerm.setLabel("Updated Label");
+        duplicatedTerm.setDescription("Updated Description");
+
+        assertExceptionMessage(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    commonServer.addVocabularyTerms(duplicatedTermRegistratorSession.getSessionToken(), new TechId(vocabularyPE.getId()),
+                            Arrays.asList(duplicatedTerm), null);
+
+                    VocabularyPE vocabularyPE = daoFactory.getVocabularyDAO().tryFindVocabularyByCode(vocabularyCode);
+                    VocabularyTermPE vocabularyTermPE = vocabularyPE.tryGetVocabularyTerm(originalTerm.getCode());
+
+                    if (duplicatedTermRegistrator.equals(SYSTEM_USER) && vocabularyPE.isManagedInternally())
+                    {
+                        assertEquals(vocabularyTermPE.getLabel(), "Updated Label");
+                        assertEquals(vocabularyTermPE.getDescription(), "Updated Description");
+                    } else
+                    {
+                        assertEquals(vocabularyTermPE.getLabel(), "Test Label");
+                        assertEquals(vocabularyTermPE.getDescription(), "Test Description");
+                    }
+
+                    assertEquals(vocabularyTermPE.getRegistrator().getUserId(), expectedTermRegistratorAfterCreation);
+                }
+            }, expectedError);
+    }
+
+    @DataProvider
+    private Object[][] providerTestUpdateAndTakeOverExistingVocabularyTerm()
+    {
+        return new Object[][] {
+                { "ORGANISM", SYSTEM_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "ORGANISM", SYSTEM_USER, TEST_USER, SYSTEM_USER, null },
+
+                { "ORGANISM", TEST_USER, SYSTEM_USER, TEST_USER, null },
+                { "ORGANISM", TEST_USER, TEST_USER, TEST_USER, null },
+
+                { "$PLATE_GEOMETRY", SYSTEM_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "$PLATE_GEOMETRY", SYSTEM_USER, TEST_USER, SYSTEM_USER,
+                        "Terms created by the system user that belong to internal vocabularies can be managed only by the system user" },
+
+                { "$PLATE_GEOMETRY", TEST_USER, SYSTEM_USER, SYSTEM_USER, null },
+                { "$PLATE_GEOMETRY", TEST_USER, TEST_USER, TEST_USER, null },
+        };
+    }
+
+    @Test(dataProvider = "providerTestUpdateAndTakeOverExistingVocabularyTerm")
+    public void testUpdateAndTakeOverExistingVocabularyTerm(String vocabularyCode, String termRegistrator, String termUpdater,
+            String expectedTermRegistratorAfterUpdate, String expectedError)
+    {
+        SessionContextDTO termRegistratorSession =
+                termRegistrator.equals(SYSTEM_USER) ? commonServer.tryToAuthenticateAsSystem()
+                        : commonServer.tryAuthenticate(termRegistrator, PASSWORD);
+        SessionContextDTO termUpdaterSession =
+                termUpdater.equals(SYSTEM_USER) ? commonServer.tryToAuthenticateAsSystem()
+                        : commonServer.tryAuthenticate(termUpdater, PASSWORD);
+
+        VocabularyPE vocabularyPE = daoFactory.getVocabularyDAO().tryFindVocabularyByCode(vocabularyCode);
+
+        VocabularyTerm term = new VocabularyTerm();
+        term.setCode("TERM-TO-TAKE-OVER");
+        term.setLabel("Test Label");
+        term.setDescription("Test Description");
+        commonServer.addVocabularyTerms(termRegistratorSession.getSessionToken(), new TechId(vocabularyPE.getId()), Arrays.asList(term),
+                null);
+
+        VocabularyTermPE termPE = vocabularyPE.tryGetVocabularyTerm(term.getCode());
+
+        VocabularyTerm updateTerm = new VocabularyTerm();
+        updateTerm.setId(termPE.getId());
+        updateTerm.setCode(termPE.getCode());
+        updateTerm.setLabel("Updated Label");
+        updateTerm.setDescription("Updated Description");
+        updateTerm.setOrdinal(termPE.getOrdinal());
+        updateTerm.setModificationDate(termPE.getModificationDate());
+        VocabularyTermBatchUpdateDetails updateDetails = new VocabularyTermBatchUpdateDetails();
+        UpdatedVocabularyTerm update = new UpdatedVocabularyTerm(updateTerm, updateDetails);
+
+        assertExceptionMessage(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    commonServer.updateVocabularyTerm(termUpdaterSession.getSessionToken(), update);
+
+                    VocabularyPE vocabularyPE = daoFactory.getVocabularyDAO().tryFindVocabularyByCode(vocabularyCode);
+                    VocabularyTermPE vocabularyTermPE = vocabularyPE.tryGetVocabularyTerm(term.getCode());
+                    assertEquals(vocabularyTermPE.getLabel(), "Updated Label");
+                    assertEquals(vocabularyTermPE.getDescription(), "Updated Description");
+                    assertEquals(vocabularyTermPE.getRegistrator().getUserId(), expectedTermRegistratorAfterUpdate);
+                }
+            }, expectedError);
+    }
+
     @Test
     public void testDeleteGroupWithPersons()
     {