Fix FELIX-4072

Field interceptors receive the right pojo object in the onGet and onSet objects

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1484696 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/CheckServiceAndFieldInterceptorHandler.java b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/CheckServiceAndFieldInterceptorHandler.java
new file mode 100644
index 0000000..23a400d
--- /dev/null
+++ b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/CheckServiceAndFieldInterceptorHandler.java
@@ -0,0 +1,131 @@
+/*
+ * 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.runtime.externalhandlers.components;
+
+import org.apache.felix.ipojo.PrimitiveHandler;
+import org.apache.felix.ipojo.architecture.ComponentTypeDescription;
+import org.apache.felix.ipojo.architecture.HandlerDescription;
+import org.apache.felix.ipojo.architecture.PropertyDescription;
+import org.apache.felix.ipojo.metadata.Element;
+import org.apache.felix.ipojo.runtime.externalhandlers.services.CheckService;
+import org.apache.felix.ipojo.runtime.externalhandlers.services.CheckServiceHandlerDescription;
+import org.osgi.framework.ServiceRegistration;
+
+import java.util.Dictionary;
+import java.util.Hashtable;
+
+public class CheckServiceAndFieldInterceptorHandler extends PrimitiveHandler implements CheckService {
+
+	ServiceRegistration sr;
+	boolean isValid;
+	int changes = 0;
+	static final String NAMESPACE = "org.apache.felix.ipojo.test.handler.checkservice";
+
+	Dictionary<String, Object> props = new Hashtable<String, Object>();
+    private String m_value;
+
+    public void configure(Element metadata, Dictionary configuration) {
+		Element[] meta = metadata.getElements("checkAndSet", NAMESPACE);
+		if(meta == null) { return;	}
+		// Get handler props
+		props.put("instance.name", configuration.get("instance.name"));
+		if(configuration.get("csh.simple") != null) { props.put("Simple", configuration.get("csh.simple")); }
+		if(configuration.get("csh.map") != null) {
+			Dictionary m = (Dictionary) configuration.get("csh.map");
+            if (m.size() > 0) {
+                props.put("Map1", m.get("a"));
+                props.put("Map2", m.get("b"));
+                props.put("Map3", m.get("c"));
+            }
+		}
+		props.put("changes", changes);
+
+        getInstanceManager().register(getPojoMetadata().getField("m_foo"), this);
+        m_value = (String) configuration.get("foo");
+
+	}
+
+	public void initializeComponentFactory(ComponentTypeDescription cd, Element metadata) {
+	    cd.addProperty(new PropertyDescription("csh.simple", "java.lang.String", null));
+        cd.addProperty(new PropertyDescription("csh.map", "java.util.Dictionary", null));
+	}
+
+	public void start() {
+		if(sr == null) {
+			sr = getInstanceManager().getContext().registerService(CheckService.class.getName(), this, props);
+		}
+		isValid = true;
+	}
+
+	public void stop() {
+		isValid = false;
+		synchronized(this) {
+			if(sr != null) { sr.unregister(); }
+		}
+	}
+
+	public boolean check() {
+        isValid = !isValid;
+		return isValid;
+	}
+
+	public Dictionary<String, Object> getProps() {
+        props.put("foo", m_value);
+		return props;
+	}
+
+	public void stateChanged(int state) {
+		if (sr != null) {
+		    changes++;
+		    props.put("changes", changes);
+		    sr.setProperties(props);
+		}
+	}
+
+	public String getName() {
+		return NAMESPACE;
+	}
+
+	public HandlerDescription getDescription() {
+		return new CheckServiceHandlerDescription(this);
+	}
+
+    @Override
+    public Object onGet(Object pojo, String fieldName, Object value) {
+        if (fieldName.equals("m_foo")) {
+            return m_value;
+        }
+        if (pojo == null) {
+            throw new IllegalStateException("pojo parameter null - FELIX-4072");
+        }
+        return value;
+    }
+
+    @Override
+    public void onSet(Object pojo, String fieldName, Object value) {
+        if (fieldName.equals("m_foo")) {
+            System.out.println("Attempt to set the foo field to " + value);
+            m_value = (String) value;
+        }
+        if (pojo == null) {
+            throw new IllegalStateException("pojo parameter null - FELIX-4072");
+        }
+    }
+}
diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/FooProviderType1.java b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/FooProviderType1.java
index a8fb40d..0d2dfb0 100644
--- a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/FooProviderType1.java
+++ b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/components/FooProviderType1.java
@@ -29,23 +29,7 @@
 	private String m_foo;

     

     private BundleContext m_context;

-    

-    private static FooProviderType1 singleton;

-    private static int count = 0;

-    

-    private static FooProviderType1 singleton(BundleContext bc) {

-        if (singleton == null) {

-            count++;

-            singleton = new FooProviderType1(bc);

-        }

-        return singleton;

-    }

-    

-    public static FooProviderType1 several(BundleContext bc) {

-        count++;

-        return new FooProviderType1(bc);

-    }

-        

+

     public FooProviderType1(BundleContext bc) {

         if (bc ==null) {

             throw new RuntimeException("Injected bundle context null");

@@ -54,43 +38,28 @@
     }

 

 	public boolean foo() {

+        // Update m_foo

+        if (m_foo.equals(VALUE)) {

+            m_foo = VALUE_2;

+        }

+

 		return true;

 	}

 

 	public Properties fooProps() {

 		Properties p = new Properties();

-		p.put("bar", new Integer(m_bar));

+		p.put("bar", m_bar);

         if(m_foo != null) {

             p.put("foo", m_foo);

         }

         p.put("context", m_context);

-        

-        p.put("count", new Integer(count));

+

+        int count = 0;

+        p.put("count", count);

 		return p;

 	}

     

-	public void testException() throws Exception {

-        String a = "foobarbaz";

-	    throw new Exception("foo"+a);

-    }

-    

-    public void testTry() {

-            String a = "foo";

-            a.charAt(0);

-    }

-    

-    public void testTry2(String s) {

-            String a = "foo";

-            a.charAt(0);

-    }

-    

-    private void nexttry(String  s) {

-        try {

-            s += "foo";

-        } catch(RuntimeException e) {

-            

-        }

-    }

+

     

 	public boolean getBoolean() { return true; }

 

@@ -100,18 +69,6 @@
 

 	public long getLong() { return 1; }

 

-	public Boolean getObject() { return new Boolean(true); }

-	

-	/**

-	 * Custom constructor.

-	 * @param bar

-	 * @param foo

-	 * @param bc

-	 */

