Fix/acs 701 (#36)

* Fix/acs 701 duplicate prefix in content model

* Build branch on Travis

* ACS-701 Remove temporary travis file change

Co-authored-by: Alan Davis <alan.davis@alfresco.com>

data-model/src/main/java/org/alfresco/service/cmr/dictionary/CustomModelService.java

@@ -33,6 +33,7 @@ import org.alfresco.repo.dictionary.CustomModelsInfo;
 import org.alfresco.repo.dictionary.M2Model;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.util.Pair;
 
 /**
  * Custom model service configuration API.
@@ -105,6 +106,14 @@ public interface CustomModelService
      */
     public NodeRef getModelNodeRef(String modelFileName);
 
+    /**
+     * Gets custom model
+     *
+     * @param modelNodeRef the {@code NodeRef} of the custom model
+     * @return m2Model the {@code M2Model} object
+     */
+    public M2Model getM2Model(NodeRef modelNodeRef);
+
     /**
      * Creates custom model
      *
@@ -167,6 +176,8 @@ public interface CustomModelService
      */
     public boolean isNamespaceUriExists(String modelNamespaceUri);
 
+    public boolean isNamespacePrefixExists(NodeRef modelNodeRef);
+
     /**
      * Whether a model with the given name exists or not
      *
@@ -175,10 +186,35 @@ public interface CustomModelService
      */
     public boolean isModelExists(String modelFileName);
 
+    /**
+     *  Gets custom models' namespace URI and prefix
+     *
+     * @param model the {@code M2Model} object
+     * @return the custom model URI and prefix as a {@code Pair<String, String}
+     * @throws CustomModelException if no namespace or more than one namespace exists
+     */
+    public Pair<String, String> getModelNamespaceUriPrefix(M2Model model);
+
+    /**
+     *  Validates the custom models' namespace prefix
+     *
+     * @param prefix the namespace prefix {@code String}
+     * @throws CustomModelException if the namespace prefix is already in use by another model
+     */
+    public void validateModelNamespacePrefix(String prefix);
+
+    /**
+     *  Validates the custom models' namespace prefix
+     *
+     * @param modelNodeRef the nodeRef of the model whose namespace prefix is to be validated {@code NodeRef}
+     * @throws CustomModelException if the namespace prefix is already in use by another model
+     */
+    public void validateModelNamespacePrefix(NodeRef modelNodeRef);
+
     /**
      * Whether the given namespace prefix has already been used or not
      *
-     * @param modelNamespaceUri the model namespace prefix
+     * @param modelNamespacePrefix the model namespace prefix
      * @return true if the prefix has been used, false otherwise
      */
     public boolean isNamespacePrefixExists(String modelNamespacePrefix);

repository/src/main/java/org/alfresco/repo/dictionary/CustomModelServiceImpl.java

@@ -271,7 +271,8 @@ public class CustomModelServiceImpl implements CustomModelService
         return nodeRefs.get(0);
     }
 
-    private M2Model getM2Model(final NodeRef modelNodeRef)
+    @Override
+    public M2Model getM2Model(final NodeRef modelNodeRef)
     {
         ContentReader reader = contentService.getReader(modelNodeRef, ContentModel.PROP_CONTENT);
         if (reader == null)
@@ -917,6 +918,43 @@ public class CustomModelServiceImpl implements CustomModelService
         return false;
     }
 
