You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2017/11/07 09:43:44 UTC
[sling-org-apache-sling-installer-factory-configuration] 04/10:
SLING-4250 : Configuration values should be compared by string comparison
SLING-4263 : Bundle location should be set to ? by default
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 <cz...@apache.org>
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
"commits@sling.apache.org" <co...@sling.apache.org>.