FELIX-4093 Dependency is ignored if @Bind does not respect naming pattern
FELIX-4094 Recognize add/remove method naming pattern
* Specification is used as default id if naming pattern does not work
* Report an explicative error when id cannot be found
* Added TestCases
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1488565 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/module/DefaultBindingModule.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/module/DefaultBindingModule.java
index d9ec4a8..39cbc09 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/module/DefaultBindingModule.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/module/DefaultBindingModule.java
@@ -250,7 +250,7 @@
.to(new AnnotationVisitorFactory() {
public AnnotationVisitor newAnnotationVisitor(BindingContext context) {
MethodNode node = (MethodNode) context.getNode();
- return new MethodBindVisitor(context.getWorkbench(), Action.BIND, node.name);
+ return new MethodBindVisitor(context.getWorkbench(), Action.BIND, node, context.getReporter());
}
});
@@ -258,7 +258,7 @@
.to(new AnnotationVisitorFactory() {
public AnnotationVisitor newAnnotationVisitor(BindingContext context) {
MethodNode node = (MethodNode) context.getNode();
- return new MethodBindVisitor(context.getWorkbench(), Action.UNBIND, node.name);
+ return new MethodBindVisitor(context.getWorkbench(), Action.UNBIND, node, context.getReporter());
}
});
@@ -266,7 +266,7 @@
.to(new AnnotationVisitorFactory() {
public AnnotationVisitor newAnnotationVisitor(BindingContext context) {
MethodNode node = (MethodNode) context.getNode();
- return new MethodBindVisitor(context.getWorkbench(), Action.MODIFIED, node.name);
+ return new MethodBindVisitor(context.getWorkbench(), Action.MODIFIED, node, context.getReporter());
}
});
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitor.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitor.java
index 6be36fc..a09a5d2 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitor.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitor.java
@@ -19,10 +19,12 @@
package org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.bind;
+import org.apache.felix.ipojo.manipulator.Reporter;
import org.apache.felix.ipojo.manipulator.metadata.annotation.ComponentWorkbench;
import org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util.Names;
import org.apache.felix.ipojo.metadata.Attribute;
import org.apache.felix.ipojo.metadata.Element;
+import org.objectweb.asm.tree.MethodNode;
import static org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util.Names.computeEffectiveMethodName;
@@ -35,11 +37,17 @@
/**
* Method name.
*/
- private String m_name;
+ private MethodNode m_node;
- public MethodBindVisitor(ComponentWorkbench workbench, Action action, String method) {
+ /**
+ * Error reporter.
+ */
+ private Reporter m_reporter;
+
+ public MethodBindVisitor(ComponentWorkbench workbench, Action action, MethodNode method, Reporter reporter) {
super(workbench, action);
- this.m_name = method;
+ this.m_node = method;
+ this.m_reporter = reporter;
}
/**
@@ -50,19 +58,28 @@
*/
public void visitEnd() {
if (m_id == null) {
- String identifier = Names.getMethodIdentifier(m_name);
+ String identifier = Names.getMethodIdentifier(m_node);
if (identifier != null) {
m_id = identifier;
} else {
- System.err.println("Cannot determine the id of the " + action.name() + " method : " + computeEffectiveMethodName(m_name));
- return;
+ if (m_specification != null) {
+ m_id = m_specification;
+ } else {
+ m_reporter.error("Cannot determine the requires identifier for the (%s) %s method: %s",
+ computeEffectiveMethodName(m_node.name),
+ action.name(),
+ "Either 'id' attribute is missing or method name do not follow the bind/set/add/modified " +
+ "naming pattern, or no specification (service interface) can be found in method signature " +
+ "or specified in annotation. Dependency will be ignored (would cause an Exception at runtime)");
+ return;
+ }
}
}
Element requires = getRequiresElement();
Element callback = new Element("callback", "");
- callback.addAttribute(new Attribute("method", computeEffectiveMethodName(m_name)));
+ callback.addAttribute(new Attribute("method", computeEffectiveMethodName(m_node.name)));
callback.addAttribute(new Attribute("type", action.name().toLowerCase()));
requires.addElement(callback);
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/Names.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/Names.java
index 390ee43..302de8e 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/Names.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/Names.java
@@ -19,7 +19,15 @@
package org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Dictionary;
+import java.util.List;
+import java.util.Map;
+
import org.apache.felix.ipojo.manipulation.MethodCreator;
+import org.objectweb.asm.Type;
+import org.objectweb.asm.tree.MethodNode;
/**
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
@@ -27,6 +35,17 @@
public class Names {
/**
+ * Excluded types when searching for a specification interface in method's arguments.
+ */
+ private static List<Type> EXCLUSIONS = new ArrayList<Type>();
+
+ static {
+ EXCLUSIONS.add(Type.getType(Map.class));
+ EXCLUSIONS.add(Type.getType(Dictionary.class));
+ EXCLUSIONS.add(Type.getType("Lorg/osgi/framework/ServiceReference;"));
+ }
+
+ /**
* Computes the real method name. This method is useful when the annotation is collected on an manipulated method
* (prefixed by <code>__M_</code>). This method just removes the prefix if found.
* @param name the collected method name
@@ -45,12 +64,13 @@
* Extract an identifier from the given method name.
* It removes some pre-defined prefixes ({@literal bind}, {@literal unbind},
* {@literal set}, {@literal unset}, {@literal modified}).
+ *
* @param method method's name
* @return the method's identifier
*/
- public static String getMethodIdentifier(final String method) {
+ public static String getMethodIdentifier(final MethodNode method) {
- String effectiveName = computeEffectiveMethodName(method);
+ String effectiveName = computeEffectiveMethodName(method.name);
if (effectiveName.startsWith("bind")) {
return effectiveName.substring("bind".length());
@@ -72,8 +92,35 @@
return effectiveName.substring("modified".length());
}
- return null;
+ if (effectiveName.startsWith("add")) {
+ return effectiveName.substring("add".length());
+ }
+ if (effectiveName.startsWith("remove")) {
+ return effectiveName.substring("remove".length());
+ }
+
+ // Try to discover the specification's type from method's parameters' type
+ Type[] arguments = Type.getArgumentTypes(method.desc);
+ return findSpecification(Arrays.asList(arguments));
+
+ }
+
+ /**
+ * Find the first type that was not excluded and consider it as the specification
+ * @param types method parameter's type
+ * @return the first non-excluded type or {@literal null} if no specification can be found
+ */
+ private static String findSpecification(final List<Type> types) {
+
+ // Find first non-excluded specification
+ // May return null if no specification is provided (likely a user error)
+ for (Type type : types) {
+ if (!EXCLUSIONS.contains(type)) {
+ return type.getClassName();
+ }
+ }
+ return null;
}
/**
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitorTestCase.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitorTestCase.java
new file mode 100644
index 0000000..b9361bc
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/bind/MethodBindVisitorTestCase.java
@@ -0,0 +1,87 @@
+/*
+ * 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 org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.bind;
+
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.anyVararg;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import org.apache.felix.ipojo.manipulator.Reporter;
+import org.apache.felix.ipojo.manipulator.metadata.annotation.ComponentWorkbench;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.MethodNode;
+
+import junit.framework.TestCase;
+
+/**
+ * User: guillaume
+ * Date: 22/05/13
+ * Time: 17:09
+ */
+public class MethodBindVisitorTestCase extends TestCase {
+
+ public void testIdentifierProvided() throws Exception {
+ Reporter reporter = mock(Reporter.class);
+ ComponentWorkbench workbench = new ComponentWorkbench(null, type());
+ MethodNode node = new MethodNode();
+ node.name = "myMethod";
+
+ MethodBindVisitor visitor = new MethodBindVisitor(workbench, Action.BIND, node, reporter);
+ visitor.visit("id", "my-identifier");
+ visitor.visitEnd();
+
+ assertNotNull(workbench.getIds().get("my-identifier"));
+ }
+
+ public void testNoIdentifierButSpecificationAsAttributeProvided() throws Exception {
+ Reporter reporter = mock(Reporter.class);
+ ComponentWorkbench workbench = new ComponentWorkbench(null, type());
+ MethodNode node = new MethodNode();
+ node.name = "notify";
+ node.desc = "()V";
+
+ MethodBindVisitor visitor = new MethodBindVisitor(workbench, Action.BIND, node, reporter);
+ visitor.visit("specification", "my.Service");
+ visitor.visitEnd();
+
+ assertNotNull(workbench.getIds().get("my.Service"));
+ }
+
+ public void testNoIdentifierAndNoSpecificationProvided() throws Exception {
+ Reporter reporter = mock(Reporter.class);
+ ComponentWorkbench workbench = new ComponentWorkbench(null, type());
+ MethodNode node = new MethodNode();
+ node.name = "notify";
+ node.desc = "()V";
+
+ MethodBindVisitor visitor = new MethodBindVisitor(workbench, Action.BIND, node, reporter);
+ visitor.visitEnd();
+
+ verify(reporter).error(anyString(), anyVararg());
+ }
+
+ private static ClassNode type() {
+ ClassNode node = new ClassNode();
+ node.name = "my/Component";
+ return node;
+ }
+
+}
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/NamesTestCase.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/NamesTestCase.java
new file mode 100644
index 0000000..f1d7035
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulator/metadata/annotation/visitor/util/NamesTestCase.java
@@ -0,0 +1,145 @@
+/*
+ * 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 org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util;
+
+import static org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util.Names.computeEffectiveMethodName;
+import static org.apache.felix.ipojo.manipulator.metadata.annotation.visitor.util.Names.getMethodIdentifier;
+
+import org.objectweb.asm.tree.MethodNode;
+
+import junit.framework.TestCase;
+
+/**
+ * User: guillaume
+ * Date: 24/05/13
+ * Time: 09:44
+ */
+public class NamesTestCase extends TestCase {
+ public void testComputeEffectiveMethodNameForNotManipulatedMethod() throws Exception {
+ assertEquals("foo", computeEffectiveMethodName("foo"));
+ }
+
+ public void testComputeEffectiveMethodNameForManipulatedMethod() throws Exception {
+ assertEquals("foo", computeEffectiveMethodName("__M_foo"));
+ }
+
+ public void testComputeEffectiveMethodNameForNullInput() throws Exception {
+ assertNull(computeEffectiveMethodName(null));
+ }
+
+ public void testBindPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "bindService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testUnbindPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "unbindService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testSetPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "setService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testUnsetPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "unsetService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testAddPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "addService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testRemovePatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "removeService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testModifiedPatternRecognition() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "modifiedService";
+ assertEquals("Service", getMethodIdentifier(node));
+ }
+
+ public void testNoPatternRecognized() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "notify";
+ node.desc = "()V";
+ assertNull(getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognized() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Lmy/Service;)V";
+ assertEquals("my.Service", getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognizedWithMap() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Lmy/Service;Ljava/util/Map;)V";
+ assertEquals("my.Service", getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognizedWithDictionary() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Lmy/Service;Ljava/util/Dictionary;)V";
+ assertEquals("my.Service", getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognizedWithServiceReference() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Lmy/Service;Lorg/osgi/framework/ServiceReference;)V";
+ assertEquals("my.Service", getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognizedOnlyMap() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Ljava/util/Map;)V";
+ assertNull(getMethodIdentifier(node));
+ }
+
+ public void testSpecificationRecognizedOnlyDictionary() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Ljava/util/Dictionary;)V";
+ assertNull(getMethodIdentifier(node));
+
+ }
+
+ public void testSpecificationRecognizedOnlyServiceReference() throws Exception {
+ MethodNode node = new MethodNode();
+ node.name = "handle";
+ node.desc = "(Lorg/osgi/framework/ServiceReference;)V";
+ assertNull(getMethodIdentifier(node));
+ }
+}