Clarify operation of `templateOptions` config key Resolves the ambiguity around single parameters of type list. Extracts the bulk of the code into a new standalone class and adds tests. Adds documentation.
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/6023536e Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/6023536e Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/6023536e Branch: refs/heads/0.7.0-incubating Commit: 6023536eb9c9fda438dc3fea42d1262a1510ae19 Parents: e26c9e3 Author: Richard Downer <[email protected]> Authored: Wed Jun 24 10:58:11 2015 +0100 Committer: Richard Downer <[email protected]> Committed: Wed Jun 24 11:03:48 2015 +0100 ---------------------------------------------------------------------- .../brooklyn/util/flags/MethodCoercions.java | 183 +++++++++++++++++++ .../util/flags/MethodCoercionsTest.java | 146 +++++++++++++++ .../location/jclouds/JcloudsLocation.java | 43 +---- 3 files changed, 333 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6023536e/core/src/main/java/brooklyn/util/flags/MethodCoercions.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/flags/MethodCoercions.java b/core/src/main/java/brooklyn/util/flags/MethodCoercions.java new file mode 100644 index 0000000..c9f00fe --- /dev/null +++ b/core/src/main/java/brooklyn/util/flags/MethodCoercions.java @@ -0,0 +1,183 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 brooklyn.util.flags; + +import brooklyn.util.exceptions.Exceptions; +import brooklyn.util.guava.Maybe; +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; +import com.google.common.reflect.TypeToken; + +import javax.annotation.Nullable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.Arrays; +import java.util.List; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * A way of binding a loosely-specified method call into a strongly-typed Java method call. + */ +public class MethodCoercions { + + /** + * Returns a predicate that matches a method with the given name, and a single parameter that + * {@link brooklyn.util.flags.TypeCoercions#tryCoerce(Object, com.google.common.reflect.TypeToken)} can process + * from the given argument. + * + * @param methodName name of the method + * @param argument argument that is intended to be given + * @return a predicate that will match a compatible method + */ + public static Predicate<Method> matchSingleParameterMethod(final String methodName, final Object argument) { + checkNotNull(methodName, "methodName"); + checkNotNull(argument, "argument"); + + return new Predicate<Method>() { + @Override + public boolean apply(@Nullable Method input) { + if (input == null) return false; + if (!input.getName().equals(methodName)) return false; + Type[] parameterTypes = input.getGenericParameterTypes(); + return parameterTypes.length == 1 + && TypeCoercions.tryCoerce(argument, TypeToken.of(parameterTypes[0])).isPresentAndNonNull(); + + } + }; + } + + /** + * Tries to find a single-parameter method with a parameter compatible with (can be coerced to) the argument, and + * invokes it. + * + * @param instance the object to invoke the method on + * @param methodName the name of the method to invoke + * @param argument the argument to the method's parameter. + * @return the result of the method call, or {@link brooklyn.util.guava.Maybe#absent()} if method could not be matched. + */ + public static Maybe<?> tryFindAndInvokeSingleParameterMethod(final Object instance, final String methodName, final Object argument) { + Class<?> clazz = instance.getClass(); + Iterable<Method> methods = Arrays.asList(clazz.getMethods()); + Optional<Method> matchingMethod = Iterables.tryFind(methods, matchSingleParameterMethod(methodName, argument)); + if (matchingMethod.isPresent()) { + Method method = matchingMethod.get(); + try { + Type paramType = method.getGenericParameterTypes()[0]; + Object coercedArgument = TypeCoercions.coerce(argument, TypeToken.of(paramType)); + return Maybe.of(method.invoke(instance, coercedArgument)); + } catch (IllegalAccessException | InvocationTargetException e) { + throw Exceptions.propagate(e); + } + } else { + return Maybe.absent(); + } + } + + /** + * Returns a predicate that matches a method with the given name, and parameters that + * {@link brooklyn.util.flags.TypeCoercions#tryCoerce(Object, com.google.common.reflect.TypeToken)} can process + * from the given list of arguments. + * + * @param methodName name of the method + * @param arguments arguments that is intended to be given + * @return a predicate that will match a compatible method + */ + public static Predicate<Method> matchMultiParameterMethod(final String methodName, final List<?> arguments) { + checkNotNull(methodName, "methodName"); + checkNotNull(arguments, "arguments"); + + return new Predicate<Method>() { + @Override + public boolean apply(@Nullable Method input) { + if (input == null) return false; + if (!input.getName().equals(methodName)) return false; + int numOptionParams = arguments.size(); + Type[] parameterTypes = input.getGenericParameterTypes(); + if (parameterTypes.length != numOptionParams) return false; + + for (int paramCount = 0; paramCount < numOptionParams; paramCount++) { + if (!TypeCoercions.tryCoerce(((List) arguments).get(paramCount), + TypeToken.of(parameterTypes[paramCount])).isPresentAndNonNull()) return false; + } + return true; + } + }; + } + + /** + * Tries to find a multiple-parameter method with each parameter compatible with (can be coerced to) the + * corresponding argument, and invokes it. + * + * @param instance the object to invoke the method on + * @param methodName the name of the method to invoke + * @param argument a list of the arguments to the method's parameters. + * @return the result of the method call, or {@link brooklyn.util.guava.Maybe#absent()} if method could not be matched. + */ + public static Maybe<?> tryFindAndInvokeMultiParameterMethod(final Object instance, final String methodName, final List<?> arguments) { + Class<?> clazz = instance.getClass(); + Iterable<Method> methods = Arrays.asList(clazz.getMethods()); + Optional<Method> matchingMethod = Iterables.tryFind(methods, matchMultiParameterMethod(methodName, arguments)); + if (matchingMethod.isPresent()) { + Method method = matchingMethod.get(); + try { + int numOptionParams = ((List)arguments).size(); + Object[] coercedArguments = new Object[numOptionParams]; + for (int paramCount = 0; paramCount < numOptionParams; paramCount++) { + Object argument = arguments.get(paramCount); + Type paramType = method.getGenericParameterTypes()[paramCount]; + coercedArguments[paramCount] = TypeCoercions.coerce(argument, TypeToken.of(paramType)); + } + return Maybe.of(method.invoke(instance, coercedArguments)); + } catch (IllegalAccessException | InvocationTargetException e) { + throw Exceptions.propagate(e); + } + } else { + return Maybe.absent(); + } + } + + /** + * Tries to find a method with each parameter compatible with (can be coerced to) the corresponding argument, and invokes it. + * + * @param instance the object to invoke the method on + * @param methodName the name of the method to invoke + * @param argument a list of the arguments to the method's parameters, or a single argument for a single-parameter method. + * @return the result of the method call, or {@link brooklyn.util.guava.Maybe#absent()} if method could not be matched. + */ + public static Maybe<?> tryFindAndInvokeBestMatchingMethod(final Object instance, final String methodName, final Object argument) { + if (argument instanceof List) { + List<?> arguments = (List<?>) argument; + + // ambiguous case: we can't tell if the user is using the multi-parameter syntax, or the single-parameter + // syntax for a method which takes a List parameter. So we try one, then fall back to the other. + + Maybe<?> maybe = tryFindAndInvokeMultiParameterMethod(instance, methodName, arguments); + if (maybe.isAbsent()) + maybe = tryFindAndInvokeSingleParameterMethod(instance, methodName, argument); + + return maybe; + } else { + return tryFindAndInvokeSingleParameterMethod(instance, methodName, argument); + } + } + +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6023536e/core/src/test/java/brooklyn/util/flags/MethodCoercionsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/util/flags/MethodCoercionsTest.java b/core/src/test/java/brooklyn/util/flags/MethodCoercionsTest.java new file mode 100644 index 0000000..4d06ca4 --- /dev/null +++ b/core/src/test/java/brooklyn/util/flags/MethodCoercionsTest.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 brooklyn.util.flags; + +import brooklyn.util.exceptions.Exceptions; +import brooklyn.util.guava.Maybe; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import java.lang.reflect.Method; +import java.util.List; + +import static org.testng.Assert.*; + +public class MethodCoercionsTest { + + private Method singleParameterMethod; + private Method multiParameterMethod; + private Method singleCollectionParameterMethod; + + @BeforeClass + public void testFixtureSetUp() { + try { + singleParameterMethod = TestClass.class.getMethod("singleParameterMethod", int.class); + multiParameterMethod = TestClass.class.getMethod("multiParameterMethod", boolean.class, int.class); + singleCollectionParameterMethod = TestClass.class.getMethod("singleCollectionParameterMethod", List.class); + } catch (NoSuchMethodException e) { + throw Exceptions.propagate(e); + } + } + + @Test + public void testMatchSingleParameterMethod() throws Exception { + Predicate<Method> predicate = MethodCoercions.matchSingleParameterMethod("singleParameterMethod", "42"); + assertTrue(predicate.apply(singleParameterMethod)); + assertFalse(predicate.apply(multiParameterMethod)); + assertFalse(predicate.apply(singleCollectionParameterMethod)); + } + + @Test + public void testTryFindAndInvokeSingleParameterMethod() throws Exception { + TestClass instance = new TestClass(); + Maybe<?> maybe = MethodCoercions.tryFindAndInvokeSingleParameterMethod(instance, "singleParameterMethod", "42"); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasSingleParameterMethodCalled()); + } + + @Test + public void testMatchMultiParameterMethod() throws Exception { + Predicate<Method> predicate = MethodCoercions.matchMultiParameterMethod("multiParameterMethod", ImmutableList.of("true", "42")); + assertFalse(predicate.apply(singleParameterMethod)); + assertTrue(predicate.apply(multiParameterMethod)); + assertFalse(predicate.apply(singleCollectionParameterMethod)); + } + + @Test + public void testTryFindAndInvokeMultiParameterMethod() throws Exception { + TestClass instance = new TestClass(); + Maybe<?> maybe = MethodCoercions.tryFindAndInvokeMultiParameterMethod(instance, "multiParameterMethod", ImmutableList.of("true", "42")); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasMultiParameterMethodCalled()); + } + + @Test + public void testTryFindAndInvokeBestMatchingMethod() throws Exception { + TestClass instance = new TestClass(); + Maybe<?> maybe = MethodCoercions.tryFindAndInvokeBestMatchingMethod(instance, "singleParameterMethod", "42"); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasSingleParameterMethodCalled()); + + instance = new TestClass(); + maybe = MethodCoercions.tryFindAndInvokeBestMatchingMethod(instance, "multiParameterMethod", ImmutableList.of("true", "42")); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasMultiParameterMethodCalled()); + + instance = new TestClass(); + maybe = MethodCoercions.tryFindAndInvokeBestMatchingMethod(instance, "singleCollectionParameterMethod", ImmutableList.of("fred", "joe")); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasSingleCollectionParameterMethodCalled()); + } +/* + @Test + public void testMatchSingleCollectionParameterMethod() throws Exception { + Predicate<Method> predicate = MethodCoercions.matchSingleCollectionParameterMethod("singleCollectionParameterMethod", ImmutableList.of("42")); + assertFalse(predicate.apply(singleParameterMethod)); + assertFalse(predicate.apply(multiParameterMethod)); + assertTrue(predicate.apply(singleCollectionParameterMethod)); + } + + @Test + public void testTryFindAndInvokeSingleCollectionParameterMethod() throws Exception { + TestClass instance = new TestClass(); + Maybe<?> maybe = MethodCoercions.tryFindAndInvokeSingleCollectionParameterMethod(instance, "singleCollectionParameterMethod", ImmutableList.of("42")); + assertTrue(maybe.isPresent()); + assertTrue(instance.wasSingleCollectionParameterMethodCalled()); + } +*/ + public static class TestClass { + + private boolean singleParameterMethodCalled; + private boolean multiParameterMethodCalled; + private boolean singleCollectionParameterMethodCalled; + + public void singleParameterMethod(int parameter) { + singleParameterMethodCalled = true; + } + + public void multiParameterMethod(boolean parameter1, int parameter2) { + multiParameterMethodCalled = true; + } + + public void singleCollectionParameterMethod(List<String> parameter) { + singleCollectionParameterMethodCalled = true; + } + + public boolean wasSingleParameterMethodCalled() { + return singleParameterMethodCalled; + } + + public boolean wasMultiParameterMethodCalled() { + return multiParameterMethodCalled; + } + + public boolean wasSingleCollectionParameterMethodCalled() { + return singleCollectionParameterMethodCalled; + } + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6023536e/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java index 7797138..02b7dde 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java @@ -25,6 +25,8 @@ import static com.google.common.base.Preconditions.checkNotNull; import static java.util.concurrent.TimeUnit.SECONDS; import static org.jclouds.compute.options.RunScriptOptions.Builder.overrideLoginCredentials; import static org.jclouds.scriptbuilder.domain.Statements.exec; + +import brooklyn.util.flags.MethodCoercions; import io.cloudsoft.winrm4j.pywinrm.Session; import io.cloudsoft.winrm4j.pywinrm.WinRMFactory; @@ -1292,45 +1294,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im Class<? extends TemplateOptions> clazz = options.getClass(); Iterable<Method> methods = Arrays.asList(clazz.getMethods()); for(final Map.Entry<String, Object> option : optionsMap.entrySet()) { - Optional<Method> methodOptional = Iterables.tryFind(methods, new Predicate<Method>() { - @Override - public boolean apply(@Nullable Method input) { - // Matches a method with the expected name, and a single parameter that TypeCoercions - // can coerce to - if (input == null) return false; - if (!input.getName().equals(option.getKey())) return false; - int numOptionParams = option.getValue() instanceof List ? ((List)option.getValue()).size() : 1; - Type[] parameterTypes = input.getGenericParameterTypes(); - if (parameterTypes.length != numOptionParams) return false; - if (numOptionParams == 1 && !(option.getValue() instanceof List) && parameterTypes.length == 1) { - return true; - } - for (int paramCount = 0; paramCount < numOptionParams; paramCount ++) { - if (!TypeCoercions.tryCoerce(((List)option.getValue()).get(paramCount), - TypeToken.of(parameterTypes[paramCount])).isPresentAndNonNull()) return false; - } - return true; - } - }); - if(methodOptional.isPresent()) { - try { - Method method = methodOptional.get(); - if (option.getValue() instanceof List) { - List<Object> parameters = Lists.newArrayList(); - int numOptionParams = ((List)option.getValue()).size(); - for (int paramCount = 0; paramCount < numOptionParams; paramCount++) { - parameters.add(TypeCoercions.coerce(((List)option.getValue()).get(paramCount), TypeToken.of(method.getGenericParameterTypes()[paramCount]))); - } - method.invoke(options, parameters.toArray()); - } else { - method.invoke(options, TypeCoercions.coerce(option.getValue(), TypeToken.of(method.getGenericParameterTypes()[0]))); - } - } catch (IllegalAccessException e) { - throw Exceptions.propagate(e); - } catch (InvocationTargetException e) { - throw Exceptions.propagate(e); - } - } else { + Maybe<?> result = MethodCoercions.tryFindAndInvokeBestMatchingMethod(options, option.getKey(), option.getValue()); + if(result.isAbsent()) { LOG.warn("Ignoring request to set template option {} because this is not supported by {}", new Object[] { option.getKey(), clazz.getCanonicalName() }); } }