+    @Override
+    public boolean isNamespacePrefixExists(NodeRef modelNodeRef)
+    {
+        ParameterCheck.mandatoryString("modelNodeRef", modelNodeRef.toString());
+
+        M2Model m2Model = getM2Model(modelNodeRef);
+        String modelNamespacePrefix = getModelNamespaceUriPrefix(m2Model).getSecond();
+        String modelNamespaceUri = getModelNamespaceUriPrefix(m2Model).getFirst();
+
+        Collection<String> prefixes = namespaceDAO.getPrefixes();
+        if (prefixes.contains(modelNamespacePrefix))
+        {
+            // Check the uri to ensure the found prefix does not belong to the model being validated
+            Collection<String> uris = namespaceDAO.getURIs();
+            if (!uris.contains(modelNamespaceUri))
+            {
+                return true;
+            }
+        }
+
+        for (CompiledModel model : getAllCustomM2Models(false))
+        {
+            M2Model existingModel = model.getM2Model();
+            String existingPrefix = getModelNamespaceUriPrefix(existingModel).getSecond();
+            if (modelNamespacePrefix.equals(existingPrefix))
+            {
+                // Get the nodeRef for the model which owns this prefix to ensure it is not the same model being validated
+                NodeRef existingModelNodeRef = getModelNodeRef(model.getModelDefinition().getName().getLocalName());
+                if (!modelNodeRef.equals(existingModelNodeRef))
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     @Override
     public boolean isModelExists(String modelFileName)
     {
@@ -937,7 +975,8 @@ public class CustomModelServiceImpl implements CustomModelService
         return false;
     }
 
-    private Pair<String, String> getModelNamespaceUriPrefix(M2Model model)
+    @Override
+    public Pair<String, String> getModelNamespaceUriPrefix(M2Model model)
     {
         ParameterCheck.mandatory("model", model);
 
@@ -963,7 +1002,8 @@ public class CustomModelServiceImpl implements CustomModelService
         }
     }
 
-    private void validateModelNamespacePrefix(String prefix)
+    @Override
+    public void validateModelNamespacePrefix(String prefix)
     {
         if (isNamespacePrefixExists(prefix))
         {
@@ -971,6 +1011,17 @@ public class CustomModelServiceImpl implements CustomModelService
         }
     }
 
+    @Override
+    public void validateModelNamespacePrefix(NodeRef modelNodeRef)
+    {
+        if (isNamespacePrefixExists(modelNodeRef))
+        {
+            M2Model m2Model = getM2Model(modelNodeRef);
+            String prefix = getModelNamespaceUriPrefix(m2Model).getSecond();
+            throw new CustomModelException.NamespaceConstraintException(MSG_NAMESPACE_PREFIX_ALREADY_IN_USE, new Object[] { prefix });
+        }
+    }
+
     /**
      * A helper method to run a unit of work in a transaction.
      *

repository/src/main/java/org/alfresco/repo/dictionary/DictionaryModelType.java

@@ -330,6 +330,11 @@ public class DictionaryModelType implements ContentServicePolicies.OnContentUpda
         
         if (beforeValue == null && afterValue != null)
         {
+            // if trying to activate the model, check the namespace prefix is not a duplicate
+            if (afterValue)
+            {
+                modelValidator.validateModelNamespacePrefix(nodeRef);
+            }
             queueModel(nodeRef);
         }
         else if (afterValue == null && beforeValue != null)
@@ -339,6 +344,11 @@ public class DictionaryModelType implements ContentServicePolicies.OnContentUpda
         }
         else if (beforeValue != null && afterValue != null && beforeValue.equals(afterValue) == false)
         {
+            // if trying to activate the model, check the namespace prefix is not a duplicate
+            if (afterValue)
+            {
+                modelValidator.validateModelNamespacePrefix(nodeRef);
+            }
             queueModel(nodeRef);
         }
     }

repository/src/main/java/org/alfresco/repo/dictionary/ModelValidator.java

@@ -25,6 +25,8 @@
  */
 package org.alfresco.repo.dictionary;
 
+import org.alfresco.service.cmr.dictionary.CustomModelException;
+import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.namespace.QName;
 
 /**
@@ -58,4 +60,12 @@ public interface ModelValidator
      * exist
      */
 	boolean canDeleteModel(QName modelName);
+
+    /**
+     * validate the namespace prefix
+     *
+     * @throws CustomModelException if the prefix already exists in another model
+     */
+	void validateModelNamespacePrefix(NodeRef modelNodeRef);
+
 }

repository/src/main/java/org/alfresco/repo/dictionary/ModelValidatorImpl.java

@@ -44,12 +44,15 @@ import org.alfresco.repo.workflow.BPMEngineRegistry;
 import org.alfresco.service.cmr.dictionary.AspectDefinition;
 import org.alfresco.service.cmr.dictionary.ClassDefinition;
 import org.alfresco.service.cmr.dictionary.ConstraintDefinition;
+import org.alfresco.service.cmr.dictionary.CustomModelException;
+import org.alfresco.service.cmr.dictionary.CustomModelService;
 import org.alfresco.service.cmr.dictionary.DictionaryException;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.ModelDefinition;
 import org.alfresco.service.cmr.dictionary.NamespaceDefinition;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.cmr.dictionary.TypeDefinition;
+import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.workflow.WorkflowDefinition;
 import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.cmr.workflow.WorkflowTaskDefinition;
@@ -79,6 +82,7 @@ public class ModelValidatorImpl implements ModelValidator
     private WorkflowService workflowService;
     private TenantService tenantService;
     private TenantAdminService tenantAdminService;
+    private CustomModelService customModelService;
     private boolean enforceTenantInNamespace = false;
 
     public void setEnforceTenantInNamespace(boolean enforceTenantInNamespace)
@@ -126,6 +130,11 @@ public class ModelValidatorImpl implements ModelValidator
         this.dictionaryService = dictionaryService;
     }
 
