Fix FELIX-4172 Updated method called twice at the bundle start
Rewrote the change detection.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1519987 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/main/java/org/apache/felix/ipojo/runtime/core/components/ImmediateConfigurableFooProvider.java b/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/main/java/org/apache/felix/ipojo/runtime/core/components/ImmediateConfigurableFooProvider.java
new file mode 100644
index 0000000..5ac95b5
--- /dev/null
+++ b/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/main/java/org/apache/felix/ipojo/runtime/core/components/ImmediateConfigurableFooProvider.java
@@ -0,0 +1,86 @@
+/*
+ * 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.core.components;
+
+import org.apache.felix.ipojo.annotations.Component;
+import org.apache.felix.ipojo.annotations.Property;
+import org.apache.felix.ipojo.annotations.Provides;
+import org.apache.felix.ipojo.annotations.Updated;
+import org.apache.felix.ipojo.runtime.core.services.FooService;
+
+import java.util.Dictionary;
+import java.util.Properties;
+
+@Component(propagation = true, immediate = true)
+@Provides
+public class ImmediateConfigurableFooProvider implements FooService {
+
+ @Property
+ private String message;
+
+ @Property(name=".private")
+ private String myPrivateLife;
+
+ private Dictionary configuration;
+ private int count;
+
+ @Updated
+ public void updated(Dictionary conf) {
+ this.configuration = conf;
+ this.count++;
+ }
+
+ public boolean foo() {
+ return true;
+ }
+
+ public Properties fooProps() {
+ Properties props = new Properties();
+ if (message == null) {
+ props.put("message", "NULL");
+ } else {
+ props.put("message", message);
+ }
+ props.put("count", count);
+
+ return props;
+ }
+
+ public boolean getBoolean() {
+ return false;
+ }
+
+ public double getDouble() {
+ return 0;
+ }
+
+ public int getInt() {
+ return count;
+ }
+
+ public long getLong() {
+ return 0;
+ }
+
+ public Boolean getObject() {
+ return null;
+ }
+
+}
diff --git a/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/test/java/org/apache/felix/ipojo/runtime/core/TestUpdated.java b/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/test/java/org/apache/felix/ipojo/runtime/core/TestUpdated.java
new file mode 100644
index 0000000..13c0434
--- /dev/null
+++ b/ipojo/runtime/core-it/ipojo-core-configuration-admin-test/src/test/java/org/apache/felix/ipojo/runtime/core/TestUpdated.java
@@ -0,0 +1,121 @@
+/*
+ * 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.core;
+
+import org.apache.felix.ipojo.runtime.core.services.FooService;
+import org.junit.Test;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.cm.Configuration;
+
+import java.io.IOException;
+import java.util.Properties;
+
+import static junit.framework.Assert.assertEquals;
+
+/**
+ * Test for FELIX-4172 Updated method called twice at the bundle start
+ */
+public class TestUpdated extends Common {
+
+ private String factoryName = "org.apache.felix.ipojo.runtime.core.components.ImmediateConfigurableFooProvider";
+
+ @Test
+ public void testNumberOfUpdatedCalls() throws IOException {
+ Properties props = new Properties();
+ props.put("message", "message");
+ props.put("propagated", "propagated");
+ props.put(".private", "wow");
+
+ Configuration configuration = admin.createFactoryConfiguration(factoryName, "?");
+ configuration.update(props);
+
+ ServiceReference ref = osgiHelper.waitForService(FooService.class.getName(),
+ "(instance.name=" + configuration.getPid() + ")",
+ 1000);
+
+ FooService fs = (FooService) osgiHelper.getServiceObject(ref);
+ assertEquals(fs.getInt(), 1);
+
+ // Update the property
+ props.put("message", "message2");
+ props.put("propagated", "propagated2");
+ props.put(".private", "wow2");
+ configuration.update(props);
+
+ grace();
+
+ assertEquals(fs.getInt(), 2);
+
+ // Remove a property
+ props.remove("propagated");
+ configuration.update(props);
+
+ grace();
+
+ assertEquals(fs.getInt(), 3);
+
+
+ configuration.delete();
+ }
+
+ @Test
+ public void testNumberOfUpdatedCallsWithManagedService() throws IOException {
+ Properties props = new Properties();
+ props.put("message", "message");
+ props.put("propagated", "propagated");
+ props.put(".private", "wow");
+ props.put("managed.service.pid", "config");
+
+ Configuration configuration = admin.createFactoryConfiguration(factoryName, "?");
+ configuration.update(props);
+
+ ServiceReference ref = osgiHelper.waitForService(FooService.class.getName(),
+ "(instance.name=" + configuration.getPid() + ")",
+ 1000);
+
+ FooService fs = (FooService) osgiHelper.getServiceObject(ref);
+ assertEquals(fs.getInt(), 1);
+
+ // Update the property using the managed service.
+
+ Configuration managedConfiguration = admin.getConfiguration("config", "?");
+ props.put("message", "message2");
+ props.put("propagated", "propagated2");
+ props.put(".private", "wow2");
+ managedConfiguration.update(props);
+
+ grace();
+
+ assertEquals(fs.getInt(), 2);
+
+ // Remove a property
+ props.remove("propagated");
+ managedConfiguration.update(props);
+
+ grace();
+
+ assertEquals(fs.getInt(), 3);
+
+ managedConfiguration.delete();
+ configuration.delete();
+ }
+
+
+}
diff --git a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
index 60fcae2..d296a3c 100644
--- a/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
+++ b/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
@@ -44,6 +44,7 @@
*/
public class ConfigurationHandler extends PrimitiveHandler implements ManagedService {
+ public static final String MANAGED_SERVICE_PID = "managed.service.pid";
/**
* List of the configurable fields.
*/
@@ -239,7 +240,7 @@
// Check if the component support ConfigurationADmin reconfiguration
m_managedServicePID = confs[0].getAttribute("pid"); // Look inside the component type description
- String instanceMSPID = (String) configuration.get("managed.service.pid"); // Look inside the instance configuration.
+ String instanceMSPID = (String) configuration.get(MANAGED_SERVICE_PID); // Look inside the instance configuration.
if (instanceMSPID != null) {
m_managedServicePID = instanceMSPID;
}
@@ -382,32 +383,112 @@
/**
* Reconfigure the component instance.
* Check if the new configuration modifies the current configuration.
- * Invokes the updated method is needed.
+ * Invokes the updated method if needed.
*
* @param configuration : the new configuration
* @see org.apache.felix.ipojo.Handler#reconfigure(java.util.Dictionary)
*/
public void reconfigure(Dictionary configuration) {
Map<String, Object> map = new LinkedHashMap<String, Object>();
+ boolean changed = false;
synchronized (this) {
info(getInstanceManager().getInstanceName() + " is reconfiguring the properties : " + configuration);
- Properties props = reconfigureProperties(configuration);
- propagate(props, m_propagatedFromInstance);
- m_propagatedFromInstance = props;
- if (getInstanceManager().getPojoObjects() != null) {
- try {
- notifyUpdated(null);
- } catch (Throwable e) {
- error("Cannot call the updated method : " + e.getMessage(), e);
+ // Is there any changes ?
+ changed = detectConfigurationChanges(configuration);
+ if (changed) {
+ Properties extra = reconfigureProperties(configuration);
+ propagate(extra, m_propagatedFromInstance);
+ m_propagatedFromInstance = extra;
+
+ if (getInstanceManager().getPojoObjects() != null) {
+ try {
+ notifyUpdated(null);
+ } catch (Throwable e) {
+ error("Cannot call the updated method : " + e.getMessage(), e);
+ }
+ }
+ // Make a snapshot of the current configuration
+ for (Property p : m_configurableProperties) {
+ map.put(p.getName(), p.getValue());
}
}
- // Make a snapshot of the current configuration
- for (Property p : m_configurableProperties) {
- map.put(p.getName(), p.getValue());
+ }
+
+ if (changed) {
+ notifyListeners(map);
+ }
+ }
+
+ private boolean detectConfigurationChanges(Dictionary configuration) {
+ Enumeration keysEnumeration = configuration.keys();
+ while (keysEnumeration.hasMoreElements()) {
+ String name = (String) keysEnumeration.nextElement();
+ Object value = configuration.get(name);
+
+ // Some properties are skipped
+ if (name.equals(Factory.INSTANCE_NAME_PROPERTY)
+ || name.equals(Constants.SERVICE_PID)
+ || name.equals(MANAGED_SERVICE_PID)) {
+ continue;
+ }
+
+ // Do we have a property.
+ Property p = getPropertyByName(name);
+ if (p != null) {
+ // Change detection based on the value.
+ if (p.getValue() == null) {
+ return true;
+ } else if (! p.getValue().equals(value)) {
+ return true;
+ }
+ } else {
+ // Was it propagated ?
+ if (m_propagatedFromCA != null) {
+ Object v = m_propagatedFromCA.get(name);
+ if (v == null || ! v.equals(value)) {
+ return true;
+ }
+ }
+ if (m_propagatedFromInstance != null) {
+ Object v = m_propagatedFromInstance.get(name);
+ if (v == null || ! v.equals(value)) {
+ return true;
+ }
+ }
}
}
- notifyListeners(map);
+
+ // A propagated property may have been removed.
+ if (m_propagatedFromCA != null) {
+ Enumeration enumeration = m_propagatedFromCA.keys();
+ while (enumeration.hasMoreElements()) {
+ String k = (String) enumeration.nextElement();
+ if (configuration.get(k) == null) {
+ return true;
+ }
+ }
+ }
+ if (m_propagatedFromInstance != null) {
+ Enumeration enumeration = m_propagatedFromInstance.keys();
+ while (enumeration.hasMoreElements()) {
+ String k = (String) enumeration.nextElement();
+ if (configuration.get(k) == null) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
+
+ private Property getPropertyByName(String name) {
+ for (Property p : m_configurableProperties) {
+ if (p.getName().equals(name)) {
+ return p;
+ }
+ }
+ return null;
}
/**
@@ -502,8 +583,8 @@
if (newProps != null) {
// Remove the name, the pid and the managed service pid props
- newProps.remove("name");
- newProps.remove("managed.service.pid");
+ newProps.remove(Factory.INSTANCE_NAME_PROPERTY);
+ newProps.remove(MANAGED_SERVICE_PID);
newProps.remove(Constants.SERVICE_PID);
// Remove all properties starting with . (config admin specification)
@@ -590,7 +671,7 @@
props.put(n, v);
}
}
- // add propagate properties to the list if propagation is enabled
+ // add propagated properties to the list if propagation is enabled
if (m_mustPropagate) {
// Start by properties from the configuration admin,
if (m_propagatedFromCA != null) {