-	public FooProviderType1(int bar, String foo, BundleContext bc) {

-	    m_bar = bar;

-	    m_foo = foo;

-	    m_context = bc;

-	}

+	public Boolean getObject() { return true; }

 

 }

diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/services/FooService.java b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/services/FooService.java
index 047c7b9..1ccb16e 100644
--- a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/services/FooService.java
+++ b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/java/org/apache/felix/ipojo/runtime/externalhandlers/services/FooService.java
@@ -22,6 +22,9 @@
 

 public interface FooService {

 

+    public static final String VALUE = "my foo is better than yours";

+    public static final String VALUE_2 = "No";

+

 	boolean foo();

 	

 	Properties fooProps();

diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/resources/metadata.xml b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/resources/metadata.xml
index f1ce7b1..0f1f66f 100644
--- a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/resources/metadata.xml
+++ b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/main/resources/metadata.xml
@@ -29,11 +29,30 @@
             architecture="false">

         <controller field="isValid" />

     </handler>

+

+    <!-- a handler with a field interceptor -->

+    <handler

+            classname="org.apache.felix.ipojo.runtime.externalhandlers.components.CheckServiceAndFieldInterceptorHandler"

+            name="checkAndSet"

+            namespace="org.apache.felix.ipojo.test.handler.checkservice"

+            architecture="false">

+        <controller field="isValid" />

+    </handler>

+

     <component

             classname="org.apache.felix.ipojo.runtime.externalhandlers.components.FooProviderType1"

             name="HANDLER-HandlerTester" architecture="true">

         <cs:check />

     </component>

+

+    <component

+            classname="org.apache.felix.ipojo.runtime.externalhandlers.components.FooProviderType1"

+            name="HANDLER-HandlerWithFieldInterceptorTester" architecture="true">

+        <cs:checkAndSet />

+        <provides/>

+    </component>

+

+

     <instance name="HandlerTest-2" component="HANDLER-HandlerTester">

         <property name="csh.simple" value="Simple" />

         <property name="csh.map">

diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/java/org/apache/felix/ipojo/runtime/externalhandlers/test/HandlerWithFieldInterceptorTest.java b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/java/org/apache/felix/ipojo/runtime/externalhandlers/test/HandlerWithFieldInterceptorTest.java
new file mode 100644
index 0000000..1cc360e
--- /dev/null
+++ b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/java/org/apache/felix/ipojo/runtime/externalhandlers/test/HandlerWithFieldInterceptorTest.java
@@ -0,0 +1,84 @@
+/*
+ * 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.runtime.externalhandlers.test;
+
+import org.apache.felix.ipojo.ComponentInstance;
+import org.apache.felix.ipojo.HandlerManagerFactory;
+import org.apache.felix.ipojo.architecture.Architecture;
+import org.apache.felix.ipojo.runtime.externalhandlers.services.CheckService;
+import org.apache.felix.ipojo.runtime.externalhandlers.services.FooService;
+import org.junit.Before;
+import org.junit.Test;
+import org.osgi.framework.ServiceReference;
+import org.ow2.chameleon.testing.helpers.Dumps;
+
+import java.util.Dictionary;
+import java.util.Properties;
+
+import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class HandlerWithFieldInterceptorTest extends Common {
+
+
+    ComponentInstance instance;
+
+    @Before
+    public void setUp() {
+        Properties props = new Properties();
+        props.put("instance.name", "HandlerTest-1");
+        props.put("csh.simple", "simple");
+        Properties p = new Properties();
+        p.put("a", "a");
+        p.put("b", "b");
+        p.put("c", "c");
+        props.put("csh.map", p);
+        props.put("foo", FooService.VALUE);
+        instance = ipojoHelper.createComponentInstance("HANDLER-HandlerWithFieldInterceptorTester", props);
+    }
+
+    @Test
+    public void testFieldInterception() {
+        // Check the availability of CheckService
+        String name = "HandlerTest-1";
+        ServiceReference ref = ipojoHelper.getServiceReferenceByName(CheckService.class.getName(), name);
+        assertNotNull("Check the check service availability", ref);
+        CheckService cs = (CheckService) osgiHelper.getServiceObject(ref);
+
+        Dictionary<String, Object> p = cs.getProps();
+        assertEquals("Assert 'simple' equality", p.get("Simple"), "simple");
+        assertEquals("Assert 'a' equality", p.get("Map1"), "a");
+        assertEquals("Assert 'b' equality", p.get("Map2"), "b");
+        assertEquals("Assert 'c' equality", p.get("Map3"), "c");
+
+        assertEquals("check foo value", FooService.VALUE, cs.getProps().get("foo"));
+
+        // Change value.
+
+        ServiceReference ref2 = ipojoHelper.getServiceReferenceByName(FooService.class.getName(), name);
+        assertNotNull("Check the foo service availability", ref2);
+        FooService fs = (FooService) osgiHelper.getServiceObject(ref2);
+
+        fs.foo(); // This trigger the changes.
+
+        assertEquals("check foo value", FooService.VALUE_2, cs.getProps().get("foo"));
+    }
+
+}
diff --git a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/resources/exam.properties b/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/resources/exam.properties
deleted file mode 100644
index d8a500d..0000000
--- a/ipojo/runtime/core-it/src/it/ipojo-core-external-handlers-test/src/test/resources/exam.properties
+++ /dev/null
@@ -1,19 +0,0 @@
-#
-# 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.
-#
-
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
index a8c650d..1589c68 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
@@ -997,7 +997,7 @@
      * @param fields the field metadata list
      * @param methods the method metadata list
      * @deprecated use {@link InstanceManager#register(FieldMetadata, FieldInterceptor)}
-     * and {@link InstanceManager#register(FieldMetadata, MethodInterceptor)} instead.
+     * and {@link InstanceManager#register(MethodMetadata, MethodInterceptor)} instead.
      */
     public void register(PrimitiveHandler handler, FieldMetadata[] fields, MethodMetadata[] methods) {
         for (int i = 0; fields != null && i < fields.length; i++) {
@@ -1111,7 +1111,7 @@
         FieldInterceptor[] list = (FieldInterceptor[]) m_fieldRegistration.get(fieldName); // Immutable list.
         for (int i = 0; list != null && i < list.length; i++) {
             // Call onGet outside of a synchronized block.
-            Object handlerResult = list[i].onGet(null, fieldName, initialValue);
+            Object handlerResult = list[i].onGet(pojo, fieldName, initialValue);
             if (handlerResult == initialValue) {
                 continue; // Non-binding case (default implementation).
             } else {
@@ -1136,7 +1136,7 @@
             }
             // Call onset outside of a synchronized block.
             for (int i = 0; list != null && i < list.length; i++) {
-                list[i].onSet(null, fieldName, result);
+                list[i].onSet(pojo, fieldName, result);
             }
         }
         return result;
@@ -1144,7 +1144,7 @@
 
     /**
      * Dispatches entry method events on registered method interceptors.
-     * This method calls the {@link PrimitiveHandler#onEntry(Object, Method, Object[])}
+     * This method calls the {@link MethodInterceptor#onEntry(Object, java.lang.reflect.Member, Object[])}
      * methods on method interceptors monitoring the method.
      * @param pojo the pojo object on which method is invoked.
      * @param methodId the method id used to compute the {@link Method} object.
@@ -1167,8 +1167,9 @@
      * The given returned object is an instance of {@link Exception} if the method thrown an
      * exception. If the given object is <code>null</code>, either the method returns <code>void</code>,
      * or the method has returned <code>null</code>
-     * This method calls the {@link PrimitiveHandler#onExit(Object, Method, Object[])} and the
-     * {@link PrimitiveHandler#onFinally(Object, Method)} methods on method interceptors monitoring the method.
+     * This method calls the {@link MethodInterceptor#onExit(Object, java.lang.reflect.Member, Object)} and the
+     * {@link MethodInterceptor#onFinally(Object, java.lang.reflect.Member)} methods on method interceptors
+     * monitoring the method.
      * @param pojo the pojo object on which method was invoked.
      * @param methodId the method id used to compute the {@link Method} object.
      * @param result the returned object.
@@ -1190,8 +1191,8 @@
     /**
      * Dispatches error method events on registered method interceptors.
      * or the method has returned <code>null</code>
-     * This method calls the {@link PrimitiveHandler#onExit(Object, Method, Object[])} and the
-     * {@link PrimitiveHandler#onFinally(Object, Method)} methods on method interceptors monitoring
+     * This method calls the {@link MethodInterceptor#onError(Object, java.lang.reflect.Member, Throwable)} and the
+     * {@link MethodInterceptor#onFinally(Object, java.lang.reflect.Member)} methods on method interceptors monitoring
      * the method.
      * @param pojo the pojo object on which the method was invoked
      * @param methodId the method id used to compute the {@link Method} object.
@@ -1276,7 +1277,7 @@
                 .get(fieldName);
         for (int i = 0; list != null && i < list.length; i++) {
              // The callback must be call outside the synchronization block.
-            list[i].onSet(null, fieldName, objectValue);
+            list[i].onSet(pojo, fieldName, objectValue);
         }
     }