This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git
The following commit(s) were added to refs/heads/master by this push:
new 3a715de8f1 FELIX-6470 : ConfigAdmin/Coordinator handling not optimal
3a715de8f1 is described below
commit 3a715de8f163cd967e412db857ac2236cd891fd2
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Tue Oct 25 16:48:14 2022 +0200
FELIX-6470 : ConfigAdmin/Coordinator handling not optimal
---
.../apache/felix/cm/impl/ConfigurationManager.java | 24 +--
.../org/apache/felix/cm/impl/CoordinatorUtil.java | 113 ++++++++++----
.../cm/integration/CoordinatorIntegrationTest.java | 171 +++++++++++++++++++++
3 files changed, 270 insertions(+), 38 deletions(-)
diff --git
a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
index 44d930a7a1..a2068de0e1 100644
---
a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
+++
b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
@@ -1057,9 +1057,9 @@ public class ConfigurationManager implements
BundleListener
* ManagedService is registered with multiple PIDs an instance of this
* class is used for each registered PID.
*/
- private class ManagedServiceUpdate implements Runnable
+ public class ManagedServiceUpdate implements Runnable
{
- private final String[] pids;
+ public final List<String> pids = new ArrayList<>();
private final ServiceReference<ManagedService> sr;
@@ -1068,7 +1068,9 @@ public class ConfigurationManager implements
BundleListener
ManagedServiceUpdate( String[] pids, ServiceReference<ManagedService>
sr, ConfigurationMap<?> configs )
{
- this.pids = pids;
+ for(final String pid : pids) {
+ this.pids.add(pid);
+ }
this.sr = sr;
this.configs = configs;
}
@@ -1124,7 +1126,7 @@ public class ConfigurationManager implements
BundleListener
}
Log.logger.log( LogService.LOG_DEBUG, "Updating service {0} with
configuration {1}@{2}", new Object[]
- { servicePid, configPid, new Long( revision ) } );
+ { servicePid, configPid, revision } );
managedServiceTracker.provideConfiguration( sr, configPid, null,
properties, revision, this.configs );
}
@@ -1132,7 +1134,7 @@ public class ConfigurationManager implements
BundleListener
@Override
public String toString()
{
- return "ManagedService Update: pid=" + Arrays.asList( pids );
+ return "ManagedService Update: pid=" + pids;
}
}
@@ -1143,7 +1145,7 @@ public class ConfigurationManager implements
BundleListener
* multiple PIDs an instance of this class is used for each registered
* PID.
*/
- private class ManagedServiceFactoryUpdate implements Runnable
+ public class ManagedServiceFactoryUpdate implements Runnable
{
private final String[] factoryPids;
@@ -1274,7 +1276,7 @@ public class ConfigurationManager implements
BundleListener
}
}
- private abstract class ConfigurationProvider<T> implements Runnable
+ public abstract class ConfigurationProvider<T> implements Runnable
{
protected final ConfigurationImpl config;
@@ -1368,7 +1370,7 @@ public class ConfigurationManager implements
BundleListener
* they are subscribed to. This may cause the configuration to be
* supplied to multiple services.
*/
- private class UpdateConfiguration extends ConfigurationProvider
+ public class UpdateConfiguration extends ConfigurationProvider
{
UpdateConfiguration( final ConfigurationImpl config )
@@ -1381,7 +1383,7 @@ public class ConfigurationManager implements
BundleListener
public void run()
{
Log.logger.log( LogService.LOG_DEBUG, "Updating configuration {0}
to revision #{1}", new Object[]
- { config.getPid(), new Long( revision ) } );
+ { config.getPid(), revision } );
final List<ServiceReference<?>> srList =
this.getHelper().getServices( getTargetedServicePid() );
if ( !srList.isEmpty() )
@@ -1449,7 +1451,7 @@ public class ConfigurationManager implements
BundleListener
* <code>ManagedService[Factory]</code> services of a configuration
* being deleted.
*/
- private class DeleteConfiguration extends ConfigurationProvider
+ public class DeleteConfiguration extends ConfigurationProvider
{
private final String configLocation;
@@ -1511,7 +1513,7 @@ public class ConfigurationManager implements
BundleListener
}
}
- private class LocationChanged extends ConfigurationProvider
+ public class LocationChanged extends ConfigurationProvider
{
private final String oldLocation;
diff --git
a/configadmin/src/main/java/org/apache/felix/cm/impl/CoordinatorUtil.java
b/configadmin/src/main/java/org/apache/felix/cm/impl/CoordinatorUtil.java
index a79b6ffe4c..455ef83f7b 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/CoordinatorUtil.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/CoordinatorUtil.java
@@ -19,7 +19,10 @@
package org.apache.felix.cm.impl;
import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import org.osgi.service.coordinator.Coordination;
import org.osgi.service.coordinator.Coordinator;
@@ -28,64 +31,120 @@ import org.osgi.service.coordinator.Participant;
/**
* Utility class for coordinations
*/
-public class CoordinatorUtil
-{
+public class CoordinatorUtil {
+
+ public static final class Notifier implements Participant {
- public static final class Notifier implements Participant
- {
private final List<Runnable> runnables = new ArrayList<Runnable>();
private final UpdateThread thread;
- public Notifier(final UpdateThread t)
- {
+ public Notifier(final UpdateThread t) {
this.thread = t;
}
- private void execute()
- {
- for(final Runnable r : runnables)
- {
+ /**
+ * Compact the notification if there is more than one operation for a
given pid
+ * @return The compacted list
+ */
+ private List<Runnable> compact() {
+ final List<Runnable> result = new ArrayList<>(this.runnables);
+ this.runnables.clear();
+
+ final Map<String, ConfigurationManager.ConfigurationProvider>
lastOperations = new HashMap<>();
+ final Map<String, Boolean> firstOperationIsCreate = new
HashMap<>();
+
+ for(int i=0;i<result.size();i++) {
+ final Runnable current = result.get(i);
+
+ if ( current instanceof
ConfigurationManager.UpdateConfiguration ) {
+ final ConfigurationManager.UpdateConfiguration up =
(ConfigurationManager.UpdateConfiguration)current;
+ final String pid =
up.getTargetedServicePid().getServicePid();
+ if ( firstOperationIsCreate.get(pid) == null ) {
+ firstOperationIsCreate.put(pid, up.revision == 1);
+ }
+ final ConfigurationManager.ConfigurationProvider last =
lastOperations.get(pid);
+ lastOperations.put(pid, up);
+ if ( last != null ) {
+ result.remove(last);
+ i--;
+ }
+ } else if ( current instanceof
ConfigurationManager.DeleteConfiguration ) {
+ final ConfigurationManager.DeleteConfiguration up =
(ConfigurationManager.DeleteConfiguration)current;
+ final String pid =
up.getTargetedServicePid().getServicePid();
+ if ( firstOperationIsCreate.get(pid) == null ) {
+ firstOperationIsCreate.put(pid, false);
+ }
+ final ConfigurationManager.ConfigurationProvider last =
lastOperations.get(pid);
+ lastOperations.put(pid, up);
+ if ( last != null ) {
+ if ( !firstOperationIsCreate.get(pid) ) {
+ firstOperationIsCreate.remove(pid);
+ result.remove(i);
+ i--;
+ lastOperations.remove(pid);
+ }
+ result.remove(last);
+ i--;
+ }
+ }
+ }
+
+ final Iterator<Runnable> iter = result.iterator();
+ while ( iter.hasNext() ) {
+ final Runnable current = iter.next();
+ if ( current instanceof
ConfigurationManager.ManagedServiceUpdate ) {
+ final ConfigurationManager.ManagedServiceUpdate up =
(ConfigurationManager.ManagedServiceUpdate)current;
+ final Iterator<String> pidIter = up.pids.iterator();
+ while ( pidIter.hasNext() ) {
+ final String pid = pidIter.next();
+ final ConfigurationManager.ConfigurationProvider last
= lastOperations.get(pid);
+ if ( last != null ) {
+ pidIter.remove();
+ }
+ }
+ if ( up.pids.isEmpty() ) {
+ iter.remove();
+ }
+ }
+ }
+
+ return result;
+ }
+
+ private void execute() {
+ for(final Runnable r : compact()) {
this.thread.schedule(r);
}
- runnables.clear();
}
@Override
- public void ended(Coordination coordination) throws Exception
- {
+ public void ended(Coordination coordination) throws Exception {
execute();
}
@Override
- public void failed(Coordination coordination) throws Exception
- {
+ public void failed(Coordination coordination) throws Exception {
execute();
}
- public void add(final Runnable t)
- {
+ public void add(final Runnable t) {
runnables.add(t);
}
}
- public static boolean addToCoordination(final Object srv, final
UpdateThread thread, final Runnable task)
- {
+ public static boolean addToCoordination(final Object srv, final
UpdateThread thread, final Runnable task) {
final Coordinator coordinator = (Coordinator) srv;
Coordination c = coordinator.peek();
- if ( c != null && !c.isTerminated() )
- {
+ if ( c != null && !c.isTerminated() ) {
Notifier n = null;
- for(final Participant p : c.getParticipants())
- {
- if ( p instanceof Notifier )
- {
+ for(final Participant p : c.getParticipants()) {
+ if ( p instanceof Notifier ) {
n = (Notifier) p;
break;
}
}
- if ( n == null )
- {
+ if ( n == null ) {
n = new Notifier(thread);
c.addParticipant(n);
}
diff --git
a/configadmin/src/test/java/org/apache/felix/cm/integration/CoordinatorIntegrationTest.java
b/configadmin/src/test/java/org/apache/felix/cm/integration/CoordinatorIntegrationTest.java
new file mode 100644
index 0000000000..c3422d6e52
--- /dev/null
+++
b/configadmin/src/test/java/org/apache/felix/cm/integration/CoordinatorIntegrationTest.java
@@ -0,0 +1,171 @@
+/*
+ * 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.
+ */
+/*
+ * Note that this test is copied from the OSGi TCK https://github.com/osgi/osgi
+ * with permission of the author Stefan Bischof.
+ */
+package org.apache.felix.cm.integration;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
+
+import java.util.ArrayList;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.Option;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.cm.Configuration;
+import org.osgi.service.cm.ConfigurationAdmin;
+import org.osgi.service.cm.ConfigurationException;
+import org.osgi.service.cm.ManagedService;
+import org.osgi.service.coordinator.Coordination;
+import org.osgi.service.coordinator.Coordinator;
+
+@RunWith(PaxExam.class)
+public class CoordinatorIntegrationTest extends ConfigurationTestBase {
+
+ @Override
+ protected Option[] additionalConfiguration() {
+ return new Option [] {
+ mavenBundle("org.apache.felix",
"org.apache.felix.coordinator", "1.0.2")
+ };
+ }
+
+ <T> T getService(Class<T> cls) {
+ ServiceReference<T> sref =
bundleContext.getServiceReference(cls);
+ if (sref == null)
+ return null;
+
+ return bundleContext.getService(sref);
+ }
+
+ void sleep() throws InterruptedException {
+ TimeUnit.MILLISECONDS.sleep(4000);
+ }
+
+ @Test
+ public void
test_deliver_existing_Configuration_to_later_registered_ManagedService()
+ throws Exception {
+ ConfigurationAdmin cm = getService(ConfigurationAdmin.class);
+ Coordinator c = getService(Coordinator.class);
+ Coordination coord = c.begin("cm-test1", 0);
+ final List<String> events = new ArrayList<>();
+
+ String pid = getClass().getName() + ".mstestpid2";
+
+ try {
+ // create the configuration
+ Dictionary<String,Object> props = new Hashtable<>();
+ props.put("key", "value");
+
+ Configuration conf = cm.getConfiguration(pid);
+ conf.update(props);
+
+ // add managed service
+ Dictionary<String,Object> msProps = new Hashtable<>();
+ msProps.put(Constants.SERVICE_PID, pid);
+
+
bundleContext.registerService(ManagedService.class.getName(),
+ new ManagedService() {
+
+ @Override
+ public void
updated(Dictionary<String, ? > properties)
+ throws
ConfigurationException {
+
events.add((String)properties.get("key2"));
+ }
+
+ }, msProps);
+
+ // update configuration
+ props.put("key2", "value2");
+ conf.update(props);
+
+ assertEquals(0, events.size());
+ } finally {
+ coord.end();
+ }
+
+ // wait and verify listener
+ sleep();
+
+ assertEquals(1, events.size());
+ // last update
+ assertEquals("value2", events.get(0));
+ }
+
+ @Test
+ public void test_create_managedservice_delete()
+ throws Exception {
+ ConfigurationAdmin cm = getService(ConfigurationAdmin.class);
+ Coordinator c = getService(Coordinator.class);
+ Coordination coord = c.begin("cm-test1", 0);
+ final List<Boolean> events = new ArrayList<>();
+
+ String pid = getClass().getName() + ".mstestpid3";
+
+ try {
+ // create the configuration
+ Dictionary<String,Object> props = new Hashtable<>();
+ props.put("key", "value");
+
+ Configuration conf = cm.getConfiguration(pid);
+ conf.update(props);
+
+ // add managed service
+ Dictionary<String,Object> msProps = new Hashtable<>();
+ msProps.put(Constants.SERVICE_PID, pid);
+
+
bundleContext.registerService(ManagedService.class.getName(),
+ new ManagedService() {
+
+ @Override
+ public void
updated(Dictionary<String, ? > properties)
+ throws
ConfigurationException {
+ events.add(properties
!= null);
+ }
+
+ }, msProps);
+
+ // update configuration
+ props.put("key2", "value2");
+ conf.update(props);
+
+ // delete configuration
+ conf.delete();
+
+ assertEquals(0, events.size());
+ } finally {
+ coord.end();
+ }
+
+ // wait and verify listener
+ sleep();
+
+ assertEquals(1, events.size());
+ assertFalse(events.get(0));// no configuration, update with null
+ }
+}
\ No newline at end of file
