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/02/15 18:31:11 UTC
[18/28] brooklyn-server git commit: address most PR comments
address most PR comments
still to do:
* version refs in deprecation notes
* geomacy's test case and parameter order
* aled's tests
* several comments from neykov re SpecParameterUnwrappingTest.java
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bb954698
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bb954698
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bb954698
Branch: refs/heads/master
Commit: bb95469820daf6bcb22b6fe80e454ebd9506a1f0
Parents: 9fc308c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon Feb 13 14:59:49 2017 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Mon Feb 13 14:59:49 2017 +0000
----------------------------------------------------------------------
.../apache/brooklyn/api/objs/Configurable.java | 2 +-
.../BrooklynComponentTemplateResolver.java | 2 +-
.../core/config/BasicConfigInheritance.java | 89 +++++++++++++++-----
.../config/internal/AbstractConfigMapImpl.java | 62 +++++++-------
.../core/mgmt/EntityManagementUtils.java | 3 +-
.../brooklyn/core/objs/BasicSpecParameter.java | 39 +++++----
.../core/objs/BrooklynObjectInternal.java | 2 +-
.../mgmt/persist/XmlMementoSerializerTest.java | 2 +-
.../org/apache/brooklyn/util/guava/Maybe.java | 19 ++++-
9 files changed, 143 insertions(+), 77 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
index 4d1d2f4..e0226b0 100644
--- a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
+++ b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
@@ -103,7 +103,7 @@ public interface Configurable {
@Deprecated
Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter);
- /** see {@link ConfigMap#findKeysDelcared(Predicate)} */
+ /** see {@link ConfigMap#findKeysDeclared(Predicate)} */
public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter);
/** see {@link ConfigMap#findKeysPresent(Predicate)} */
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
index 4a0e1ff..adeb7a7 100644
--- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
+++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
@@ -409,7 +409,7 @@ public class BrooklynComponentTemplateResolver {
//
// I plan to propose a change on dev@brooklyn, to replace `@SetFromFlag`!
- // need to de-dupe? (can't use Set bc FCKVR doesn't impl equals/hashcode)
+ // need to de-dupe? (can't use Set bc FCKAVR doesn't impl equals/hashcode)
// TODO merge *bagFlags* with existing spec params, merge yaml w yaml parent params elsewhere
List<FlagConfigKeyAndValueRecord> allKeys = MutableList.of();
allKeys.addAll(FlagUtils.findAllParameterConfigKeys(spec.getParameters(), bagFlags));
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java
index d63d330..4d66293 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigInheritance.java
@@ -49,6 +49,9 @@ public class BasicConfigInheritance implements ConfigInheritance {
public static final String CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE = "deep_merge";
public static final String CONFLICT_RESOLUTION_STRATEGY_OVERWRITE = "overwrite";
+ /** This class allows us to have constant instances of {@link ConfigInheritance}
+ * which have no internal fields (ie they don't extend {@link BasicConfigInheritance})
+ * so are pretty on serialization, but otherwise act identically. */
public static abstract class DelegatingConfigInheritance implements ConfigInheritance {
protected abstract ConfigInheritance getDelegate();
@@ -69,20 +72,24 @@ public class BasicConfigInheritance implements ConfigInheritance {
public boolean equals(Object obj) {
return super.equals(obj) || getDelegate().equals(obj);
}
-
+
+ @Override
+ public int hashCode() {
+ return getDelegate().hashCode();
+ }
+
// standard deserialization method
protected ConfigInheritance readResolve() {
return returnEquivalentConstant(this);
}
}
-
+
private static ConfigInheritance returnEquivalentConstant(ConfigInheritance candidate) {
for (ConfigInheritance knownMode: Arrays.asList(
NOT_REINHERITED, NOT_REINHERITED_ELSE_DEEP_MERGE, NEVER_INHERITED, OVERWRITE, BasicConfigInheritance.DEEP_MERGE)) {
if (candidate.equals(knownMode)) return knownMode;
}
- if (candidate.equals(new BasicConfigInheritance(false, CONFLICT_RESOLUTION_STRATEGY_OVERWRITE, true, true))) {
- // ignore the ancestor flag for this mode
+ if (candidate.equals(NEVER_INHERITED_OLD)) {
return NEVER_INHERITED;
}
return candidate;
@@ -105,7 +112,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
* (ie the container does not expect it) to a container which does expect it, but it will not be passed down further.
* If the inheritor also defines a value the parent's value is ignored irrespective
* (as in {@link #OVERWRITE}; see {@link #NOT_REINHERITED_ELSE_DEEP_MERGE} if merging is desired). */
- public static ConfigInheritance NOT_REINHERITED = new NotReinherited();
+ public static final ConfigInheritance NOT_REINHERITED = new NotReinherited();
private static class NotReinheritedElseDeepMerge extends DelegatingConfigInheritance {
final static BasicConfigInheritance DELEGATE = new BasicConfigInheritance(false, CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE, false, true);
@@ -113,7 +120,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
}
/** As {@link #NOT_REINHERITED} but in cases where a value is inherited because a parent did not recognize it,
* if the inheritor also defines a value the two values should be merged. */
- public static ConfigInheritance NOT_REINHERITED_ELSE_DEEP_MERGE = new NotReinheritedElseDeepMerge();
+ public static final ConfigInheritance NOT_REINHERITED_ELSE_DEEP_MERGE = new NotReinheritedElseDeepMerge();
private static class NeverInherited extends DelegatingConfigInheritance {
final static BasicConfigInheritance DELEGATE = new BasicConfigInheritance(false, CONFLICT_RESOLUTION_STRATEGY_OVERWRITE, true, false);
@@ -121,7 +128,11 @@ public class BasicConfigInheritance implements ConfigInheritance {
}
/** Indicates that a key's value should never be inherited, even if inherited from a value set on a container that does not know the key.
* (Most usages will prefer {@link #NOT_REINHERITED}.) */
- public static ConfigInheritance NEVER_INHERITED = new NeverInherited();
+ public static final ConfigInheritance NEVER_INHERITED = new NeverInherited();
+
+ /** In case we deserialize an old copy; the last arg (ancestor inherit default) is irrelevant
+ * so accept this as a synonym for {@link #NEVER_INHERITED}. */
+ private static final ConfigInheritance NEVER_INHERITED_OLD = new BasicConfigInheritance(false, CONFLICT_RESOLUTION_STRATEGY_OVERWRITE, true, true);
private static class Overwrite extends DelegatingConfigInheritance {
final static BasicConfigInheritance DELEGATE = new BasicConfigInheritance(true, CONFLICT_RESOLUTION_STRATEGY_OVERWRITE, false, true);
@@ -129,7 +140,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
}
/** Indicates that if a key has a value at both an ancestor and a descendant, the descendant and his descendants
* will prefer the value at the descendant. */
- public static ConfigInheritance OVERWRITE = new Overwrite();
+ public static final ConfigInheritance OVERWRITE = new Overwrite();
private static class DeepMerge extends DelegatingConfigInheritance {
final static BasicConfigInheritance DELEGATE = new BasicConfigInheritance(true, CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE, false, true);
@@ -138,7 +149,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
/** Indicates that if a key has a value at both an ancestor and a descendant, the descendant and his descendants
* should attempt to merge the values. If the values are not mergeable behaviour is undefined
* (and often the descendant's value will simply overwrite). */
- public static ConfigInheritance DEEP_MERGE = new DeepMerge();
+ public static final ConfigInheritance DEEP_MERGE = new DeepMerge();
// support conversion from these legacy fields
@SuppressWarnings("deprecation")
@@ -169,17 +180,27 @@ public class BasicConfigInheritance implements ConfigInheritance {
/** a symbol indicating a conflict-resolution-strategy understood by the implementation.
* in {@link BasicConfigInheritance} supported values are
* {@link #CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE} and {@link #CONFLICT_RESOLUTION_STRATEGY_OVERWRITE}.
- * subclasses may pass null or a different string if they provide a custom implementaton
+ * subclasses may pass null or a different string if they provide a custom implementation
* of {@link #resolveWithParentCustomStrategy(ConfigValueAtContainer, ConfigValueAtContainer, org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext)} */
@Nullable protected final String conflictResolutionStrategy;
/** @deprecated since 0.10.0 when this was introduced, now renamed {@link #localDefaultResolvesWithAncestorValue} */
@Deprecated protected final Boolean useLocalDefaultValue;
/** whether a local default value should be considered for resolution in the presence of an ancestor value.
- * can use true with overwrite to mean don't inherit, or true with merge to mean local default merged on top of inherited
+ * can use true with {@link #CONFLICT_RESOLUTION_STRATEGY_OVERWRITE} to mean don't inherit,
+ * or true with {@link #CONFLICT_RESOLUTION_STRATEGY_DEEP_MERGE} to mean local default merged on top of inherited
* (but be careful here, if local default is null in a merge it will delete ancestor values).
* <p>
- * in most cases this is false, meaning a default value is ignored if the parent has a value.
+ * in most cases this is false, meaning a default value is ignored if the parent has a value,
+ * but it can be used when a key supplies a default which should conflict-resolve with an ancestor value:
+ * a trivial example is when not reinheriting, a default should conflict-resolve (overwriting) an explicit ancestor value.
+ * more interesting potentially, this could indicate
+ * that a default value is being introduced which should be merged/combined with ancestors;
+ * we don't use this (config inheritance is complex enough and we don't have a compelling use case
+ * to expose more complexity to users) but having this as a concept, and the related
+ * {@link #ancestorDefaultInheritable} specifying (in this case) whether a local default should
+ * resolve/merge/combine with ancestor defaults in addition to ancestor explicit values,
+ * means the logic in this refers to the right control dimensions rather than taking shortcuts.
* <p>
* null should not be used. a boxed object is taken (as opposed to a primitive boolean) only in order to support migration.
*/
@@ -187,7 +208,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
protected final Boolean localDefaultResolvesWithAncestorValue;
/** whether a default set in an ancestor container's key definition will be considered as the
- * local default value at descendants who don't define any other value (nothing set locally and local default is null);
+ * local default value at descendants who don't define any other value (nothing set locally and local default is null).
* <p>
* if true (now the usual behaviour), if an ancestor defines a default and a descendant doesn't, the ancestor's value will be taken as a default.
* if it is also the case that localDefaultResolvesWithAncestorValue is true at the <i>ancestor</i> then a descendant who
@@ -197,6 +218,21 @@ public class BasicConfigInheritance implements ConfigInheritance {
* if this is false, ancestor defaults are completely ignored; prior to 0.10.0 this was the normal behaviour,
* but it caused surprises where default values in parameters did not take effect.
* <p>
+ * as currently we only really use {@link #localDefaultResolvesWithAncestorValue} true when we
+ * wish to block reinheritance, this is false in that case,
+ * and true in all other cases (where we wish to have ancestor inheritance, but only if there
+ * is no local value that should trump);
+ * however logically there are other possibilities, such as a local default expecting to
+ * merge/combine with an ancestor value, in which case we'd probably want this to be true
+ * to indicate the local default should combine with the ancestor default,
+ * but it could also make sense for this to be false, meaning the local default would
+ * merge with an explicit ancestor value but not with an ancestor default
+ * (see javadoc on {@link #localDefaultResolvesWithAncestorValue} for more info).
+ * <p>
+ * it gets complex and we've tried to simplify the config modes that we actually use,
+ * but have made efforts in the code to account for the different logical possibilities,
+ * hence the complexity of this.
+ * <p>
* null should not be used. a boxed object is taken (as opposed to a primitive boolean) only in order to support migration.
*/
@Nonnull
@@ -232,6 +268,8 @@ public class BasicConfigInheritance implements ConfigInheritance {
private boolean isSameRootInstanceAs(ConfigInheritance other) {
if (other==null) return false;
if (this==other) return true;
+ // we should consider the argument the same if it is a delegate to us,
+ // e.g. for when this method is invoked by the delegate comparing against its definition
if (other instanceof DelegatingConfigInheritance) return isSameRootInstanceAs( ((DelegatingConfigInheritance)other).getDelegate() );
return false;
}
@@ -350,6 +388,7 @@ public class BasicConfigInheritance implements ConfigInheritance {
// standard deserialization method
private ConfigInheritance readResolve() {
try {
+ // uses reflection because fields are declared final
if (useLocalDefaultValue!=null) {
// move away from useLocalDefaultValue to localDefaultResolvesWithAncestorValue
@@ -382,19 +421,29 @@ public class BasicConfigInheritance implements ConfigInheritance {
}
@Override
+ public int hashCode() {
+ return Objects.hash(conflictResolutionStrategy, isReinherited, localDefaultResolvesWithAncestorValue, ancestorDefaultInheritable);
+ }
+
+ @Override
public boolean equals(Object obj) {
if (obj==null) return false;
- if (obj instanceof DelegatingConfigInheritance) return equals( ((DelegatingConfigInheritance)obj).getDelegate() );
- if (obj.getClass().equals(BasicConfigInheritance.class)) {
- BasicConfigInheritance b = (BasicConfigInheritance)obj;
- return Objects.equals(conflictResolutionStrategy, b.conflictResolutionStrategy) &&
- Objects.equals(isReinherited, b.isReinherited) &&
- Objects.equals(getLocalDefaultResolvesWithAncestorValue(), b.getLocalDefaultResolvesWithAncestorValue()) &&
- Objects.equals(getAncestorDefaultInheritable(), b.getAncestorDefaultInheritable());
+ if (obj instanceof DelegatingConfigInheritance) {
+ return equals( ((DelegatingConfigInheritance)obj).getDelegate() );
+ }
+ if (obj instanceof BasicConfigInheritance) {
+ return equalsAfterResolvingDelegate((BasicConfigInheritance)obj);
}
return false;
}
+ protected boolean equalsAfterResolvingDelegate(BasicConfigInheritance b) {
+ return (Objects.equals(conflictResolutionStrategy, b.conflictResolutionStrategy) &&
+ Objects.equals(isReinherited, b.isReinherited) &&
+ Objects.equals(getLocalDefaultResolvesWithAncestorValue(), b.getLocalDefaultResolvesWithAncestorValue()) &&
+ Objects.equals(getAncestorDefaultInheritable(), b.getAncestorDefaultInheritable()));
+ }
+
@Override
public String toString() {
return super.toString()+"[reinherit="+isReinherited()+"; strategy="+getConflictResolutionStrategy()+"; localDefaultResolvesWithAncestor="+localDefaultResolvesWithAncestorValue+"]";
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
index a7de989..010b91b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java
@@ -360,35 +360,35 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
queryKey.hasDefaultValue() ? coerceFn.apply(Maybe.of((Object)queryKey.getDefaultValue())) :
Maybe.<T>absent();
- if (ownKey instanceof ConfigKeySelfExtracting) {
-
- Function<TContainer, Maybe<Object>> lookupFn = new Function<TContainer, Maybe<Object>>() {
- @Override public Maybe<Object> apply(TContainer input) {
- // lookup against ownKey as it may do extra resolution (eg grab *.* subkeys if a map)
- Maybe<Object> result = getRawValueAtContainer(input, ownKey);
- if (!raw) result = resolveRawValueFromContainer(input, ownKey, result);
- return result;
- }
- };
- Function<TContainer, TContainer> parentFn = new Function<TContainer, TContainer>() {
- @Override public TContainer apply(TContainer input) {
- return getParentOfContainer(input);
- }
- };
- AncestorContainerAndKeyValueIterator<TContainer, T> ckvi = new AncestorContainerAndKeyValueIterator<TContainer,T>(
- getContainer(), keyFn, lookupFn, coerceFn, parentFn);
-
- return ConfigInheritances.resolveInheriting(
- getContainer(), ownKey, coerceFn.apply(lookupFn.apply(getContainer())), defaultValue,
- ckvi, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance());
-
- } else {
- String message = "Config key "+ownKey+" of "+getBrooklynObject()+" is not a ConfigKeySelfExtracting; cannot retrieve value; returning default";
- LOG.warn(message);
- return ReferenceWithError.newInstanceThrowingError(new BasicConfigValueAtContainer<TContainer,T>(getContainer(), ownKey, null, false,
- defaultValue),
- new IllegalStateException(message));
+ if (ownKey instanceof ConfigKeySelfExtracting) {
+
+ Function<TContainer, Maybe<Object>> lookupFn = new Function<TContainer, Maybe<Object>>() {
+ @Override public Maybe<Object> apply(TContainer input) {
+ // lookup against ownKey as it may do extra resolution (eg grab *.* subkeys if a map)
+ Maybe<Object> result = getRawValueAtContainer(input, ownKey);
+ if (!raw) result = resolveRawValueFromContainer(input, ownKey, result);
+ return result;
}
+ };
+ Function<TContainer, TContainer> parentFn = new Function<TContainer, TContainer>() {
+ @Override public TContainer apply(TContainer input) {
+ return getParentOfContainer(input);
+ }
+ };
+ AncestorContainerAndKeyValueIterator<TContainer, T> ckvi = new AncestorContainerAndKeyValueIterator<TContainer,T>(
+ getContainer(), keyFn, lookupFn, coerceFn, parentFn);
+
+ return ConfigInheritances.resolveInheriting(
+ getContainer(), ownKey, coerceFn.apply(lookupFn.apply(getContainer())), defaultValue,
+ ckvi, InheritanceContext.RUNTIME_MANAGEMENT, getDefaultRuntimeInheritance());
+
+ } else {
+ String message = "Config key "+ownKey+" of "+getBrooklynObject()+" is not a ConfigKeySelfExtracting; cannot retrieve value; returning default";
+ LOG.warn(message);
+ return ReferenceWithError.newInstanceThrowingError(new BasicConfigValueAtContainer<TContainer,T>(getContainer(), ownKey, null, false,
+ defaultValue),
+ new IllegalStateException(message));
+ }
}
@Override
@@ -436,10 +436,10 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
@Override
public Set<ConfigKey<?>> findKeysPresent(Predicate<? super ConfigKey<?>> filter) {
- return findKeys(filter, KeyFindingMode.PRESENT_BUT_RESOLVED);
+ return findKeys(filter, KeyFindingMode.PRESENT_AND_RESOLVED);
}
- protected enum KeyFindingMode { DECLARED_OR_PRESENT, PRESENT_BUT_RESOLVED, PRESENT_NOT_RESOLVED }
+ protected enum KeyFindingMode { DECLARED_OR_PRESENT, PRESENT_AND_RESOLVED, PRESENT_NOT_RESOLVED }
@SuppressWarnings("deprecation")
protected Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter, KeyFindingMode mode) {
@@ -461,7 +461,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i
if (getParent()!=null) {
switch (mode) {
case DECLARED_OR_PRESENT: result.addAll( getParentInternal().config().getInternalConfigMap().findKeysDeclared(filter) ); break;
- case PRESENT_BUT_RESOLVED: result.addAll( getParentInternal().config().getInternalConfigMap().findKeysPresent(filter) ); break;
+ case PRESENT_AND_RESOLVED: result.addAll( getParentInternal().config().getInternalConfigMap().findKeysPresent(filter) ); break;
case PRESENT_NOT_RESOLVED: result.addAll( getParentInternal().config().getInternalConfigMap().findKeys(filter) ); break;
default:
throw new IllegalStateException("Unsupported key finding mode: "+mode);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
index 0a8b580..cfee58f 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/EntityManagementUtils.java
@@ -318,8 +318,7 @@ public class EntityManagementUtils {
// prevent merge only if a location is defined at both levels
((spec.getLocations().isEmpty() && spec.getLocationSpecs().isEmpty()) ||
(Iterables.getOnlyElement(spec.getChildren()).getLocations().isEmpty()) && Iterables.getOnlyElement(spec.getChildren()).getLocationSpecs().isEmpty())
- // TODO what should we do with parameters? currently clobbers due to EntitySpec.parameters(...) behaviour.
-// && (spec.getParameters().isEmpty() || Iterables.getOnlyElement(spec.getChildren()).getParameters().isEmpty())
+ // parameters are collapsed on merge so don't need to be considered here
;
}
/** @deprecated since 0.9.0 use {@link #canUnwrapEntity(EntitySpec)} */ @Deprecated
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java
index 5344bc1..aa5150e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java
@@ -52,9 +52,12 @@ import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.text.StringPredicates;
import org.apache.brooklyn.util.time.Duration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import com.google.common.annotations.Beta;
import com.google.common.base.Function;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
@@ -67,6 +70,8 @@ import com.google.common.reflect.TypeToken;
public class BasicSpecParameter<T> implements SpecParameter<T>{
private static final long serialVersionUID = -4728186276307619778L;
+ private static final Logger log = LoggerFactory.getLogger(BasicSpecParameter.class);
+
private final String label;
/** pinning may become a priority or other more expansive indicator */
@@ -142,7 +147,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
@Override
public String toString() {
- return Objects.toStringHelper(this)
+ return MoreObjects.toStringHelper(this)
.add("label", label)
.add("pinned", pinned)
.add("type", configKey)
@@ -249,12 +254,12 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
hasRuntimeInheritance = true;
runtimeInheritance = parseInheritance(inputDef.get("inheritance.runtime"), loader);
} else if (inputDef.containsKey("inheritance.parent")) {
- // this alias will be deprecated
+ log.warn("Using deprecated key 'inheritance.parent' for "+inputDef+"; replace with 'inheritance.runtime'");
hasRuntimeInheritance = true;
runtimeInheritance = parseInheritance(inputDef.get("inheritance.parent"), loader);
} else {
hasRuntimeInheritance = false;
- runtimeInheritance = parseInheritance(null, loader);
+ runtimeInheritance = null;
}
boolean hasTypeInheritance = inputDef.containsKey("inheritance.type");
@@ -421,8 +426,8 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
/**
* Adds the given list of {@link SpecParameter parameters} to the provided
- * {@link AbstractBrooklynObjectSpec spec} or generates a list from the
- * spec if the provided list is empty.
+ * {@link AbstractBrooklynObjectSpec spec}, and if spec has no parameters it
+ * also generates a list from the spec
*
* @see EntitySpec#parameters(List)
*/
@@ -445,24 +450,24 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
}
List<SpecParameter<?>> result = MutableList.<SpecParameter<?>>of();
+
for (SpecParameter<?> p: newParams) {
+ final SpecParameter<?> existingP = existingToKeep.remove(p.getConfigKey().getName());
if (p instanceof SpecParameterForInheritance) {
- SpecParameter<?> existingP = existingToKeep.remove(p.getConfigKey().getName());
if (existingP!=null) {
p = ((SpecParameterForInheritance<?>)p).resolveWithAncestor(existingP);
} else {
- // TODO find config keys on the type (not available as parameters)
+ ConfigKey<?> configKeyExtendedByThisParameter = null;
+ // TODO find any matching config key declared on the type
/* we don't currently do this due to low priority; all it means if there is a config key in java,
* and a user wishes to expose it as a parameter, they have to redeclare everything;
- * nothing from the config key in java will be inherited */
- p = ((SpecParameterForInheritance<?>)p).resolveWithAncestor((ConfigKey<?>)null);
+ * none of the fields from the config key in java will be inherited */
+ p = ((SpecParameterForInheritance<?>)p).resolveWithAncestor(configKeyExtendedByThisParameter);
}
- result.add(p);
- } else {
- existingToKeep.remove(p.getConfigKey().getName());
- result.add(p);
}
+ result.add(p);
}
+
result.addAll(existingToKeep.values());
return result;
}
@@ -494,9 +499,9 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
return new BasicSpecParameter<>(
hasLabelSet ? getLabel() : ancestor.getLabel(),
- hasPinnedSet ? isPinned() : ancestor.isPinned(),
- resolveWithAncestorConfigKey(ancestor.getConfigKey()),
- hasType ? getSensor() : ancestor.getSensor());
+ hasPinnedSet ? isPinned() : ancestor.isPinned(),
+ resolveWithAncestorConfigKey(ancestor.getConfigKey()),
+ hasType ? getSensor() : ancestor.getSensor());
}
/** as {@link #resolveWithAncestor(SpecParameter)} but where the param redefines/extends a config key coming from a java supertype, rather than a parameter */
@@ -507,7 +512,7 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{
SpecParameter<?> resolveWithAncestor(ConfigKey<?> ancestor) {
if (ancestor==null) return new BasicSpecParameter<>(getLabel(), isPinned(), getConfigKey(), getSensor());
- // TODO probably want to do this (but it could get expensive!)
+ // TODO probably want to do this (but it could get expensive! - limited caching could help)
// Set<Class<?>> types = MutableSet.<Class<?>>builder()
// .add(spec.getImplementation())
// .add(spec.getType())
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/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 49b637a..86daf1f 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
@@ -153,7 +153,7 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable {
/** This is currently the only way to get some rolled up collections and raw,
* and also to test for the presence of a value (without any default).
* As more accessors are added callers may be asked to migrate.
- * Callers may also consider using {@link #findKeysDeprecated(com.google.common.base.Predicate)}
+ * Callers may also consider using {@link #findKeysDeclared(com.google.common.base.Predicate)}
* although efficiency should be considered (this gives direct access whereas that does lookups and copies). */
@Beta // TODO provide more accessors and deprecate this
ConfigMapWithInheritance<? extends BrooklynObject> getInternalConfigMap();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java
index 68aa938..a2ffb16 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java
@@ -799,7 +799,7 @@ public class XmlMementoSerializerTest {
public void testConfigInheritanceVals() throws Exception {
ConfigInheritance val = BasicConfigInheritance.NEVER_INHERITED;
- ConfigInheritance newVal = assertSerializeAndDeserialize(val); // TODO this line fails
+ ConfigInheritance newVal = assertSerializeAndDeserialize(val);
Assert.assertSame(val, newVal);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bb954698/utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java b/utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java
index 5766e6b..2dfe627 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/guava/Maybe.java
@@ -459,14 +459,27 @@ public abstract class Maybe<T> implements Serializable, Supplier<T> {
return Objects.hashCode(31, get());
}
+ /** Two {@link Maybe} instances are equal if both present wrapping the same value,
+ * or if both are absent for any reason.
+ * <p>
+ * Specifically, in cases of absences, the reasons for absence are not compared.
+ * This could be revisited if there is compelling reason to do so, but in the main
+ * the cause of an absence is interesting for giving information to the user.
+ * Note this is different to the behaviour of {@link Optional} which says absences
+ * are only equal if they are the same instance.
+ */
@Override
public boolean equals(Object obj) {
if (!(obj instanceof Maybe)) return false;
Maybe<?> other = (Maybe<?>)obj;
- if (!isPresent())
- return !other.isPresent();
- if (!other.isPresent())
+ if (!isPresent()) {
+ if (other.isPresent()) return false;
+ // could compare exceptions; see javadoc
+ return true;
+ }
+ if (!other.isPresent()) {
return false;
+ }
return Objects.equal(get(), other.get());
}