Revert "This closes apache#772"

This reverts commit 9795c47, reversing
changes made to c694439.

Author: graeme.miller

camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java

@@ -20,7 +20,6 @@
 
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
-import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.entity.stock.BasicEntity;
 import org.apache.brooklyn.test.Asserts;
@@ -56,44 +55,12 @@ public void testAddTypes() throws Exception {
         Asserts.assertEquals(item, Iterables.getOnlyElement(itemsInstalled), "Wrong item; installed: "+itemsInstalled);
     }
 
-    @Test // test disabled as "broken" in super but works here
+    @Test // test broken in super works here
+    // TODO" comment at https://issues.apache.org/jira/browse/BROOKLYN-343
     public void testSameCatalogReferences() {
         super.testSameCatalogReferences();
     }
-
-    @Test
-    public void testUpdatingItemAllowedIfEquivalentUnderRewrite() {
-        String symbolicName = "my.catalog.app.id.duplicate";
-        // forward reference supported here (but not in super)
-        // however the plan is rewritten meaning a second install requires special comparison
-        // (RegisteredTypes "equivalent plan" methods)
-        addForwardReferencePlan(symbolicName);
-
-        // delete one but not the other to prevent resolution and thus rewrite until later validation phase,
-        // thus initial addition will compare unmodified plan from here against modified plan added above;
-        // replacement will then succeed only if we've correctly recorded equivalence tags 
-        deleteCatalogEntity("forward-referenced-entity");
 
-        addForwardReferencePlan(symbolicName);
-    }
-
-    protected void addForwardReferencePlan(String symbolicName) {
-        addCatalogItems(
-            "brooklyn.catalog:",
-            "  id: " + symbolicName,
-            "  version: " + TEST_VERSION,
-            "  itemType: entity",
-            "  items:",
-            "  - id: " + symbolicName,
-            "    itemType: entity",
-            "    item:",
-            "      type: forward-referenced-entity",
-            "  - id: " + "forward-referenced-entity",
-            "    itemType: entity",
-            "    item:",
-            "      type: " + TestEntity.class.getName());
-    }
-    
     // also runs many other tests from super, here using the osgi/type-registry appraoch
 
 }

camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java

@@ -571,6 +571,7 @@ public void testSameCatalogReferences() {
             "          spec: ",
             "            $brooklyn:entitySpec:",
             "              type: referenced-entity");
+
     }
 
     @Test

core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java

@@ -907,16 +907,12 @@ private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, Managed
                 sourcePlanYaml = planInterpreter.itemYaml;
             }
 
-            BasicTypeImplementationPlan plan = new BasicTypeImplementationPlan(format, sourcePlanYaml);
             BasicRegisteredType type = (BasicRegisteredType) RegisteredTypes.newInstance(
                 RegisteredTypeKind.UNRESOLVED,
-                symbolicName, version, plan,
+                symbolicName, version, new BasicTypeImplementationPlan(format, sourcePlanYaml),
                 superTypes, aliases, tags, containingBundle==null ? null : containingBundle.getVersionedName().toString(), 
                 MutableList.<OsgiBundleWithUrl>copyOf(libraryBundles), 
                 displayName, description, catalogIconUrl, catalogDeprecated, catalogDisabled);
-            RegisteredTypes.notePlanEquivalentToThis(type, plan);
-            // record original source in case it was changed
-            RegisteredTypes.notePlanEquivalentToThis(type, new BasicTypeImplementationPlan(format, sourceYaml));
 
             ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(type, force);
 
