SLING-4272 : Issues in handling of configurations wrt update handling and write back
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1648985 13f79535-47bb-0310-9956-ffa450edef68
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 a5f1e11..2b27df6 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
@@ -22,6 +22,7 @@
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.apache.sling.installer.factories.configuration.impl.Coordinator.Operation;
import org.osgi.service.cm.Configuration;
import org.osgi.service.cm.ConfigurationAdmin;
@@ -44,7 +45,7 @@
@SuppressWarnings("unchecked")
@Override
public void execute(final InstallationContext ctx) {
- synchronized ( ConfigTaskCreator.getLock() ) {
+ synchronized ( Coordinator.SHARED ) {
// Get or create configuration, but do not
// update if the new one has the same values.
boolean created = false;
@@ -81,6 +82,8 @@
this.getLogger().debug("Configuration " + config.getPid()
+ " " + (created ? "created" : "updated")
+ " from " + getResource());
+ final Operation op = new Coordinator.Operation(config.getPid(), config.getFactoryPid(), false);
+ Coordinator.SHARED.add(op);
} else {
this.setFinishedState(ResourceState.IGNORED, this.getCompositeAliasPid());
}
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 9cdc7b5..e0bf6ea 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
@@ -45,7 +45,7 @@
@Override
@SuppressWarnings("unchecked")
public void execute(final InstallationContext ctx) {
- synchronized ( ConfigTaskCreator.getLock() ) {
+ synchronized ( Coordinator.SHARED ) {
try {
final Configuration cfg = getConfiguration();
if (cfg == null) {
@@ -54,12 +54,16 @@
if ( !ConfigUtil.isSameData(cfg.getProperties(), this.getResource().getDictionary()) ) {
this.getLogger().debug("Configuration has changed after it has been installed!");
} else {
+ final Coordinator.Operation op = new Coordinator.Operation(cfg.getPid(), cfg.getFactoryPid(), true);
+
this.getLogger().debug("Deleting config {} ({})", getCompositePid(), getResource());
cfg.delete();
ctx.log("Deleted configuration {} from resource {}", getCompositePid(), getResource());
+
+ Coordinator.SHARED.add(op);
}
}
- } catch (Exception e) {
+ } catch (final Exception e) {
this.getLogger().debug("Exception during removal of config " + this.getResource() + " : " + e.getMessage() + ". Retrying later.", e);
}
// we always set to uninstalled as the resource really has been deleted
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 bb01629..23a59e0 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
@@ -39,6 +39,8 @@
import org.osgi.service.cm.ConfigurationAdmin;
import org.osgi.service.cm.ConfigurationEvent;
import org.osgi.service.cm.ConfigurationListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Task creator for configurations.
@@ -46,6 +48,9 @@
public class ConfigTaskCreator
implements InstallTaskFactory, ConfigurationListener, ResourceTransformer {
+ /** Logger. */
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
/** Configuration admin. */
private final ConfigurationAdmin configAdmin;
@@ -91,7 +96,7 @@
*/
@SuppressWarnings("unchecked")
public void configurationEvent(final ConfigurationEvent event) {
- synchronized ( ConfigTaskCreator.getLock() ) {
+ synchronized ( Coordinator.SHARED ) {
final String id;
final String pid;
if (event.getFactoryPid() == null ) {
@@ -103,13 +108,19 @@
id = event.getFactoryPid() + '.' + event.getPid();
}
if ( event.getType() == ConfigurationEvent.CM_DELETED ) {
- this.changeListener.resourceRemoved(InstallableResource.TYPE_CONFIG, id);
+ final Coordinator.Operation op = Coordinator.SHARED.get(event.getPid(), event.getFactoryPid(), true);
+ if ( op == null ) {
+ this.changeListener.resourceRemoved(InstallableResource.TYPE_CONFIG, id);
+ } else {
+ this.logger.debug("Ignoring configuration event for {}:{}", event.getPid(), event.getFactoryPid());
+ }
} else if ( event.getType() == ConfigurationEvent.CM_UPDATED ) {
try {
final Configuration config = ConfigUtil.getConfiguration(configAdmin,
event.getFactoryPid(),
event.getPid());
- if ( config != null ) {
+ final Coordinator.Operation op = Coordinator.SHARED.get(event.getPid(), event.getFactoryPid(), false);
+ if ( config != null && op == null ) {
final boolean persist = ConfigUtil.toBoolean(config.getProperties().get(ConfigurationConstants.PROPERTY_PERSISTENCE), true);
if ( persist ) {
final Dictionary<String, Object> dict = ConfigUtil.cleanConfiguration(config.getProperties());
@@ -129,6 +140,8 @@
}
this.changeListener.resourceAddedOrUpdated(InstallableResource.TYPE_CONFIG, id, null, dict, attrs);
}
+ } else {
+ this.logger.debug("Ignoring configuration event for {}:{}", event.getPid(), event.getFactoryPid());
}
} catch ( final Exception ignore) {
// ignore for now
@@ -242,10 +255,4 @@
}
return path.replace('\\', '/');
}
-
- private static final Object LOCK = new Object();
-
- public static Object getLock() {
- return LOCK;
- }
}
diff --git a/src/main/java/org/apache/sling/installer/factories/configuration/impl/Coordinator.java b/src/main/java/org/apache/sling/installer/factories/configuration/impl/Coordinator.java
new file mode 100644
index 0000000..c2c4b17
--- /dev/null
+++ b/src/main/java/org/apache/sling/installer/factories/configuration/impl/Coordinator.java
@@ -0,0 +1,131 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Coordinator service.
+ *
+ * All operations should be synced on the {@link #SHARED} instance.
+ */
+public class Coordinator {
+
+ /**
+ * Shared instance for syncing and keeping track of operations.
+ */
+ public static final Coordinator SHARED = new Coordinator();
+
+ /**
+ * Entries expire after this number of milliseconds (defaults to 5 secs)
+ */
+ private static final long EXPIRY = 5000;
+
+ /**
+ * An operation
+ */
+ public static final class Operation {
+ public final String pid;
+ public final String factoryPid;
+ public final boolean isDelete;
+ public final long created;
+
+ public Operation(final String pid, final String factoryPid, final boolean isDelete) {
+ created = System.currentTimeMillis();
+ this.pid = pid;
+ this.factoryPid = factoryPid;
+ this.isDelete = isDelete;
+ }
+
+ @Override
+ public String toString() {
+ return "Operation [pid=" + pid + ", factoryPid=" + factoryPid
+ + ", isDelete=" + isDelete + ", created=" + created + "]";
+ }
+ }
+
+ /**
+ * Logger
+ */
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+ /**
+ * The list of operations.
+ */
+ private final List<Operation> operations = new ArrayList<Coordinator.Operation>();
+
+ /**
+ * Private constructor
+ */
+ private Coordinator() {
+ // private constructor
+ }
+
+ public void add(final Operation op) {
+ this.cleanup();
+ this.operations.add(op);
+ logger.debug("Adding {}", op);
+ }
+
+ public Operation get(final String pid, final String factoryPid, final boolean isDelete) {
+ this.cleanup();
+ logger.debug("Searching {} : {} - {}", new Object[] {pid, factoryPid, isDelete});
+ Operation result = null;
+ final Iterator<Operation> i = this.operations.iterator();
+ while ( i.hasNext() ) {
+ final Operation op = i.next();
+ if ( op.isDelete == isDelete ) {
+ if ( op.pid.equals(pid) ) {
+ if ( (op.factoryPid == null && factoryPid == null)
+ || (op.factoryPid != null && op.factoryPid.equals(factoryPid)) ) {
+ result = op;
+ i.remove();
+ break;
+ }
+ }
+ }
+ }
+ logger.debug("Result ({} : {} - {}) : {}", new Object[] {pid, factoryPid, isDelete, result});
+ return result;
+ }
+
+ /**
+ * Clean up the list of operations.
+ * Remove all entries which are older then the {@link #EXPIRY}
+ */
+ private void cleanup() {
+ final long time = System.currentTimeMillis() - EXPIRY;
+ final Iterator<Operation> i = this.operations.iterator();
+ while ( i.hasNext() ) {
+ final Operation op = i.next();
+ if ( op.created <= time ) {
+ logger.debug("Deleting expired {}", op);
+ i.remove();
+ } else {
+ break;
+ }
+ }
+ }
+}