+    public void setCustomModelService(CustomModelService customModelService)
+    {
+        this.customModelService = customModelService;
+    }
+
     private void checkCustomModelNamespace(M2Model model, String tenantDomain)
     {
         if(tenantDomain != null && !tenantDomain.equals("") && enforceTenantInNamespace)
@@ -526,4 +535,27 @@ public class ModelValidatorImpl implements ModelValidator
         }
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void validateModelNamespacePrefix(NodeRef modelNodeRef)
+    {
+        String errorMsg = "Namespace error: ";
+        try
+        {
+            M2Model m2Model = customModelService.getM2Model(modelNodeRef);
+            // if there is no model then there is no namespace prefix to validate
+            if (m2Model != null)
+            {
+                errorMsg = "Duplicate namespace prefix: ";
+                customModelService.validateModelNamespacePrefix(modelNodeRef);
+            }
+        }
+        catch (CustomModelException ce)
+        {
+            logger.error(errorMsg + ce);
+            throw ce;
+        }
+    }
 }

repository/src/main/resources/alfresco/core-services-context.xml

@@ -611,6 +611,7 @@
         <property name="transactionService" ref="transactionService"/>
         <property name="qnameDAO" ref="qnameDAO"/>
         <property name="enforceTenantInNamespace" value="${models.enforceTenantInNamespace}"/>
+        <property name="customModelService" ref="customModelService"/>
     </bean>
 
     <bean id="dictionaryDAO" class="org.alfresco.repo.dictionary.DictionaryDAOImpl">

repository/src/main/resources/alfresco/minimal-context.xml

@@ -25,6 +25,8 @@
 
     <!-- Required by RepositoryStartStopTest which may apply the CalendarAllDayEventDatesCorrectingPatch -->
     <import resource="classpath:alfresco/calendar-services-context.xml"/>
+    <import resource="classpath:alfresco/custom-model-management-services-context.xml"/>
+    <import resource="classpath:alfresco/download-services-context.xml"/>
 
     <!--
          Import all modules and related components.

repository/src/test/java/org/alfresco/AppContext03TestSuite.java

@@ -43,6 +43,9 @@ import org.junit.runners.Suite;
     // needs a clean DB to run
     org.alfresco.repo.calendar.CalendarServiceImplTest.class,
 
+    // needs a clean(ish) db to run, otherwise fails with workspace issues
+    org.alfresco.repo.dictionary.ModelValidatorTest.class,
+
     org.alfresco.RepositoryStartupTest.class,
     org.alfresco.repo.content.cleanup.ContentStoreCleanerTest.class,
     org.alfresco.repo.content.RoutingContentServiceTest.class,
