TUSCANY-3020 - when definitions are aggregated together check that there are no duplicates. DOne here rather than later at the resolve phase as there is a better chance of reporting some sensible context to the user about which definitions.xml file is in error.

git-svn-id: http://svn.us.apache.org/repos/asf/tuscany@802762 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 113 insertions, 30 deletions

diff --git a/java/sca/modules/definitions-xml/src/main/java/org/apache/tuscany/sca/definitions/xml/DefinitionsDocumentProcessor.java b/java/sca/modules/definitions-xml/src/main/java/org/apache/tuscany/sca/definitions/xml/DefinitionsDocumentProcessor.java
index 7d87a8a659..699920dd6f 100644
--- a/java/sca/modules/definitions-xml/src/main/java/org/apache/tuscany/sca/definitions/xml/DefinitionsDocumentProcessor.java
+++ b/java/sca/modules/definitions-xml/src/main/java/org/apache/tuscany/sca/definitions/xml/DefinitionsDocumentProcessor.java
@@ -145,7 +145,7 @@ public class DefinitionsDocumentProcessor implements URLArtifactProcessor<Defini
                     // QName name = reader.getName();
                     Object model = extensionProcessor.read(reader);
                     if (model instanceof Definitions) {
-                        DefinitionsUtil.aggregate((Definitions)model, definitions);
+                        DefinitionsUtil.aggregate((Definitions)model, definitions, monitor);
                         return definitions;
                     } else {
                         error("ContributionReadException", model, null);
@@ -175,27 +175,7 @@ public class DefinitionsDocumentProcessor implements URLArtifactProcessor<Defini
         }
     }
 
-    // FIXME: [rfeng] We need to validate the definitions against the Conformance Items
-    // defined by the SCA Policy Framework Spec V1.1
-    private static void stripDuplicates(Definitions definitions) {
-        Set<Intent> intents = new HashSet<Intent>(definitions.getIntents());
-        Set<PolicySet> policySets = new HashSet<PolicySet>(definitions.getPolicySets());
-
-        Set<BindingType> bindingTypes = new HashSet<BindingType>(definitions.getBindingTypes());
-        Set<ImplementationType> implementationTypes = new HashSet<ImplementationType>(definitions.getImplementationTypes());
-
-        definitions.getIntents().clear();
-        definitions.getIntents().addAll(intents);
-        definitions.getPolicySets().clear();
-        definitions.getPolicySets().addAll(policySets);
-        definitions.getBindingTypes().clear();
-        definitions.getBindingTypes().addAll(bindingTypes);
-        definitions.getImplementationTypes().clear();
-        definitions.getImplementationTypes().addAll(implementationTypes);
-    }
-
     public void resolve(Definitions scaDefinitions, ModelResolver resolver) throws ContributionResolveException {
-        stripDuplicates(scaDefinitions);
         extensionProcessor.resolve(scaDefinitions, resolver);
     }
 
diff --git a/java/sca/modules/definitions/META-INF/MANIFEST.MF b/java/sca/modules/definitions/META-INF/MANIFEST.MF
index 51582ad834..c503375f74 100644
--- a/java/sca/modules/definitions/META-INF/MANIFEST.MF
+++ b/java/sca/modules/definitions/META-INF/MANIFEST.MF
@@ -13,8 +13,10 @@ Bnd-LastModified: 1225397089625
 Bundle-ManifestVersion: 2

 Bundle-License: http://www.apache.org/licenses/LICENSE-2.0.txt

 Bundle-Description: Apache Tuscany SCA Definitions

-Import-Package: javax.xml.namespace,org.apache.tuscany.sca.definitions

- ;version="2.0.0",org.apache.tuscany.sca.definitions.util;version="2.0.0",

+Import-Package: javax.xml.namespace,

+ org.apache.tuscany.sca.definitions;version="2.0.0",

+ org.apache.tuscany.sca.definitions.util;version="2.0.0",

+ org.apache.tuscany.sca.monitor;version="2.0.0",

  org.apache.tuscany.sca.policy;version="2.0.0"

 Bundle-SymbolicName: org.apache.tuscany.sca.definitions

 Bundle-DocURL: http://www.apache.org/