@@ -1585,7 +1581,7 @@ public ReferenceWithError<RegisteredType> resolve(RegisteredType typeToValidate)
                 }
 
                 if (!Objects.equal(guesser.getPlanYaml(), yaml)) {
-                    RegisteredTypes.changePlanNotingEquivalent(resultT, 
+                    RegisteredTypes.changePlan(resultT, 
                         new BasicTypeImplementationPlan(null /* CampTypePlanTransformer.FORMAT */, guesser.getPlanYaml()));
                     changedSomething = true;
                 }

core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java

@@ -367,7 +367,7 @@ private boolean assertSameEnoughToAllowReplacing(RegisteredType oldType, Registe
         }
         if (Objects.equals(oldType.getContainingBundle(), type.getContainingBundle())) {
             // if named bundles equal then contents must be the same (due to bundle checksum); bail out early
-            if (!samePlan(oldType, type)) {
+            if (!oldType.getPlan().equals(type.getPlan())) {
                 String msg = "Cannot add "+type+" to catalog; different plan in "+oldType+" from same bundle "+
                     type.getContainingBundle()+" is already present";
                 log.debug(msg+"\n"+
@@ -383,7 +383,7 @@ private boolean assertSameEnoughToAllowReplacing(RegisteredType oldType, Registe
         }
 
         // different bundles, either anonymous or same item in two named bundles
-        if (!samePlan(oldType, type)) {
+        if (!oldType.getPlan().equals(type.getPlan())) {
             // if plan is different, fail
             String msg = "Cannot add "+type+" in "+type.getContainingBundle()+" to catalog; different plan in "+oldType+" from bundle "+
                 oldType.getContainingBundle()+" is already present (throwing)";
@@ -425,10 +425,6 @@ private boolean assertSameEnoughToAllowReplacing(RegisteredType oldType, Registe
             + "item  is already present in different bundle "+oldType.getContainingBundle());
     }
 
-    private boolean samePlan(RegisteredType oldType, RegisteredType type) {
-        return RegisteredTypes.arePlansEquivalent(oldType, type);
-    }
-
     @Beta // API stabilising
     public void delete(VersionedName type) {
         RegisteredType registeredTypeRemoved = localRegisteredTypes.remove(type.toString());

core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java

@@ -53,7 +53,6 @@
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.guava.Maybe.Absent;
-import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.NaturalOrderComparator;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.text.VersionComparator;
@@ -63,7 +62,6 @@
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
-import com.google.common.base.Objects;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ComparisonChain;
@@ -601,57 +599,11 @@ public static String getIconUrl(BrooklynObject object) {
         return item.getIconUrl();
     }
 
-    /** @deprecated since 0.12.0 see {@link #changePlanNotingEquivalent(RegisteredType, TypeImplementationPlan)} */
-    @Deprecated
     public static RegisteredType changePlan(RegisteredType type, TypeImplementationPlan plan) {
         ((BasicRegisteredType)type).implementationPlan = plan;
         return type;
     }
-    
-    /** Changes the plan set on the given type, returning it,
-     * and also recording that comparisons checking {@link #arePlansEquivalent(RegisteredType, RegisteredType)}
-     * should consider the two plans equivalent.
-     */
-    @Beta  // would prefer not to need this, but currently it is needed due to how resolver rewrites plan
-           // and we then check plan equality when considering a re-install, else we spuriously fail on
-           // identical re-installs (eg with dns-etc-hosts-generator)
-    public static RegisteredType changePlanNotingEquivalent(RegisteredType type, TypeImplementationPlan plan) {
-        RegisteredTypes.notePlanEquivalentToThis(type, type.getPlan());
-        RegisteredTypes.notePlanEquivalentToThis(type, plan);
-        ((BasicRegisteredType)type).implementationPlan = plan;
-        return type;
-    }
-    
-    private static String tagForEquivalentPlan(String input) {
-        // plans may be trimmed by yaml parser so do that before checking equivalence
-        // it does mean a format change will be ignored
-        return "equivalent-plan("+Streams.getMd5Checksum(Streams.newInputStreamWithContents(input.trim()))+")";
-    }
-    
-    @Beta
-    public static void notePlanEquivalentToThis(RegisteredType type, TypeImplementationPlan plan) {
-        Object data = plan.getPlanData();
-        if (data==null) throw new IllegalStateException("No plan data for "+plan+" noted equivalent to "+type);
-        if (!(data instanceof String)) throw new IllegalStateException("Expected plan for equivalent to "+type+" to be a string; was "+data);
-        ((BasicRegisteredType)type).tags.add(tagForEquivalentPlan((String)data));
-    }
 
-    @Beta
-    public static boolean arePlansEquivalent(RegisteredType type1, RegisteredType type2) {
-        String plan1 = getImplementationDataStringForSpec(type1);
-        String plan2 = getImplementationDataStringForSpec(type2);
-        if (Strings.isNonBlank(plan1) && Strings.isNonBlank(plan2)) {
-            String p2tag = tagForEquivalentPlan(plan2);
-            String p1tag = tagForEquivalentPlan(plan1);
-            // allow same plan under trimming,
-            // or any recorded tag in either direction
-            if (Objects.equal(p1tag, p2tag)) return true;
-            if (type1.getTags().contains(p2tag)) return true;
-            if (type2.getTags().contains(p1tag)) return true;
-        }
-        return Objects.equal(type1.getPlan(), type2.getPlan());
-    }
-    
     public static boolean isTemplate(RegisteredType type) {
         if (type==null) return false;
         return type.getTags().contains(BrooklynTags.CATALOG_TEMPLATE);

rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java

@@ -175,8 +175,10 @@ public void testRegisterCustomEntityTopLevelSyntaxWithBundleWhereEntityIsFromCor
         assertEquals(item.getIconUrl(), "classpath:/org/apache/brooklyn/test/osgi/entities/icon.gif");
 
         // an InterfacesTag should be created for every catalog item
-        Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
-        List<String> actualInterfaces = traitsMapTag.get("traits");
+        assertEquals(entityItem.getTags().size(), 1);
+        Object tag = entityItem.getTags().iterator().next();
+        @SuppressWarnings("unchecked")
+        List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
         List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class);
         assertEquals(actualInterfaces.size(), expectedInterfaces.size());
         for (Class<?> expectedInterface : expectedInterfaces) {
@@ -811,9 +813,10 @@ public void testOsgiBundleWithBom() throws Exception {
         assertEquals(item.getIconUrl(), "classpath:/org/apache/brooklyn/test/osgi/entities/icon.gif");
 
         // an InterfacesTag should be created for every catalog item
+        assertEquals(entityItem.getTags().size(), 1);
+        Object tag = entityItem.getTags().iterator().next();
         @SuppressWarnings("unchecked")
-        Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
-        List<String> actualInterfaces = traitsMapTag.get("traits");
+        List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
         List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class);
         assertEquals(actualInterfaces.size(), expectedInterfaces.size());
         for (Class<?> expectedInterface : expectedInterfaces) {
@@ -886,8 +889,10 @@ public void testOsgiBundleWithBomNotInBrooklynNamespace() throws Exception {
         assertEquals(item.getIconUrl(), "classpath:" + iconPath);
 
         // an InterfacesTag should be created for every catalog item
-        Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
-        List<String> actualInterfaces = traitsMapTag.get("traits");
+        assertEquals(entityItem.getTags().size(), 1);
+        Object tag = entityItem.getTags().iterator().next();
+        @SuppressWarnings("unchecked")
+        List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
         List<String> expectedInterfaces = ImmutableList.of(Entity.class.getName(), BrooklynObject.class.getName(), Identifiable.class.getName(), Configurable.class.getName());
         assertTrue(actualInterfaces.containsAll(expectedInterfaces), "actual="+actualInterfaces);