changeset 643:a558e3037749

Make MultipleServiceTracker handle dependencies going away Add support in MultipleServiceTracker for dependencies going away and notifying when that happens. Reviewed-by: jerboaa, neugens, vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-September/003275.html
author Omair Majid <omajid@redhat.com>
date Wed, 26 Sep 2012 19:36:51 -0400
parents 56669e535899
children 21042beca200
files common/core/src/main/java/com/redhat/thermostat/common/MultipleServiceTracker.java common/core/src/test/java/com/redhat/thermostat/common/MultipleServiceTrackerTest.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/osgi/Activator.java
diffstat 5 files changed, 69 insertions(+), 57 deletions(-) [+]
line wrap: on
line diff
--- a/common/core/src/main/java/com/redhat/thermostat/common/MultipleServiceTracker.java	Wed Sep 26 19:36:51 2012 -0400
+++ b/common/core/src/main/java/com/redhat/thermostat/common/MultipleServiceTracker.java	Wed Sep 26 19:36:51 2012 -0400
@@ -41,6 +41,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
@@ -48,63 +49,45 @@
 import org.osgi.util.tracker.ServiceTrackerCustomizer;
 
 /**
- * 
- * This class is intended to be used within BundleActivator implementations that require
- * some action be taken only after certain services have appeared.  It is not actually
- * an extension to the ServiceTracker class, but embeds a number of ServiceTracker objects
- * (one per required service) nonetheless.
- *
+ * A {@link ServiceTracker} for multiple services. This class is intended to be
+ * used within BundleActivator implementations that require some action be taken
+ * only after certain services have appeared.
  */
 public class MultipleServiceTracker {
 
     public interface Action {
-        public void doIt(Map<String, Object> services);
+        public void dependenciesAvailable(Map<String, Object> services);
+        public void dependenciesUnavailable();
     }
 
     class InternalServiceTrackerCustomizer implements ServiceTrackerCustomizer {
 
-        private static final String OBJECT_CLASS = "objectClass";
-        private ServiceTracker tracker;
-
-        void setTracker(ServiceTracker tracker) {
-            this.tracker = tracker;
-        }
-
         @Override
         public Object addingService(ServiceReference reference) {
-            ensureTracker();
+            Object service = context.getService(reference);
             services.put(getServiceClassName(reference), context.getService(reference));
             if (allServicesReady()) {
-                action.doIt(services);
+                action.dependenciesAvailable(services);
             }
-            return tracker.addingService(reference);
+            return service;
         }
 
         @Override
         public void modifiedService(ServiceReference reference, Object service) {
             // We don't actually need to do anything here.
-            ensureTracker();
-            tracker.modifiedService(reference, service);
         }
 
         @Override
         public void removedService(ServiceReference reference, Object service) {
-            ensureTracker();
+            if (servicesWillBecomeNotReady(getServiceClassName(reference))) {
+                action.dependenciesUnavailable();
+            }
             services.put(getServiceClassName(reference), null);
-            tracker.removedService(reference, service);
-        }
-
-        private void ensureTracker() {
-            if (tracker == null) {
-                // This class is used only internally, and is initialized within the constructor.  The trackers that could
-                // be generating events cannot be opened except by calling open on the enclosing class, so this should
-                // never ever ever ever happen.
-                throw new IllegalStateException("Trackers should not be opened before this guy has been set.");
-            }
+            context.ungetService(reference);
         }
 
         private String getServiceClassName(ServiceReference reference) {
-            return ((String[]) reference.getProperty(OBJECT_CLASS))[0];
+            return ((String[]) reference.getProperty(org.osgi.framework.Constants.OBJECTCLASS))[0];
         }
     }
 
@@ -113,17 +96,16 @@
     private Action action;
     private BundleContext context;
 
-    public MultipleServiceTracker(BundleContext context, Class[] classes, Action action) {
-        action.getClass();
-        context.getClass();
-        classes.getClass(); // Harmless call to cause NPE if passed null.
+    public MultipleServiceTracker(BundleContext context, Class<?>[] classes, Action action) {
+        Objects.requireNonNull(context);
+        Objects.requireNonNull(classes);
+        Objects.requireNonNull(action);
         this.context = context;
         services = new HashMap<>();
         trackers = new ArrayList<>();
-        for (Class clazz: classes) {
+        for (Class<?> clazz: classes) {
             InternalServiceTrackerCustomizer tc = new InternalServiceTrackerCustomizer();
             ServiceTracker tracker = new ServiceTracker(context, clazz.getName(), tc);
-            tc.setTracker(tracker);
             trackers.add(tracker);
             services.put(clazz.getName(), null);
         }
@@ -150,4 +132,8 @@
         }
         return true;
     }
+
+    private boolean servicesWillBecomeNotReady(String serviceName) {
+        return (allServicesReady() && services.containsKey(serviceName));
+    }
 }
--- a/common/core/src/test/java/com/redhat/thermostat/common/MultipleServiceTrackerTest.java	Wed Sep 26 19:36:51 2012 -0400
+++ b/common/core/src/test/java/com/redhat/thermostat/common/MultipleServiceTrackerTest.java	Wed Sep 26 19:36:51 2012 -0400
@@ -59,7 +59,6 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.any;
 
 import static org.powermock.api.mockito.PowerMockito.verifyNew;
 import static org.powermock.api.mockito.PowerMockito.whenNew;
@@ -72,6 +71,8 @@
     private BundleContext context;
     private ServiceTracker objectTracker, stringTracker;
     private ServiceReference objectReference, stringReference;
+    private final Object OBJECT = new Object();
+    private final String STRING = "foo";
 
     @Before
     public void setUp() throws Exception {
@@ -79,9 +80,8 @@
         context = mock(BundleContext.class);
 
         objectReference = mock(ServiceReference.class);
-        String[] objObjectClassProperty = {Object.class.getName()};
-        when(objectReference.getProperty(eq("objectClass"))).thenReturn(objObjectClassProperty);
-        when(context.getService(objectReference)).thenReturn(new Object());
+        when(objectReference.getProperty(eq("objectClass"))).thenReturn(new String[] { OBJECT.getClass().getName() });
+        when(context.getService(objectReference)).thenReturn(OBJECT);
         objectTracker = mock(ServiceTracker.class);
         whenNew(ServiceTracker.class).
                 withParameterTypes(BundleContext.class, String.class, ServiceTrackerCustomizer.class).
@@ -89,9 +89,8 @@
                         isA(ServiceTrackerCustomizer.class)).thenReturn(objectTracker);
 
         stringReference = mock(ServiceReference.class);
-        String[] stringObjectClassProperty = {String.class.getName()};
-        when(stringReference.getProperty(eq("objectClass"))).thenReturn(stringObjectClassProperty);
-        when(context.getService(stringReference)).thenReturn("foo");
+        when(stringReference.getProperty(eq("objectClass"))).thenReturn(new String[] { STRING.getClass().getName() });
+        when(context.getService(stringReference)).thenReturn(STRING);
         stringTracker = mock(ServiceTracker.class);
         whenNew(ServiceTracker.class).
                 withParameterTypes(BundleContext.class, String.class, ServiceTrackerCustomizer.class).
@@ -115,7 +114,10 @@
         verify(objectTracker).open();
 
         customizer.addingService(objectReference);
-        verify(action).doIt(any(Map.class));
+        verify(action).dependenciesAvailable(isA(Map.class));
+
+        customizer.removedService(objectReference, OBJECT);
+        verify(action).dependenciesUnavailable();
     }
 
     @Test
