Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@
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;

import com.google.common.base.Optional;

/**
* {@link PlanInterpreter} which understands the $brooklyn DSL
*/
Expand Down Expand Up @@ -157,15 +156,6 @@ public Object evaluateOn(Object o, FunctionWithArgs f, boolean deepEvaluation) {
if (f.getArgs()==null)
throw new IllegalStateException("Invalid function-only expression '"+f.getFunction()+"'");

Class<?> clazz;
if (o instanceof Class) {
clazz = (Class<?>)o;
} else {
clazz = o.getClass();
}
if (!(clazz.getPackage().getName().startsWith(BrooklynDslCommon.class.getPackage().getName())))
throw new IllegalArgumentException("Not permitted to invoke function on '"+clazz+"' (outside allowed package scope)");

String fn = f.getFunction();
fn = Strings.removeFromStart(fn, "$brooklyn:");
if (fn.startsWith("function.")) {
Expand All @@ -175,19 +165,22 @@ public Object evaluateOn(Object o, FunctionWithArgs f, boolean deepEvaluation) {
o = BrooklynDslCommon.Functions.class;
fn = Strings.removeFromStart(fn, "function.");
}
List<Object> args = new ArrayList<>();
for (Object arg: f.getArgs()) {
args.add( deepEvaluation ? evaluate(arg, true) : arg );
}
try {
List<Object> args = new ArrayList<>();
for (Object arg: f.getArgs()) {
args.add( deepEvaluation ? evaluate(arg, true) : arg );
if (o instanceof BrooklynDslDeferredSupplier && !(o instanceof DslCallable)) {
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);
}
Optional<Object> v = Reflections.invokeMethodWithArgs(o, fn, args);
if (v.isPresent()) return v.get();
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
throw Exceptions.propagate(new InvocationTargetException(e, "Error invoking '"+fn+"' on '"+o+"'"));
throw Exceptions.propagate(new InvocationTargetException(e, "Error invoking '"+fn+"' on '"+o+"' with arguments "+args+""));
}

throw new IllegalArgumentException("No such function '"+fn+"' on "+o);
}

}
Original file line number Diff line number Diff line change
@@ -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 DslCallable {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm considering changing this to an annotation. When applied on:

  • A class will work as now - let the DSL evaluator know to use the object as is, stop evaluation
  • On a method - white-list the method for DSL calls. Deprecate non-annotated method calls. This will make it very easy to inspect what is executable by the DSL.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea. Do you want to make it part of this PR or do it subsequently?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one. Don't want to introduce APIs that will go away in master.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After spiking the changes decided to keep DslCallable interface for first case and introduce a DslAccessible annotation for second. They have different use-cases, better not to mix them in the same annotation.

Will send an email to dev@brooklyn.apache.org discussing the DslAccessible change and implement it in a separate PR.


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*
* 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 java.lang.reflect.InvocationTargetException;
import java.util.List;
import java.util.concurrent.Callable;

import org.apache.brooklyn.api.mgmt.Task;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.BrooklynDslCommon;
import org.apache.brooklyn.core.entity.EntityInternal;
import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.Reflections;

import com.google.common.base.Joiner;
import com.google.common.base.Objects;

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 String fnName;
private List<?> args;

public DslDeferredFunctionCall(BrooklynDslDeferredSupplier<?> o, String fn, List<Object> args) {
this.object = o;
this.fnName = fn;
this.args = args;
}

@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);
}

@Override
public Task<Object> newTask() {
return Tasks.builder()
.displayName("Deferred function call " + object + "." + fnName + "(" + toString(args) + ")")
.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
.dynamic(false)
.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);
}

}).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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this code reachable? This class is only constructed when object is DslCallable in which case the above branch applies, isn't it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm - missed the !

.context(((EntityInternal)entity()).getExecutionContext())
.deep(true)
.immediately(immediate)
.recursive(false)
.getMaybe();
if (resultMaybe.isAbsent()) {
return resultMaybe;
} 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;
} else {
return resolve(result, immediate);
}
}
}

protected Object invokeOn(Object obj) {
return invokeOn(obj, fnName, args);
}