diff --git a/java/sca/modules/definitions/pom.xml b/java/sca/modules/definitions/pom.xml
index e4daa3a9c9..f26d802bbb 100644
--- a/java/sca/modules/definitions/pom.xml
+++ b/java/sca/modules/definitions/pom.xml
@@ -47,6 +47,13 @@
             <version>2.0-SNAPSHOT</version>
             <scope>test</scope>
         </dependency>
+        
+        <dependency>
+            <groupId>org.apache.tuscany.sca</groupId>
+            <artifactId>tuscany-monitor</artifactId>
+            <version>2.0-SNAPSHOT</version>
+            <scope>test</scope>
+        </dependency>        
     </dependencies>
 
 </project>
diff --git a/java/sca/modules/definitions/src/main/java/org/apache/tuscany/sca/definitions/util/DefinitionsUtil.java b/java/sca/modules/definitions/src/main/java/org/apache/tuscany/sca/definitions/util/DefinitionsUtil.java
index 2e9775e955..ba5a792c64 100644
--- a/java/sca/modules/definitions/src/main/java/org/apache/tuscany/sca/definitions/util/DefinitionsUtil.java
+++ b/java/sca/modules/definitions/src/main/java/org/apache/tuscany/sca/definitions/util/DefinitionsUtil.java
@@ -19,7 +19,14 @@
 
 package org.apache.tuscany.sca.definitions.util;
 
+import java.util.HashSet;
+
 import org.apache.tuscany.sca.definitions.Definitions;