@@ -58,7 +61,6 @@ import org.junit.runners.Suite;
     org.alfresco.repo.descriptor.DescriptorServiceTest.class,
     org.alfresco.repo.dictionary.DictionaryModelTypeTest.class,
     org.alfresco.repo.dictionary.DictionaryRepositoryBootstrapTest.class,
-    org.alfresco.repo.dictionary.ModelValidatorTest.class,
     org.alfresco.repo.dictionary.types.period.PeriodTest.class,
     org.alfresco.repo.exporter.RepositoryExporterComponentTest.class,
     org.alfresco.repo.i18n.MessageServiceImplTest.class,

repository/src/test/java/org/alfresco/repo/dictionary/ModelValidatorTest.java

@@ -25,6 +25,7 @@
  */
 package org.alfresco.repo.dictionary;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 
@@ -37,7 +38,11 @@ import org.alfresco.repo.domain.qname.QNameDAO;
 import org.alfresco.repo.node.archive.NodeArchiveService;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
+import org.alfresco.service.cmr.dictionary.CustomModelDefinition;
+import org.alfresco.service.cmr.dictionary.CustomModelException;
+import org.alfresco.service.cmr.dictionary.CustomModelService;
 import org.alfresco.service.cmr.dictionary.DictionaryException;
+import org.alfresco.service.cmr.dictionary.NamespaceDefinition;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.model.FileInfo;
 import org.alfresco.service.cmr.repository.ContentService;
@@ -51,6 +56,7 @@ import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.GUID;
+import org.alfresco.util.Pair;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.Test;
@@ -63,6 +69,11 @@ import org.springframework.context.ApplicationContext;
  */
 public class ModelValidatorTest
 {
+    private static final String TEST_MODEL_URI_PART1 = "http://www.alfresco.org/model/testmodelvalidatoramespace";
+    private static final String TEST_MODEL_URI_PART2 = "/1.0";
+    private static final String TEST_MODEL_DESC = "This is test custom model desc";
+    private static final String TEST_MODEL_AUTHOR = "John Doe";
+
     private ApplicationContext ctx;
 
     private String testNamespace;
@@ -78,6 +89,7 @@ public class ModelValidatorTest
     private VersionService versionService;
     private TransactionService transactionService;
     private NodeArchiveService nodeArchiveService;
+    private CustomModelService customModelService;
 
     private M2Model model;
     private QName modelQName;
@@ -100,6 +112,7 @@ public class ModelValidatorTest
         this.versionService = (VersionService)ctx.getBean("VersionService");
         this.transactionService = (TransactionService)ctx.getBean("TransactionService");
         this.nodeArchiveService = (NodeArchiveService)ctx.getBean("nodeArchiveService");
+        this.customModelService = (CustomModelService)ctx.getBean("customModelService");
 
         this.modelName = "modelvalidatortest" + System.currentTimeMillis();
         addModel();
@@ -508,4 +521,123 @@ public class ModelValidatorTest
             //expected
         }
     }
