You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2017/07/21 23:54:44 UTC
[1/2] brooklyn-server git commit: record a checksum so that on plan
rewrite we don't block reinstallation
Repository: brooklyn-server
Updated Branches:
refs/heads/master c69443942 -> 9795c4746
record a checksum so that on plan rewrite we don't block reinstallation
with test case that fails without the equivalent plan checks, but works with them
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bb9d1f89
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bb9d1f89
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bb9d1f89
Branch: refs/heads/master
Commit: bb9d1f89e495b39a59cd7a9c05726e8500b19d7f
Parents: c694439
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri Jul 21 10:10:11 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri Jul 21 23:22:28 2017 +0100
----------------------------------------------------------------------
.../CatalogYamlEntityOsgiTypeRegistryTest.java | 37 ++++++++++++++-
.../brooklyn/catalog/CatalogYamlEntityTest.java | 1 -
.../catalog/internal/BasicBrooklynCatalog.java | 8 +++-
.../core/typereg/BasicBrooklynTypeRegistry.java | 8 +++-
.../brooklyn/core/typereg/RegisteredTypes.java | 48 ++++++++++++++++++++
.../rest/resources/CatalogResourceTest.java | 17 +++----
6 files changed, 101 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
index ac11898..35c074d 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.camp.brooklyn.catalog;
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;
@@ -55,12 +56,44 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends CatalogYamlEntityTest
Asserts.assertEquals(item, Iterables.getOnlyElement(itemsInstalled), "Wrong item; installed: "+itemsInstalled);
}
- @Test // test broken in super works here
- // TODO" comment at https://issues.apache.org/jira/browse/BROOKLYN-343
+ @Test // test disabled as "broken" in super but works here
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
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index 9d907d9..4633817 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@ -571,7 +571,6 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
" spec: ",
" $brooklyn:entitySpec:",
" type: referenced-entity");
-
}
@Test
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
index 9eea402..affb4e7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
@@ -907,12 +907,16 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
sourcePlanYaml = planInterpreter.itemYaml;
}
+ BasicTypeImplementationPlan plan = new BasicTypeImplementationPlan(format, sourcePlanYaml);
BasicRegisteredType type = (BasicRegisteredType) RegisteredTypes.newInstance(
RegisteredTypeKind.UNRESOLVED,
- symbolicName, version, new BasicTypeImplementationPlan(format, sourcePlanYaml),
+ symbolicName, version, plan,
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);
@@ -1581,7 +1585,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
}
if (!Objects.equal(guesser.getPlanYaml(), yaml)) {
- RegisteredTypes.changePlan(resultT,
+ RegisteredTypes.changePlanNotingEquivalent(resultT,
new BasicTypeImplementationPlan(null /* CampTypePlanTransformer.FORMAT */, guesser.getPlanYaml()));
changedSomething = true;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
index bd2b056..60142a7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
@@ -367,7 +367,7 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry {
}
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 (!oldType.getPlan().equals(type.getPlan())) {
+ if (!samePlan(oldType, type)) {
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 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry {
}
// different bundles, either anonymous or same item in two named bundles
- if (!oldType.getPlan().equals(type.getPlan())) {
+ if (!samePlan(oldType, type)) {
// 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,6 +425,10 @@ public class BasicBrooklynTypeRegistry implements BrooklynTypeRegistry {
+ "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());
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
index 6519edf..360df02 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
@@ -53,6 +53,7 @@ import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer.JavaCla
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;
@@ -62,6 +63,7 @@ import org.slf4j.LoggerFactory;
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;
@@ -599,11 +601,57 @@ public class RegisteredTypes {
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);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb9d1f89/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
index 0a91d59..abdd1ed 100644
--- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
+++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java
@@ -175,10 +175,8 @@ public class CatalogResourceTest extends BrooklynRestResourceTest {
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")
- List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
+ Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
+ List<String> actualInterfaces = traitsMapTag.get("traits");
List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class);
assertEquals(actualInterfaces.size(), expectedInterfaces.size());
for (Class<?> expectedInterface : expectedInterfaces) {
@@ -813,10 +811,9 @@ public class CatalogResourceTest extends BrooklynRestResourceTest {
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")
- List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
+ Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
+ List<String> actualInterfaces = traitsMapTag.get("traits");
List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class);
assertEquals(actualInterfaces.size(), expectedInterfaces.size());
for (Class<?> expectedInterface : expectedInterfaces) {
@@ -889,10 +886,8 @@ public class CatalogResourceTest extends BrooklynRestResourceTest {
assertEquals(item.getIconUrl(), "classpath:" + iconPath);
// an InterfacesTag should be created for every catalog item
- assertEquals(entityItem.getTags().size(), 1);
- Object tag = entityItem.getTags().iterator().next();
- @SuppressWarnings("unchecked")
- List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits");
+ Map<String, List<String>> traitsMapTag = Iterables.getOnlyElement(Iterables.filter(entityItem.getTags(), Map.class));
+ List<String> actualInterfaces = traitsMapTag.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);
[2/2] brooklyn-server git commit: This closes #772
Posted by he...@apache.org.
This closes #772
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/9795c474
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/9795c474
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/9795c474
Branch: refs/heads/master
Commit: 9795c4746dfeaef484f8a26af51cd0ce6eddbe90
Parents: c694439 bb9d1f8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Sat Jul 22 00:54:33 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sat Jul 22 00:54:33 2017 +0100
----------------------------------------------------------------------
.../CatalogYamlEntityOsgiTypeRegistryTest.java | 37 ++++++++++++++-
.../brooklyn/catalog/CatalogYamlEntityTest.java | 1 -
.../catalog/internal/BasicBrooklynCatalog.java | 8 +++-
.../core/typereg/BasicBrooklynTypeRegistry.java | 8 +++-
.../brooklyn/core/typereg/RegisteredTypes.java | 48 ++++++++++++++++++++
.../rest/resources/CatalogResourceTest.java | 17 +++----
6 files changed, 101 insertions(+), 18 deletions(-)
----------------------------------------------------------------------