This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to annotated tag org.apache.sling.installer.factory.configuration-1.1.0 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-installer-factory-configuration.git
commit adfde86db081e4520a4b680b87b515517fc5d5c6 Author: Carsten Ziegeler <[email protected]> AuthorDate: Mon Dec 29 10:24:15 2014 +0000 SLING-4250 : Configuration values should be compared by string comparison SLING-4263 : Bundle location should be set to ? by default git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/installer/factories/configuration@1648302 13f79535-47bb-0310-9956-ffa450edef68 --- pom.xml | 10 +-- .../configuration/ConfigurationConstants.java | 10 +++ .../configuration/impl/AbstractConfigTask.java | 10 ++- .../configuration/impl/ConfigInstallTask.java | 20 +++-- .../configuration/impl/ConfigRemoveTask.java | 5 +- .../configuration/impl/ConfigTaskCreator.java | 21 ++--- .../factories/configuration/impl/ConfigUtil.java | 69 ++++++++++++---- ...nfigurationConstants.java => package-info.java} | 13 +--- .../configuration/impl/ConfigUtilTest.java | 91 ++++++++++++++++++++++ 9 files changed, 188 insertions(+), 61 deletions(-) diff --git a/pom.xml b/pom.xml index 9938245..2221d25 100644 --- a/pom.xml +++ b/pom.xml @@ -51,12 +51,6 @@ <Bundle-Activator> org.apache.sling.installer.factories.configuration.impl.Activator </Bundle-Activator> - <Export-Package> - org.apache.sling.installer.factories.configuration;version=1.0.0 - </Export-Package> - <Private-Package> - org.apache.sling.installer.factories.configuration.impl.* - </Private-Package> </instructions> </configuration> </plugin> @@ -82,5 +76,9 @@ <version>1.0.0</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + </dependency> </dependencies> </project> diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java b/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java index 93afb48..c762c00 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java @@ -28,4 +28,14 @@ public abstract class ConfigurationConstants { * by clients creating the configuration. */ public static final String PROPERTY_PERSISTENCE = "org.apache.sling.installer.configuration.persist"; + + /** + * This property defines the value to be used as a bundle location if a configuration + * is created by the installer. This property is a string value defaulting to "?". + * If this property contains the empty string, {@code null} is used as the value. + * + * The property should be used, if a configuration should be bound to a specific client. + * @since 1.1 + */ + public static final String PROPERTY_BUNDLE_LOCATION = "org.apache.sling.installer.configuration.bundlelocation"; } diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/AbstractConfigTask.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/AbstractConfigTask.java index c1d82e5..c5f4d1d 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/AbstractConfigTask.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/AbstractConfigTask.java @@ -92,9 +92,13 @@ abstract class AbstractConfigTask extends InstallTask { return this.getResource().getDictionary(); } - protected Configuration getConfiguration(final ConfigurationAdmin ca, - final boolean createIfNeeded) + protected Configuration getConfiguration() throws IOException, InvalidSyntaxException { - return ConfigUtil.getConfiguration(ca, this.factoryPid, (this.factoryPid != null && this.aliasPid != null ? this.aliasPid : this.configPid), createIfNeeded); + return ConfigUtil.getConfiguration(this.configAdmin, this.factoryPid, (this.factoryPid != null && this.aliasPid != null ? this.aliasPid : this.configPid)); + } + + protected Configuration createConfiguration(final String location) + throws IOException, InvalidSyntaxException { + return ConfigUtil.createConfiguration(this.configAdmin, this.factoryPid, (this.factoryPid != null && this.aliasPid != null ? this.aliasPid : this.configPid), location); } } diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java index 775d605..a5f1e11 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigInstallTask.java @@ -21,6 +21,7 @@ package org.apache.sling.installer.factories.configuration.impl; import org.apache.sling.installer.api.tasks.InstallationContext; import org.apache.sling.installer.api.tasks.ResourceState; import org.apache.sling.installer.api.tasks.TaskResourceGroup; +import org.apache.sling.installer.factories.configuration.ConfigurationConstants; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; @@ -44,28 +45,33 @@ public class ConfigInstallTask extends AbstractConfigTask { @Override public void execute(final InstallationContext ctx) { synchronized ( ConfigTaskCreator.getLock() ) { - final ConfigurationAdmin ca = this.getConfigurationAdmin(); - // Get or create configuration, but do not // update if the new one has the same values. boolean created = false; try { - Configuration config = getConfiguration(ca, false); + String location = (String)this.getResource().getDictionary().get(ConfigurationConstants.PROPERTY_BUNDLE_LOCATION); + if ( location == null ) { + location = "?"; // default + } else if ( location.length() == 0 ) { + location = null; + } + + Configuration config = getConfiguration(); if (config == null) { created = true; - config = getConfiguration(ca, true); + + config = createConfiguration(location); } else { if (ConfigUtil.isSameData(config.getProperties(), getResource().getDictionary())) { this.getLogger().debug("Configuration {} already installed with same data, update request ignored: {}", config.getPid(), getResource()); config = null; + } else { + config.setBundleLocation(location); } } if (config != null) { - if (config.getBundleLocation() != null) { - config.setBundleLocation(null); - } config.update(getDictionary()); ctx.log("Installed configuration {} from resource {}", config.getPid(), getResource()); if ( this.factoryPid != null ) { diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigRemoveTask.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigRemoveTask.java index a7eb86c..9cdc7b5 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigRemoveTask.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigRemoveTask.java @@ -42,13 +42,12 @@ public class ConfigRemoveTask extends AbstractConfigTask { /** * @see org.apache.sling.installer.api.tasks.InstallTask#execute(org.apache.sling.installer.api.tasks.InstallationContext) */ + @Override @SuppressWarnings("unchecked") public void execute(final InstallationContext ctx) { synchronized ( ConfigTaskCreator.getLock() ) { - final ConfigurationAdmin ca = this.getConfigurationAdmin(); - try { - final Configuration cfg = getConfiguration(ca, false); + final Configuration cfg = getConfiguration(); if (cfg == null) { this.getLogger().debug("Cannot delete config , pid={} not found, ignored ({})", getCompositePid(), getResource()); } else { diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java index fe61cf8..f3fdd94 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigTaskCreator.java @@ -47,10 +47,10 @@ public class ConfigTaskCreator implements InstallTaskFactory, ConfigurationListener, ResourceTransformer { /** Configuration admin. */ - private ConfigurationAdmin configAdmin; + private final ConfigurationAdmin configAdmin; /** Resource change listener. */ - private ResourceChangeListener changeListener; + private final ResourceChangeListener changeListener; public ConfigTaskCreator(final ResourceChangeListener listener, final ConfigurationAdmin configAdmin) { this.changeListener = listener; @@ -107,20 +107,11 @@ public class ConfigTaskCreator try { final Configuration config = ConfigUtil.getConfiguration(configAdmin, event.getFactoryPid(), - event.getPid(), - false); + event.getPid()); if ( config != null ) { - final Dictionary<String, Object> dict = ConfigUtil.cleanConfiguration(config.getProperties()); - boolean persist = true; - final Object persistProp = dict.get(ConfigurationConstants.PROPERTY_PERSISTENCE); - if ( persistProp != null ) { - if (persistProp instanceof Boolean) { - persist = ((Boolean) persistProp).booleanValue(); - } else { - persist = Boolean.valueOf(String.valueOf(persistProp)); - } - } + final boolean persist = ConfigUtil.toBoolean(config.getProperties().get(ConfigurationConstants.PROPERTY_PERSISTENCE), true); if ( persist ) { + final Dictionary<String, Object> dict = ConfigUtil.cleanConfiguration(config.getProperties()); final Map<String, Object> attrs = new HashMap<String, Object>(); attrs.put(Constants.SERVICE_PID, event.getPid()); if ( event.getFactoryPid() == null ) { @@ -186,7 +177,7 @@ public class ConfigTaskCreator final String cString = pid.substring(n + 1); boolean useExtendedPid = false; try { - if ( ConfigUtil.getConfiguration(this.configAdmin, fString, fString + '.' + cString, false) != null ) { + if ( ConfigUtil.getConfiguration(this.configAdmin, fString, fString + '.' + cString) != null ) { useExtendedPid = true; } } catch ( final Exception ignore) { diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java index 43e4826..3982efe 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtil.java @@ -19,7 +19,6 @@ package org.apache.sling.installer.factories.configuration.impl; import java.io.IOException; -import java.util.Arrays; import java.util.Dictionary; import java.util.Enumeration; import java.util.HashSet; @@ -37,6 +36,11 @@ import org.osgi.service.cm.ConfigurationAdmin; abstract class ConfigUtil { /** + * This property marks the configuration as being deleted. + */ + public static final String PROPERTY_DELETE_MARKER = "org.apache.sling.installer.configuration.deleted"; + + /** * This property has been used in older versions to keep track where the * configuration has been installed from. */ @@ -76,29 +80,32 @@ abstract class ConfigUtil { final Set<String> keysA = collectKeys(a); final Set<String> keysB = collectKeys(b); if ( keysA.size() == keysB.size() && keysA.containsAll(keysB) ) { + result = true; for(final String key : keysA ) { final Object valA = a.get(key); final Object valB = b.get(key); if ( valA.getClass().isArray() ) { - if ( !Arrays.equals((Object[])valA, (Object[])valB) ) { - return result; + final Object[] arrA = (Object[])valA; + final Object[] arrB = (Object[])valB; + + if ( arrA.length != arrB.length ) { + result = false; + break; } - } else if ( valA instanceof Number ) { - // JCR only supports Long but not Integer - // therefore we have to add a special case here! - if ( ! (valB instanceof Number) ) { - return result; - } - if ( !(String.valueOf(valA).equals(String.valueOf(valB))) ) { - return result; + for(int i=0; i<arrA.length; i++) { + if ( !(String.valueOf(arrA[i]).equals(String.valueOf(arrB[i]))) ) { + result = false; + break; + } } } else { - if ( !a.get(key).equals(b.get(key)) ) { - return result; + // we always do a string comparison + if ( !(String.valueOf(valA).equals(String.valueOf(valB))) ) { + result = false; + break; } } } - result = true; } } return result; @@ -132,14 +139,30 @@ abstract class ConfigUtil { public static Configuration getConfiguration(final ConfigurationAdmin ca, final String factoryPid, + final String configPid) + throws IOException, InvalidSyntaxException { + return getOrCreateConfiguration(ca, factoryPid, configPid, null, false); + } + + public static Configuration createConfiguration(final ConfigurationAdmin ca, + final String factoryPid, + final String configPid, + final String location) + throws IOException, InvalidSyntaxException { + return getOrCreateConfiguration(ca, factoryPid, configPid, location, true); + } + + private static Configuration getOrCreateConfiguration(final ConfigurationAdmin ca, + final String factoryPid, final String configPid, + final String location, final boolean createIfNeeded) - throws IOException, InvalidSyntaxException { + throws IOException, InvalidSyntaxException { Configuration result = null; if (factoryPid == null) { if (createIfNeeded) { - result = ca.getConfiguration(configPid, null); + result = ca.getConfiguration(configPid, location); } else { String filter = "(" + Constants.SERVICE_PID + "=" + encode(configPid) + ")"; @@ -165,7 +188,7 @@ abstract class ConfigUtil { if (configs == null || configs.length == 0) { if (createIfNeeded) { - result = ca.createFactoryConfiguration(factoryPid, null); + result = ca.createFactoryConfiguration(factoryPid, location); } } else { result = configs[0]; @@ -177,4 +200,16 @@ abstract class ConfigUtil { return result; } + + public static boolean toBoolean(final Object obj, final boolean defaultValue) { + boolean result = defaultValue; + if ( obj != null ) { + if (obj instanceof Boolean) { + result = ((Boolean) obj).booleanValue(); + } else { + result = Boolean.valueOf(String.valueOf(obj)); + } + } + return result; + } } diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java b/src/main/java/org/apache/sling/installer/factories/configuration/package-info.java similarity index 65% copy from src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java copy to src/main/java/org/apache/sling/installer/factories/configuration/package-info.java index 93afb48..2068cb5 100644 --- a/src/main/java/org/apache/sling/installer/factories/configuration/ConfigurationConstants.java +++ b/src/main/java/org/apache/sling/installer/factories/configuration/package-info.java @@ -16,16 +16,9 @@ * specific language governing permissions and limitations * under the License. */ + +@Version("1.1.0") package org.apache.sling.installer.factories.configuration; -public abstract class ConfigurationConstants { +import aQute.bnd.annotation.Version; - /** - * This property defines if a configuration should be persisted by the - * installer. This property is a boolean value defaulting to true. - * - * The property should be used, if a configuration should not be persisted - * by clients creating the configuration. - */ - public static final String PROPERTY_PERSISTENCE = "org.apache.sling.installer.configuration.persist"; -} diff --git a/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java b/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java new file mode 100644 index 0000000..ee0b2b1 --- /dev/null +++ b/src/test/java/org/apache/sling/installer/factories/configuration/impl/ConfigUtilTest.java @@ -0,0 +1,91 @@ +/* + * 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.sling.installer.factories.configuration.impl; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Dictionary; +import java.util.Enumeration; +import java.util.Hashtable; + +import org.junit.Test; + +public class ConfigUtilTest { + + @Test public void testIsSameDataEmptyAndNullDictionaries() throws Exception { + final Dictionary<String, Object> a = new Hashtable<String, Object>(); + final Dictionary<String, Object> b = new Hashtable<String, Object>(); + + assertTrue(ConfigUtil.isSameData(a, b)); + assertTrue(ConfigUtil.isSameData(b, a)); + assertFalse(ConfigUtil.isSameData(null, a)); + assertFalse(ConfigUtil.isSameData(null, null)); + assertFalse(ConfigUtil.isSameData(b, null)); + } + + @Test public void testIsSameDataSameDictionaries() throws Exception { + final Dictionary<String, Object> a = new Hashtable<String, Object>(); + final Dictionary<String, Object> b = new Hashtable<String, Object>(); + + a.put("a", "value"); + a.put("b", 1); + a.put("c", 2L); + a.put("d", true); + a.put("e", 1.1); + + final Enumeration<String> e = a.keys(); + while ( e.hasMoreElements() ) { + final String name = e.nextElement(); + b.put(name, a.get(name)); + } + + assertTrue(ConfigUtil.isSameData(a, b)); + assertTrue(ConfigUtil.isSameData(b, a)); + + final Enumeration<String> e1 = a.keys(); + while ( e1.hasMoreElements() ) { + final String name = e1.nextElement(); + b.put(name, a.get(name).toString()); + } + + assertTrue(ConfigUtil.isSameData(a, b)); + assertTrue(ConfigUtil.isSameData(b, a)); + } + + @Test public void testIsSameDataArrays() throws Exception { + final Dictionary<String, Object> a = new Hashtable<String, Object>(); + final Dictionary<String, Object> b = new Hashtable<String, Object>(); + + a.put("a", new String[] {"1", "2", "3"}); + b.put("a", a.get("a")); + + a.put("b", new Integer[] {1,2,3}); + b.put("b", a.get("b")); + + a.put("c", new Long[] {1L,2L,3L}); + b.put("c", a.get("c")); + + a.put("d", new Integer[] {1,2,3}); + b.put("d", new String[] {"1", "2", "3"}); + + assertTrue(ConfigUtil.isSameData(a, b)); + assertTrue(ConfigUtil.isSameData(b, a)); + } +} -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
