Rename DslCallable to DslFunctionSource and some cleanup Addresses review comments.
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/60c5c6fa Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/60c5c6fa Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/60c5c6fa Branch: refs/heads/master Commit: 60c5c6fa97ae3bd33548832123e5f50c5532b128 Parents: b75b45c Author: Svetoslav Neykov <svetoslav.ney...@cloudsoftcorp.com> Authored: Tue Dec 13 16:42:57 2016 +0200 Committer: Svetoslav Neykov <svetoslav.ney...@cloudsoftcorp.com> Committed: Tue Dec 13 16:42:57 2016 +0200 ---------------------------------------------------------------------- .../spi/dsl/BrooklynDslInterpreter.java | 7 +- .../camp/brooklyn/spi/dsl/DslCallable.java | 26 ------ .../spi/dsl/DslDeferredFunctionCall.java | 95 ++++++++++---------- .../brooklyn/spi/dsl/DslFunctionSource.java | 26 ++++++ .../brooklyn/spi/dsl/methods/DslComponent.java | 4 +- .../camp/brooklyn/spi/dsl/DslYamlTest.java | 32 ++++++- .../spi/dsl/methods/DslTestObjects.java | 7 +- .../brooklyn/util/core/task/ValueResolver.java | 1 - 8 files changed, 115 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslInterpreter.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslInterpreter.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslInterpreter.java index 16d5c5a..ab20c3b 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslInterpreter.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslInterpreter.java @@ -33,7 +33,6 @@ import org.apache.brooklyn.camp.spi.resolve.interpret.PlanInterpretationNode; import org.apache.brooklyn.camp.spi.resolve.interpret.PlanInterpretationNode.Role; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; -import org.apache.brooklyn.util.javalang.Reflections; import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -170,12 +169,14 @@ public class BrooklynDslInterpreter extends PlanInterpreterAdapter { args.add( deepEvaluation ? evaluate(arg, true) : arg ); } try { - if (o instanceof BrooklynDslDeferredSupplier && !(o instanceof DslCallable)) { + // TODO Could move argument resolve in DslDeferredFunctionCall freeing each Deffered implementation + // having to handle it separately. The shortcoming is that will lose the eager evaluation we have here. + if (o instanceof BrooklynDslDeferredSupplier && !(o instanceof DslFunctionSource)) { return new DslDeferredFunctionCall((BrooklynDslDeferredSupplier<?>) o, fn, args); } else { // Would prefer to keep the invocation logic encapsulated in DslDeferredFunctionCall, but // for backwards compatibility will evaluate as much as possible eagerly (though it shouldn't matter in theory). - return DslDeferredFunctionCall.invokeOn(o, fn, args); + return DslDeferredFunctionCall.invokeOn(o, fn, args).get(); } } catch (Exception e) { Exceptions.propagateIfFatal(e); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslCallable.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslCallable.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslCallable.java deleted file mode 100644 index 4eeddb7..0000000 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslCallable.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2016 The Apache Software Foundation. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.brooklyn.camp.brooklyn.spi.dsl; - -import org.apache.brooklyn.util.core.task.DeferredSupplier; - -/** - * Marker interface so the evaluator can tell apart objects which are {@link DeferredSupplier} - * but which expect DSL methods called on them instead of the value they supply. - */ -public interface DslCallable { - -} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java index ad8c4b1..638f36f 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java @@ -37,12 +37,11 @@ public class DslDeferredFunctionCall extends BrooklynDslDeferredSupplier<Object> private static final long serialVersionUID = 3243262633795112155L; - // TODO should this be some of the super types? - private BrooklynDslDeferredSupplier<?> object; + private Object object; private String fnName; private List<?> args; - public DslDeferredFunctionCall(BrooklynDslDeferredSupplier<?> o, String fn, List<Object> args) { + public DslDeferredFunctionCall(Object o, String fn, List<Object> args) { this.object = o; this.fnName = fn; this.args = args; @@ -50,15 +49,7 @@ public class DslDeferredFunctionCall extends BrooklynDslDeferredSupplier<Object> @Override public Maybe<Object> getImmediately() { - Maybe<?> obj = resolveImmediate(object); - if (obj.isPresent()) { - if (obj.isNull()) { - throw new IllegalArgumentException("Deferred function call, " + object + - " evaluates to null (when calling " + fnName + "(" + toString(args) + "))"); - } - return Maybe.of(invokeOn(obj.get())); - } - return Maybe.absent("Could not evaluate immediately " + object); + return invokeOnDeferred(object, true); } @Override @@ -70,52 +61,37 @@ public class DslDeferredFunctionCall extends BrooklynDslDeferredSupplier<Object> .body(new Callable<Object>() { @Override public Object call() throws Exception { - Object obj = DslDeferredFunctionCall.this.resolveBlocking(object).orNull(); - if (obj == null) { - throw new IllegalArgumentException("Deferred function call, " + object + - " evaluates to null (when calling " + fnName + "(" + DslDeferredFunctionCall.toString(args) + "))"); - } - return invokeOn(obj); + return invokeOnDeferred(object, false).get(); } }).build(); } - protected Maybe<?> resolveImmediate(Object object) { - return resolve(object, true); - } - protected Maybe<?> resolveBlocking(Object object) { - return resolve(object, false); - } - protected Maybe<?> resolve(Object object, boolean immediate) { - if (object instanceof DslCallable || object == null) { - return Maybe.of(object); - } - Maybe<?> resultMaybe = Tasks.resolving(object, Object.class) - .context(((EntityInternal)entity()).getExecutionContext()) - .deep(true) - .immediately(immediate) - .recursive(false) - .getMaybe(); - if (resultMaybe.isAbsent()) { - return resultMaybe; + protected Maybe<Object> invokeOnDeferred(Object obj, boolean immediate) { + Maybe<?> resolvedMaybe = resolve(obj, immediate); + if (resolvedMaybe.isPresent()) { + Object instance = resolvedMaybe.get(); + + if (instance == null) { + throw new IllegalArgumentException("Deferred function call, " + object + + " evaluates to null (when calling " + fnName + "(" + toString(args) + "))"); + } + + return invokeOn(instance); } else { - // No nice way to figure out whether the object is deferred. Try to resolve it - // until it matches the input value as a poor man's replacement. - Object result = resultMaybe.get(); - if (result == object) { - return resultMaybe; + if (immediate) { + return Maybe.absent("Could not evaluate immediately " + obj); } else { - return resolve(result, immediate); + return Maybe.absent(Maybe.getException(resolvedMaybe)); } } } - protected Object invokeOn(Object obj) { + protected Maybe<Object> invokeOn(Object obj) { return invokeOn(obj, fnName, args); } - protected static Object invokeOn(Object obj, String fnName, List<?> args) { + protected static Maybe<Object> invokeOn(Object obj, String fnName, List<?> args) { Object instance = obj; List<?> instanceArgs = args; Maybe<Method> method = Reflections.getMethodFromArgs(instance, fnName, instanceArgs); @@ -148,12 +124,39 @@ public class DslDeferredFunctionCall extends BrooklynDslDeferredSupplier<Object> try { // Value is most likely another BrooklynDslDeferredSupplier - let the caller handle it, - return Reflections.invokeMethodFromArgs(instance, m, instanceArgs); + return Maybe.of(Reflections.invokeMethodFromArgs(instance, m, instanceArgs)); } catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) { + // If the method is there but not executable for whatever reason fail with a fatal error, don't return an absent. throw Exceptions.propagate(new InvocationTargetException(e, "Error invoking '"+fnName+"("+toString(instanceArgs)+")' on '"+instance+"'")); } } else { - throw new IllegalArgumentException("No such function '"+fnName+"("+toString(args)+")' on "+obj); + return Maybe.absent(new IllegalArgumentException("No such function '"+fnName+"("+toString(args)+")' on "+obj)); + } + } + + protected Maybe<?> resolve(Object object, boolean immediate) { + if (object instanceof DslFunctionSource || object == null) { + return Maybe.of(object); + } + + Maybe<?> resultMaybe = Tasks.resolving(object, Object.class) + .context(((EntityInternal)entity()).getExecutionContext()) + .deep(true) + .immediately(immediate) + .recursive(false) + .getMaybe(); + + if (resultMaybe.isPresent()) { + // No nice way to figure out whether the object is deferred. Try to resolve it + // until it matches the input value as a poor man's replacement. + Object result = resultMaybe.get(); + if (result == object) { + return resultMaybe; + } else { + return resolve(result, immediate); + } + } else { + return resultMaybe; } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslFunctionSource.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslFunctionSource.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslFunctionSource.java new file mode 100644 index 0000000..ac85603 --- /dev/null +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslFunctionSource.java @@ -0,0 +1,26 @@ +/* + * Copyright 2016 The Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.brooklyn.camp.brooklyn.spi.dsl; + +import org.apache.brooklyn.util.core.task.DeferredSupplier; + +/** + * Marker interface so the evaluator can tell apart objects which are {@link DeferredSupplier} + * but which expect DSL methods called on them instead of the value they supply. + */ +public interface DslFunctionSource { + +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java index d6859fa..37b9c59 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java @@ -31,7 +31,7 @@ import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.api.sensor.Sensor; import org.apache.brooklyn.camp.brooklyn.BrooklynCampConstants; import org.apache.brooklyn.camp.brooklyn.spi.dsl.BrooklynDslDeferredSupplier; -import org.apache.brooklyn.camp.brooklyn.spi.dsl.DslCallable; +import org.apache.brooklyn.camp.brooklyn.spi.dsl.DslFunctionSource; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Entities; @@ -63,7 +63,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.Iterables; import com.google.common.util.concurrent.Callables; -public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements DslCallable { +public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements DslFunctionSource { private static final long serialVersionUID = -7715984495268724954L; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java index 65fd66d..aca1aba 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java @@ -38,6 +38,7 @@ import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.exceptions.CompoundRuntimeException; import org.apache.brooklyn.util.guava.Maybe; import org.testng.annotations.Test; @@ -351,7 +352,7 @@ public class DslYamlTest extends AbstractYamlTest { assertEquals(getConfigEventually(app, DEST), "myvalue"); } - @Test(groups="WIP") // config accepts strings only, no suppliers + @Test(groups="WIP") // attributeWhenReady accepts strings only, no suppliers public void testDslAttributeWhenReadyWithDeferredArg() throws Exception { final Entity app = createAndStartApplication( "services:", @@ -367,7 +368,7 @@ public class DslYamlTest extends AbstractYamlTest { assertEquals(getConfigEventually(app, DEST), "myvalue"); } - @Test(groups="WIP") // config accepts strings only, no suppliers + @Test(groups="WIP") // attributeWhenReady accepts strings only, no suppliers public void testDslAttributeWhenReadyOnEntityWithDeferredArg() throws Exception { final Entity app = createAndStartApplication( "services:", @@ -505,6 +506,20 @@ public class DslYamlTest extends AbstractYamlTest { assertEquals(replacementFn.apply("Broooklyn"), "Brooklyn"); } + @Test + public void testDslNonDeferredInvalidMethod() throws Exception { + try { + createAndStartApplication( + "services:", + "- type: " + BasicApplication.class.getName(), + " brooklyn.config:", + " dest: $brooklyn:self().invalidMethod()"); + Asserts.shouldHaveFailedPreviously("Non-existing non-deferred method should fail deployment"); + } catch (CompoundRuntimeException e) { + Asserts.expectedFailureContains(e, "No such function 'invalidMethod()'"); + } + } + public static class InaccessibleType { public static void isEvaluated() {} } @@ -548,6 +563,17 @@ public class DslYamlTest extends AbstractYamlTest { } @Test + public void testDeferredDslChainingDslComponent() throws Exception { + final Entity app = createAndStartApplication( + "services:", + "- type: " + BasicApplication.class.getName(), + " brooklyn.config:", + " dest: $brooklyn:config(\"targetValue\").self().attributeWhenReady(\"entity.id\")"); + app.config().set(ConfigKeys.newConfigKey(TestDslSupplierValue.class, "targetValue"), new TestDslSupplierValue()); + assertEquals(getConfigEventually(app, DEST), app.getId()); + } + + @Test public void testDeferredDslChainingOnConfigNoFunction() throws Exception { final Entity app = createAndStartApplication( "services:", @@ -710,7 +736,7 @@ public class DslYamlTest extends AbstractYamlTest { Task<T> result = ((EntityInternal)entity).getExecutionContext().submit(new Callable<T>() { @Override public T call() throws Exception { - // TODO Move the getNonBlocking call out of the task after #280 is merged. + // TODO Move the getNonBlocking call out of the task after #480 is merged. // Currently doesn't work because no execution context available. Maybe<T> immediateValue = ((EntityInternal)entity).config().getNonBlocking(configKey); T blockingValue = entity.config().get(configKey); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslTestObjects.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslTestObjects.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslTestObjects.java index 9cd1dbd..60225af 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslTestObjects.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslTestObjects.java @@ -15,7 +15,7 @@ */ package org.apache.brooklyn.camp.brooklyn.spi.dsl.methods; -import org.apache.brooklyn.camp.brooklyn.spi.dsl.DslCallable; +import org.apache.brooklyn.camp.brooklyn.spi.dsl.DslFunctionSource; import org.apache.brooklyn.util.core.task.DeferredSupplier; import org.apache.brooklyn.util.core.task.ImmediateSupplier; import org.apache.brooklyn.util.guava.Maybe; @@ -38,6 +38,9 @@ public class DslTestObjects { public boolean isSupplierEvaluated() { return true; } + public DslComponent self() { + return BrooklynDslCommon.self(); + } } public static class TestDslSupplier implements DeferredSupplier<Object>, ImmediateSupplier<Object> { @@ -58,7 +61,7 @@ public class DslTestObjects { } } - public static class DslTestCallable implements DslCallable, DeferredSupplier<TestDslSupplier>, ImmediateSupplier<TestDslSupplier> { + public static class DslTestCallable implements DslFunctionSource, DeferredSupplier<TestDslSupplier>, ImmediateSupplier<TestDslSupplier> { @Override public Maybe<TestDslSupplier> getImmediately() { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/60c5c6fa/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java index 4dcf5be..3b00252 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java @@ -31,7 +31,6 @@ import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.mgmt.TaskAdaptable; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; -import org.apache.brooklyn.core.mgmt.rebind.ImmediateDeltaChangeListener; import org.apache.brooklyn.util.core.flags.TypeCoercions; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe;
