This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit a5830edd791b232e5b7987cdbcad8d4664c570f0 Author: Alex Heneveld <[email protected]> AuthorDate: Fri May 10 19:20:26 2024 +0100 better resolution of workflow inputs - prevent confusion with subkeys having dot qualifiers and being omitted when used in sensor/policy definition - don't coerce a supplier to a map - allow unknown types under type key in maps, since we now use our deserializers for it - allow access to selected workflow.util vars even outwith workflow - return entity predicate where required, and better coercion from boolean to conditions - remove deprecated findKeys method - better error messages - more tests --- .../org/apache/brooklyn/api/objs/Configurable.java | 4 -- .../brooklyn/camp/brooklyn/ObjectsYamlTest.java | 5 -- .../brooklyn/camp/brooklyn/WorkflowYamlTest.java | 28 +++++++++ .../apache/brooklyn/core/config/ConfigUtils.java | 10 +--- .../config/internal/AbstractConfigMapImpl.java | 25 ++++---- .../core/internal/BrooklynPropertiesImpl.java | 5 -- .../mgmt/internal/DeferredBrooklynProperties.java | 5 -- .../objs/AbstractConfigurationSupportInternal.java | 7 +-- .../core/objs/BasicConfigurableObject.java | 12 +--- .../brooklyn/core/objs/BrooklynObjectInternal.java | 6 +- .../resolve/jackson/AsPropertyIfAmbiguous.java | 8 +-- .../core/resolve/jackson/BeanWithTypeUtils.java | 13 ++++- .../resolve/jackson/CommonTypesSerialization.java | 4 +- .../jackson/JsonSymbolDependentDeserializer.java | 12 +++- .../workflow/WorkflowExpressionResolution.java | 49 ++++++++++++++-- .../brooklyn/core/workflow/WorkflowPolicy.java | 2 +- .../brooklyn/core/workflow/WorkflowSensor.java | 14 +++++ .../brooklyn/util/core/flags/TypeCoercions.java | 4 ++ .../util/core/predicates/DslPredicates.java | 66 +++++++++++++--------- .../workflow/WorkflowInputOutputExtensionTest.java | 34 +++++++++-- .../brooklyn/util/core/internal/FlagUtilsTest.java | 5 -- .../java/org/apache/brooklyn/config/ConfigMap.java | 10 ---- 22 files changed, 212 insertions(+), 116 deletions(-) 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 5187d00fe9..2f7fb3c820 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 @@ -96,10 +96,6 @@ public interface Configurable { */ <T> T set(HasConfigKey<T> key, Task<T> val); - /** @deprecated since 0.11.0 see {@link ConfigMap#findKeys(Predicate)} */ - @Deprecated - Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter); - /** see {@link ConfigMap#findKeysDeclared(Predicate)} */ public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java index 32749897db..31cf646aba 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java @@ -194,11 +194,6 @@ public class ObjectsYamlTest extends AbstractYamlTest { return set(key.getConfigKey(), val); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return findKeysDeclared(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return MutableSet.copyOf(Iterables.filter(bag.getAllConfigAsConfigKeyMap().keySet(), filter)); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java index f5d93fb499..02f9517fd7 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java @@ -88,6 +88,7 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; +import org.apache.brooklyn.util.core.task.Tasks; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; @@ -425,6 +426,33 @@ public class WorkflowYamlTest extends AbstractYamlTest { if (extraChecks!=null) extraChecks.accept(policy); } + @Test + public void testWorkflowPolicyInputs() throws Exception { + Entity app = createAndStartApplication( + "services:", + "- type: " + BasicEntity.class.getName(), + " id: main_entity", + " brooklyn.policies:", + " - type: workflow-policy", + " brooklyn.config:", + " name: Set myWorkflowSensor", + " input:", + " x: 42", + " id: set-my-workflow-sensor", + " steps:", + " - set-sensor myWorkflowSensor = ${x}", + " - return ${x}", + ""); + + Stopwatch sw = Stopwatch.createStarted(); + waitForApplicationTasks(app); + + Entity entity = app.getChildren().iterator().next(); + WorkflowPolicy policy = (WorkflowPolicy) entity.policies().asList().stream().filter(p -> p instanceof WorkflowPolicy).findAny().get(); + Asserts.assertEquals(Entities.submit(entity, Tasks.create("test", policy::runOnceNow)).getUnchecked(), 42); + EntityAsserts.assertAttributeEquals(entity, Sensors.newSensor(Object.class, "myWorkflowSensor"), 42); + } + ClassLogWatcher lastLogWatcher; Object invokeWorkflowStepsWithLogging(String ...stepLines) throws Exception { diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigUtils.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigUtils.java index 617fa46e67..3ea73ece1b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigUtils.java @@ -21,17 +21,15 @@ package org.apache.brooklyn.core.config; import java.io.File; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; +import com.google.common.base.Predicate; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; -import org.apache.brooklyn.core.config.ConfigUtils; -import org.apache.brooklyn.core.config.WrappedConfigKey; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.exceptions.Exceptions; @@ -41,8 +39,6 @@ import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Predicate; - @SuppressWarnings({"unchecked"}) public class ConfigUtils { @@ -83,7 +79,7 @@ public class ConfigUtils { public static BrooklynProperties filterFor(BrooklynProperties properties, Predicate<? super String> filter) { BrooklynProperties result = BrooklynProperties.Factory.newEmpty(); - Set<ConfigKey<?>> keys = properties.findKeys(ConfigPredicates.nameSatisfies(filter)); + Set<ConfigKey<?>> keys = properties.findKeysPresent(ConfigPredicates.nameSatisfies(filter)); for (ConfigKey<?> key : keys) { result.put(key, properties.getConfig(key)); } @@ -92,7 +88,7 @@ public class ConfigUtils { public static BrooklynProperties filterForPrefix(BrooklynProperties properties, String prefix) { BrooklynProperties result = BrooklynProperties.Factory.newEmpty(); - Set<ConfigKey<?>> keys = properties.findKeys(ConfigPredicates.nameSatisfies(StringPredicates.startsWith(prefix))); + Set<ConfigKey<?>> keys = properties.findKeysPresent(ConfigPredicates.nameSatisfies(StringPredicates.startsWith(prefix))); for (ConfigKey<?> key : keys) { result.put(key, properties.getConfig(key)); } 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 80ba2f8c59..f7610e56a3 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 @@ -18,16 +18,25 @@ */ package org.apache.brooklyn.core.config.internal; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.Future; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.common.reflect.TypeToken; -import java.util.function.Consumer; import org.apache.brooklyn.api.mgmt.ExecutionContext; import org.apache.brooklyn.api.mgmt.TaskFactory; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.config.ConfigInheritance; -import org.apache.brooklyn.config.ConfigInheritance.ConfigInheritanceContext; import org.apache.brooklyn.config.ConfigInheritances; import org.apache.brooklyn.config.ConfigInheritances.BasicConfigValueAtContainer; import org.apache.brooklyn.config.ConfigKey; @@ -57,11 +66,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import java.util.*; -import java.util.concurrent.Future; - public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> implements ConfigMapWithInheritance<TContainer> { /* @@ -526,11 +530,6 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i return result; } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return findKeys(filter, KeyFindingMode.PRESENT_NOT_RESOLVED); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return findKeys(filter, KeyFindingMode.DECLARED_OR_PRESENT); @@ -569,7 +568,7 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i switch (mode) { case DECLARED_OR_PRESENT: inherited = getParentInternal().config().getInternalConfigMap().findKeysDeclared(filter); break; case PRESENT_AND_RESOLVED: inherited = getParentInternal().config().getInternalConfigMap().findKeysPresent(filter); break; - case PRESENT_NOT_RESOLVED: inherited = getParentInternal().config().getInternalConfigMap().findKeys(filter); break; + case PRESENT_NOT_RESOLVED: inherited = ((AbstractConfigMapImpl) getParentInternal().config().getInternalConfigMap()).findKeys(filter, KeyFindingMode.PRESENT_NOT_RESOLVED); break; default: throw new IllegalStateException("Unsupported key finding mode: "+mode); } diff --git a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java index 026c8c0fc0..6e4704e8a7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/internal/BrooklynPropertiesImpl.java @@ -508,11 +508,6 @@ public class BrooklynPropertiesImpl implements BrooklynProperties { return getAllConfigLocalRaw(); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return findKeysDeclared(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { Set<ConfigKey<?>> result = new LinkedHashSet<ConfigKey<?>>(); diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java index b391bb28cc..526071e0a7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/DeferredBrooklynProperties.java @@ -235,11 +235,6 @@ public class DeferredBrooklynProperties implements BrooklynProperties { return new DeferredBrooklynProperties(submap, mgmt); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return delegate.findKeys(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return delegate.findKeysDeclared(filter); diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java index 0391458f21..b1ae127a23 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java @@ -245,11 +245,6 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb getConfigsInternal().setLocalConfig(MutableMap.<ConfigKey<?>,Object>of()); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return getConfigsInternal().findKeys(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return getConfigsInternal().findKeysDeclared(filter); @@ -272,7 +267,7 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb @SuppressWarnings("deprecation") @Override - // see super; we aspire to depreate this due to poor treatment of inheritance + // see super; we aspire to depreate this due to poor treatment of inheritance, and ambiguity about map subkeys; public ConfigBag getBag() { ConfigBag result = ConfigBag.newInstance(); AbstractConfigMapImpl<? extends BrooklynObject> ci = getConfigsInternal(); diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java index 3c28fc0e47..d81383b87d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicConfigurableObject.java @@ -20,6 +20,8 @@ package org.apache.brooklyn.core.objs; import java.util.Set; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.objs.Configurable; @@ -34,16 +36,13 @@ import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.text.Identifiers; -import com.google.common.base.Predicate; -import com.google.common.collect.Iterables; - /** * A parent class for ancilliary objects that do not require the full heavy lifting of {@link AbstractBrooklynObject} * or similar, but wish to use {@link ConfigKey} and {@link Configurable} in their construction, via the * {@code $brooklyn:object} method of the CAMP DSL. * <p> * Type coercion of values will occur when the {@link ConfigMap} is accessed, but resolving of {@link Task tasks} and other - * deferred operations are assumed to have occurred prior to calling {@link #setConfig(ConfigKey, Object)} i.e. at + * deferred operations are assumed to have occurred prior to calling {@link org.apache.brooklyn.api.objs.Configurable.ConfigurationSupport#set(ConfigKey, Object)} i.e. at * object construction. */ public class BasicConfigurableObject implements Configurable, Identifiable, ManagementContextInjectable, HasBrooklynManagementContext { @@ -118,11 +117,6 @@ public class BasicConfigurableObject implements Configurable, Identifiable, Mana return set(key.getConfigKey(), val); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return findKeysDeclared(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return MutableSet.copyOf(Iterables.filter(config.getAllConfigAsConfigKeyMap().keySet(), filter)); 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 953f106d7a..84820a0be4 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 @@ -80,8 +80,12 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { * It does not identify the container where it is defined, meaning URLs and deferred config values * cannot be resolved in the context of the appropriate ancestor. * <p> + * Also there is ambiguity about map subkeys (caller probably wants map = { key = val } but this gives map.key = val. + * <p> * For these reasons it is recommended to use a different accessor, - * and callers should be advised this beta method may be removed. + * and callers should be advised this beta method may be removed. + * <p> + * Consider code such as WorkflowPollCallable.getFlatBag */ @Beta // TODO deprecate. used fairly extensively, mostly in tests. a bit more care will be needed to refactor. diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java index b266cd2b69..2eccb9f6af 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/AsPropertyIfAmbiguous.java @@ -66,10 +66,10 @@ public class AsPropertyIfAmbiguous { /** @deprecated since 1.1 now use transform fn, and prefer wrapped in parens */ public static final String CONFLICTING_TYPE_NAME_PROPERTY_PREFIX = "@"; - // we can change this to false to allow e.g. deserialize "{type: unknown}" as a map when an object is expected; - // however it is probably more useful to return that as an error, because usually it is an error, - // and have a special way of permitting it in places - public static final boolean THROW_ON_OBJECT_EXPECTED_AND_INVALID_TYPE_KEY_SUPPLIED = true; + // almost always this can be true, but sometimes if deserializing to an object or map, + // more often now that we modify map deserializers, it might be necessary to ignore a "type":"unknown" entry, + // rather than failing because 'unknown' isn't known as a type + public static final boolean THROW_ON_OBJECT_EXPECTED_AND_INVALID_TYPE_KEY_SUPPLIED = false; public interface HasBaseType { JavaType getBaseType(); diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java index 5df6c020f9..a48870fd74 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java @@ -196,7 +196,18 @@ public class BeanWithTypeUtils { try { // needed for a few things, mainly where a bean has a type field that conflicts with the type here, // tryCoercer -20-wrong-bean uses this - return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); + T result = convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); + if (wrongTypeResult!=null) { + if (result==null || !type.getRawType().isInstance(result)) { + LOG.warn("Wrong type returned coercing "+mapOrListToSerializeThenDeserialize+" to "+type+"; got "+wrongTypeResult+" / "+result); + // prefer the original if both get it wrong + return wrongTypeResult; + } else { + LOG.warn("Wrong type returned coercing "+mapOrListToSerializeThenDeserialize+" to "+type+" shallow, but succeeded on deep; got "+wrongTypeResult+" / "+result); + return result; + } + } + return result; } catch (Exception e2) { Exceptions.propagateIfFatal(e2); if (wrongTypeResult!=null) return wrongTypeResult; diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java index f294ee80f8..f070956bc9 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java @@ -460,7 +460,9 @@ public class CommonTypesSerialization { return super.convertStringToObject(value, p, ctxt); } catch (Exception e) { Exceptions.propagateIfFatal(e); - if (BrooklynObjectSerialization.this.mgmt instanceof NonDeploymentManagementContext) { + if (BrooklynObjectSerialization.this.mgmt==null) { + LOG.warn("Reference to BrooklynObject " + value + " outwith task context or without mgmt set; replacing with 'null': "+e); + } else if (BrooklynObjectSerialization.this.mgmt instanceof NonDeploymentManagementContext) { LOG.warn("Reference to BrooklynObject " + value + " which is unknown or not yet known, using NonDeployment context; replacing with 'null': "+e); } else { LOG.warn("Reference to BrooklynObject " + value + " which is unknown or no longer available; replacing with 'null': "+e); diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java index 5b2c5fff2e..18bf7568ef 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java @@ -32,11 +32,15 @@ import java.util.function.Function; import org.apache.brooklyn.util.core.units.Range; import org.apache.brooklyn.util.core.xstream.ImmutableSetConverter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils.createBeanDeserializer; public abstract class JsonSymbolDependentDeserializer extends JsonDeserializer<Object> implements ContextualDeserializer { + private static final Logger log = LoggerFactory.getLogger(JsonSymbolDependentDeserializer.class); + public static final Set<JsonToken> SIMPLE_TOKENS = ImmutableSet.of( JsonToken.VALUE_STRING, JsonToken.VALUE_NUMBER_FLOAT, @@ -71,7 +75,13 @@ public abstract class JsonSymbolDependentDeserializer extends JsonDeserializer<O type = ctxt.getContextualType(); } if (isTypeReplaceableByDefault()) { - type = getDefaultType(); + JavaType type2 = getDefaultType(); + if (type!=null && !type.getRawClass().isAssignableFrom(type2.getRawClass())) { + // keep old type if it's not compatible; it might fail subsequently, but at least it won't be incompatible + log.warn("Default type for "+type+" reported as "+type2+", in "+this+", which is not compatible; may fail subsequently"); + // but change it, as if we leave it as an interface it will almost certainly fail + } + type = type2; } return this; diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java index 9831f2dccb..7443d91b92 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java @@ -143,6 +143,25 @@ public class WorkflowExpressionResolution { return null; } + /** allow access to selected static things in a non-workflow context; e.g. time */ + public static class WorkflowWithoutContextFreemarkerModel implements TemplateHashModel { + @Override + public TemplateModel get(String key) throws TemplateModelException { + if ("workflow".equals(key)) { + return new TemplateHashModel() { + @Override + public TemplateModel get(String key) throws TemplateModelException { + if ("util".equals(key)) return new WorkflowStaticUtilModel(); + return null; + } + @Override public boolean isEmpty() throws TemplateModelException { return false; } + }; + } + return null; + } + @Override public boolean isEmpty() throws TemplateModelException { return false; } + } + public class WorkflowFreemarkerModel implements TemplateHashModel, TemplateProcessor.UnwrappableTemplateModel { @Override public Maybe<Object> unwrap() { @@ -358,9 +377,9 @@ public class WorkflowExpressionResolution { } } - class WorkflowUtilModel implements TemplateHashModel { + static class WorkflowStaticUtilModel implements TemplateHashModel { - WorkflowUtilModel() {} + WorkflowStaticUtilModel() {} @Override public TemplateModel get(String key) throws TemplateModelException { @@ -373,7 +392,7 @@ public class WorkflowExpressionResolution { if ("now_nice".equals(key)) return TemplateProcessor.wrapAsTemplateModel(Time.makeDateString(Instant.now())); if ("random".equals(key)) return TemplateProcessor.wrapAsTemplateModel(Math.random()); - return ifNoMatches(); + return null; } @Override @@ -382,6 +401,20 @@ public class WorkflowExpressionResolution { } } + class WorkflowUtilModel extends WorkflowStaticUtilModel { + + WorkflowUtilModel() {} + @Override + public TemplateModel get(String key) throws TemplateModelException { + + // could put workflow-specific context things here + + TemplateModel result = super.get(key); + if (result!=null) return result; + return ifNoMatches(); + } + } + AllowBrooklynDslMode defaultAllowBrooklynDsl = null; //AllowBrooklynDslMode.ALL; @@ -406,6 +439,7 @@ public class WorkflowExpressionResolution { return inResolveStackEntry("resolve-coercing", expression, () -> { boolean triedCoercion = false; List<Exception> exceptions = MutableList.of(); + List<Exception> deferredExceptions = MutableList.of(); if (expression instanceof String) { try { // prefer simple coercion if it's a string coming in @@ -424,7 +458,7 @@ public class WorkflowExpressionResolution { RegisteredTypes.getClassLoadingContext(context.getEntity()), true /* needed for wrapped resolved holders */); } catch (Exception e) { Exceptions.propagateIfFatal(e); - exceptions.add(e); + deferredExceptions.add(e); } } @@ -439,6 +473,7 @@ public class WorkflowExpressionResolution { } } + exceptions.addAll(deferredExceptions); throw Exceptions.propagate(exceptions.iterator().next()); }); } @@ -660,6 +695,10 @@ public class WorkflowExpressionResolution { return new WorkflowFreemarkerModel(); } + public static TemplateHashModel newWorkflowFreemarkerModel(WorkflowExpressionResolution context) { + return context==null ? new WorkflowWithoutContextFreemarkerModel() : context.newWorkflowFreemarkerModel(); + } + public WorkflowExecutionContext getWorkflowExecutionContext() { return context; } @@ -689,7 +728,7 @@ public class WorkflowExpressionResolution { BiFunction<String, WorkflowExpressionResolution, Object> fn = context.getManagementContext().getScratchpad().get(WORKFLOW_CUSTOM_INTERPOLATION_FUNCTION); if (fn!=null) result = fn.apply(expression, this); else result = TemplateProcessor.processTemplateContentsForWorkflow("workflow", expression, - newWorkflowFreemarkerModel(), true, false, errorMode); + newWorkflowFreemarkerModel(this), true, false, errorMode); } finally { if (ourWait) interruptClear(); diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java index 58e445e296..11333f2c45 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java @@ -153,7 +153,7 @@ public class WorkflowPolicy<T> extends AbstractPolicy implements WorkflowCommonC } protected WorkflowPollCallable newWorkflowPollCallable() { - return new WorkflowPollCallable(WorkflowContextType.POLICY, getDisplayName() + " (policy)", config().getBag(), this); + return new WorkflowPollCallable(WorkflowContextType.POLICY, getDisplayName() + " (policy)", this); } @Override diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowSensor.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowSensor.java index 2a9c08b8e8..fc564d91b5 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowSensor.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowSensor.java @@ -32,10 +32,13 @@ import org.apache.brooklyn.api.objs.EntityAdjunct; import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.SubElementConfigKey; +import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl; import org.apache.brooklyn.core.effector.AddSensorInitializer; import org.apache.brooklyn.core.effector.AddSensorInitializerAbstractProto; import org.apache.brooklyn.core.entity.EntityInitializers; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; +import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal; import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.workflow.steps.appmodel.EntityValueToSet; @@ -158,6 +161,17 @@ public class WorkflowSensor<T> extends AbstractAddTriggerableSensor<T> implement private final Map<String,Object> params; private final WorkflowExecutionContext.WorkflowContextType wcType; + protected WorkflowPollCallable(WorkflowExecutionContext.WorkflowContextType wcType, String workflowCallableName, BrooklynObject entityOrAdjunct) { + this(wcType, workflowCallableName, getFlatBag(entityOrAdjunct), entityOrAdjunct); + } + static ConfigBag getFlatBag(BrooklynObject entityOrAdjunct) { + ConfigBag result = ConfigBag.newInstance(); + entityOrAdjunct.config().findKeysPresent(x -> true).forEach(k -> { + while (k instanceof SubElementConfigKey) k = ((SubElementConfigKey)k).parent; + result.put((ConfigKey)k, ((ConfigurationSupportInternal)entityOrAdjunct.config()).getRaw(k).or(k.getDefaultValue()) ); + }); + return result; + } protected WorkflowPollCallable(WorkflowExecutionContext.WorkflowContextType wcType, String workflowCallableName, ConfigBag params, BrooklynObject entityOrAdjunct) { this.wcType = wcType; this.workflowCallableName = workflowCallableName; diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java index 35b1752690..51cee03008 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java @@ -36,6 +36,7 @@ import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.util.JavaGroovyEquivalents; import org.apache.brooklyn.util.core.ClassLoaderUtils; import org.apache.brooklyn.util.core.predicates.DslPredicates; +import org.apache.brooklyn.util.core.task.DeferredSupplier; import org.apache.brooklyn.util.core.task.Tasks; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; @@ -388,6 +389,9 @@ public class TypeCoercions { if (input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrBoxedObject(input)) { return null; } + if (input instanceof DeferredSupplier) { + return null; + } // input is a complex type / bean boolean toMap = Map.class.isAssignableFrom(type.getRawType()); boolean toBeanWithType = !toMap && Reflections.findFieldMaybe(type.getRawType(), "type").isPresentAndNonNull(); diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java index d185c7a3ff..e28bab8317 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java @@ -18,10 +18,30 @@ */ package org.apache.brooklyn.util.core.predicates; +import java.io.IOException; +import java.lang.reflect.Method; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import javax.annotation.Nullable; + import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.BeanProperty; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.util.TokenBuffer; import com.google.common.annotations.Beta; @@ -68,18 +88,6 @@ import org.apache.brooklyn.util.text.WildcardGlobs; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nullable; -import java.io.IOException; -import java.lang.reflect.Method; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.util.*; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.BiFunction; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.regex.Pattern; - public class DslPredicates { private static final Logger LOG = LoggerFactory.getLogger(DslPredicates.class); @@ -95,9 +103,7 @@ public class DslPredicates { TypeCoercions.registerAdapter(String.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); TypeCoercions.registerAdapter(Boolean.class, DslPredicate.class, DslPredicates::always); -// TypeCoercions.registerAdapter(DeferredSupplier.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); -// TypeCoercions.registerAdapter(WorkflowExpressionResolution.WrappedUnresolvedExpression.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); - // not sure why above don't work, but below does + // use this to map more types of objects TypeCoercions.registerAdapter("60-expression-to-predicate", new TryCoercer() { @Override public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { @@ -127,8 +133,14 @@ public class DslPredicates { /** always returns false */ NEVER, } - static <T> T unwrapped(WrappedValue<T> t) { - return WrappedValue.get(t); + static <T> T unwrapped(WrappedValue<T> t, Class<T> type) { + return TypeCoercions.coerce(WrappedValue.get(t), type); + } + static <T> T unwrapped(WrappedValue<T> t, TypeToken<T> type) { + return TypeCoercions.coerce(WrappedValue.get(t), type); + } + static DslPredicate<?> unwrappedPredicate(WrappedValue t) { + return unwrapped(t, DslPredicate.class); } static Object unwrappedObject(Object t) { @@ -345,7 +357,7 @@ public class DslPredicates { } } - public Object implicitEqualsUnwrapped() { return unwrapped(implicitEquals); } + public Object implicitEqualsUnwrapped() { return unwrapped(implicitEquals, Object.class); } public boolean apply(T input) { Maybe<Object> result = resolveTargetAgainstInput(input); @@ -497,9 +509,8 @@ public class DslPredicates { if (assertCondition!=null) failOnAssertCondition(result, checker); checker.check(implicitEquals, result, (implicitTestSpec, value) -> { - // if a condition somehow gets put into the implicit equals, e.g. via an expression returning an expression, then recognize it as a condition - Object test = unwrapped(implicitTestSpec); + Object test = unwrapped(implicitTestSpec, Object.class); if (test instanceof DslPredicate) { return nestedPredicateCheck((DslPredicate) test, result); } @@ -517,8 +528,8 @@ public class DslPredicates { return DslPredicates.coercedEqual(implicitTestSpec, value); }); checker.check(equals, result, DslPredicates::coercedEqual); - checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(unwrapped(test), Pattern.DOTALL).matcher(v).matches())); - checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(unwrapped(test), v))); + checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(unwrapped(test, String.class), Pattern.DOTALL).matcher(v).matches())); + checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(unwrapped(test, String.class), v))); checker.check(inRange, result, (test,value) -> // current Range only supports Integer, but this code will support any @@ -549,10 +560,10 @@ public class DslPredicates { return nestedPredicateCheck(test, Maybe.of(computedSize)); }); - checker.checkTest(not, test -> !nestedPredicateCheck(unwrapped(test), result)); - checker.checkTest(check, test -> nestedPredicateCheck(unwrapped(test), result)); - checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(unwrapped(p), result))); - checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(unwrapped(p), result))); + checker.checkTest(not, test -> !nestedPredicateCheck(unwrapped(test, DslPredicate.class), result)); + checker.checkTest(check, test -> nestedPredicateCheck(unwrapped(test, DslPredicate.class), result)); + checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(unwrapped(p, DslPredicate.class), result))); + checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(unwrapped(p, DslPredicate.class), result))); checker.check(javaInstanceOf, result, this::checkJavaInstanceOf); } @@ -963,6 +974,7 @@ public class DslPredicates { @Override public JavaType getDefaultType() { + if (type!=null && DslEntityPredicate.class.isAssignableFrom(type.getRawClass())) return ctxt.constructType(DslEntityPredicateDefault.class); return ctxt.constructType(DslPredicateDefault.class); } diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java index 508b735b7e..c52e31534d 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.core.workflow; import com.fasterxml.jackson.core.JsonProcessingException; +import com.google.common.collect.Iterables; import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.mgmt.Task; @@ -81,8 +82,14 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor } @Test - public void testMapOutputAndInputFromLastStep() { - doTestMapOutputAndInput(cfg -> { + public void testMapOutputFromExplicitOutput() { + doTestMapOutput(cfg -> cfg.put(WorkflowEffector.OUTPUT, + MutableMap.of("c", "${c}", "d", "${d}", "e", "${e}") )); + } + + @Test + public void testMapOutputFromLastStep() { + doTestMapOutput(cfg -> { List<Object> step = cfg.get(WorkflowEffector.STEPS); step.add("let map result = { c: ${c}, d: ${d}, e: ${e} }"); cfg.put(WorkflowEffector.STEPS, step); @@ -91,12 +98,27 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor } @Test - public void testMapOutputAndInputFromExplicitOutput() { - doTestMapOutputAndInput(cfg -> cfg.put(WorkflowEffector.OUTPUT, - MutableMap.of("c", "${c}", "d", "${d}", "e", "${e}") )); + public void testMapOutputFromReturn() { + doTestMapOutput(cfg -> { + List<Object> step = cfg.get(WorkflowEffector.STEPS); + step.add("let map result = { c: ${c}, d: ${d}, e: ${e} }"); + step.add("return ${result}"); + cfg.put(WorkflowEffector.STEPS, step); + }); + } + + @Test + public void testMapOutputFromExplicitInput() { + doTestMapOutput(cfg -> { + List<Object> step = cfg.get(WorkflowEffector.STEPS); + step.add("let map result = { c: ${c}, d: ${d}, e: ${E} }"); + cfg.put(WorkflowEffector.STEPS, step); + cfg.put(WorkflowEffector.OUTPUT, "${result}"); + cfg.put(WorkflowEffector.INPUT, MutableMap.of("E", "b")); + }); } - public void doTestMapOutputAndInput(Consumer<ConfigBag> mod) { + public void doTestMapOutput(Consumer<ConfigBag> mod) { loadTypes(); BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class)); diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java index f22db62e2e..e915702c02 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/FlagUtilsTest.java @@ -404,11 +404,6 @@ public class FlagUtilsTest { return set(key.getConfigKey(), val); } - @Override @Deprecated - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter) { - return findKeysDeclared(filter); - } - @Override public Set<ConfigKey<?>> findKeysDeclared(Predicate<? super ConfigKey<?>> filter) { return MutableSet.copyOf(Iterables.filter(bag.getAllConfigAsConfigKeyMap().keySet(), filter)); diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java index 2045c690c4..147b6eca47 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java +++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigMap.java @@ -64,16 +64,6 @@ public interface ConfigMap { /** returns a read-only map of all local config keys with their raw (unresolved+uncoerced) contents */ public Map<ConfigKey<?>,Object> getAllConfigLocalRaw(); - /** @deprecated since 0.11.0 use {@link #findKeysDeclared(Predicate)} or {@link #findKeysPresent(Predicate)} - * <p> - * this method is like the above but it does not compare against reference keys in the container / type context. - * there are no known cases where that is the desired behaviour, so callers are being forced to migrate to - * one of the above other two methods. if keys in the map are needed and not the reference keys, - * let the brooklyn developers know to keep this functionality! */ - @Deprecated - // XXX - public Set<ConfigKey<?>> findKeys(Predicate<? super ConfigKey<?>> filter); - /** returns all keys present in the map matching the given filter predicate; see ConfigPredicates for common predicates. * if the map is associated with a container or type context where reference keys are defined, * those keys are included in the result whether or not present in the map (unlike {@link #findKeysPresent(Predicate)}) */