protected static Object invokeOn(Object obj, String fnName, List<?> args) {
checkCallAllowed(obj, fnName, args);

Maybe<Object> v;
try {
v = Reflections.invokeMethodFromArgs(obj, fnName, args);
} catch (IllegalArgumentException | IllegalAccessException | InvocationTargetException e) {
Exceptions.propagateIfFatal(e);
throw Exceptions.propagate(new InvocationTargetException(e, "Error invoking '"+fnName+"("+toString(args)+")' on '"+obj+"'"));
}
if (v.isPresent()) {
// Value is most likely another BrooklynDslDeferredSupplier - let the caller handle it,
return v.get();
} else {
throw new IllegalArgumentException("No such function '"+fnName+"("+toString(args)+")' on "+obj);
}
}

private static void checkCallAllowed(Object obj, String fnName2, List<?> args2) {
Class<?> clazz;
if (obj instanceof Class) {
clazz = (Class<?>)obj;
} else {
clazz = obj.getClass();
}
if (!(clazz.getPackage().getName().startsWith(BrooklynDslCommon.class.getPackage().getName())))
throw new IllegalArgumentException("Not permitted to invoke function on '"+clazz+"' (outside allowed package scope)");
}

@Override
public int hashCode() {
return Objects.hashCode(object, fnName, args);
}

@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (obj == null || getClass() != obj.getClass()) return false;
DslDeferredFunctionCall that = DslDeferredFunctionCall.class.cast(obj);
return Objects.equal(this.object, that.object) &&
Objects.equal(this.fnName, that.fnName) &&
Objects.equal(this.args, that.args);
}

@Override
public String toString() {
return object + "." + fnName + "(" + toString(args) + ")";
}

private static String toString(List<?> args) {
if (args == null) return "";
return Joiner.on(", ").join(args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.Reflections;
import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
import org.apache.brooklyn.util.text.StringFunctions.RegexReplacer;
import org.apache.brooklyn.util.text.Strings;
import org.apache.commons.beanutils.BeanUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -626,25 +627,17 @@ public String toString() {
public static class Functions {
public static Object regexReplacement(final Object pattern, final Object replacement) {
if (resolved(pattern, replacement)) {
return new RegexReplacer(String.valueOf(pattern), String.valueOf(replacement));
return new org.apache.brooklyn.util.text.StringFunctions.RegexReplacer(String.valueOf(pattern), String.valueOf(replacement));
} else {
return new DslRegexReplacer(pattern, replacement);
}
}

public static class RegexReplacer implements Function<String, String> {
private final String pattern;
private final String replacement;

/** @deprecated since 0.11.0; use {@link org.apache.brooklyn.util.text.StringFunctions.RegexReplacer} instead */
@Deprecated
public static class RegexReplacer extends org.apache.brooklyn.util.text.StringFunctions.RegexReplacer {
public RegexReplacer(String pattern, String replacement) {
this.pattern = pattern;
this.replacement = replacement;
}

@Nullable
@Override
public String apply(@Nullable String s) {
return s == null ? null : Strings.replaceAllRegex(s, pattern, replacement);
super(pattern, replacement);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.entity.Entities;
Expand Down Expand Up @@ -62,7 +63,7 @@
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.Callables;

public class DslComponent extends BrooklynDslDeferredSupplier<Entity> {
public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements DslCallable {

private static final long serialVersionUID = -7715984495268724954L;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.brooklyn.camp.brooklyn.dsl;
package org.apache.brooklyn.camp.brooklyn.spi.dsl;

import static com.google.common.base.Preconditions.checkNotNull;
import static org.testng.Assert.assertEquals;
Expand All @@ -24,7 +24,6 @@
import java.util.NoSuchElementException;
import java.util.Random;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

Expand All @@ -33,13 +32,12 @@
import org.apache.brooklyn.api.mgmt.Task;
import org.apache.brooklyn.api.sensor.AttributeSensor;
import org.apache.brooklyn.camp.brooklyn.BrooklynCampConstants;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.BrooklynDslDeferredSupplier;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.BrooklynDslCommon;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.entity.EntityInternal;
import org.apache.brooklyn.core.objs.BasicSpecParameter;
import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
import org.apache.brooklyn.core.objs.BasicSpecParameter;
import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
import org.apache.brooklyn.core.test.entity.TestApplication;
import org.apache.brooklyn.core.test.entity.TestEntity;
Expand Down
Loading