+
+    /**
+     * For ACS-701
+     * Tests that validating the model namespace prefix succeeds for both inactive and active models with a
+     * unique namespace prefix.
+     */
+    @Test
+    public void testValidatePrefixForModelWithValidPrefix() throws Exception
+    {
+        // authenticate
+        AuthenticationUtil.pushAuthentication();
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+
+        // create and validate models
+        final String inactiveModelName = "testCustomModel1" + System.currentTimeMillis();
+        final String activeModelName = "testCustomModel2" + System.currentTimeMillis();
+
+        // create inactive model with unique prefix
+        final String inactiveModelPrefix = "testmodelvalidatorpfx1" + "-" + System.currentTimeMillis();
+        createAndVerifyTestModel(inactiveModelName, inactiveModelPrefix, false);
+
+        // create active model with unique preifx
+        final String activeModelPrefix = "testmodelvalidatorpfx2" + "-" + System.currentTimeMillis();
+        createAndVerifyTestModel(activeModelName, activeModelPrefix, true);
+
+        // validate model prefixes
+        validatePrefix(inactiveModelName);
+        validatePrefix(activeModelName);
+    }
+
+    /**
+     * For ACS-701
+     * Tests that validating the model namespace prefix throws an error for a model with
+     *  a prefix in use by another model.
+     */
+    @Test
+    public void testValidatePrefixForModelWithDuplicatePrefix() throws Exception
+    {
+        // authenticate
+        AuthenticationUtil.pushAuthentication();
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+
+        // create and validate models
+        final String customModel1Name = "testCustomModel1" + System.currentTimeMillis();
+        final String customModel2Name = "testCustomModel2" + System.currentTimeMillis();
+        final String modelPrefix = "acs701-prefix01" + "-" + System.currentTimeMillis();
+
+        // create model with unique prefix
+        createAndVerifyTestModel(customModel1Name, modelPrefix, false);
+        validatePrefix(customModel1Name);
+
+        try
+        {
+            // create model with duplicate prefix
+            createAndVerifyTestModel(customModel2Name, modelPrefix, true);
+            fail("Expected a CustomModelException for model with a duplicate namespace prefix");
+        }
+        catch (CustomModelException ex)
+        {
+            // expected exception
+        }
+    }
+
+    private void createAndVerifyTestModel(String testModelName, String prefix, boolean activate)
+    {
+        // Create the M2Model
+        String uri = TEST_MODEL_URI_PART1 + System.currentTimeMillis() + TEST_MODEL_URI_PART2;
+        Pair<String, String> namespacePair = new Pair<String, String>(uri, prefix);
+        M2Model model = M2Model.createModel(namespacePair.getSecond() + QName.NAMESPACE_PREFIX + testModelName);
+        model.createNamespace(namespacePair.getFirst(), namespacePair.getSecond());
+        model.setDescription(TEST_MODEL_DESC);
+        model.setAuthor(TEST_MODEL_AUTHOR);
+
+        // Create the model definition
+        CustomModelDefinition modelDefinition = createModel(model, activate);
+
+        // Assert model is created as expected
+        assertNotNull(modelDefinition);
+        assertEquals(testModelName, modelDefinition.getName().getLocalName());
+
+        NamespaceDefinition namespaceDefinition = modelDefinition.getNamespaces().iterator().next();
+        assertNotNull(namespaceDefinition);
+        assertEquals(namespacePair.getFirst(), namespaceDefinition.getUri());
+        assertEquals(namespacePair.getSecond(), namespaceDefinition.getPrefix());
+
+        // Assert model activation status
+        NodeRef modelNodeRef = customModelService.getModelNodeRef(testModelName);
+        boolean isActive = Boolean.TRUE.equals(nodeService.getProperty(modelNodeRef, ContentModel.PROP_MODEL_ACTIVE));
+        assertEquals(activate, isActive);
+    }
+
+    private CustomModelDefinition createModel(final M2Model m2Model, final boolean activate)
+    {
+        RetryingTransactionCallback<CustomModelDefinition> createModelCallback = new RetryingTransactionCallback<CustomModelDefinition>()
+        {
+            public CustomModelDefinition execute() throws Throwable
+            {
+                CustomModelDefinition cmd;
+                cmd = customModelService.createCustomModel(m2Model, activate);
+                return cmd;
+            }
+        };
+        return transactionService.getRetryingTransactionHelper().doInTransaction(createModelCallback, false, true);
+    }
+
+    private void validatePrefix(String modelName)
+    {
+        // validate the model namespace prefix
+        RetryingTransactionCallback<Void> validateInactiveModelPrefixCallback = new RetryingTransactionCallback<Void>()
+        {
+            public Void execute() throws Throwable
+            {
+                NodeRef modelNodeRef = customModelService.getModelNodeRef(modelName);
+                modelValidator.validateModelNamespacePrefix(modelNodeRef);
+                return null;
+            }
+        };
+        transactionService.getRetryingTransactionHelper().doInTransaction(validateInactiveModelPrefixCallback, false, true);
+    }
 }