diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index 119bddad80..79210525d0 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -591,6 +591,7 @@ + diff --git a/source/java/org/alfresco/repo/dictionary/ModelValidatorImpl.java b/source/java/org/alfresco/repo/dictionary/ModelValidatorImpl.java index b6d43daa73..c74aa1071a 100644 --- a/source/java/org/alfresco/repo/dictionary/ModelValidatorImpl.java +++ b/source/java/org/alfresco/repo/dictionary/ModelValidatorImpl.java @@ -25,6 +25,7 @@ */ package org.alfresco.repo.dictionary; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -45,6 +46,7 @@ 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.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; @@ -71,6 +73,7 @@ public class ModelValidatorImpl implements ModelValidator private static final Log logger = LogFactory.getLog(ModelValidatorImpl.class); private DictionaryDAO dictionaryDAO; + private DictionaryService dictionaryService; private QNameDAO qnameDAO; private NamespaceService namespaceService; private TransactionService transactionService; @@ -119,6 +122,11 @@ public class ModelValidatorImpl implements ModelValidator this.tenantAdminService = tenantAdminService; } + public void setDictionaryService(DictionaryService dictionaryService) + { + this.dictionaryService = dictionaryService; + } + private void checkCustomModelNamespace(M2Model model, String tenantDomain) { if(tenantDomain != null && !tenantDomain.equals("") && enforceTenantInNamespace) @@ -477,8 +485,37 @@ public class ModelValidatorImpl implements ModelValidator { throw new AlfrescoRuntimeException("Failed to validate model update - found non-incrementally updated " + modelDiff.getElementType() + " '" + modelDiff.getElementName() + "'"); } + + if(modelDiff.getDiffType().equals(M2ModelDiff.DIFF_CREATED)) + { + if (modelDiff.getElementType().equals(M2ModelDiff.TYPE_NAMESPACE)) + { + ModelDefinition importedModel = dictionaryService.getModelByNamespaceUri(modelDiff.getNamespaceDefinition().getUri()); + if(importedModel != null && !model.getNamespaces().isEmpty()) + { + checkCircularDependency(importedModel, model, importedModel.getName().getLocalName()); + } + } + } } // TODO validate that any deleted constraints are not being referenced - else currently will become anon - or push down into model compilation (check backwards compatibility ...) } + + private void checkCircularDependency(ModelDefinition model, M2Model existingModel, String parentPrefixedName) throws AlfrescoRuntimeException + { + for (NamespaceDefinition importedNamespace : model.getImportedNamespaces()) + { + ModelDefinition md = null; + if ((md = dictionaryService.getModelByNamespaceUri(importedNamespace.getUri())) != null) + { + if (existingModel.getNamespace(importedNamespace.getUri()) != null) + { + throw new AlfrescoRuntimeException("Failed to validate model update - found circular dependency. You can't set parent " + parentPrefixedName + " as it's model already depends on " + existingModel.getName()); + } + checkCircularDependency(md, existingModel, parentPrefixedName); + } + } + } + } diff --git a/source/test-java/org/alfresco/repo/dictionary/DictionaryModelTypeTest.java b/source/test-java/org/alfresco/repo/dictionary/DictionaryModelTypeTest.java index 70776dbbbf..17a7029f69 100644 --- a/source/test-java/org/alfresco/repo/dictionary/DictionaryModelTypeTest.java +++ b/source/test-java/org/alfresco/repo/dictionary/DictionaryModelTypeTest.java @@ -1142,11 +1142,90 @@ public class DictionaryModelTypeTest extends BaseSpringTest // Delete the model DictionaryModelTypeTest.this.nodeService.deleteNode(modelNode); return null; + } + }); + } + + public void testCircularDependencyDetected() throws Exception + { + // Create modelA + txn = transactionService.getUserTransaction(); + txn.begin(); + final NodeRef modelANode = createActiveModel("dictionary/modelA.xml"); + txn.commit(); + + //Create modelB + txn = transactionService.getUserTransaction(); + txn.begin(); + final NodeRef modelBNode = createActiveModel("dictionary/modelB.xml"); + txn.commit(); + + // update model A to introduce circular dependency + try + { + transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback() { + public Void execute() throws Exception { + ContentWriter contentWriter = DictionaryModelTypeTest.this.contentService.getWriter(modelANode, ContentModel.PROP_CONTENT, true); + contentWriter.putContent(Thread.currentThread().getContextClassLoader().getResourceAsStream("dictionary/modelAupdated.xml")); + return null; + } + }); + fail("Validation should fail as a circular dependency was introduced"); + } catch(AlfrescoRuntimeException e) + { + assertTrue(e.getMessage().contains("Failed to validate model update - found circular dependency. You can't set parent model-B as it's model already depends on prefixA:model-A")); + } + + // update model A to introduce circular dependency - also add a second namespace + try + { + transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback() { + public Void execute() throws Exception { + ContentWriter contentWriter = DictionaryModelTypeTest.this.contentService.getWriter(modelANode, ContentModel.PROP_CONTENT, true); + contentWriter.putContent(Thread.currentThread().getContextClassLoader().getResourceAsStream("dictionary/modelAupdatedWithSecondNamespace.xml")); + return null; + } + }); + fail("Validation should fail as a circular dependency was introduced"); + } catch(AlfrescoRuntimeException e) + { + assertTrue(e.getMessage().contains("Failed to validate model update - found circular dependency. You can't set parent model-B as it's model already depends on prefixA:model-A")); + } + + //delete both created models + finally { + transactionService.getRetryingTransactionHelper().doInTransaction( + new RetryingTransactionCallback() { + public Object execute() throws Exception { + // Delete the model + DictionaryModelTypeTest.this.nodeService.deleteNode(modelANode); + DictionaryModelTypeTest.this.nodeService.deleteNode(modelBNode); + return null; } }); } } + private NodeRef createActiveModel(String contentFile) + { + PropertyMap properties = new PropertyMap(1); + properties.put(ContentModel.PROP_MODEL_ACTIVE, true); + NodeRef modelNode = this.nodeService.createNode( + this.rootNodeRef, + ContentModel.ASSOC_CHILDREN, + QName.createQName(NamespaceService.ALFRESCO_URI, "dictionaryModels"), + ContentModel.TYPE_DICTIONARY_MODEL, + properties).getChildRef(); + assertNotNull(modelNode); + + ContentWriter contentWriter = this.contentService.getWriter(modelNode, ContentModel.PROP_CONTENT, true); + contentWriter.setEncoding("UTF-8"); + contentWriter.setMimetype(MimetypeMap.MIMETYPE_XML); + contentWriter.putContent(Thread.currentThread().getContextClassLoader().getResourceAsStream(contentFile)); + return modelNode; + } +} + /* MNT-15345 test */ public void testImportingSameNamespaceFailsWithinSingleModel() throws Exception { diff --git a/source/test-resources/dictionary/modelA.xml b/source/test-resources/dictionary/modelA.xml new file mode 100644 index 0000000000..932a963c5a --- /dev/null +++ b/source/test-resources/dictionary/modelA.xml @@ -0,0 +1,24 @@ + + + + Administrator + + + + + + + + + + + Model-A Type A + cm:content + + + + + + + + \ No newline at end of file diff --git a/source/test-resources/dictionary/modelAupdated.xml b/source/test-resources/dictionary/modelAupdated.xml new file mode 100644 index 0000000000..b7a5e1c318 --- /dev/null +++ b/source/test-resources/dictionary/modelAupdated.xml @@ -0,0 +1,31 @@ + + Administrator + + + + + + + + + + + + Model-A Type A + cm:content + + + + + + + Model-A Type B + prefixB:TypeB + + + + + + + + \ No newline at end of file diff --git a/source/test-resources/dictionary/modelAupdatedWithSecondNamespace.xml b/source/test-resources/dictionary/modelAupdatedWithSecondNamespace.xml new file mode 100644 index 0000000000..f9dd64b526 --- /dev/null +++ b/source/test-resources/dictionary/modelAupdatedWithSecondNamespace.xml @@ -0,0 +1,40 @@ + + Administrator + + + + + + + + + + + + + Model-A Type A + cm:content + + + + + + + Model-A Type A + cm:content + + + + + + + Model-A Type B + prefixB:TypeB + + + + + + + + \ No newline at end of file diff --git a/source/test-resources/dictionary/modelB.xml b/source/test-resources/dictionary/modelB.xml new file mode 100644 index 0000000000..a87c4312eb --- /dev/null +++ b/source/test-resources/dictionary/modelB.xml @@ -0,0 +1,22 @@ + + Administrator + + + + + + + + + + + Model-B TypeB + prefixA:TypeA + + + + + + + + \ No newline at end of file