You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by cs...@apache.org on 2018/02/05 09:45:05 UTC
[3/3] aries-rsa git commit: [ARIES-1763] Correctly close exports when
service goes away
[ARIES-1763] Correctly close exports when service goes away
Project: http://git-wip-us.apache.org/repos/asf/aries-rsa/repo
Commit: http://git-wip-us.apache.org/repos/asf/aries-rsa/commit/55e2c1cb
Tree: http://git-wip-us.apache.org/repos/asf/aries-rsa/tree/55e2c1cb
Diff: http://git-wip-us.apache.org/repos/asf/aries-rsa/diff/55e2c1cb
Branch: refs/heads/master
Commit: 55e2c1cb4767d5155a171e349514c91a7ac466f3
Parents: a5c8d7d
Author: Christian Schneider <cs...@adobe.com>
Authored: Mon Feb 5 10:44:54 2018 +0100
Committer: Christian Schneider <cs...@adobe.com>
Committed: Mon Feb 5 10:44:54 2018 +0100
----------------------------------------------------------------------
.../aries/rsa/core/ExportRegistrationImpl.java | 18 ++++++-
.../aries/rsa/core/RemoteServiceAdminCore.java | 23 +++++++--
.../core/DistributionProviderTrackerTest.java | 34 +++++++-----
.../rsa/core/RemoteServiceAdminCoreTest.java | 54 ++++++++------------
.../exporter/TopologyManagerExport.java | 24 ++++-----
5 files changed, 91 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aries-rsa/blob/55e2c1cb/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java
----------------------------------------------------------------------
diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java b/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java
index 81bb859..a02ff3a 100644
--- a/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java
+++ b/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java
@@ -21,6 +21,7 @@ package org.apache.aries.rsa.core;
import java.io.Closeable;
import java.io.IOException;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.Map;
import java.util.Set;
@@ -30,6 +31,7 @@ import org.osgi.framework.ServiceReference;
import org.osgi.service.remoteserviceadmin.EndpointDescription;
import org.osgi.service.remoteserviceadmin.ExportReference;
import org.osgi.service.remoteserviceadmin.ExportRegistration;
+import org.osgi.service.remoteserviceadmin.RemoteConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -110,6 +112,7 @@ public class ExportRegistrationImpl implements ExportRegistration {
}
public final void close() {
+ closeHandler.onClose(this);
synchronized (this) {
if (closed) {
return;
@@ -117,7 +120,6 @@ public class ExportRegistrationImpl implements ExportRegistration {
closed = true;
}
- closeHandler.onClose(this);
if (exportReference != null) {
exportReference.close();
}
@@ -181,9 +183,21 @@ public class ExportRegistrationImpl implements ExportRegistration {
return null;
}
ServiceReference<?> sref = getExportReference().getExportedService();
- EndpointDescription epd = new EndpointDescription(sref, properties);
+
+ HashMap<String, Object> props = new HashMap<>(properties);
+ EndpointDescription oldEpd = getExportReference().getExportedEndpoint();
+ copyIfNull(props, oldEpd, RemoteConstants.ENDPOINT_ID);
+ copyIfNull(props, oldEpd, RemoteConstants.SERVICE_IMPORTED_CONFIGS);
+
+ EndpointDescription epd = new EndpointDescription(sref, props);
exportReference = new ExportReferenceImpl(sref, epd);
this.sender.notifyUpdate(this.getExportReference());
return epd;
}
+
+ private void copyIfNull(HashMap<String, Object> props, EndpointDescription oldEpd, String key) {
+ if (props.get(key) == null) {
+ props.put(key, oldEpd.getProperties().get(key));
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/aries-rsa/blob/55e2c1cb/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java
----------------------------------------------------------------------
diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java
index 98fca14..f8df7c8 100644
--- a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java
+++ b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java
@@ -39,6 +39,7 @@ import org.apache.aries.rsa.util.EndpointHelper;
import org.apache.aries.rsa.util.StringPlus;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceEvent;
import org.osgi.framework.ServiceListener;
import org.osgi.framework.ServiceReference;
import org.osgi.framework.ServiceRegistration;
@@ -84,13 +85,26 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin {
this.packageUtil = packageUtil;
this.closeHandler = new CloseHandler() {
public void onClose(ExportRegistration exportReg) {
- removeExportRegistration((ExportRegistrationImpl) exportReg);
+ removeExportRegistration(exportReg);
}
public void onClose(ImportRegistration importReg) {
removeImportRegistration((ImportRegistrationImpl) importReg);
}
};
+ createServiceListener();
+ }
+
+ // listen for exported services being unregistered so we can close the export
+ protected void createServiceListener() {
+ this.exportedServiceListener = new ServiceListener() {
+ public void serviceChanged(ServiceEvent event) {
+ if (event.getType() == ServiceEvent.UNREGISTERING) {
+ removeServiceExports(event.getServiceReference());
+ }
+ }
+ };
+ this.bctx.addServiceListener(exportedServiceListener);
}
@Override
@@ -216,6 +230,9 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin {
return null;
}
return new ExportRegistrationImpl(serviceReference, endpoint, closeHandler, eventProducer);
+ } catch (IllegalArgumentException e) {
+ // TCK expects this for garbage input
+ throw e;
} catch (Exception e) {
return new ExportRegistrationImpl(e, closeHandler, eventProducer);
}
@@ -476,14 +493,14 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin {
*
* @param eri the export registration to remove
*/
- protected void removeExportRegistration(ExportRegistrationImpl eri) {
+ protected void removeExportRegistration(ExportRegistration eri) {
synchronized (exportedServices) {
for (Iterator<Collection<ExportRegistration>> it = exportedServices.values().iterator(); it.hasNext();) {
Collection<ExportRegistration> value = it.next();
for (Iterator<ExportRegistration> it2 = value.iterator(); it2.hasNext();) {
ExportRegistration er = it2.next();
if (er.equals(eri)) {
- eventProducer.notifyRemoval(eri.getExportReferenceAlways());
+ eventProducer.notifyRemoval(eri.getExportReference());
it2.remove();
if (value.isEmpty()) {
it.remove();
http://git-wip-us.apache.org/repos/asf/aries-rsa/blob/55e2c1cb/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java
----------------------------------------------------------------------
diff --git a/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java b/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java
index 1468b8b..a18dae5 100644
--- a/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java
+++ b/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java
@@ -18,6 +18,11 @@
*/
package org.apache.aries.rsa.core;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.anyString;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+
import java.util.Dictionary;
import org.apache.aries.rsa.spi.DistributionProvider;
@@ -29,6 +34,7 @@ import org.osgi.framework.Filter;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServiceFactory;
+import org.osgi.framework.ServiceListener;
import org.osgi.framework.ServiceReference;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.remoteserviceadmin.RemoteConstants;
@@ -45,19 +51,21 @@ public class DistributionProviderTrackerTest {
DistributionProvider provider = c.createMock(DistributionProvider.class);
ServiceReference<DistributionProvider> providerRef = c.createMock(ServiceReference.class);
- EasyMock.expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn("");
- EasyMock.expect(providerRef.getProperty(RemoteConstants.REMOTE_CONFIGS_SUPPORTED)).andReturn("");
+ expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn("");
+ expect(providerRef.getProperty(RemoteConstants.REMOTE_CONFIGS_SUPPORTED)).andReturn("");
BundleContext context = c.createMock(BundleContext.class);
String filterSt = String.format("(objectClass=%s)", DistributionProvider.class.getName());
Filter filter = FrameworkUtil.createFilter(filterSt);
- EasyMock.expect(context.createFilter(filterSt)).andReturn(filter);
- EasyMock.expect(context.getService(providerRef)).andReturn(provider);
+ expect(context.createFilter(filterSt)).andReturn(filter);
+ expect(context.getService(providerRef)).andReturn(provider);
ServiceRegistration rsaReg = c.createMock(ServiceRegistration.class);
- EasyMock.expect(context.registerService(EasyMock.isA(String.class), EasyMock.isA(ServiceFactory.class),
+ expect(context.registerService(EasyMock.isA(String.class), EasyMock.isA(ServiceFactory.class),
EasyMock.isA(Dictionary.class)))
.andReturn(rsaReg).atLeastOnce();
-
+ context.addServiceListener(anyObject(ServiceListener.class));
+ expectLastCall().anyTimes();
+
final BundleContext apiContext = c.createMock(BundleContext.class);
c.replay();
DistributionProviderTracker tracker = new DistributionProviderTracker(context) {
@@ -83,19 +91,21 @@ public class DistributionProviderTrackerTest {
DistributionProvider provider = c.createMock(DistributionProvider.class);
ServiceReference<DistributionProvider> providerRef = c.createMock(ServiceReference.class);
- EasyMock.expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn(null);
- EasyMock.expect(providerRef.getProperty(RemoteConstants.REMOTE_CONFIGS_SUPPORTED)).andReturn(null);
+ expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn(null);
+ expect(providerRef.getProperty(RemoteConstants.REMOTE_CONFIGS_SUPPORTED)).andReturn(null);
BundleContext context = c.createMock(BundleContext.class);
String filterSt = String.format("(objectClass=%s)", DistributionProvider.class.getName());
Filter filter = FrameworkUtil.createFilter(filterSt);
- EasyMock.expect(context.createFilter(filterSt)).andReturn(filter);
- EasyMock.expect(context.getService(providerRef)).andReturn(provider);
+ expect(context.createFilter(filterSt)).andReturn(filter);
+ expect(context.getService(providerRef)).andReturn(provider);
ServiceRegistration rsaReg = c.createMock(ServiceRegistration.class);
- EasyMock.expect(context.registerService(EasyMock.isA(String.class), EasyMock.isA(ServiceFactory.class),
+ expect(context.registerService(EasyMock.isA(String.class), EasyMock.isA(ServiceFactory.class),
EasyMock.isA(Dictionary.class)))
.andReturn(rsaReg).atLeastOnce();
-
+ context.addServiceListener(anyObject(ServiceListener.class));
+ expectLastCall().anyTimes();
+
final BundleContext apiContext = c.createMock(BundleContext.class);
c.replay();
DistributionProviderTracker tracker = new DistributionProviderTracker(context) {
http://git-wip-us.apache.org/repos/asf/aries-rsa/blob/55e2c1cb/rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java
----------------------------------------------------------------------
diff --git a/rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java b/rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java
index 09dffc2..6200d45 100644
--- a/rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java
+++ b/rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java
@@ -20,7 +20,10 @@ package org.apache.aries.rsa.core;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.hamcrest.Matchers.array;
import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -28,7 +31,7 @@ import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
-import java.lang.reflect.Field;
+import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Dictionary;
@@ -50,6 +53,7 @@ import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.InvalidSyntaxException;
+import org.osgi.framework.ServiceListener;
import org.osgi.framework.ServiceReference;
import org.osgi.framework.Version;
import org.osgi.service.remoteserviceadmin.EndpointDescription;
@@ -93,9 +97,7 @@ public class RemoteServiceAdminCoreTest {
};
};
rsaCore = new RemoteServiceAdminCore(rsaContext, apiContext, eventProducer, provider, packageUtil) {
- protected void createExportedServicesListener() {
- // Skip
- }
+ protected void createServiceListener() {};
};
}
@@ -224,10 +226,8 @@ public class RemoteServiceAdminCoreTest {
Map<String, Object> edProps = endpoint.getProperties();
assertEquals("http://something", edProps.get("endpoint.id"));
assertNotNull(edProps.get("service.imported"));
- assertTrue(Arrays.equals(new String[] {"java.lang.Runnable"},
- (Object[]) edProps.get("objectClass")));
- assertTrue(Arrays.equals(new String[] {MYCONFIG},
- (Object[]) edProps.get("service.imported.configs")));
+ assertThat((String[]) edProps.get("objectClass"), array(equalTo("java.lang.Runnable")));
+ assertThat((String[]) edProps.get("service.imported.configs"), array(equalTo(MYCONFIG)));
// Ask to export the same service again, this should not go through the whole process again but simply return
// a copy of the first instance.
@@ -239,22 +239,17 @@ public class RemoteServiceAdminCoreTest {
assertEquals(ereg.getExportReference().getExportedEndpoint().getProperties(),
ereg2.getExportReference().getExportedEndpoint().getProperties());
- // Look at the exportedServices data structure
- Map<Map<String, Object>, Collection<ExportRegistration>> exportedServices = getInternalExportedServices();
-
- assertEquals("One service was exported", 1, exportedServices.size());
- Collection<ExportRegistration> firstRegs = exportedServices.values().iterator().next();
- assertEquals("There are 2 export registrations (identical copies)",
- 2, firstRegs.size());
+ assertNumExports(2);
- // Unregister one of the exports
- rsaCore.removeExportRegistration((ExportRegistrationImpl) eregs.get(0));
- assertEquals("One service was exported", 1, exportedServices.size());
- assertEquals("There 1 export registrations left", 1, firstRegs.size());
+ ereg.close();
+ assertNumExports(1);
+
+ ereg2.close();
+ assertNumExports(0);
+ }
- // Unregister the other export
- rsaCore.removeExportRegistration((ExportRegistrationImpl) eregs2.get(0));
- assertEquals("No more exported services", 0, exportedServices.size());
+ private void assertNumExports(int expectedNum) {
+ assertThat("Number of export references", rsaCore.getExportedServices().size(), equalTo(expectedNum));
}
@Test
@@ -328,22 +323,15 @@ public class RemoteServiceAdminCoreTest {
c.verify();
}
- private Map<Map<String, Object>, Collection<ExportRegistration>> getInternalExportedServices()
- throws NoSuchFieldException, IllegalAccessException {
- Field field = RemoteServiceAdminCore.class.getDeclaredField("exportedServices");
- field.setAccessible(true);
- Map<Map<String, Object>, Collection<ExportRegistration>> exportedServices =
- (Map<Map<String, Object>, Collection<ExportRegistration>>) field.get(rsaCore);
- return exportedServices;
- }
-
- private Endpoint createEndpoint(final Map<String, Object> sProps) {
+ private Endpoint createEndpoint(final Map<String, Object> sProps) throws IOException {
Map<String, Object> eProps = new HashMap<String, Object>(sProps);
eProps.put("endpoint.id", "http://something");
eProps.put("service.imported.configs", new String[] {MYCONFIG});
final EndpointDescription epd = new EndpointDescription(eProps);
Endpoint er = c.createMock(Endpoint.class);
- expect(er.description()).andReturn(epd);
+ expect(er.description()).andReturn(epd).anyTimes();
+ er.close();
+ expectLastCall();
return er;
}
http://git-wip-us.apache.org/repos/asf/aries-rsa/blob/55e2c1cb/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/exporter/TopologyManagerExport.java
----------------------------------------------------------------------
diff --git a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/exporter/TopologyManagerExport.java b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/exporter/TopologyManagerExport.java
index 257c924..0d62b2b 100644
--- a/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/exporter/TopologyManagerExport.java
+++ b/topology-manager/src/main/java/org/apache/aries/rsa/topologymanager/exporter/TopologyManagerExport.java
@@ -88,11 +88,15 @@ public class TopologyManagerExport implements ServiceListener {
// track all service registrations so we can export any services that are configured to be exported
// ServiceListener events may be delivered out of order, concurrently, re-entrant, etc. (see spec or docs)
public void serviceChanged(ServiceEvent event) {
- LOG.info("Received ServiceEvent type: {}, sref: {}", getTypeName(event), event.getServiceReference());
ServiceReference<?> sref = event.getServiceReference();
+ if (!shouldExport(sref)) {
+ LOG.debug("Skipping service {}", sref);
+ return;
+ }
+ LOG.info("Received ServiceEvent type: {}, sref: {}", getTypeName(event), sref);
switch (event.getType()) {
case ServiceEvent.REGISTERED:
- exportInBackground(sref);
+ doExport(sref);
break;
case ServiceEvent.MODIFIED:
@@ -151,11 +155,6 @@ public class TopologyManagerExport implements ServiceListener {
}
private void doExport(final ServiceReference<?> sref) {
- Map<String, ?> addProps = policy.additionalParameters(sref);
- if (!shouldExport(sref, addProps)) {
- LOG.debug("Skipping service {}", sref);
- return;
- }
LOG.debug("Exporting service {}", sref);
toBeExported.add(sref);
if (endpointRepo.size() == 0) {
@@ -167,15 +166,14 @@ public class TopologyManagerExport implements ServiceListener {
for (RemoteServiceAdmin remoteServiceAdmin : endpointRepo.keySet()) {
ServiceExportsRepository repo = endpointRepo.get(remoteServiceAdmin);
- Collection<ExportRegistration> regs = exportService(remoteServiceAdmin, sref, addProps);
+ Collection<ExportRegistration> regs = exportService(remoteServiceAdmin, sref);
repo.addService(sref, regs);
}
}
- private static Collection<ExportRegistration> exportService(
+ private Collection<ExportRegistration> exportService(
final RemoteServiceAdmin rsa,
- final ServiceReference<?> sref,
- final Map<String, ?> addProps) {
+ final ServiceReference<?> sref) {
// abort if the service was unregistered by the time we got here
// (we check again at the end, but this optimization saves unnecessary heavy processing)
if (sref.getBundle() == null) {
@@ -184,6 +182,7 @@ public class TopologyManagerExport implements ServiceListener {
}
LOG.debug("exporting Service {} using RemoteServiceAdmin {}", sref, rsa.getClass().getName());
+ Map<String, ?> addProps = policy.additionalParameters(sref);
Collection<ExportRegistration> exportRegs = rsa.exportService(sref, addProps);
// process successful/failed registrations
@@ -209,7 +208,8 @@ public class TopologyManagerExport implements ServiceListener {
return exportRegs;
}
- private boolean shouldExport(ServiceReference<?> sref, Map<String, ?> addProps) {
+ private boolean shouldExport(ServiceReference<?> sref) {
+ Map<String, ?> addProps = policy.additionalParameters(sref);
List<String> exported= StringPlus.normalize(sref.getProperty(RemoteConstants.SERVICE_EXPORTED_INTERFACES));
List<String> addExported = StringPlus.normalize(addProps.get(RemoteConstants.SERVICE_EXPORTED_INTERFACES));
return sizeOf(exported) + sizeOf(addExported) > 0;