make LocationRegistry be a facade to TypeRegistry for most things
instead of the messy old way where catalog added/removed in the location
registry;
means it works for things added directly to TypeRegistry now (and those
tests pass)
one minor change to backwards compatibility, the internal
LocationDefinition ID now contains the version.
it isn't used and in other cases is a random string anyway. this will fix
bugs where mulitple versions
of the same location would result in just one being returned.
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/c5af5495
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/c5af5495
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/c5af5495
Branch: refs/heads/master
Commit: c5af5495549829462f8658468897a5d064398838
Parents: b016e67
Author: Alex Heneveld <[email protected]>
Authored: Mon Jul 3 14:03:29 2017 +0100
Committer: Alex Heneveld <[email protected]>
Committed: Mon Jul 3 17:30:26 2017 +0100
----------------------------------------------------------------------
.../brooklyn/api/location/LocationRegistry.java | 20 ++-
.../catalog/CatalogOsgiYamlLocationTest.java | 10 +-
.../catalog/CatalogYamlLocationTest.java | 21 ++-
.../catalog/internal/BasicBrooklynCatalog.java | 11 +-
.../core/location/BasicLocationRegistry.java | 168 ++++++++++---------
.../core/typereg/BasicBrooklynTypeRegistry.java | 14 +-
.../core/location/LocationManagementTest.java | 10 +-
.../core/location/LocationRegistryTest.java | 13 +-
.../command/support/CloudExplorerSupport.java | 2 +-
.../rest/resources/LocationResource.java | 2 +-
.../rest/resources/LocationResourceTest.java | 4 +-
11 files changed, 154 insertions(+), 121 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/api/src/main/java/org/apache/brooklyn/api/location/LocationRegistry.java
----------------------------------------------------------------------
diff --git
a/api/src/main/java/org/apache/brooklyn/api/location/LocationRegistry.java
b/api/src/main/java/org/apache/brooklyn/api/location/LocationRegistry.java
index 7804afc..bbf6f52 100644
--- a/api/src/main/java/org/apache/brooklyn/api/location/LocationRegistry.java
+++ b/api/src/main/java/org/apache/brooklyn/api/location/LocationRegistry.java
@@ -27,6 +27,7 @@ import javax.annotation.Nullable;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.mgmt.LocationManager;
+import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
import org.apache.brooklyn.util.guava.Maybe;
/**
@@ -37,19 +38,32 @@ import org.apache.brooklyn.util.guava.Maybe;
public interface LocationRegistry {
/** map of ID (possibly randomly generated) to the definition (spec, name,
id, and props;
- * where spec is the spec as defined, for instance possibly another
named:xxx location) */
+ * where spec is the spec as defined, for instance possibly another
named:xxx location).
+ * optionally will also return all things from other sources where this
can look up. */
+ public Map<String,LocationDefinition> getDefinedLocations(boolean
includeThingsWeAreFacadeFor);
+
+ /** @deprecated since 0.12.0 use {@link #getDefinedLocations(boolean)}
passing <code>true</code>.
+ * some clients might only want the things actually defined here, which is
cheaper. */
+ @Deprecated
public Map<String,LocationDefinition> getDefinedLocations();
/** returns a LocationDefinition given its ID (usually a random string),
or null if none */
public LocationDefinition getDefinedLocationById(String id);
/** returns a LocationDefinition given its name (e.g. for named locations,
supply the bit after the "named:" prefix),
- * or null if none */
+ * looking inthe {@link BrooklynTypeRegistry} for registered items, then
in this list, or null if none */
public LocationDefinition getDefinedLocationByName(String name);
- /** adds or updates the given defined location */
+ /** as {@link #updateDefinedLocationNonPersisted(LocationDefinition)}
+ * @deprecated since 0.12.0 use {@link
#updateDefinedLocationNonPersisted(LocationDefinition)};
+ * it's exactly the same, just the name makes it clear */
+ @Deprecated
public void updateDefinedLocation(LocationDefinition l);
+ /** adds or updates the given defined location in this registry; note it
is not persisted;
+ * callers should consider adding to the {@link BrooklynTypeRegistry}
instead */
+ public void updateDefinedLocationNonPersisted(LocationDefinition l);
+
/** removes the defined location from the registry (applications running
there are unaffected) */
public void removeDefinedLocation(String id);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlLocationTest.java
----------------------------------------------------------------------
diff --git
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlLocationTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlLocationTest.java
index 94b2c50..d986a51 100644
---
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlLocationTest.java
+++
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlLocationTest.java
@@ -75,6 +75,14 @@ public class CatalogOsgiYamlLocationTest extends
AbstractYamlTest {
assertAdded(symbolicName, SIMPLE_LOCATION_TYPE);
assertOsgi(symbolicName);
removeAndAssert(symbolicName);
+
+ // and do it again; the OSGi registry doesn't add it;
+ // it only works because the location registry initializes itself on
first call
+ // by reading from the catalog
+ symbolicName = "my.catalog.location.id.load.2";
+ addCatalogLocation(symbolicName, SIMPLE_LOCATION_TYPE,
getOsgiLibraries());
+ assertAdded(symbolicName, SIMPLE_LOCATION_TYPE);
+ assertOsgi(symbolicName);
}
@Test
@@ -103,7 +111,7 @@ public class CatalogOsgiYamlLocationTest extends
AbstractYamlTest {
// Item added to catalog should automatically be available in location
registry
LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocationByName(symbolicName);
- assertEquals(def.getId(), symbolicName);
+ assertEquals(def.getId(), symbolicName+":"+TEST_VERSION);
assertEquals(def.getName(), symbolicName);
LocationSpec<?> spec = mgmt().getTypeRegistry().createSpec(item, null,
LocationSpec.class);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
----------------------------------------------------------------------
diff --git
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
index 026b2e8..80db3f3 100644
---
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
+++
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
@@ -95,7 +95,6 @@ public class CatalogYamlLocationTest extends AbstractYamlTest
{
// Item added to catalog should automatically be available in location
registry
LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocationByName(symbolicName);
- assertEquals(def.getId(), symbolicName);
assertEquals(def.getName(), symbolicName);
LocationSpec spec = mgmt().getTypeRegistry().createSpec(item, null,
LocationSpec.class);
@@ -171,9 +170,9 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
addCatalogItems(yaml);
- Map<String, LocationDefinition> defs =
mgmt().getLocationRegistry().getDefinedLocations();
- LocationDefinition def1 = checkNotNull(defs.get("loc1"), "loc1
missing; has %s", defs.keySet());
- LocationDefinition def2 = checkNotNull(defs.get("loc2"), "loc2
missing; has %s", defs.keySet());
+ Map<String, LocationDefinition> defs =
mgmt().getLocationRegistry().getDefinedLocations(true);
+ LocationDefinition def1 = checkNotNull(defs.get("loc1:0.1.2"), "loc1
missing; has %s", defs.keySet());
+ LocationDefinition def2 = checkNotNull(defs.get("loc2:0.1.2"), "loc2
missing; has %s", defs.keySet());
LocationSpec<? extends Location> spec1 =
mgmt().getLocationRegistry().getLocationSpec(def1).get();
LocationSpec<? extends Location> spec2 =
mgmt().getLocationRegistry().getLocationSpec(def2).get();
@@ -234,15 +233,15 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
addCatalogItems(yaml2);
addCatalogItems(yaml3);
- LocationDefinition def1 =
mgmt().getLocationRegistry().getDefinedLocations().get("loc1");
+ LocationDefinition def1 =
mgmt().getLocationRegistry().getDefinedLocationByName("loc1");
LocationSpec<? extends Location> spec1 =
mgmt().getLocationRegistry().getLocationSpec(def1).get();
assertEquals(spec1.getDisplayName(), "My name in item metadata");
- LocationDefinition def2 =
mgmt().getLocationRegistry().getDefinedLocations().get("loc2");
+ LocationDefinition def2 =
mgmt().getLocationRegistry().getDefinedLocationByName("loc2");
LocationSpec<? extends Location> spec2 =
mgmt().getLocationRegistry().getLocationSpec(def2).get();
assertEquals(spec2.getDisplayName(), "My name in top-level metadata");
- LocationDefinition def3b =
mgmt().getLocationRegistry().getDefinedLocations().get("loc3b");
+ LocationDefinition def3b =
mgmt().getLocationRegistry().getDefinedLocationByName("loc3b");
LocationSpec<? extends Location> spec3b =
mgmt().getLocationRegistry().getLocationSpec(def3b).get();
assertEquals(spec3b.getDisplayName(), "My name within item 3b");
}
@@ -261,7 +260,7 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
addCatalogItems(yaml);
- LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocations().get("loc1");
+ LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocationByName("loc1");
LocationSpec<? extends Location> spec =
mgmt().getLocationRegistry().getLocationSpec(def).get();
assertEquals(spec.getDisplayName(), "My name in top-level");
}
@@ -280,7 +279,7 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
addCatalogItems(yaml);
- LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocations().get("loc1");
+ LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocationByName("loc1");
LocationSpec<? extends Location> spec =
mgmt().getLocationRegistry().getLocationSpec(def).get();
assertEquals(spec.getDisplayName(), "My name in item metadata");
}
@@ -299,7 +298,7 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
addCatalogItems(yaml);
- LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocations().get("loc1");
+ LocationDefinition def =
mgmt().getLocationRegistry().getDefinedLocationByName("loc1");
LocationSpec<? extends Location> spec =
mgmt().getLocationRegistry().getLocationSpec(def).get();
assertEquals(spec.getDisplayName(), "My name within item");
}
@@ -403,7 +402,7 @@ public class CatalogYamlLocationTest extends
AbstractYamlTest {
}
private void assertLocationRegistryCount(int size) {
-
Asserts.assertThat(mgmt().getLocationRegistry().getDefinedLocations().keySet(),
CollectionFunctionals.sizeEquals(size));
+
Asserts.assertThat(mgmt().getLocationRegistry().getDefinedLocations(true).keySet(),
CollectionFunctionals.sizeEquals(size));
}
private void assertLocationManagerInstancesCount(int size) {
Asserts.assertThat(mgmt().getLocationManager().getLocations(),
CollectionFunctionals.sizeEquals(size));
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/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 0d858e0..40655bc 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
@@ -330,11 +330,6 @@ public class BasicBrooklynCatalog implements
BrooklynCatalog {
if (log.isTraceEnabled()) {
log.trace("Scheduling item for persistence removal: {}",
itemDto.getId());
}
- if (itemDto.getCatalogItemType() == CatalogItemType.LOCATION) {
- @SuppressWarnings("unchecked")
- CatalogItem<Location,LocationSpec<?>> locationItem =
(CatalogItem<Location, LocationSpec<?>>) itemDto;
-
((BasicLocationRegistry)mgmt.getLocationRegistry()).removeDefinedLocation(locationItem);
- }
mgmt.getRebindManager().getChangeListener().onUnmanaged(itemDto);
}
@@ -1629,11 +1624,7 @@ public class BasicBrooklynCatalog implements
BrooklynCatalog {
}
private void onAdditionUpdateOtherRegistries(CatalogItemDtoAbstract<?, ?>
itemDto) {
- if (itemDto.getCatalogItemType() == CatalogItemType.LOCATION) {
- @SuppressWarnings("unchecked")
- CatalogItem<Location,LocationSpec<?>> locationItem =
(CatalogItem<Location, LocationSpec<?>>) itemDto;
-
((BasicLocationRegistry)mgmt.getLocationRegistry()).updateDefinedLocation(locationItem);
- }
+ // nothing needed (previously updated BasicLocationRegistry but now
that is a facade)
}
/** returns item DTO if item is an allowed duplicate, or null if it should
be added (there is no duplicate),
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java
----------------------------------------------------------------------
diff --git
a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java
b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java
index a761513..cbfd140 100644
---
a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java
+++
b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java
@@ -47,6 +47,7 @@ import org.apache.brooklyn.core.internal.BrooklynProperties;
import org.apache.brooklyn.core.location.internal.LocationInternal;
import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager;
import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
import org.apache.brooklyn.location.localhost.LocalhostLocationResolver;
import org.apache.brooklyn.util.collections.MutableList;
@@ -71,74 +72,46 @@ import com.google.common.collect.Sets;
/**
* See {@link LocationRegistry} for general description.
* <p>
- * TODO The relationship between the catalog and the location registry is a
bit messy.
- * For all existing code, the location registry is the definitive way to
resolve
- * locations.
+ * This implementation unfortunately has to deal with location definitions
coming from three places:
+ *
+ * <li> brooklyn.properties
+ * <li> {@link BrooklynCatalog}
+ * <li> {@link BrooklynTypeRegistry}
+ * <p>
+ * The first is read on init in the call to {@link #updateDefinedLocations()}.
+ * The latter two are <i>not</i> loaded here but are treated as lookups to
{@link BrooklynTypeRegistry}.
* <p>
- * Any location item added to the catalog must therefore be registered here.
- * Whenever an item is added to the catalog, it will automatically call
- * {@link #updateDefinedLocation(RegisteredType)}. Similarly, when a location
- * is deleted from the catalog it will call {@link
#removeDefinedLocation(RegisteredType)}.
+ * (Prior to 0.12.0 items in {@link BrooklynCatalog} were scanned on init
here, and then
+ * it updated this on additions and removals there. That is no longer done
with type registry.)
* <p>
- * However, the location item in the catalog has an unparsed blob of YAML,
which contains
- * important things like the type and the config of the location. This is only
parsed when
- * {@link BrooklynCatalog#createSpec(CatalogItem)} is called. We therefore
jump through
- * some hoops to wire together the catalog and the registry.
+ * We should consider deprecating this as a place where locations can be
defined (switch to type registry only).
* <p>
- * To add a location to the catalog, and then to resolve a location that is in
the catalog,
- * it goes through the following steps:
- *
- * <ol>
- * <li>Call {@link BrooklynCatalog#addItems(String)}
- * <ol>
- * <li>This automatically calls {@link
#updateDefinedLocation(RegisteredType)}
- * <li>A LocationDefinition is creating, using as its id the {@link
RegisteredType#getSymbolicName()}.
- * The definition's spec is {@code
brooklyn.catalog:<symbolicName>:<version>},
- * </ol>
- * <li>A blueprint can reference the catalog item using its symbolic name,
- * such as the YAML {@code location: my-new-location}.
- * (this feels similar to the "named locations").
- * <ol>
- * <li>This automatically calls {@link #getLocationManaged(String)}.
- * <li>The LocationDefinition is found by lookig up this name.
- * <li>The {@link LocationDefiniton.getSpec()} is retrieved; the right
{@link LocationResolver} is
- * found for it.
- * <li>This uses the {@link CatalogLocationResolver}, because the spec
starts with {@code brooklyn.catalog:}.
- * <li>This resolver extracts from the spec the
<symobolicName>:<version>, and looks up the
- * location item using the {@link BrooklynTypeRegistry}.
- * <li>It then creates a {@link LocationSpec} by calling {@link
BrooklynTypeRegistry#createSpec(RegisteredType)}.
- * <ol>
- * <li>This first tries to use the type (that is in the YAML) as a
simple Java class.
- * <li>If that fails, it will resolve the type using {@link
#resolve(String, Boolean, Map)}, which
- * returns an actual location object.
- * <li>It extracts from that location object the appropriate
metadata to create a {@link LocationSpec},
- * returns the spec and discards the location object.
- * </ol>
- * <li>The resolver creates the {@link Location} from the {@link
LocationSpec}
- * </ol>
- * </ol>
- *
- * TODO we should change the registry to be a pass-through facade on top of
the catalog,
- * and shift to preferring catalog access mechanisms.
- * this brings it in line with how we do other things;
- * also this does not understand versions.
- * to do this we will need to:
- * <li> update the catalog on addition, setting a plan (ensuring
serialization);
- * in case of a definition CHANGED in brooklyn.properties give an error
if it means the plan has changed
- * (user could then remove from brooklyn.properties, if persistence on,
or apply the update in the catalog;
- * ie similar semantics to defining an initial catalog via the CLI)
- * <li> find and return the RegisteredType from the type-registry/catalog here
+ * However this is used for items from brooklyn.properties -- we could create
a bundle to store those
+ * to help with migration, and then deprecate/block changes to those after the
bundle is created.
+ * However properties are the only way currently to set default data used for
various clouds;
+ * either we'd need to deprecate that also, or find a way to support that in a
bundled world.
+ * <p>
+ * This is also used in a few other places, such as clocker and the server
pool entity.
+ * Things here are never persisted though, so it is those callers'
responsibility to recreate.
+ * Alternatively (better) would be for them to add items to the catalog
instead of here.
+ * <p>
+ * Or we keep this lying around for now. Probably not worth the pain above.
* <p>
- * Once done, update the UI use /v1/catalog/locations instead of /v1/locations
- * (currently the latter is the only way to list locations known in the
LocationRegistry
- * ie those from brookln.properties.)
+ * Also note the location item in the catalog has an unparsed blob of YAML,
which contains
+ * important things like the type and the config of the location. This is only
parsed when
+ * {@link BrooklynTypeRegistry#createSpec(RegisteredType)} is called, so we do
that for
+ * lookup possibly more often than one might expect. (If a time hole this
could be cached.)
+ * We also need to make sure some of the resolvers here can be used to
identify types, so
+ * when loading types, the spec creation routines are able to call this to
+ * {@link #resolve(String, Boolean, Map)} (that returns a {@link Location} not
a spec,
+ * but the {@link Location} instance is discarded).
+ * <p>
+ * Finally note /v1/catalog/locations does not list items stored here;
+ * only /v1/locations does that.
*/
@SuppressWarnings({"rawtypes","unchecked"})
public class BasicLocationRegistry implements LocationRegistry {
- // TODO save / serialize
- // (we persist live locations, ie those in the LocationManager, but not
"catalog" locations, ie those in this Registry)
-
public static final Logger log =
LoggerFactory.getLogger(BasicLocationRegistry.class);
/**
@@ -194,29 +167,68 @@ public class BasicLocationRegistry implements
LocationRegistry {
}
@Override
- public Map<String,LocationDefinition> getDefinedLocations() {
+ public Map<String, LocationDefinition> getDefinedLocations(boolean
includeThingsWeAreFacadeFor) {
+ Map<String, LocationDefinition> result = MutableMap.of();
synchronized (definedLocations) {
- return
ImmutableMap.<String,LocationDefinition>copyOf(definedLocations);
+ result.putAll(definedLocations);
+ }
+ for (RegisteredType rt:
mgmt.getTypeRegistry().getMatching(RegisteredTypePredicates.IS_LOCATION)) {
+ result.put(rt.getId(), newDefinedLocation(rt));
}
+ return result;
+ }
+
+ @Override @Deprecated
+ public Map<String,LocationDefinition> getDefinedLocations() {
+ return getDefinedLocations(true);
}
@Override
public LocationDefinition getDefinedLocationById(String id) {
- return definedLocations.get(id);
+ return getDefinedLocation(id, true);
}
@Override
public LocationDefinition getDefinedLocationByName(String name) {
+ return getDefinedLocation(name, false);
+ }
+
+ private LocationDefinition getDefinedLocation(String nameOrId, boolean
isId) {
+ RegisteredType lt = mgmt.getTypeRegistry().get(nameOrId,
RegisteredTypeLoadingContexts.withSpecSuperType(null, LocationSpec.class));
+ if (lt!=null) {
+ return newDefinedLocation(lt);
+ }
+
synchronized (definedLocations) {
- for (LocationDefinition l: definedLocations.values()) {
- if (l.getName().equals(name)) return l;
+ if (isId) {
+ LocationDefinition ld = definedLocations.get(nameOrId);
+ if (ld!=null) {
+ return ld;
+ }
+ } else {
+ for (LocationDefinition l: definedLocations.values()) {
+ if (l.getName().equals(nameOrId)) return l;
+ }
}
- return null;
}
+
+ // fall back to ignoring supertypes, in case they weren't set
+ lt = mgmt.getTypeRegistry().get(nameOrId);
+ if (lt!=null) {
+ log.warn("Location registry only found "+nameOrId+" when ignoring
supertypes; check it is correctly resolved.");
+ return newDefinedLocation(lt);
+ }
+
+ return null;
}
- @Override
+ @Override @Deprecated
public void updateDefinedLocation(LocationDefinition l) {
+ updateDefinedLocationNonPersisted(l);
+ }
+
+ @Override
+ public void updateDefinedLocationNonPersisted(LocationDefinition l) {
synchronized (definedLocations) {
definedLocations.put(l.getId(), l);
}
@@ -226,7 +238,9 @@ public class BasicLocationRegistry implements
LocationRegistry {
* Converts the given item from the catalog into a LocationDefinition, and
adds it
* to the registry (overwriting anything already registered with the id
* {@link CatalogItem#getCatalogItemId()}.
+ * @deprecated since 0.12.0 this class is a facade so method no longer
wanted
*/
+ @Deprecated // not used
public void updateDefinedLocation(CatalogItem<Location, LocationSpec<?>>
item) {
String id = item.getCatalogItemId();
String symbolicName = item.getSymbolicName();
@@ -241,17 +255,22 @@ public class BasicLocationRegistry implements
LocationRegistry {
* Converts the given item from the catalog into a LocationDefinition, and
adds it
* to the registry (overwriting anything already registered with the id
* {@link RegisteredType#getId()}.
+ * @deprecated since 0.12.0 this class is a facade so method no longer
wanted
*/
+ @Deprecated // not used
public void updateDefinedLocation(RegisteredType item) {
+ BasicLocationDefinition locDefinition = newDefinedLocation(item);
+ updateDefinedLocation(locDefinition);
+ }
+
+ protected BasicLocationDefinition newDefinedLocation(RegisteredType item) {
String id = item.getId();
- String symbolicName = item.getSymbolicName();
String spec = CatalogLocationResolver.createLegacyWrappedReference(id);
- Map<String, Object> config = ImmutableMap.<String, Object>of();
- BasicLocationDefinition locDefinition = new
BasicLocationDefinition(symbolicName, symbolicName, spec, config);
-
- updateDefinedLocation(locDefinition);
+ return new BasicLocationDefinition(id, item.getSymbolicName(), spec,
ImmutableMap.<String, Object>of());
}
+ /** @deprecated since 0.12.0 this class is a facade so method no longer
wanted */
+ @Deprecated // not used
public void removeDefinedLocation(CatalogItem<Location, LocationSpec<?>>
item) {
removeDefinedLocation(item.getSymbolicName());
}
@@ -290,11 +309,6 @@ public class BasicLocationRegistry implements
LocationRegistry {
}
if (log.isDebugEnabled())
log.debug("Found "+count+" defined locations from properties
(*.named.* syntax): "+definedLocations.values());
-
- for (RegisteredType item:
mgmt.getTypeRegistry().getMatching(RegisteredTypePredicates.IS_LOCATION)) {
- updateDefinedLocation(item);
- count++;
- }
}
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/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 f7f71e5..f1a3e66 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
@@ -19,6 +19,7 @@
package org.apache.brooklyn.core.typereg;
import java.util.Map;
+import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Set;
@@ -28,6 +29,7 @@ import org.apache.brooklyn.api.catalog.BrooklynCatalog;
import org.apache.brooklyn.api.catalog.CatalogItem;
import org.apache.brooklyn.api.catalog.CatalogItem.CatalogItemType;
import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
+import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
import org.apache.brooklyn.api.typereg.RegisteredType;
@@ -35,7 +37,9 @@ import
org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan;
import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext;
import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
import org.apache.brooklyn.core.catalog.internal.CatalogItemBuilder;
+import org.apache.brooklyn.core.catalog.internal.CatalogItemDtoAbstract;
import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
+import org.apache.brooklyn.core.location.BasicLocationRegistry;
import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
import org.apache.brooklyn.test.Asserts;
@@ -419,14 +423,18 @@ public class BasicBrooklynTypeRegistry implements
BrooklynTypeRegistry {
@Beta // API stabilising
public void delete(VersionedName type) {
- if (localRegisteredTypes.remove(type.toString()) != null) {
+ RegisteredType registeredTypeRemoved =
localRegisteredTypes.remove(type.toString());
+ if (registeredTypeRemoved != null) {
return ;
}
- // TODO may need to support version-less here?
// legacy deletion (may call back to us, but max once)
mgmt.getCatalog().deleteCatalogItem(type.getSymbolicName(),
type.getVersionString());
- // if nothing deleted, throw NoSuchElement
+ // currently the above will succeed or throw; but if we delete that,
we need to enable the code below
+// if (Strings.isBlank(type.getVersionString()) ||
BrooklynCatalog.DEFAULT_VERSION.equals(type.getVersionString())) {
+// throw new IllegalStateException("Deleting items with unspecified
version (argument DEFAULT_VERSION) not supported.");
+// }
+// throw new NoSuchElementException("No catalog item found with id
"+type);
}
public void delete(RegisteredType type) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/core/src/test/java/org/apache/brooklyn/core/location/LocationManagementTest.java
----------------------------------------------------------------------
diff --git
a/core/src/test/java/org/apache/brooklyn/core/location/LocationManagementTest.java
b/core/src/test/java/org/apache/brooklyn/core/location/LocationManagementTest.java
index ed6758a..da85a9e 100644
---
a/core/src/test/java/org/apache/brooklyn/core/location/LocationManagementTest.java
+++
b/core/src/test/java/org/apache/brooklyn/core/location/LocationManagementTest.java
@@ -106,23 +106,23 @@ public class LocationManagementTest extends
BrooklynAppUnitTestSupport {
@Test
public void testManagedLocationsNamedCreateAndCleanup() {
-
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations().keySet(),
CollectionFunctionals.sizeEquals(0));
+
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations(true).keySet(),
CollectionFunctionals.sizeEquals(0));
Asserts.assertThat(mgmt.getCatalog().getCatalogItems(),
CollectionFunctionals.sizeEquals(0));
Asserts.assertThat(locationManager.getLocations(),
CollectionFunctionals.sizeEquals(0));
- mgmt.getLocationRegistry().updateDefinedLocation( new
BasicLocationDefinition("lh1", "localhost", null) );
+ mgmt.getLocationRegistry().updateDefinedLocationNonPersisted( new
BasicLocationDefinition("lh1", "localhost", null) );
-
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations().keySet(),
CollectionFunctionals.sizeEquals(1));
+
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations(true).keySet(),
CollectionFunctionals.sizeEquals(1));
Asserts.assertThat(locationManager.getLocations(),
CollectionFunctionals.sizeEquals(0));
// currently such defined locations do NOT appear in catalog -- see
CatalogYamlLocationTest
Asserts.assertThat(mgmt.getCatalog().getCatalogItems(),
CollectionFunctionals.sizeEquals(0));
Location loc = mgmt.getLocationRegistry().getLocationManaged("lh1");
-
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations().keySet(),
CollectionFunctionals.sizeEquals(1));
+
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations(true).keySet(),
CollectionFunctionals.sizeEquals(1));
Asserts.assertThat(locationManager.getLocations(),
CollectionFunctionals.sizeEquals(1));
mgmt.getLocationManager().unmanage(loc);
-
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations().keySet(),
CollectionFunctionals.sizeEquals(1));
+
Asserts.assertThat(mgmt.getLocationRegistry().getDefinedLocations(true).keySet(),
CollectionFunctionals.sizeEquals(1));
Asserts.assertThat(locationManager.getLocations(),
CollectionFunctionals.sizeEquals(0));
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java
----------------------------------------------------------------------
diff --git
a/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java
b/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java
index bd27f81..b9c8d5a 100644
---
a/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java
+++
b/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java
@@ -53,9 +53,9 @@ public class LocationRegistryTest {
properties.put("brooklyn.location.named.foo",
"byon:(hosts=\"[email protected].{1,2,3,4}\")");
properties.put("brooklyn.location.named.foo.privateKeyFile",
"~/.ssh/foo.id_rsa");
mgmt = LocalManagementContextForTests.newInstance(properties);
- log.info("foo properties gave defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations());
+ log.info("foo properties gave defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations(true));
locdef = mgmt.getLocationRegistry().getDefinedLocationByName("foo");
- Assert.assertNotNull(locdef, "Expected 'foo' location; but had
"+mgmt.getLocationRegistry().getDefinedLocations());
+ Assert.assertNotNull(locdef, "Expected 'foo' location; but had
"+mgmt.getLocationRegistry().getDefinedLocations(true));
Assert.assertEquals(locdef.getConfig().get("privateKeyFile"),
"~/.ssh/foo.id_rsa");
}
@@ -67,7 +67,7 @@ public class LocationRegistryTest {
mgmt = LocalManagementContextForTests.newInstance(properties);
locdef = mgmt.getLocationRegistry().getDefinedLocationByName("foo");
- log.info("testResovlesBy has defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations());
+ log.info("testResovlesBy has defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations(true));
LocationSpec<?> ls =
mgmt.getLocationRegistry().getLocationSpec("named:foo").get();
Location l = mgmt.getLocationManager().createLocation(ls);
@@ -127,7 +127,7 @@ public class LocationRegistryTest {
BrooklynProperties properties = BrooklynProperties.Factory.newEmpty();
properties.put("brooklyn.location.named.bar", "named:bar");
mgmt = LocalManagementContextForTests.newInstance(properties);
- log.info("bar properties gave defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations());
+ log.info("bar properties gave defined locations:
"+mgmt.getLocationRegistry().getDefinedLocations(true));
try {
mgmt.getLocationRegistry().getLocationSpec("bar").get();
Asserts.shouldHaveFailedPreviously("Circular reference gave a
location");
@@ -137,7 +137,7 @@ public class LocationRegistryTest {
}
protected boolean findLocationMatching(String regex) {
- for (LocationDefinition d:
mgmt.getLocationRegistry().getDefinedLocations().values()) {
+ for (LocationDefinition d:
mgmt.getLocationRegistry().getDefinedLocations(true).values()) {
if (d.getName()!=null && d.getName().matches(regex)) return true;
}
return false;
@@ -159,8 +159,7 @@ public class LocationRegistryTest {
properties.put(LocalhostLocationResolver.LOCALHOST_ENABLED.getName(),
false);
mgmt = LocalManagementContextForTests.newInstance(properties);
- log.info("RESOLVERS:
"+mgmt.getLocationRegistry().getDefinedLocations());
- log.info("DEFINED LOCATIONS:
"+mgmt.getLocationRegistry().getDefinedLocations());
+ log.info("DEFINED LOCATIONS:
"+mgmt.getLocationRegistry().getDefinedLocations(true));
Assert.assertFalse( findLocationMatching("localhost") );
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/launcher-common/src/main/java/org/apache/brooklyn/launcher/command/support/CloudExplorerSupport.java
----------------------------------------------------------------------
diff --git
a/launcher-common/src/main/java/org/apache/brooklyn/launcher/command/support/CloudExplorerSupport.java
b/launcher-common/src/main/java/org/apache/brooklyn/launcher/command/support/CloudExplorerSupport.java
index dc4000b..e35517d 100644
---
a/launcher-common/src/main/java/org/apache/brooklyn/launcher/command/support/CloudExplorerSupport.java
+++
b/launcher-common/src/main/java/org/apache/brooklyn/launcher/command/support/CloudExplorerSupport.java
@@ -104,7 +104,7 @@ public abstract class CloudExplorerSupport implements
Callable<Void> {
locs.add(loc);
} else if (allLocations) {
// Find all named locations that point at different target clouds
- Map<String, LocationDefinition> definedLocations =
managementContext.getLocationRegistry().getDefinedLocations();
+ Map<String, LocationDefinition> definedLocations =
managementContext.getLocationRegistry().getDefinedLocations(true);
for (LocationDefinition locationDef : definedLocations.values()) {
Location loc =
managementContext.getLocationManager().createLocation(
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LocationResource.java
----------------------------------------------------------------------
diff --git
a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LocationResource.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LocationResource.java
index 55159fa..052114a 100644
---
a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LocationResource.java
+++
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LocationResource.java
@@ -83,7 +83,7 @@ public class LocationResource extends
AbstractBrooklynRestResource implements Lo
}
}
};
- return
FluentIterable.from(brooklyn().getLocationRegistry().getDefinedLocations().values())
+ return
FluentIterable.from(brooklyn().getLocationRegistry().getDefinedLocations(true).values())
.transform(transformer)
.filter(LocationSummary.class)
.toSortedList(nameOrSpecComparator());
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5af5495/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/LocationResourceTest.java
----------------------------------------------------------------------
diff --git
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/LocationResourceTest.java
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/LocationResourceTest.java
index c61c395..d4a484d 100644
---
a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/LocationResourceTest.java
+++
b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/LocationResourceTest.java
@@ -180,7 +180,7 @@ public class LocationResourceTest extends
BrooklynRestResourceTest {
@Test(dependsOnMethods = { "testAddLegacyLocationDefinition" })
@Deprecated
public void testDeleteLocation() {
- final int size = getLocationRegistry().getDefinedLocations().size();
+ final int size =
getLocationRegistry().getDefinedLocations(true).size();
URI expectedLocationUri = URI.create("/locations/"+legacyLocationName);
Response response = client().path(expectedLocationUri).delete();
@@ -188,7 +188,7 @@ public class LocationResourceTest extends
BrooklynRestResourceTest {
Asserts.succeedsEventually(new Runnable() {
@Override
public void run() {
-
assertEquals(getLocationRegistry().getDefinedLocations().size(), size - 1);
+
assertEquals(getLocationRegistry().getDefinedLocations(true).size(), size - 1);
}
});
}