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 2092bf45f62b1c7b3e0937ec46ab2fb19d215f65 Author: Carsten Ziegeler <[email protected]> AuthorDate: Fri Jan 2 09:32:41 2015 +0000 SLING-4272 : Issues in handling of configurations wrt update handling and write back git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/installer/factories/configuration@1648985 13f79535-47bb-0310-9956-ffa450edef68 --- .../configuration/impl/ConfigInstallTask.java | 5 +- .../configuration/impl/ConfigRemoveTask.java | 8 +- .../configuration/impl/ConfigTaskCreator.java | 25 ++-- .../factories/configuration/impl/Coordinator.java | 131 +++++++++++++++++++++ 4 files changed, 157 insertions(+), 12 deletions(-) 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.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.apache.sling.installer.factories.configuration.impl.Coordinator.Operation; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; @@ -44,7 +45,7 @@ public class ConfigInstallTask extends AbstractConfigTask { @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 @@ public class ConfigInstallTask extends AbstractConfigTask { 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 @@ public class ConfigRemoveTask extends AbstractConfigTask { @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 @@ public class ConfigRemoveTask extends AbstractConfigTask { 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.Configuration; 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 @@ import org.osgi.service.cm.ConfigurationListener; 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 @@ public class ConfigTaskCreator */ @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 @@ public class ConfigTaskCreator 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 @@ public class ConfigTaskCreator } 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 @@ public class ConfigTaskCreator } 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; + } + } + } +} -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
