You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by dr...@apache.org on 2017/05/03 11:13:07 UTC

[3/4] brooklyn-server git commit: sort out ORIGINAL_SPEC and NAMED_SPEC_NAME, deprecating the latter

sort out ORIGINAL_SPEC and NAMED_SPEC_NAME, deprecating the latter

and ensuring the former is always set to the originally requested spec, with tests


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/1e8873a8
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/1e8873a8
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/1e8873a8

Branch: refs/heads/master
Commit: 1e8873a8d52a03267cbbbd06af1e70ba9a57fcf3
Parents: e2319e5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Tue Feb 28 16:01:05 2017 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Tue Feb 28 16:01:05 2017 +0000

----------------------------------------------------------------------
 .../camp/brooklyn/catalog/CatalogYamlLocationTest.java  |  3 +++
 .../brooklyn/core/location/BasicLocationRegistry.java   | 12 ++++++++++--
 .../brooklyn/core/location/CatalogLocationResolver.java |  3 +--
 .../core/location/internal/LocationConfigMap.java       |  1 -
 .../core/location/internal/LocationInternal.java        |  6 +++---
 .../brooklyn/rest/transform/LocationTransformer.java    | 10 +++++-----
 6 files changed, 22 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/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 01fb484..026b2e8 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
@@ -34,6 +34,7 @@ import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
 import org.apache.brooklyn.core.config.BasicConfigKey;
 import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.location.internal.LocationInternal;
 import org.apache.brooklyn.core.typereg.RegisteredTypePredicates;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.entity.stock.BasicEntity;
@@ -328,6 +329,8 @@ public class CatalogYamlLocationTest extends AbstractYamlTest {
         assertEquals(location.getConfig(new BasicConfigKey<String>(String.class, "config1")), "config1");
         assertEquals(location.getConfig(new BasicConfigKey<String>(String.class, "config2")), "config2 override");
         assertEquals(location.getConfig(new BasicConfigKey<String>(String.class, "config3")), "config3");
+        
+        Asserts.assertEquals(location.getConfig(LocationInternal.ORIGINAL_SPEC), locTypeInYaml);
     }
 
     private void addCatalogLocation(String symbolicName, String locationType, List<String> libraries) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/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 e68c0dc..a761513 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
@@ -347,7 +347,12 @@ public class BasicLocationRegistry implements LocationRegistry {
             .putAll(locationFlags)
             .putIfAbsentAndNotNull(LocationInternal.NAMED_SPEC_NAME, ld.getName())
             .putIfAbsentAndNotNull(LocationInternal.ORIGINAL_SPEC, ld.getName());
-        return getLocationSpec(ld.getSpec(), newLocationFlags.getAllConfigRaw());        
+        Maybe<LocationSpec<? extends Location>> result = getLocationSpec(ld.getSpec(), newLocationFlags.getAllConfigRaw());
+        if (result.isPresent()) {
+            result.get().configure(LocationInternal.NAMED_SPEC_NAME, ld.getName());
+            result.get().configure(LocationInternal.ORIGINAL_SPEC, ld.getName());
+        }
+        return result;
     }
 
     @Override