@@ -140,11 +142,21 @@
 
         customizer.addingService(objectReference);
         customizer.addingService(stringReference);
-        verify(action).doIt(serviceMap.capture());
+        verify(action).dependenciesAvailable(serviceMap.capture());
         
         Map caputerServices = serviceMap.getValue();
         Assert.assertTrue(caputerServices.containsKey(Object.class.getName()));
         Assert.assertTrue(caputerServices.containsKey(String.class.getName()));
         Assert.assertEquals(2, caputerServices.size());
+
+        customizer.removedService(objectReference, OBJECT);
+
+        verify(action).dependenciesUnavailable();
+
+        customizer.removedService(stringReference, STRING);
+
+        // this second invocation of removedServices must not trigger dependenciesUnavailable again.
+        // it should still have been invoked exactly 1 time
+        verify(action).dependenciesUnavailable();
     }
 }
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java	Wed Sep 26 19:36:51 2012 -0400
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java	Wed Sep 26 19:36:51 2012 -0400
@@ -55,20 +55,28 @@
     class RegisterLauncherAction implements Action {
 
         private BundleContext context;
+        private ServiceReference registryReference;
 
         RegisterLauncherAction(BundleContext context) {
             this.context = context;
         }
+
         @Override
-        public void doIt(Map<String, Object> services) {
+        public void dependenciesAvailable(Map<String, Object> services) {
             
-            ServiceReference reference = context.getServiceReference(OSGiRegistry.class);
-            OSGiRegistry bundleService = (OSGiRegistry) context.getService(reference);
+            registryReference = context.getServiceReference(OSGiRegistry.class);
+            OSGiRegistry bundleService = (OSGiRegistry) context.getService(registryReference);
             LauncherImpl launcher = new LauncherImpl(context,
                     new CommandContextFactory(context), bundleService);
             launcherServiceRegistration = context.registerService(Launcher.class.getName(), launcher, null);
         }
-        
+
+        @Override
+        public void dependenciesUnavailable() {
+            launcherServiceRegistration.unregister();
+            context.ungetService(registryReference);
+        }
+
     }
 
     @SuppressWarnings("rawtypes")
@@ -85,9 +93,6 @@
     @Override
     public void stop(BundleContext context) throws Exception {
         super.stop(context);
-        if (launcherServiceRegistration != null) {
-            launcherServiceRegistration.unregister();
-        }
         if (tracker != null) {
             tracker.close();
         }
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java	Wed Sep 26 19:36:51 2012 -0400
+++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java	Wed Sep 26 19:36:51 2012 -0400
@@ -123,11 +123,12 @@
                 actionCaptor.capture());
         Action action = actionCaptor.getValue();
 
-        action.doIt(any(Map.class));
+        action.dependenciesAvailable(isA(Map.class));
         verify(context).registerService(eq(Launcher.class.getName()), isA(Launcher.class), (Dictionary) isNull());
 
         activator.stop(context);
-        verify(launcherServiceRegistration).unregister();
+        // osgi will take care of unregistration on bundle stop
+        // verify(launcherServiceRegistration).unregister();
         verify(tracker).close();
     }
 }
