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 2016/01/15 00:54:30 UTC

[16/24] incubator-brooklyn git commit: parameter-inheritance now marked with TODO, and prev behaviour largely restored

parameter-inheritance now marked with TODO, and prev behaviour largely restored


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/7ffc2162
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/7ffc2162
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/7ffc2162

Branch: refs/heads/master
Commit: 7ffc21624ef6c1887f704207862381c1eb965dff
Parents: aa9a2d7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Jan 14 14:32:55 2016 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Jan 14 15:51:24 2016 +0000

----------------------------------------------------------------------
 .../internal/AbstractBrooklynObjectSpec.java    | 24 +++++++++++++++++++-
 .../BrooklynEntityDecorationResolver.java       |  3 +++
 .../core/mgmt/EntityManagementUtils.java        | 13 ++++++++---
 3 files changed, 36 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ffc2162/brooklyn-server/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/brooklyn-server/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
index d03eec1..560c499 100644
--- a/brooklyn-server/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
+++ b/brooklyn-server/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
@@ -38,6 +38,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.Beta;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
@@ -108,8 +109,23 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly
         return self();
     }
     
-    /** adds the given parameters */
+    // TODO which semantics are correct? replace has been the behaviour;
+    // add breaks tests and adds unwanted parameters,
+    // but replacing will cause some desired parameters to be lost.
+    // i (AH) think ideally the caller should remove any parameters which
+    // have been defined as config keys, and then add the others;
+    // or actually we should always add, since this is really defining the config keys,
+    // and maybe extend the SpecParameter object to be able to advertise whether
+    // it is a CatalogConfig or merely a config key, maybe introducing displayable, or even priority 
+    // (but note part of the reason for CatalogConfig.priority is that java reflection doesn't preserve field order) .
+    // see also comments on the camp SpecParameterResolver.
+    @Beta
     public SpecT parameters(List<? extends SpecParameter<?>> parameters) {
+        return parametersReplace(parameters);
+    }
+    /** adds the given parameters */
+    @Beta
+    public SpecT parametersAdd(List<? extends SpecParameter<?>> parameters) {
         // parameters follows immutable pattern, unlike the other fields
         Builder<SpecParameter<?>> result = ImmutableList.<SpecParameter<?>>builder();
         if (this.parameters!=null)
@@ -118,6 +134,12 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly
         this.parameters = result.build();
         return self();
     }
+    /** replaces parameters with the given */
+    @Beta
+    public SpecT parametersReplace(List<? extends SpecParameter<?>> parameters) {
+        this.parameters = ImmutableList.copyOf( checkNotNull(parameters, "parameters") );
+        return self();
+    }
 
     /**
      * @return The type (often an interface) this spec represents and which will be instantiated from it 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ffc2162/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java b/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java
index 6734875..4913cb1 100644
--- a/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java
+++ b/brooklyn-server/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java
@@ -186,6 +186,9 @@ public abstract class BrooklynEntityDecorationResolver<DT> {
         @Override
         public void decorate(EntitySpec<?> entitySpec, ConfigBag attrs) {
             List<? extends SpecParameter<?>> explicitParams = buildListOfTheseDecorationsFromEntityAttributes(attrs);
+            // TODO see discussion at EntitySpec.parameters; 
+            // maybe we should instead inherit always, or 
+            // inherit except where it is set as config and then add the new explicit ones
             if (!explicitParams.isEmpty()) {
                 entitySpec.parameters(explicitParams);
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ffc2162/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
----------------------------------------------------------------------
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
index 7dcd9a2..c0868ef 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
@@ -251,7 +251,8 @@ public class EntityManagementUtils {
         
         wrappedChild.locations(wrapperParent.getLocations());
         
-        wrappedChild.parameters(wrapperParent.getParameters());
+        if (!wrapperParent.getParameters().isEmpty())
+            wrappedChild.parametersReplace(wrapperParent.getParameters());
         
         if (wrappedChild.getCatalogItemId()==null) {
             wrappedChild.catalogItemId(wrapperParent.getCatalogItemId());
@@ -302,12 +303,18 @@ public class EntityManagementUtils {
      * @see #WRAPPER_APP_MARKER for an overview */
     public static boolean canUnwrapEntity(EntitySpec<? extends Entity> spec) {
         return isWrapperApp(spec) && hasSingleChild(spec) &&
-            //equivalent to no keys starting with "brooklyn."
+            // these "brooklyn.*" items on the app rather than the child absolutely prevent unwrapping
+            // as their semantics could well be different whether they are on the parent or the child
             spec.getEnrichers().isEmpty() &&
             spec.getEnricherSpecs().isEmpty() &&
             spec.getInitializers().isEmpty() &&
             spec.getPolicies().isEmpty() &&
-            spec.getPolicySpecs().isEmpty();
+            spec.getPolicySpecs().isEmpty() &&
+            // these items prevent merge only if they are defined at both levels
+            (spec.getLocations().isEmpty() || Iterables.getOnlyElement(spec.getChildren()).getLocations().isEmpty())
+            // TODO what should we do with parameters? currently clobbers due to EntitySpec.parameters(...) behaviour.
+//            && (spec.getParameters().isEmpty() || Iterables.getOnlyElement(spec.getChildren()).getParameters().isEmpty())
+            ;
     }
     /** @deprecated since 0.9.0 use {@link #canUnwrapEntity(EntitySpec)} */ @Deprecated
     public static boolean canPromoteChildrenInWrappedApplication(EntitySpec<? extends Application> spec) {