@@ -372,7 +377,10 @@ public class BasicLocationRegistry implements LocationRegistry {
 
             if (resolver != null) {
                 try {
-                    return (Maybe) Maybe.of(resolver.newLocationSpecFromString(spec, locationFlags, this));
+                    LocationSpec<? extends Location> specO = resolver.newLocationSpecFromString(spec, locationFlags, this);
+                    specO.configure(LocationInternal.ORIGINAL_SPEC, spec);
+                    specO.configure(LocationInternal.NAMED_SPEC_NAME, spec);
+                    return (Maybe) Maybe.of(specO);
                 } catch (RuntimeException e) {
                      return Maybe.absent(Suppliers.ofInstance(e));
                 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java b/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java
index da66172..239fd16 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java
@@ -67,8 +67,7 @@ public class CatalogLocationResolver implements LocationResolver {
         }
         
         LocationSpec<?> origLocSpec = managementContext.getTypeRegistry().createSpec(item, null, LocationSpec.class);
-        LocationSpec<?> locSpec = LocationSpec.create(origLocSpec).configure(locationFlags);
-        return locSpec;
+        return LocationSpec.create(origLocSpec).configure(locationFlags);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
index 689f5f2..51e2e61 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
@@ -44,7 +44,6 @@ import com.google.common.base.Predicate;
 
 public class LocationConfigMap extends AbstractConfigMapImpl<Location> {
 
-    @SuppressWarnings("unused")
     private static final Logger log = LoggerFactory.getLogger(LocationConfigMap.class);
     
     public LocationConfigMap(AbstractLocation loc) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationInternal.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationInternal.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationInternal.java
index 79b8eaa..9ed1767 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationInternal.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationInternal.java
@@ -37,10 +37,10 @@ import com.google.common.annotations.Beta;
  */
 public interface LocationInternal extends BrooklynObjectInternal, Location {
 
-    // TODO original_spec seems to be the reference inside the named_spec -- do we really ever want that?
     public static final ConfigKey<String> ORIGINAL_SPEC = ConfigKeys.newStringConfigKey("spec.original", "The original spec used to instantiate a location");
-    public static final ConfigKey<String> FINAL_SPEC = ConfigKeys.newStringConfigKey("spec.final", "The actual spec (in a chain) which instantiates a location");
-    public static final ConfigKey<String> NAMED_SPEC_NAME = ConfigKeys.newStringConfigKey("spec.named.name", "The name on the (first) named spec in a chain");
+    /** @deprecated since 0.11.0 use {@link #ORIGINAL_SPEC}; the distinction was unclear and the implementation inconsistent */ @Deprecated
+    public static final ConfigKey<String> NAMED_SPEC_NAME = ConfigKeys.newStringConfigKey("spec.named.name", "The name on the (first) named spec in a chain, either original spec or as defined in that");
+    public static final ConfigKey<String> FINAL_SPEC = ConfigKeys.newStringConfigKey("spec.final", "The actual spec (in a chain) which instantiates a location, ie is not a reference to another one");
     
     /**
      * Registers the given extension for the given type. If an extension already existed for

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/1e8873a8/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/LocationTransformer.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/LocationTransformer.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/LocationTransformer.java
index c083ad9..fa3fb2c 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/LocationTransformer.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/LocationTransformer.java
@@ -70,12 +70,13 @@ public class LocationTransformer {
         // mgmt not actually needed
         Map<String, Object> config = MutableMap.copyOf(explicitConfig==null ? null : explicitConfig.getAllConfig());
         if (spec!=null && (level==LocationDetailLevel.FULL_EXCLUDING_SECRET || level==LocationDetailLevel.FULL_INCLUDING_SECRET)) {
-            // full takes from any resolved spec AND from explicit config
-            // TODO include flags? esp need provider, region, and endpoint
+            // full takes from any resolved spec ie inherited, AND explicit config and flags;
+            // would be nice to distinguish flags from config, but more important to make sure flags are supplied
+            // (ideally would return yaml, using yoml)
             config = ConfigBag.newInstance(spec.getFlags()).putAll(spec.getConfig()).putAll(config).getAllConfig();
         } else if (level==LocationDetailLevel.LOCAL_EXCLUDING_SECRET) {
-            // in local mode, just make sure display name is set
-            // TODO include provider, region, and endpoint ?
+            // in local mode, make sure any inherited display name is set, but otherwise _no_ inherited
+            // NB this may not include provider, region, and endpoint -- use 'full' for that
             if (spec!=null && !explicitConfig.containsKey(LocationConfigKeys.DISPLAY_NAME) ) {
                 if (Strings.isNonBlank((String) spec.getFlags().get(LocationConfigKeys.DISPLAY_NAME.getName()))){
                     config.put(LocationConfigKeys.DISPLAY_NAME.getName(), spec.getFlags().get(LocationConfigKeys.DISPLAY_NAME.getName()));
@@ -148,7 +149,6 @@ public class LocationTransformer {
             // walk parent locations
             // TODO not sure this is the best strategy, or if it's needed, as the spec config is inherited anyway... 
             if (spec==null) {
-                // TODO should be "named spec" ?
                 Maybe<Object> originalSpec = ((LocationInternal)lp).config().getRaw(LocationInternal.ORIGINAL_SPEC);
                 if (originalSpec.isPresent())
                     spec = Strings.toString(originalSpec.get());