+import org.apache.tuscany.sca.monitor.Monitor;
+import org.apache.tuscany.sca.policy.BindingType;
+import org.apache.tuscany.sca.policy.ImplementationType;
+import org.apache.tuscany.sca.policy.Intent;
+import org.apache.tuscany.sca.policy.PolicySet;
 
 /**
  * Some utility functions to deal with SCADefinitions
@@ -28,11 +35,67 @@ import org.apache.tuscany.sca.definitions.Definitions;
  */
 public class DefinitionsUtil {
     
-    public static void aggregate(Definitions source, Definitions target) {
-        target.getIntents().addAll(source.getIntents());
-        target.getPolicySets().addAll(source.getPolicySets());
-        target.getBindingTypes().addAll(source.getBindingTypes());
-        target.getImplementationTypes().addAll(source.getImplementationTypes());
+    /**
+     * Add the source set of definitions into the target set of definitions checking that 
+     * definitions artifacts are unique in the process
+     * 
+     * @param source the input definitions collection
+     * @param target the definition collection into which source will aggregated
+     */
+    public static void aggregate(Definitions source, Definitions target, Monitor monitor) {
+        
+        HashSet<Intent> intents = new HashSet<Intent>(target.getIntents());
+        for(Intent intent : source.getIntents()){           
+            if (intents.contains(intent)){
+                Monitor.error(monitor, 
+                              target, 
+                              "definitions-validation-messages", 
+                              "DuplicateIntent", 
+                              intent.getName().toString());
+            } else {          
+                target.getIntents().add(intent);
+            }
+        }
+        
+        HashSet<PolicySet> policySets = new HashSet<PolicySet>(target.getPolicySets());
+        for(PolicySet policySet : source.getPolicySets()){           
+            if (policySets.contains(policySet)){
+                Monitor.error(monitor, 
+                              target, 
+                              "definitions-validation-messages", 
+                              "DuplicatePolicySet", 
+                              policySet.getName().toString());
+            } else {          
+                target.getPolicySets().add(policySet);
+            }
+        }        
+
+        HashSet<BindingType> bindingTypes = new HashSet<BindingType>(target.getBindingTypes());
+        for(BindingType bindingType : source.getBindingTypes()){           
+            if (bindingTypes.contains(bindingType)){
+                Monitor.error(monitor, 
+                              target, 
+                              "definitions-validation-messages", 
+                              "DuplicateBindingType", 
+                              bindingType.getType().toString());
+            } else {          
+                target.getBindingTypes().add(bindingType);
+            }
+        }   
+
+        HashSet<ImplementationType> implementationTypes = new HashSet<ImplementationType>(target.getImplementationTypes());
+        for(ImplementationType implementationType : source.getImplementationTypes()){           
+            if (implementationTypes.contains(implementationType)){
+                Monitor.error(monitor, 
+                              target, 
+                              "definitions-validation-messages", 
+                              "DuplicateImplementationType", 
+                              implementationType.getType().toString());
+            } else {          
+                target.getImplementationTypes().add(implementationType);
+            }
+        }        
+      
         target.getBindings().addAll(source.getBindings());
     }
     
diff --git a/java/sca/modules/definitions/src/main/resources/definitions-validation-messages.properties b/java/sca/modules/definitions/src/main/resources/definitions-validation-messages.properties
new file mode 100644
index 0000000000..4cf9c01da5
--- /dev/null
+++ b/java/sca/modules/definitions/src/main/resources/definitions-validation-messages.properties
@@ -0,0 +1,25 @@
+#
+#
+#    Licensed to the Apache Software Foundation (ASF) under one
+#    or more contributor license agreements. See the NOTICE file
+#    distributed with this work for additional information
+#    regarding copyright ownership. The ASF licenses this file
+#    to you under the Apache License, Version 2.0 (the
+#    "License"); you may not use this file except in compliance
+#    with the License. You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing,
+#    software distributed under the License is distributed on an
+#    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+#    KIND, either express or implied. See the License for the
+#    specific language governing permissions and limitations
+#    under the License.
+#
+#
+
+DuplicateIntent = Duplicate intent {0} found in domain
+DuplicatePolicysSet = Duplicate policy set {0} found in domain
+DuplicateImplementationType = Duplicate implementation type {0} found in domain
+DuplicateBindingType = Duplicate binding type {0} found in domain
diff --git a/java/sca/modules/node-impl/src/main/java/org/apache/tuscany/sca/node/impl/NodeFactoryImpl.java b/java/sca/modules/node-impl/src/main/java/org/apache/tuscany/sca/node/impl/NodeFactoryImpl.java
index 41531a7b10..db0c44ad39 100644
--- a/java/sca/modules/node-impl/src/main/java/org/apache/tuscany/sca/node/impl/NodeFactoryImpl.java
+++ b/java/sca/modules/node-impl/src/main/java/org/apache/tuscany/sca/node/impl/NodeFactoryImpl.java
@@ -433,9 +433,11 @@ public class NodeFactoryImpl extends NodeFactory {
         systemDefinitions = definitionsFactory.createDefinitions();
 
         DefinitionsExtensionPoint definitionsExtensionPoint = extensionPoints.getExtensionPoint(DefinitionsExtensionPoint.class);
+        monitor.pushContext("Extension points definitions");
         for(Definitions defs: definitionsExtensionPoint.getDefinitions()) {
-            DefinitionsUtil.aggregate(defs, systemDefinitions);
+            DefinitionsUtil.aggregate(defs, systemDefinitions, monitor);
         }
+        monitor.popContext();
 
         // create a system contribution to hold the definitions. The contribution
         // will be extended later with definitions from application contributions
@@ -482,11 +484,14 @@ public class NodeFactoryImpl extends NodeFactory {
         // each contribution so that for unresolved items the resolution
         // processing will look in the system contribution
         for (Contribution contribution: contributions) {
+            monitor.pushContext("Contribution: " + contribution.getURI());
             // aggregate definitions
             for (Artifact artifact : contribution.getArtifacts()) {
                 Object model = artifact.getModel();
                 if (model instanceof Definitions) {
-                    DefinitionsUtil.aggregate((Definitions)model, systemDefinitions);
+                    monitor.pushContext("Definitions: " + artifact.getLocation());
+                    DefinitionsUtil.aggregate((Definitions)model, systemDefinitions, monitor);
+                    monitor.popContext();
                 }
             }
 
@@ -497,6 +502,7 @@ public class NodeFactoryImpl extends NodeFactory {
             DefaultImport defaultImport = contributionFactory.createDefaultImport();
             defaultImport.setModelResolver(systemContribution.getModelResolver());
             contribution.getImports().add(defaultImport);
+            monitor.popContext();
         }
 
         ExtensibleModelResolver modelResolver = new ExtensibleModelResolver(new Contributions(contributions), modelResolvers, modelFactories);