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/04/21 22:55:57 UTC
[38/39] brooklyn-server git commit: minor cleanups following #338
minor cleanups following #338
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/88a09bae
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/88a09bae
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/88a09bae
Branch: refs/heads/master
Commit: 88a09bae443548cab24bd585b7e84f40125b0d4c
Parents: a65aae7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri Apr 21 17:07:34 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri Apr 21 23:45:53 2017 +0100
----------------------------------------------------------------------
.../internal/AbstractBrooklynObjectSpec.java | 55 ++++++++++--------
.../brooklyn/api/objs/BrooklynObject.java | 9 ++-
.../brooklyn/catalog/CatalogYamlEntityTest.java | 39 +++++--------
.../core/catalog/internal/CatalogItemDo.java | 5 ++
.../core/mgmt/rebind/RebindIteration.java | 9 ++-
.../core/objs/AbstractBrooklynObject.java | 59 ++++++++++++--------
.../core/objs/BrooklynObjectInternal.java | 1 +
7 files changed, 98 insertions(+), 79 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
index d88078a..adbb48b 100644
--- a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
+++ b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
@@ -22,10 +22,8 @@ import static com.google.common.base.Preconditions.checkNotNull;
import java.io.Serializable;
import java.lang.reflect.Modifier;
-import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Collections;
-import java.util.Deque;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -36,6 +34,7 @@ import org.apache.brooklyn.api.objs.BrooklynObject;
import org.apache.brooklyn.api.objs.SpecParameter;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
+import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.slf4j.Logger;
@@ -69,7 +68,7 @@ public abstract class AbstractBrooklynObjectSpec<T, SpecT extends AbstractBrookl
private final Class<? extends T> type;
private String displayName;
private String catalogItemId;
- private Deque<String> catalogItemIdSearchPath = new ArrayDeque<>();
+ private Collection<String> catalogItemIdSearchPath = MutableSet.of();
private Set<Object> tags = MutableSet.of();
private List<SpecParameter<?>> parameters = ImmutableList.of();
@@ -117,16 +116,30 @@ public abstract class AbstractBrooklynObjectSpec<T, SpecT extends AbstractBrookl
* Set the immediate catalog item ID of this object, and the search path of other catalog items used to define it.
*/
public synchronized SpecT catalogItemIdAndSearchPath(String catalogItemId, Collection<String> searchPath) {
+ // TODO if ID is null should we really ignore search path?
if (catalogItemId != null) {
catalogItemId(catalogItemId);
- catalogItemIdSearchPath.clear();
- catalogItemIdSearchPath.addAll(searchPath);
+ synchronized (catalogItemIdSearchPath) {
+ catalogItemIdSearchPath.clear();
+ if (searchPath!=null) {
+ catalogItemIdSearchPath.addAll(searchPath);
+ }
+ }
}
return self();
}
+ public synchronized SpecT addSearchPath(List<String> searchPath) {
+ if (searchPath!=null) {
+ synchronized (catalogItemIdSearchPath) {
+ catalogItemIdSearchPath.addAll(searchPath);
+ }
+ }
+ return self();
+ }
+
/**
- * @deprecated since 0.11.0, use {@link #stackCatalogItemId(String)} instead
+ * @deprecated since 0.11.0, most callers would want {@link #stackCatalogItemId(String)} instead, though semantics are different
*/
@Deprecated
@Beta
@@ -139,7 +152,7 @@ public abstract class AbstractBrooklynObjectSpec<T, SpecT extends AbstractBrookl
protected Object readResolve() {
if (catalogItemIdSearchPath == null) {
- catalogItemIdSearchPath = new ArrayDeque<>();
+ catalogItemIdSearchPath = MutableList.of();
}
return this;
}
@@ -160,7 +173,13 @@ public abstract class AbstractBrooklynObjectSpec<T, SpecT extends AbstractBrookl
public SpecT stackCatalogItemId(String val) {
if (null != val) {
if (null != catalogItemId && !catalogItemId.equals(val)) {
- catalogItemIdSearchPath.addFirst(catalogItemId);
+ synchronized (catalogItemIdSearchPath) {
+ Set<String> newPath = MutableSet.of();
+ newPath.add(catalogItemId);
+ newPath.addAll(catalogItemIdSearchPath);
+ catalogItemIdSearchPath.clear();
+ catalogItemIdSearchPath.addAll(newPath);
+ }
}
catalogItemId(val);
}
@@ -253,26 +272,16 @@ public abstract class AbstractBrooklynObjectSpec<T, SpecT extends AbstractBrookl
return displayName;
}
+ /** Same as {@link BrooklynObject#getCatalogItemId()}. */
public final String getCatalogItemId() {
return catalogItemId;
}
- /**
- * An immutable list of ids of catalog items that define this item.
- * Wrapping items are first in the list (i.e. wrapping items precede wrapped items),
- * for example, for a spec of an item of type Z, where the catalog is:
- * <pre>
- * items:
- * - id: X
- * - id: Y
- * item: X
- * - id: Z
- * item: Y
- * </pre>
- * then the spec will have getCatalogId() of Z and getCatalogItemIdSearchPath() of Y, X.
- */
+ /** Same as {@link BrooklynObject#getCatalogItemIdSearchPath()}. */
public final List<String> getCatalogItemIdSearchPath() {
- return ImmutableList.copyOf(catalogItemIdSearchPath);
+ synchronized (catalogItemIdSearchPath) {
+ return ImmutableList.copyOf(catalogItemIdSearchPath);
+ }
}
public final Set<Object> getTags() {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java
index d72fe6a..265813a 100644
--- a/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java
+++ b/api/src/main/java/org/apache/brooklyn/api/objs/BrooklynObject.java
@@ -60,8 +60,10 @@ public interface BrooklynObject extends Identifiable, Configurable {
String getCatalogItemId();
/**
- * An immutable list of ids of catalog items that define this item.
- * e.g. if the catalog item is defined as a Z where
+ * An immutable list of ids of catalog items that this item depends on in some way,
+ * with the item that directly defines it implicit, but other items it references explicit.
+ * Wrapping items are first in the list (i.e. wrapping items precede wrapped items),
+ * so for example, if the catalog is:
* <pre>
* items:
* - id: X
@@ -70,7 +72,8 @@ public interface BrooklynObject extends Identifiable, Configurable {
* - id: Z
* item: Y
* </pre>
- * then the list will contain X, Y.
+ * the spec for Z will have getCatalogId() of Z and getCatalogItemIdSearchPath() of Y, X.
+ * (The self catalog ID is implicit at the head of the search path.)
*/
List<String> getCatalogItemIdSearchPath();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/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 847e461..bc9f9ad 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
@@ -22,7 +22,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
-import static org.testng.Assert.fail;
import org.apache.brooklyn.api.catalog.BrooklynCatalog;
import org.apache.brooklyn.api.entity.Entity;
@@ -39,8 +38,7 @@ import org.apache.brooklyn.core.test.entity.TestEntityImpl;
import org.apache.brooklyn.core.typereg.RegisteredTypes;
import org.apache.brooklyn.entity.stock.BasicApplication;
import org.apache.brooklyn.entity.stock.BasicEntity;
-import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.osgi.OsgiTestResources;
+import org.apache.brooklyn.test.Asserts;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -50,15 +48,6 @@ import com.google.common.collect.Iterables;
public class CatalogYamlEntityTest extends AbstractYamlTest {
-
- private static final String SIMPLE_ENTITY_TYPE = OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_SIMPLE_ENTITY;
- private static final String MORE_ENTITIES_POM_PROPERTIES_PATH =
- "META-INF/maven/org.apache.brooklyn.test.resources.osgi/brooklyn-test-osgi-more-entities/pom.properties";
-
- @Override
- protected boolean disableOsgi() {
- return false;
- }
@Test
public void testAddCatalogItemVerySimple() throws Exception {
@@ -205,7 +194,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
RegisteredType referrer = mgmt().getTypeRegistry().get(referrerSymbolicName, TEST_VERSION);
String planYaml = RegisteredTypes.getImplementationDataStringForSpec(referrer);
- Assert.assertTrue(planYaml.contains("services"), "expected services in: "+planYaml);
+ Asserts.assertStringContains(planYaml, "services");
Entity app = createAndStartApplication("services:",
"- type: " + ver(referrerSymbolicName, TEST_VERSION));
@@ -334,10 +323,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
" - type: " + BasicEntity.class.getName(),
" brooklyn.children:",
" - type: " + ver(referrerSymbolicName, TEST_VERSION));
- fail("Expected to throw");
+ Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
- Exceptions.propagateIfFatal(e);
- assertTrue(e.getMessage().contains(referrerSymbolicName), "message was: "+e);
+ Asserts.expectedFailureContains(e, referrerSymbolicName);
}
}
@@ -384,9 +372,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
try {
addCatalogEntity(IdAndVersion.of(symbolicName, TEST_VERSION + "-update"), symbolicName);
- fail("Catalog addition expected to fail due to recursive reference to " + symbolicName);
+ Asserts.shouldHaveFailedPreviously("Catalog addition expected to fail due to recursive reference to " + symbolicName);
} catch (IllegalStateException e) {
- assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e);
+ Asserts.expectedFailureContains(e, "recursive", symbolicName);
}
}
@@ -400,9 +388,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
String versionedId = CatalogUtils.getVersionedId(symbolicName, TEST_VERSION);
try {
addCatalogEntity(IdAndVersion.of(symbolicName, TEST_VERSION + "-update"), versionedId);
- fail("Catalog addition expected to fail due to recursive reference to " + versionedId);
+ Asserts.shouldHaveFailedPreviously("Catalog addition expected to fail due to recursive reference to " + versionedId);
} catch (IllegalStateException e) {
- assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e);
+ Asserts.expectedFailureContains(e, "recursive", symbolicName, versionedId);
}
}
@@ -419,9 +407,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
try {
addCatalogEntity(IdAndVersion.of(callerSymbolicName, TEST_VERSION), calleeSymbolicName);
- fail();
+ Asserts.shouldHaveFailedPreviously();
} catch (IllegalStateException e) {
- assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e);
+ Asserts.expectedFailureContains(e, "recursive");
}
}
@@ -450,9 +438,9 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
" - type: " + BasicEntity.class.getName(),
" brooklyn.children:",
" - type: " + calleeSymbolicName);
- fail();
+ Asserts.shouldHaveFailedPreviously();
} catch (IllegalStateException e) {
- assertTrue(e.toString().contains("recursive"), "Unexpected error message: "+e);
+ Asserts.expectedFailureContains(e, "recursive");
}
}
@@ -606,7 +594,8 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
String yaml = Joiner.on("\n").join(
"name: simple-app-yaml",
- "location: localhost",
+ "location:",
+ "localhost: { latitude: 0, longitude: 0 }", // prevent host geo lookup delay (slowing down test on my network)
"services:",
" - type: "+id+":"+version);
Entity app = createAndStartApplication(yaml);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java
index 0738d76..c263e59 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java
@@ -135,6 +135,11 @@ public class CatalogItemDo<T,SpecT> implements CatalogItem<T,SpecT>, BrooklynObj
public void setCatalogItemIdAndSearchPath(String catalogItemId, List<String> ids) {
itemDto.setCatalogItemIdAndSearchPath(catalogItemId, ids);
}
+
+ @Override
+ public void addSearchPath(List<String> searchPath) {
+ itemDto.addSearchPath(searchPath);
+ }
@Override
public List<String> getCatalogItemIdSearchPath() {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
index e4fbaa6..ad12c52 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
@@ -1124,8 +1124,7 @@ public abstract class RebindIteration {
*/
protected Policy newPolicy(PolicyMemento memento) {
String id = memento.getId();
- LoadedClass<? extends Policy> loaded = load(Policy.class, memento.getType(), memento.getCatalogItemId(),
- memento.getCatalogItemIdSearchPath(), id);
+ LoadedClass<? extends Policy> loaded = load(Policy.class, memento);
Class<? extends Policy> policyClazz = loaded.clazz;
Policy policy;
@@ -1151,7 +1150,7 @@ public abstract class RebindIteration {
policy = invokeConstructor(null, policyClazz, new Object[] {flags});
}
- setCatalogItemIds(policy, memento.getCatalogItemId(), memento.getCatalogItemIdSearchPath());
+ setCatalogItemIds(policy, loaded.catalogItemId, loaded.searchPath);
return policy;
}
@@ -1186,7 +1185,7 @@ public abstract class RebindIteration {
enricher = invokeConstructor(reflections, enricherClazz, new Object[] {flags});
}
- setCatalogItemIds(enricher, memento.getCatalogItemId(), memento.getCatalogItemIdSearchPath());
+ setCatalogItemIds(enricher, loaded.catalogItemId, loaded.searchPath);
return enricher;
}
@@ -1210,7 +1209,7 @@ public abstract class RebindIteration {
"; type="+feedClazz);
}
- setCatalogItemIds(feed, memento.getCatalogItemId(), memento.getCatalogItemIdSearchPath());
+ setCatalogItemIds(feed, loaded.catalogItemId, loaded.searchPath);
return feed;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java
index 9a409ca..9ba5a61 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java
@@ -18,23 +18,12 @@
*/
package org.apache.brooklyn.core.objs;
-import java.util.ArrayDeque;
+import java.util.Collection;
import java.util.Collections;
-import java.util.Deque;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import com.google.common.collect.ImmutableList;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
-
import org.apache.brooklyn.api.internal.ApiObjectsFactory;
import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.core.entity.AbstractEntity;
@@ -42,9 +31,19 @@ import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl;
import org.apache.brooklyn.core.objs.proxy.InternalFactory;
import org.apache.brooklyn.core.relations.ByObjectBasicRelationSupport;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.core.flags.SetFromFlag;
import org.apache.brooklyn.util.text.Identifiers;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
@@ -57,7 +56,7 @@ public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
private String id = Identifiers.makeRandomLowercaseId(10);
private String catalogItemId;
- private Deque<String> searchPath = new ArrayDeque<>();
+ private Collection<String> searchPath = MutableSet.of();
/** callers (only in TagSupport) should synchronize on this for all access */
@SetFromFlag("tags")
@@ -86,18 +85,15 @@ public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
_legacyConstruction = true;
}
- // TODO this will be overridden if the spec has a catalog item ID
- // correct behaviour should be to inherit context's search path, perhaps, though maybe that's better done as spec?
- // in any case, should not define it as _the_ catalog item ID; also see assignment based on parent
- // in CatalogUtils.setCatalogItemIdOnAddition
- setCatalogItemId(ApiObjectsFactory.get().getCatalogItemIdFromContext());
+ // inherit any context for search purposes (though maybe that should be done when creating the spec?)
+ addSearchPath(MutableList.<String>of().appendIfNotNull(ApiObjectsFactory.get().getCatalogItemIdFromContext()));
// rely on sub-class to call configure(properties), because otherwise its fields will not have been initialised
}
protected Object readResolve() {
if (searchPath == null) {
- searchPath = new ArrayDeque<>();
+ searchPath = MutableList.of();
}
return this;
}
@@ -210,22 +206,39 @@ public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
@Override
public void setCatalogItemIdAndSearchPath(String catalogItemId, List<String> ids) {
setCatalogItemId(catalogItemId);
- searchPath.clear();
- searchPath.addAll(ids);
+ synchronized (searchPath) {
+ searchPath.clear();
+ searchPath.addAll(ids);
+ }
+ }
+
+ @Override
+ public void addSearchPath(List<String> ids) {
+ synchronized (searchPath) {
+ searchPath.addAll(ids);
+ }
}
@Override
public void stackCatalogItemId(String id) {
if (null != id) {
if (null != catalogItemId && !catalogItemId.equals(id)) {
- searchPath.addFirst(catalogItemId);
+ synchronized (searchPath) {
+ Set<String> newPath = MutableSet.of();
+ newPath.add(catalogItemId);
+ newPath.addAll(searchPath);
+ searchPath.clear();
+ searchPath.addAll(newPath);
+ }
}
setCatalogItemId(id);
}
}
public List<String> getCatalogItemIdSearchPath() {
- return ImmutableList.copyOf(searchPath);
+ synchronized (searchPath) {
+ return ImmutableList.copyOf(searchPath);
+ }
}
@Override
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/88a09bae/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java
index d7fe17b..44ed3b8 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java
@@ -39,6 +39,7 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable {
void setCatalogItemId(String id);
void setCatalogItemIdAndSearchPath(String catalogItemId, List<String> searchPath);
+ void addSearchPath(List<String> searchPath);
/**
* Moves the current catalog item id onto the start of the search path,