incubator-sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bdelacre...@apache.org
Subject 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/
Date Wed, 02 Sep 2009 09:04:08 GMT
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() {



Mime
View raw message