--- a/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/osgi/Activator.java	Wed Sep 26 19:36:51 2012 -0400
+++ b/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/osgi/Activator.java	Wed Sep 26 19:36:51 2012 -0400
@@ -40,6 +40,7 @@
 
 import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceRegistration;
 
 import com.redhat.thermostat.client.osgi.service.ApplicationService;
 import com.redhat.thermostat.client.osgi.service.VmInformationService;
@@ -62,14 +63,21 @@
         };
         
         Action action = new Action() {
+
+            private ServiceRegistration registration;
+
             @Override
-            public void doIt(Map<String, Object> services) {
+            public void dependenciesAvailable(Map<String, Object> services) {
                 ThreadCollectorFactory collectorFactory = (ThreadCollectorFactory) services.get(ThreadCollectorFactory.class.getName());
                 ApplicationService applicationService = (ApplicationService) services.get(ApplicationService.class.getName());
                 ThreadViewProvider viewFactory = (ThreadViewProvider) services.get(ThreadViewProvider.class.getName());
                 
                 VmInformationService vmInfoService = new ThreadInformationService(applicationService, collectorFactory, viewFactory);
-                context.registerService(VmInformationService.class.getName(), vmInfoService, null);
+                registration = context.registerService(VmInformationService.class.getName(), vmInfoService, null);
+            }
+            @Override
+            public void dependenciesUnavailable() {
+                registration.unregister();
             }
         };