Fix issue Felix-1024
iPOJO creates a suitable constructor if none found.
Fix issue Felix-1025
Extender callbacks are no more called with the lock, and so avoid deadlocks.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@762084 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/handler/extender/src/main/java/org/apache/felix/ipojo/handler/extender/BundleTracker.java b/ipojo/handler/extender/src/main/java/org/apache/felix/ipojo/handler/extender/BundleTracker.java
index b3a3572..e940dff 100644
--- a/ipojo/handler/extender/src/main/java/org/apache/felix/ipojo/handler/extender/BundleTracker.java
+++ b/ipojo/handler/extender/src/main/java/org/apache/felix/ipojo/handler/extender/BundleTracker.java
@@ -100,20 +100,23 @@
/**
* Call this method to start the tracking of active bundles.
**/
- public synchronized void open() {
- if (!m_open) {
- m_open = true;
+ public void open() {
+ synchronized (this) {
+ if (!m_open) {
+ m_open = true;
- m_context.addBundleListener(m_listener);
+ m_context.addBundleListener(m_listener);
+ }
+ }
Bundle[] bundles = m_context.getBundles();
for (int i = 0; i < bundles.length; i++) {
if (bundles[i].getState() == Bundle.ACTIVE) {
- m_bundleSet.add(bundles[i]);
- addedBundle(bundles[i]);
+ if (m_bundleSet.add(bundles[i])) {
+ addedBundle(bundles[i]);
+ }
}
}
- }
}
/**
diff --git a/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java b/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
index eababe1..3f641b9 100644
--- a/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
+++ b/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
@@ -195,7 +195,7 @@
return new AnnotationCollector(md);
}
} else {
- // Avoid constructors.
+ // no constructors.
if (!(name.startsWith("_get") || // Avoid getter method
name.startsWith("_set") || // Avoid setter method
name.equals("_setComponentManager") || // Avoid the set method
diff --git a/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java b/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
index 44a2761..6f5305b 100644
--- a/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
+++ b/ipojo/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/MethodCreator.java
@@ -120,6 +120,18 @@
* method.
*/
private List m_visitedMethods = new ArrayList();
+
+ /**
+ * Set to <code>true</code> when a suitable constructor
+ * is found. If not set to <code>true</code> at the end
+ * of the visit, the manipulator injects a constructor.
+ */
+ private boolean m_foundSuitableConstructor = false;
+
+ /**
+ * Name of the super class.
+ */
+ private String m_superclass;
/**
* Constructor.
@@ -148,6 +160,7 @@
*/
public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
m_owner = name;
+ m_superclass = superName;
addPOJOInterface(version, access, name, signature, superName, interfaces);
addIMField();
}
@@ -179,8 +192,10 @@
Type[] args = Type.getArgumentTypes(desc);
if (args.length == 0) {
generateEmptyConstructor(access, signature, exceptions, md.getAnnotations());
+ m_foundSuitableConstructor = true;
} else if (args.length == 1 && args[0].getClassName().equals("org.osgi.framework.BundleContext")) {
generateBCConstructor(access, signature, exceptions, md.getAnnotations());
+ m_foundSuitableConstructor = true;
} else {
// Do nothing, the constructor does not match.
return cv.visitMethod(access, name, desc, signature, exceptions);
@@ -535,6 +550,11 @@
// Add the getComponentInstance
createGetComponentInstanceMethod();
+
+ // Need to inject a constructor?
+ if (! m_foundSuitableConstructor) { // No adequate constructor, create one.
+ createSimpleConstructor();
+ }
m_methods.clear();
m_methodFlags.clear();
@@ -543,6 +563,31 @@
}
/**
+ * Creates a simple constructor with an instance manager
+ * in argument if no suitable constructor is found during
+ * the visit.
+ */
+ private void createSimpleConstructor() {
+ MethodVisitor mv = cv.visitMethod(ACC_PUBLIC, "<init>",
+ "(Lorg/apache/felix/ipojo/InstanceManager;)V", null, null);
+ mv.visitCode();
+
+ // Super call
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, m_superclass, "<init>", "()V");
+
+ // Call set instance manager
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitVarInsn(ALOAD, 1);
+ mv.visitMethodInsn(INVOKEVIRTUAL, m_owner, "_setInstanceManager",
+ "(Lorg/apache/felix/ipojo/InstanceManager;)V");
+
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(0, 0);
+ mv.visitEnd();
+ }
+
+ /**
* Create the setter method for the __cm field.
*/
private void createSetInstanceManagerMethod() {
diff --git a/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/ASimpleParentClass.java b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/ASimpleParentClass.java
new file mode 100644
index 0000000..ef42322
--- /dev/null
+++ b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/ASimpleParentClass.java
@@ -0,0 +1,11 @@
+package org.apache.felix.ipojo.test.scenarios.component;
+
+public class ASimpleParentClass {
+
+ protected String name;
+
+ public ASimpleParentClass() {
+ name = "hello";
+ }
+
+}
diff --git a/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructor.java b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructor.java
new file mode 100644
index 0000000..59f2ef7
--- /dev/null
+++ b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructor.java
@@ -0,0 +1,31 @@
+package org.apache.felix.ipojo.test.scenarios.component;
+
+import java.util.Properties;
+
+import org.apache.felix.ipojo.test.scenarios.manipulation.service.CheckService;
+
+public class NoEmptyConstructor implements CheckService {
+
+
+ private String name;
+
+ public NoEmptyConstructor(final String n) {
+ name = n;
+ }
+
+ public boolean check() {
+ return name != null;
+ }
+
+ public Properties getProps() {
+ Properties props = new Properties();
+ if (name == null) {
+ props.put("name", "NULL");
+ } else {
+ props.put("name", name);
+ }
+ return props;
+ }
+
+
+}
diff --git a/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructorWithParentClass.java b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructorWithParentClass.java
new file mode 100644
index 0000000..409692b
--- /dev/null
+++ b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/component/NoEmptyConstructorWithParentClass.java
@@ -0,0 +1,27 @@
+package org.apache.felix.ipojo.test.scenarios.component;
+
+import java.util.Properties;
+
+import org.apache.felix.ipojo.test.scenarios.manipulation.service.CheckService;
+
+public class NoEmptyConstructorWithParentClass extends ASimpleParentClass implements CheckService {
+
+ public NoEmptyConstructorWithParentClass(final String n) {
+ name = n;
+ }
+
+ public boolean check() {
+ return name != null;
+ }
+
+ public Properties getProps() {
+ Properties props = new Properties();
+ if (name == null) {
+ props.put("name", "NULL");
+ } else {
+ props.put("name", name);
+ }
+ return props;
+ }
+
+}
diff --git a/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/manipulation/SeveralConstructorTest.java b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/manipulation/SeveralConstructorTest.java
index 4978128..f2c8ddf 100644
--- a/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/manipulation/SeveralConstructorTest.java
+++ b/ipojo/tests/manipulator/creation/src/main/java/org/apache/felix/ipojo/test/scenarios/manipulation/SeveralConstructorTest.java
@@ -11,11 +11,14 @@
private IPOJOHelper helper;
- private ComponentInstance ci;
+ private ComponentInstance ci, ci2, ci3;
public void setUp() {
helper = new IPOJOHelper(this);
ci = helper.createComponentInstance("org.apache.felix.ipojo.test.scenarios.component.SeveralConstructors");
+ ci2 = helper.createComponentInstance("org.apache.felix.ipojo.test.scenarios.component.NoEmptyConstructor");
+ ci3 = helper.createComponentInstance("org.apache.felix.ipojo.test.scenarios.component.NoEmptyConstructorWithParentClass");
+
}
public void tearDown() {
@@ -29,6 +32,23 @@
assertTrue("Check assignation", cs.check());
String name = (String) cs.getProps().get("name");
assertEquals("Check message", "hello world", name);
+ //assertNull("Check message", name);
+ }
+
+ public void testNoEmptyConstructor() {
+ ServiceReference ref = helper.getServiceReferenceByName(CheckService.class.getName(), ci2.getInstanceName());
+ CheckService cs = (CheckService) getServiceObject(ref);
+ assertFalse("Check assignation", cs.check());
+ String name = (String) cs.getProps().get("name");
+ assertEquals("Check message", "NULL", name);
+ }
+
+ public void testNoEmptyConstructorWithAParentClass() {
+ ServiceReference ref = helper.getServiceReferenceByName(CheckService.class.getName(), ci3.getInstanceName());
+ CheckService cs = (CheckService) getServiceObject(ref);
+ assertTrue("Check assignation", cs.check()); // super set name to "hello"
+ String name = (String) cs.getProps().get("name");
+ assertEquals("Check message", "hello", name);
}
}
diff --git a/ipojo/tests/manipulator/creation/src/main/resources/metadata.xml b/ipojo/tests/manipulator/creation/src/main/resources/metadata.xml
index 2b51486..5d2c2de 100644
--- a/ipojo/tests/manipulator/creation/src/main/resources/metadata.xml
+++ b/ipojo/tests/manipulator/creation/src/main/resources/metadata.xml
@@ -63,4 +63,11 @@
<component classname="org.apache.felix.ipojo.test.scenarios.component.SeveralConstructors">
<provides/>
</component>
+ <!-- No Empty constructor -->
+ <component classname="org.apache.felix.ipojo.test.scenarios.component.NoEmptyConstructor">
+ <provides/>
+ </component>
+ <component classname="org.apache.felix.ipojo.test.scenarios.component.NoEmptyConstructorWithParentClass">
+ <provides/>
+ </component>
</ipojo>