You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2009/09/02 11:04:08 UTC
svn commit: r810428 - in /sling/trunk/installer/osgi:
installer/src/main/java/org/apache/sling/osgi/installer/
installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/
it/src/test/java/org/apache/sling/osgi/installer/it/
Author: bdelacretaz
Date: Wed Sep 2 09:04:08 2009
New Revision: 810428
URL: http://svn.apache.org/viewvc?rev=810428&view=rev
Log:
SLING-1078 - ConfigInstallTask ignores configs which make no actual changes
Modified:
sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/InstallableResource.java
sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/ConfigInstallTask.java
sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/ConfigInstallTest.java
sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/OsgiInstallerTestBase.java
Modified: sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/InstallableResource.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/InstallableResource.java?rev=810428&r1=810427&r2=810428&view=diff
==============================================================================
--- sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/InstallableResource.java (original)
+++ sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/InstallableResource.java Wed Sep 2 09:04:08 2009
@@ -27,6 +27,7 @@
import java.security.NoSuchAlgorithmException;
import java.util.Dictionary;
import java.util.Enumeration;
+import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
@@ -161,16 +162,26 @@
final BigInteger bigInt = new BigInteger(1, d.digest());
return new String(bigInt.toString(16));
}
+
+ /** Compute digest on all keys of supplied data */
+ public static String computeDigest(Dictionary<String, Object> data) throws IOException, NoSuchAlgorithmException {
+ return computeDigest(data, null);
+ }
/** Digest is needed to detect changes in data, and must not depend on dictionary ordering */
- public static String computeDigest(Dictionary<String, Object> data) throws IOException, NoSuchAlgorithmException {
+ public static String computeDigest(Dictionary<String, Object> data, Set<String> keysToIgnore) throws IOException, NoSuchAlgorithmException {
final MessageDigest d = MessageDigest.getInstance(DIGEST_TYPE);
final ByteArrayOutputStream bos = new ByteArrayOutputStream();
final ObjectOutputStream oos = new ObjectOutputStream(bos);
final SortedSet<String> sortedKeys = new TreeSet<String>();
- for(Enumeration<String> e = data.keys(); e.hasMoreElements(); ) {
- sortedKeys.add(e.nextElement());
+ if(data != null) {
+ for(Enumeration<String> e = data.keys(); e.hasMoreElements(); ) {
+ final String key = e.nextElement();
+ if(keysToIgnore == null || !keysToIgnore.contains(key)) {
+ sortedKeys.add(key);
+ }
+ }
}
for(String key : sortedKeys) {
oos.writeObject(key);
Modified: sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/ConfigInstallTask.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/ConfigInstallTask.java?rev=810428&r1=810427&r2=810428&view=diff
==============================================================================
--- sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/ConfigInstallTask.java (original)
+++ sling/trunk/installer/osgi/installer/src/main/java/org/apache/sling/osgi/installer/impl/tasks/ConfigInstallTask.java Wed Sep 2 09:04:08 2009
@@ -18,8 +18,13 @@
*/
package org.apache.sling.osgi.installer.impl.tasks;
+import java.io.IOException;
+import java.security.NoSuchAlgorithmException;
import java.util.Dictionary;
+import java.util.HashSet;
+import java.util.Set;
+import org.apache.sling.osgi.installer.InstallableResource;
import org.apache.sling.osgi.installer.impl.OsgiInstallerContext;
import org.apache.sling.osgi.installer.impl.RegisteredResource;
import org.osgi.service.cm.Configuration;
@@ -33,6 +38,13 @@
static final String CONFIG_PATH_KEY = "_jcr_config_path";
public static final String [] CONFIG_EXTENSIONS = { ".cfg", ".properties" };
+ /** Configuration properties to ignore when comparing configs */
+ public static Set<String> ignoredProperties = new HashSet<String>();
+ static {
+ ignoredProperties.add("service.pid");
+ ignoredProperties.add(CONFIG_PATH_KEY);
+ }
+
public ConfigInstallTask(RegisteredResource r) {
super(r);
}
@@ -42,7 +54,8 @@
return TaskOrder.CONFIG_INSTALL_ORDER + pid.getCompositePid();
}
- @Override
+ @SuppressWarnings("unchecked")
+ @Override
public void execute(OsgiInstallerContext ctx) throws Exception {
final ConfigurationAdmin ca = ctx.getConfigurationAdmin();
@@ -55,8 +68,6 @@
return;
}
- logExecution(ctx);
-
// Convert data to a configuration Dictionary
Dictionary<String, Object> dict = resource.getDictionary();
@@ -72,22 +83,48 @@
dict.put(ALIAS_KEY, pid.getFactoryPid());
}
- // Get or create configuration
+ // Get or create configuration, but do not
+ // update if the new one has the same values.
boolean created = false;
Configuration config = getConfiguration(ca, pid, false, ctx);
if(config == null) {
created = true;
config = getConfiguration(ca, pid, true, ctx);
+ } else {
+ if(isSameData(config.getProperties(), resource.getDictionary())) {
+ if(ctx.getLogService() != null) {
+ ctx.getLogService().log(LogService.LOG_DEBUG,
+ "Configuration " + config.getPid()
+ + " already installed with same data, update request ignored: "
+ + resource);
+ }
+ config = null;
+ }
}
- if (config.getBundleLocation() != null) {
- config.setBundleLocation(null);
- }
- config.update(dict);
- if(ctx.getLogService() != null) {
- ctx.getLogService().log(LogService.LOG_INFO,
- "Configuration " + config.getPid()
- + " " + (created ? "created" : "updated")
- + " from " + resource);
+
+ if(config != null) {
+ logExecution(ctx);
+ if (config.getBundleLocation() != null) {
+ config.setBundleLocation(null);
+ }
+ config.update(dict);
+ if(ctx.getLogService() != null) {
+ ctx.getLogService().log(LogService.LOG_INFO,
+ "Configuration " + config.getPid()
+ + " " + (created ? "created" : "updated")
+ + " from " + resource);
+ }
}
}
+
+ /** True if a and b represent the same config data, ignoring "non-configuration" keys in the dictionaries */
+ boolean isSameData(Dictionary<String, Object>a, Dictionary<String, Object>b) throws NoSuchAlgorithmException, IOException {
+ boolean result = false;
+ if(a != null && b != null) {
+ final String da = InstallableResource.computeDigest(a, ignoredProperties);
+ final String db = InstallableResource.computeDigest(b, ignoredProperties);
+ result = da.equals(db);
+ }
+ return result;
+ }
}
\ No newline at end of file
Modified: sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/ConfigInstallTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/ConfigInstallTest.java?rev=810428&r1=810427&r2=810428&view=diff
==============================================================================
--- sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/ConfigInstallTest.java (original)
+++ sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/ConfigInstallTest.java Wed Sep 2 09:04:08 2009
@@ -22,6 +22,8 @@
import java.util.Dictionary;
import java.util.Hashtable;
+import java.util.LinkedList;
+import java.util.List;
import org.apache.sling.osgi.installer.InstallableResource;
import org.apache.sling.osgi.installer.OsgiInstaller;
@@ -32,13 +34,19 @@
import org.ops4j.pax.exam.Option;
import org.ops4j.pax.exam.junit.JUnit4TestRunner;
import org.osgi.framework.Bundle;
+import org.osgi.framework.ServiceRegistration;
import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.ConfigurationAdmin;
+import org.osgi.service.cm.ConfigurationEvent;
+import org.osgi.service.cm.ConfigurationListener;
@RunWith(JUnit4TestRunner.class)
-public class ConfigInstallTest extends OsgiInstallerTestBase {
+public class ConfigInstallTest extends OsgiInstallerTestBase implements ConfigurationListener {
private final static long TIMEOUT = 5000L;
+ private List<ConfigurationEvent> events = new LinkedList<ConfigurationEvent>();
+ private ServiceRegistration serviceRegistration;
@org.ops4j.pax.exam.junit.Configuration
public static Option[] configuration() {
@@ -48,14 +56,24 @@
@Before
public void setUp() {
setupInstaller();
+ events.clear();
+ serviceRegistration = bundleContext.registerService(ConfigurationListener.class.getName(), this, null);
}
@After
public void tearDown() {
super.tearDown();
+ if(serviceRegistration != null) {
+ serviceRegistration.unregister();
+ serviceRegistration = null;
+ }
}
- @Test
+ public void configurationEvent(ConfigurationEvent e) {
+ events.add(e);
+ }
+
+ @Test
public void testInstallAndRemoveConfig() throws Exception {
final Dictionary<String, Object> cfgData = new Hashtable<String, Object>();
cfgData.put("foo", "bar");
@@ -136,4 +154,33 @@
waitForConfiguration("Config must be removed once ConfigurationAdmin restarts",
cfgPid, TIMEOUT, false);
}
+
+ @Test
+ public void testReinstallSameConfig() throws Exception {
+ final Dictionary<String, Object> cfgData = new Hashtable<String, Object>();
+ cfgData.put("foo", "bar");
+ final String cfgPid = getClass().getSimpleName() + ".reinstall." + System.currentTimeMillis();
+ assertNull("Config " + cfgPid + " must not be found before test", findConfiguration(cfgPid));
+
+ // Install config directly
+ ConfigurationAdmin ca = waitForConfigAdmin(true);
+ final Configuration c = ca.getConfiguration(cfgPid);
+ c.update(cfgData);
+ waitForConfigValue("After manual installation", cfgPid, TIMEOUT, "foo", "bar");
+ assertEquals("Expected one ConfigurationEvent after manual install", 1, events.size());
+
+ long nOps = installer.getCounters()[OsgiInstaller.OSGI_TASKS_COUNTER];
+ installer.addResource(getInstallableResource(cfgPid, cfgData));
+ waitForInstallerAction(OsgiInstaller.WORKER_THREAD_BECOMES_IDLE_COUNTER, 1);
+ assertEquals("Registering a Configuration that's already installed must not generate OSGi tasks",
+ nOps, installer.getCounters()[OsgiInstaller.OSGI_TASKS_COUNTER]);
+ assertEquals("Expected one ConfigurationEvent after (ignored) install via OsgiInstaller", 1, events.size());
+
+ // Reinstalling with a change must be executed
+ cfgData.put("foo", "changed");
+ installer.addResource(getInstallableResource(cfgPid, cfgData));
+ waitForConfigValue("After changing value", cfgPid, TIMEOUT, "foo", "changed");
+ final Condition cond = new Condition() { public boolean isTrue() { return events.size() == 2; }};
+ waitForCondition("Expected two ConfigurationEvents since beginning of test", TIMEOUT, cond);
+ }
}
Modified: sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/OsgiInstallerTestBase.java
URL: http://svn.apache.org/viewvc/sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/OsgiInstallerTestBase.java?rev=810428&r1=810427&r2=810428&view=diff
==============================================================================
--- sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/OsgiInstallerTestBase.java (original)
+++ sling/trunk/installer/osgi/it/src/test/java/org/apache/sling/osgi/installer/it/OsgiInstallerTestBase.java Wed Sep 2 09:04:08 2009
@@ -65,6 +65,10 @@
public static final String URL_SCHEME = "OsgiInstallerTest";
+ static interface Condition {
+ boolean isTrue() throws Exception;
+ }
+
@SuppressWarnings("unchecked")
protected <T> T getService(Class<T> clazz) {
final ServiceReference ref = bundleContext.getServiceReference(clazz.getName());
@@ -153,6 +157,29 @@
return null;
}
+ protected void waitForCondition(String info, long timeoutMsec, Condition c) throws Exception {
+ final long end = System.currentTimeMillis() + timeoutMsec;
+ do {
+ if(c.isTrue()) {
+ return;
+ }
+ Thread.sleep(100L);
+ } while(System.currentTimeMillis() < end);
+ fail("WaitForCondition failed: " + info);
+ }
+
+ protected void waitForConfigValue(String info, String pid, long timeoutMsec, String key, String value) throws Exception {
+ final long end = System.currentTimeMillis() + timeoutMsec;
+ do {
+ final Configuration c = waitForConfiguration(info, pid, timeoutMsec, true);
+ if(value.equals(c.getProperties().get(key))) {
+ return;
+ }
+ Thread.sleep(100L);
+ } while(System.currentTimeMillis() < end);
+ fail("Did not get " + key + "=" + value + " for config " + pid);
+ }
+
protected Configuration waitForConfiguration(String info, String pid, long timeoutMsec, boolean shouldBePresent) throws Exception {
if(info == null) {
info = "";
@@ -245,7 +272,8 @@
return result;
}
- protected void waitForConfigAdmin(boolean shouldBePresent) {
+ protected ConfigurationAdmin waitForConfigAdmin(boolean shouldBePresent) {
+ ConfigurationAdmin result = null;
if(configAdminTracker == null) {
synchronized (this) {
configAdminTracker = new ServiceTracker(bundleContext, ConfigurationAdmin.class.getName(), null);
@@ -256,13 +284,13 @@
final int timeout = 5;
final long waitUntil = System.currentTimeMillis() + (timeout * 1000L);
do {
- boolean isPresent = configAdminTracker.getService() != null;
- if(isPresent == shouldBePresent) {
- return;
- }
- sleep(100L);
+ result = (ConfigurationAdmin)configAdminTracker.getService();
+ boolean isPresent = result != null;
+ assertEquals("Expected ConfigurationAdmin to be " + (shouldBePresent ? "present" : "absent"),
+ shouldBePresent, isPresent);
} while(System.currentTimeMillis() < waitUntil);
- fail("ConfigurationAdmin service not available after waiting " + timeout + " seconds");
+
+ return result;
}
protected void resetCounters() {