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 737adf8b208f64664562e8be8b9e0e7417998762 Author: Alex Heneveld <[email protected]> AuthorDate: Thu May 30 14:03:23 2024 +0100 easier checking if config is null, and allow explicit null values in workflow allows values when set, but does not make it easy to set null --- .../org/apache/brooklyn/api/objs/Configurable.java | 3 ++ .../brooklyn/camp/brooklyn/ObjectsYamlTest.java | 9 +++-- .../config/internal/AbstractConfigMapImpl.java | 14 +++++++- .../core/internal/BrooklynPropertiesImpl.java | 26 ++++++++++++--- .../mgmt/internal/DeferredBrooklynProperties.java | 10 ++++-- .../objs/AbstractConfigurationSupportInternal.java | 6 +++- .../core/objs/BasicConfigurableObject.java | 4 +++ .../brooklyn/core/objs/BrooklynObjectInternal.java | 1 + .../workflow/WorkflowExpressionResolution.java | 30 ++++++++++------- .../WorkflowStepInstanceExecutionContext.java | 21 +++++++++--- .../brooklyn/util/core/config/ConfigBag.java | 22 +++++++++--- .../brooklyn/util/core/text/TemplateProcessor.java | 39 ++++++++++++++-------- .../workflow/WorkflowConfigSensorEffectorTest.java | 16 ++++----- .../brooklyn/util/core/internal/FlagUtilsTest.java | 12 ++----- .../java/org/apache/brooklyn/config/ConfigMap.java | 1 + .../exceptions/PropagatedRuntimeException.java | 12 ++++++- 16 files changed, 157 insertions(+), 69 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 2f7fb3c820..eac7cacb88 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 @@ -27,6 +27,7 @@ import org.apache.brooklyn.config.ConfigMap; import com.google.common.annotations.Beta; import com.google.common.base.Predicate; +import org.apache.brooklyn.util.guava.Maybe; /** * Something that has mutable config, such as an entity or policy. @@ -101,5 +102,7 @@ public interface Configurable { /** see {@link ConfigMap#findKeysPresent(Predicate)} */ public Set<ConfigKey<?>> findKeysPresent(Predicate<? super ConfigKey<?>> filter); + + public <T> Maybe<T> getMaybe(ConfigKey<T> ck); } } 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 31cf646aba..eeb6327a9c 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 @@ -44,6 +44,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.core.flags.TypeCoercions; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.time.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -158,13 +159,11 @@ public class ObjectsYamlTest extends AbstractYamlTest { private class BasicConfigurationSupport implements ConfigurationSupport { private final ConfigBag bag = new ConfigBag(); - @Override - public <T> T get(ConfigKey<T> key) { + @Override public <T> T get(ConfigKey<T> key) { return bag.get(key); } - - @Override - public <T> T get(HasConfigKey<T> key) { + @Override public <T> Maybe<T> getMaybe(ConfigKey<T> ck) { return bag.getMaybe(ck); } + @Override public <T> T get(HasConfigKey<T> key) { return get(key.getConfigKey()); } 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 f7610e56a3..c2a8e44d26 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 @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.concurrent.Future; @@ -58,6 +59,7 @@ import org.apache.brooklyn.util.core.internal.ConfigKeySelfExtracting; 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.exceptions.PropagatedRuntimeException; import org.apache.brooklyn.util.exceptions.ReferenceWithError; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.Maybe.MaybeSupplier; @@ -108,7 +110,17 @@ public abstract class AbstractConfigMapImpl<TContainer extends BrooklynObject> i @Override public <T> T getConfig(ConfigKey<T> key) { - return getConfigImpl(key, false).getWithoutError().get(); +// return getConfigImpl(key, false).getWithoutError().get(); + return getConfigMaybe(key).orNull(); + } + + @Override + public <T> Maybe<T> getConfigMaybe(ConfigKey<T> key) { + ReferenceWithError<ConfigValueAtContainer<TContainer, T>> vo = getConfigImpl(key, false); + ConfigValueAtContainer<TContainer, T> v = vo.getWithoutError(); + if (v.isValueExplicitlySet()) return Maybe.of(v.get()); + if (v.getDefaultValue()!=null) return v.getDefaultValue(); + return Maybe.absent(() -> PropagatedRuntimeException.asRuntimeException(vo.getError(), () -> new NoSuchElementException("No value or default for '"+key.getName()+"'"))); } @Override 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 6e4704e8a7..143b55e801 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 @@ -463,17 +463,26 @@ public class BrooklynPropertiesImpl implements BrooklynProperties { return getConfig(key.getConfigKey(), defaultValue); } + @Override + public <T> Maybe<T> getConfigMaybe(ConfigKey<T> key) { + return getConfigMaybe(key, null,Maybe.absent("Key not present and no default value")); + } + @Override public <T> T getConfig(ConfigKey<T> key, T defaultValue) { + return getConfigMaybe(key, defaultValue==null ? null : Maybe.of(defaultValue), null).orNull(); + } + public <T> Maybe<T> getConfigMaybe(ConfigKey<T> key, Maybe<T> defaultValueForceOverrideIfPresent, + Maybe<T> defaultValueIfNoKeyDefault) { // TODO does not support MapConfigKey etc where entries use subkey notation; for now, access using submap if (!containsKey(key.getName())) { - if (defaultValue!=null) return defaultValue; - return key.getDefaultValue(); + if (defaultValueForceOverrideIfPresent!=null && defaultValueForceOverrideIfPresent.isPresent()) return defaultValueForceOverrideIfPresent; + return Maybe.ofDisallowingNull(key.getDefaultValue()).or(defaultValueIfNoKeyDefault!=null ? defaultValueIfNoKeyDefault : Maybe.absent()); } - Object value = get(key.getName()); - if (value==null) return null; + Maybe<Object> value = getMaybe(key.getName()); + if (value.isAbsent()) return Maybe.castAbsent(value); // shouldn't happen // no evaluation / key extraction here - return TypeCoercions.coerce(value, key.getTypeToken()); + return Maybe.ofAllowingNull(TypeCoercions.coerce(value.get(), key.getTypeToken())); } @Override @@ -606,6 +615,13 @@ public class BrooklynPropertiesImpl implements BrooklynProperties { return contents.get(key); } } + + protected Maybe<Object> getMaybe(String key) { + synchronized (contents) { + if (!contents.containsKey(key)) return Maybe.absent(); + return Maybe.ofAllowingNull(contents.get(key)); + } + } protected Object putImpl(String key, Object value) { synchronized (contents) { 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 526071e0a7..9403d17c6d 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 @@ -130,8 +130,14 @@ public class DeferredBrooklynProperties implements BrooklynProperties { @Override public <T> T getConfig(ConfigKey<T> key) { - T raw = delegate.getConfig(key); - return resolve(key, raw); + return getConfigMaybe(key).orNull(); + } + + @Override + public <T> Maybe<T> getConfigMaybe(ConfigKey<T> key) { + Maybe<T> raw = delegate.getConfigMaybe(key); + if (raw.isAbsent()) return Maybe.castAbsent(raw); + return Maybe.ofAllowingNull(resolve(key, raw.get())); } @Override 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 b1ae127a23..f8c9c19f4f 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 @@ -178,7 +178,11 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb @Override public <T> T get(ConfigKey<T> key) { // validation done by getConfig call below - return (T) getConfigsInternal().getConfig(key); + return getConfigsInternal().getConfig(key); + } + + public <T> Maybe<T> getMaybe(ConfigKey<T> key) { + return getConfigsInternal().getConfigMaybe(key); } @SuppressWarnings("unchecked") 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 d81383b87d..d70deced28 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 @@ -34,6 +34,7 @@ import org.apache.brooklyn.core.mgmt.ManagementContextInjectable; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.SetFromFlag; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.Identifiers; /** @@ -90,6 +91,9 @@ public class BasicConfigurableObject implements Configurable, Identifiable, Mana return config.get(key); } + @Override + public <T> Maybe<T> getMaybe(ConfigKey<T> ck) { return config.getMaybe(ck); } + @Override public <T> T get(HasConfigKey<T> key) { return get(key.getConfigKey()); 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 84820a0be4..6ab9902e62 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 @@ -29,6 +29,7 @@ import org.apache.brooklyn.api.objs.Configurable; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.config.ConfigMap.ConfigMapWithInheritance; +import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.task.ImmediateSupplier; import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateUnsupportedException; 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 7443d91b92..633ddbfc2b 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 @@ -190,22 +190,22 @@ public class WorkflowExpressionResolution { return ifNoMatches(); } - Object candidate = null; + Maybe candidate = Maybe.absent(); if (stage.after(WorkflowExpressionStage.STEP_PRE_INPUT)) { //somevar -> workflow.current_step.output.somevar WorkflowStepInstanceExecutionContext currentStep = context.currentStepInstance; if (currentStep != null && stage.after(WorkflowExpressionStage.STEP_OUTPUT)) { if (currentStep.getOutput() instanceof Map) { - candidate = ((Map) currentStep.getOutput()).get(key); - if (candidate != null) return TemplateProcessor.wrapAsTemplateModel(candidate); + candidate = getMapMaybe((Map) currentStep.getOutput(), key); + if (candidate.isPresent()) return TemplateProcessor.wrapAsTemplateModelAllowingNull(candidate); } } //somevar -> workflow.current_step.input.somevar try { if (currentStep!=null) { - candidate = currentStep.getInput(key, Object.class); + candidate = currentStep.getInputMaybe(key, Object.class); } } catch (Throwable t) { Exceptions.propagateIfFatal(t); @@ -241,33 +241,33 @@ public class WorkflowExpressionResolution { // // settled on (d) effectively; we allow local references, and fail on recursive references, with exceptions. // the main exception, handled here, is if we are setting an input - candidate = null; + candidate = Maybe.absent(); errors.add(t); } } - if (candidate != null) return TemplateProcessor.wrapAsTemplateModel(candidate); + if (candidate.isPresent()) return TemplateProcessor.wrapAsTemplateModelAllowingNull(candidate); } //workflow.previous_step.output.somevar if (stage.after(WorkflowExpressionStage.WORKFLOW_INPUT)) { Object prevStepOutput = context.getPreviousStepOutput(); if (prevStepOutput instanceof Map) { - candidate = ((Map) prevStepOutput).get(key); - if (candidate != null) return TemplateProcessor.wrapAsTemplateModel(candidate); + candidate = getMapMaybe((Map) prevStepOutput, key); + if (candidate.isPresent()) return TemplateProcessor.wrapAsTemplateModelAllowingNull(candidate); } } //workflow.scratch.somevar if (stage.after(WorkflowExpressionStage.WORKFLOW_INPUT)) { - candidate = context.getWorkflowScratchVariables().get(key); - if (candidate != null) return TemplateProcessor.wrapAsTemplateModel(candidate); + candidate = getMapMaybe(context.getWorkflowScratchVariables(), key); + if (candidate.isPresent()) return TemplateProcessor.wrapAsTemplateModelAllowingNull(candidate); } //workflow.input.somevar if (context.input.containsKey(key)) { - candidate = context.getInput(key); + candidate = context.getInputMaybe(key, TypeToken.of(Object.class), candidate); // the subtlety around step input above doesn't apply here as workflow inputs are not resolved with freemarker - if (candidate != null) return TemplateProcessor.wrapAsTemplateModel(candidate); + if (candidate.isPresent()) return TemplateProcessor.wrapAsTemplateModelAllowingNull(candidate); } if (!errors.isEmpty()) Exceptions.propagate("Errors resolving "+key, errors); @@ -281,6 +281,12 @@ public class WorkflowExpressionResolution { } } + public static <K,V> Maybe<V> getMapMaybe(Map<K,V> map, K key) { + if (map==null) return Maybe.absent(); + if (map.containsKey(key)) return Maybe.ofAllowingNull(map.get(key)); + return Maybe.absent(); + } + class WorkflowExplicitModel implements TemplateHashModel, TemplateProcessor.UnwrappableTemplateModel { @Override public Maybe<Object> unwrap() { diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java index 3ede6cc447..3d1a4d19ce 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.exceptions.LossySerializingThrowable; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.javalang.Boxing; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -163,6 +164,9 @@ public class WorkflowStepInstanceExecutionContext { public <T> T getInput(String key, Class<T> type) { return getInput(key, TypeToken.of(type)); } + public <T> Maybe<T> getInputMaybe(String key, Class<T> type) { + return getInputMaybe(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key, TypeToken.of(type)); + } /** Returns the resolved value of the given key, converting to the given type. * Stores the resolved input so if re-resolved it returns the same. * (Input is not resolved until first access because some implementations, such as 'let', might handle errors in resolution. @@ -177,23 +181,30 @@ public class WorkflowStepInstanceExecutionContext { return input.containsKey(key); } public <T> T getInput(WorkflowExpressionResolution.WorkflowExpressionStage stage, String key, TypeToken<T> type) { - if (inputResolved.containsKey(key)) return (T)inputResolved.get(key); + return getInputMaybe(stage, key, type).orNull(); + } + public <T> Maybe<T> getInputMaybe(WorkflowExpressionResolution.WorkflowExpressionStage stage, String key, TypeToken<T> type) { + if (inputResolved.containsKey(key)) return Maybe.ofAllowingNull((T)inputResolved.get(key)); + + Maybe<Object> vm = WorkflowExpressionResolution.getMapMaybe(input, key); + if (vm.isAbsent()) return Maybe.castAbsent(vm); - Object v = input.get(key); + if (vm.isNull()) return Maybe.ofAllowingNull(null); + Object v = vm.get(); T v2; try { v2 = WorkflowExpressionResolution.allowingRecursionWhenSetting(context, WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key, () -> context.resolve(stage, v, type)); } catch (Exception e) { - throw Exceptions.propagateAnnotated("Cannot resolve input "+ - (Boxing.isPrimitiveOrStringOrBoxedObject(v) ? "'"+v+"'" : v.getClass().getName() + " ("+v+")"), e); + throw Exceptions.propagateAnnotated("Cannot resolve input " + + (Boxing.isPrimitiveOrStringOrBoxedObject(v) ? "'" + v + "'" : v.getClass().getName() + " (" + v + ")"), e); } if (REMEMBER_RESOLVED_INPUT) { if (!Objects.equals(v, v2)) { inputResolved.put(key, v2); } } - return v2; + return Maybe.of(v2); } /** Returns the unresolved value of the given key */ public Object getInputRaw(String key) { diff --git a/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java b/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java index 0ccbe5cfa4..2aa18c2586 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/config/ConfigBag.java @@ -371,11 +371,16 @@ public class ConfigBag { } /** returns the value of this config key, falling back to its default (use containsKey to see whether it was contained); + * and null if not found; * also marks it as having been used (use peek to prevent marking as used) */ public <T> T get(ConfigKey<T> key) { return get(key, true); } + /** as get, but wrapped as a maybe so explicit null can be investigated */ + public <T> Maybe<T> getMaybe(ConfigKey<T> key) { + return getMaybe(key, true); + } /** gets a value from a string-valued key or null; ConfigKey is preferred, but this is useful in some contexts (e.g. setting from flags) */ public Object getStringKey(String key) { @@ -535,33 +540,40 @@ public class ConfigBag { } protected <T> T get(ConfigKey<T> key, boolean markUsed) { + return getMaybe(key, markUsed).orNull(); + } + protected <T> Maybe<T> getMaybe(ConfigKey<T> key, boolean markUsed) { // TODO for now, no evaluation -- maps / closure content / other smart (self-extracting) keys are NOT supported // (need a clean way to inject that behaviour, as well as desired TypeCoercions) // this method, and the coercion, is not synchronized, nor does it need to be, because the "get" is synchronized. Maybe<Object> val = getKeyMaybeUncoercedIfPresent(key, markUsed); - return coerceFirstNonNullKeyValue(key, val.orNull()); + if (val.isNull()) return Maybe.ofAllowingNull(null); + return coerceFirstNonNullKeyValueMaybe(key, val.orNull()); } /** If the key is set, return the type-correct (coerced) value, but not any default; otherwise returns absent */ public <T> Maybe<T> ifPresent(ConfigKey<T> key) { Maybe<Object> val = getKeyMaybeUncoercedIfPresent(key, true); - if (val.isAbsent()) return ((Maybe<T>)val); - return Maybe.of(coerceFirstNonNullKeyValue(key, val.get())); + if (val.isAbsent() || val.isNull()) return ((Maybe<T>)val); // don't allow default, and do allow null + return coerceFirstNonNullKeyValueMaybe(key, val.orNull()); } /** returns the first non-null value to be the type indicated by the key, or the keys default value if no non-null values are supplied */ public static <T> T coerceFirstNonNullKeyValue(ConfigKey<T> key, Object ...values) { + return coerceFirstNonNullKeyValueMaybe(key, values).orNull(); + } + public static <T> Maybe<T> coerceFirstNonNullKeyValueMaybe(ConfigKey<T> key, Object ...values) { for (Object o: values) { if (o != null) { try { - return TypeCoercions.coerce(o, key.getTypeToken()); + return Maybe.ofAllowingNull(TypeCoercions.coerce(o, key.getTypeToken())); } catch (Exception e) { throw Exceptions.propagate("Error resolving "+key.getName()+" value "+o+" as "+key.getTypeToken()+": "+e, e); } } } try { - return TypeCoercions.coerce(key.getDefaultValue(), key.getTypeToken()); + return Maybe.ofDisallowingNull(TypeCoercions.coerce(key.getDefaultValue(), key.getTypeToken())); } catch (Exception e) { throw Exceptions.propagate("Error resolving "+key.getName()+" default value "+key.getDefaultValue()+" as "+key.getTypeToken()+": "+e, e); } diff --git a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java index b620ca11ef..1ab41b167a 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java @@ -37,6 +37,7 @@ import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.Identifiable; import org.apache.brooklyn.api.sensor.AttributeSensor; +import org.apache.brooklyn.core.config.BasicConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.effector.EffectorBase; import org.apache.brooklyn.core.entity.BrooklynConfigKeys; @@ -85,6 +86,14 @@ public class TemplateProcessor { static BrooklynFreemarkerUnwrappableObjectWrapper BROOKLYN_WRAPPER = new BrooklynFreemarkerUnwrappableObjectWrapper(); public static TemplateModel wrapAsTemplateModel(Object o) throws TemplateModelException { return BROOKLYN_WRAPPER.wrap(o); } + public static TemplateModel wrapAsTemplateModelAllowingNull(Maybe<?> m) throws TemplateModelException { + if (m.isPresent()) { + Object o = m.get(); + if (o==null) return TemplateModel.NOTHING; + return BROOKLYN_WRAPPER.wrap(o); + } + return null; + } public static Maybe<Object> unwrapTemplateModelMaybe(TemplateModel templateModel) { return BROOKLYN_WRAPPER.unwrapMaybe(templateModel); } static ThreadLocalStack<Map<TemplateModel,Object>> TEMPLATE_MODEL_UNWRAP_CACHE = new ThreadLocalStack<>(); @@ -111,6 +120,8 @@ public class TemplateProcessor { result = ((UnwrappableTemplateModel) model).unwrap(); if (result.isPresent()) return result; } + if (TemplateModel.NOTHING.getClass().isInstance(model)) return Maybe.ofAllowingNull(null); + Maybe<Map<TemplateModel, Object>> unwrappingMapM = TEMPLATE_MODEL_UNWRAP_CACHE.peek(); if (unwrappingMapM.isAbsent()) { return Maybe.absent("This thread does not support unwrapping"); @@ -457,13 +468,14 @@ public class TemplateProcessor { @Override public TemplateModel get(String key) throws TemplateModelException { try { - Object result = entity.getConfig(ConfigKeys.builder(Object.class).name(key).build()); + BasicConfigKey<Object> ck = ConfigKeys.builder(Object.class).name(key).build(); + Maybe<Object> result = entity.config().getMaybe(ck); - if (result==null) - result = mgmt.getConfig().getConfig(ConfigKeys.builder(Object.class).name(key).build()); + if (result.isAbsent()) + result = mgmt.getConfig().getConfigMaybe(ck); - if (result!=null) - return wrapAsTemplateModel( result ); + if (result.isPresent()) + return wrapAsTemplateModelAllowingNull( result ); } catch (Exception e) { throw handleModelError("Error accessing config '"+key+"' on "+entity, e); @@ -496,10 +508,9 @@ public class TemplateProcessor { @Override public TemplateModel get(String key) throws TemplateModelException { try { - Object result = mgmt.getConfig().getConfig(ConfigKeys.builder(Object.class).name(key).build()); + Maybe<Object> result = mgmt.getConfig().getConfigMaybe(ConfigKeys.builder(Object.class).name(key).build()); - if (result!=null) - return wrapAsTemplateModel( result ); + if (result.isPresent()) return wrapAsTemplateModelAllowingNull( result ); } catch (Exception e) { Exceptions.propagateIfFatal(e); @@ -535,15 +546,14 @@ public class TemplateProcessor { @Override public TemplateModel get(String key) throws TemplateModelException { try { - Object result = null; + Maybe<Object> result = null; - result = location.getConfig(ConfigKeys.builder(Object.class).name(key).build()); + result = location.config().getMaybe(ConfigKeys.builder(Object.class).name(key).build()); - if (result==null && mgmt!=null) - result = mgmt.getConfig().getConfig(ConfigKeys.builder(Object.class).name(key).build()); + if (result.isAbsent() && mgmt!=null) + result = mgmt.getConfig().getConfigMaybe(ConfigKeys.builder(Object.class).name(key).build()); - if (result!=null) - return wrapAsTemplateModel( result ); + if (result.isPresent()) return wrapAsTemplateModelAllowingNull( result ); } catch (Exception e) { Exceptions.propagateIfFatal(e); @@ -617,6 +627,7 @@ public class TemplateProcessor { @Override public TemplateModel get(String key) throws TemplateModelException { + // TODO could support null here, like we do for sensors and workflow Object result; try { result = diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowConfigSensorEffectorTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowConfigSensorEffectorTest.java index d1b81f6231..48c7d465cf 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowConfigSensorEffectorTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowConfigSensorEffectorTest.java @@ -201,15 +201,13 @@ public class WorkflowConfigSensorEffectorTest extends RebindTestFixture<TestAppl "let y = ${x} ?? missing", "return ${y}")); Asserts.assertEquals(lastInvocation.getUnchecked(), "missing"); - Asserts.assertFailsWith(() -> runWorkflow(MutableList.of("let entity x = chilldx", - "return ${x}")), - // would be nice not to cast it to null if not found, and maybe for return null to be allowed (?); - // might want a different way to look up an entity (functional?) -// Asserts.expectedFailureContainsIgnoreCase("entity", "not known", "chilldx") -// .and(Asserts.expectedFailureDoesNotContainIgnoreCase("${x}", "null")) - // but for now it is useful to be able to do lookups, and null lets us tell it wasn't found - Asserts.expectedFailureContainsIgnoreCase("${x}", "null") - ); + + Asserts.assertEquals(runWorkflow(MutableList.of("let entity x = chilldx", "return ${x}")), null); + // previously the above failed; it set x as null, but couldn't return it. + // now it can return null, so it works. + // we might want to be stricter that entities not found throw an error, + // or have a cleaner way to find an entity (possibly with more search control?), + // at which point we can provide more semantics } } 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 e915702c02..638240fb47 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 @@ -372,15 +372,9 @@ public class FlagUtilsTest { } private class BasicConfigurationSupport implements ConfigurationSupport { - @Override - public <T> T get(ConfigKey<T> key) { - return bag.get(key); - } - - @Override - public <T> T get(HasConfigKey<T> key) { - return get(key.getConfigKey()); - } + @Override public <T> T get(ConfigKey<T> key) { return bag.get(key); } + @Override public <T> Maybe<T> getMaybe(ConfigKey<T> ck) { return bag.getMaybe(ck); } + @Override public <T> T get(HasConfigKey<T> key) { return get(key.getConfigKey()); } @Override public <T> T set(ConfigKey<T> key, T val) { 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 147b6eca47..67bc66eab2 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 @@ -33,6 +33,7 @@ public interface ConfigMap { /** returns the config, resolved and inheritance rules applied, with default value as per the key, or null */ public <T> T getConfig(ConfigKey<T> key); + public <T> Maybe<T> getConfigMaybe(ConfigKey<T> key); /** @see #getConfig(ConfigKey), with default value as per the key, or null */ public <T> T getConfig(HasConfigKey<T> key); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java index 50fbe2c2b6..9514c8ff9b 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/exceptions/PropagatedRuntimeException.java @@ -18,7 +18,8 @@ */ package org.apache.brooklyn.util.exceptions; -import org.apache.brooklyn.util.text.Strings; +import java.util.function.Supplier; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,4 +79,13 @@ public class PropagatedRuntimeException extends RuntimeException { return causeEmbeddedInMessage; } + public static RuntimeException asRuntimeException(Throwable t) { + return asRuntimeException(t, null); + } + public static RuntimeException asRuntimeException(Throwable t, Supplier<RuntimeException> ifNull) { + if (t==null) return ifNull==null ? null : ifNull.get(); + if (t instanceof RuntimeException) return (RuntimeException) t; + return new PropagatedRuntimeException(t); + } + }
