Modify ModuleInfoBundleTracker to track RESOLVED bundles 38/27138/5
authorTom Pantelis <tpanteli@brocade.com>
Sun, 13 Sep 2015 03:55:29 +0000 (23:55 -0400)
committerGerrit Code Review <gerrit@opendaylight.org>
Tue, 29 Sep 2015 13:43:20 +0000 (13:43 +0000)
I've seen issues where a yang-generated class that exists in another
bundle isn't found on startup when a module config is pushed.
Specifically I've seen it when registering RPC implementations. The
BindingDOMRpcProviderServiceAdapter uses the
BindingToNormalizedNodeCodec get the RPC schema and it can fail to
get the RPC input class.

The BindingToNormalizedNodeCodec calls the BindingRuntimeContext to
obtain schema classes which in turn uses the ClassLoadingStrategy OSGi
service to load classes. The backing implementation of
ClassLoadingStrategy is the ModuleInfoBackedContext supplied by the
config manager bundle. This is backed by the ModuleInfoBundleTracker
which scrapes yang files from bundles. However it listens for ACTIVE
bundles. A while ago the GlobalBundleScanningSchemaServiceImpl was
(correctly) changed to listen for RESOLVED bundles which fixed startup
timing issues. It makes sense to also change the
ModuleInfoBundleTracker to listen for RESOLVED bundles so all existing
yang models are loaded on startup prior to use.

The ModuleFactoryBundleTracker piggy-backs the ModuleInfoBundleTracker
to load ModuleFactory instances needed by the config system. It
registers ModuleFactory instances as OSGi services, which are consumed by the
BundleContextBackedModuleFactoriesResolver, however this fails for a
RESOLVED bundle b/c it doesn't have a BundleContext yet (apparently this
is set when the bundle is started/activated). To fix this, I refactored
BundleContextBackedModuleFactoriesResolver and
ModuleFactoryBundleTracker a bit. The ModuleFactoryBundleTracker no
longer registers ModuleFactory instances as OSGi services. Instead it
maintains a list of ModuleFactory/Bundle entries which the
BundleContextBackedModuleFactoriesResolver directly uses to build the
resulting factories map for the ConfigRegistry. A bundle may still not
have a BundleContext at that point so, to safe guard against that, I
added a busy wait for BundleContext.

Change-Id: Ia7bd39f635e3473e6e84011163a0768865c9a931
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ExtensibleBundleTracker.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTracker.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolverTest.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTrackerTest.java

index e8639d5..e9af468 100644 (file)
@@ -8,14 +8,12 @@
 package org.opendaylight.controller.config.manager.impl.osgi;
 
 import java.util.AbstractMap;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Map.Entry;
 import org.opendaylight.controller.config.manager.impl.factoriesresolver.ModuleFactoriesResolver;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -26,42 +24,27 @@ public class BundleContextBackedModuleFactoriesResolver implements
         ModuleFactoriesResolver {
     private static final Logger LOG = LoggerFactory
             .getLogger(BundleContextBackedModuleFactoriesResolver.class);
-    private final BundleContext bundleContext;
+    private ModuleFactoryBundleTracker moduleFactoryBundleTracker;
 
-    public BundleContextBackedModuleFactoriesResolver(
-            BundleContext bundleContext) {
-        this.bundleContext = bundleContext;
+    public BundleContextBackedModuleFactoriesResolver() {
+    }
+
+    public void setModuleFactoryBundleTracker(ModuleFactoryBundleTracker moduleFactoryBundleTracker) {
+        this.moduleFactoryBundleTracker = moduleFactoryBundleTracker;
     }
 
     @Override
     public Map<String, Map.Entry<ModuleFactory, BundleContext>> getAllFactories() {
-        Collection<ServiceReference<ModuleFactory>> serviceReferences;
-        try {
-            serviceReferences = bundleContext.getServiceReferences(
-                    ModuleFactory.class, null);
-        } catch (InvalidSyntaxException e) {
-            throw new IllegalStateException(e);
-        }
-        Map<String, Map.Entry<ModuleFactory, BundleContext>> result = new HashMap<>(serviceReferences.size());
-        for (ServiceReference<ModuleFactory> serviceReference : serviceReferences) {
-            ModuleFactory factory = bundleContext.getService(serviceReference);
-            // null if the service is not registered, the service object
-            // returned by a ServiceFactory does not
-            // implement the classes under which it was registered or the
-            // ServiceFactory threw an exception.
-            if(factory == null) {
-                throw new NullPointerException("ServiceReference of class" + serviceReference.getClass() + "not found.");
-            }
-
-            String moduleName = factory.getImplementationName();
+        Map<String, Map.Entry<ModuleFactory, BundleContext>> result = new HashMap<>();
+        for(Entry<ModuleFactory, BundleContext> entry: moduleFactoryBundleTracker.getModuleFactoryEntries()) {
+            ModuleFactory factory = entry.getKey();
+            BundleContext bundleContext = entry.getValue();
+            String moduleName = factory .getImplementationName();
             if (moduleName == null || moduleName.isEmpty()) {
-                throw new IllegalStateException(
-                        "Invalid implementation name for " + factory);
-            }
-            if (serviceReference.getBundle() == null || serviceReference.getBundle().getBundleContext() == null) {
-                throw new NullPointerException("Bundle context of " + factory + " ModuleFactory not found.");
+                throw new IllegalStateException("Invalid implementation name for " + factory);
             }
-            LOG.debug("Reading factory {} {}", moduleName, factory);
+
+            LOG.debug("Processing factory {} {}", moduleName, factory);
 
             Map.Entry<ModuleFactory, BundleContext> conflicting = result.get(moduleName);
             if (conflicting != null) {
@@ -71,10 +54,10 @@ public class BundleContextBackedModuleFactoriesResolver implements
                 LOG.error(error);
                 throw new IllegalArgumentException(error);
             } else {
-                result.put(moduleName, new AbstractMap.SimpleImmutableEntry<>(factory,
-                        serviceReference.getBundle().getBundleContext()));
+                result.put(moduleName, new AbstractMap.SimpleImmutableEntry<>(factory, bundleContext));
             }
         }
+
         return result;
     }
 }
index 79788c7..7fbd488 100644 (file)
@@ -9,7 +9,6 @@ package org.opendaylight.controller.config.manager.impl.osgi;
 
 import static org.opendaylight.controller.config.manager.impl.util.OsgiRegistrationUtil.registerService;
 import static org.opendaylight.controller.config.manager.impl.util.OsgiRegistrationUtil.wrap;
-
 import java.lang.management.ManagementFactory;
 import java.util.Arrays;
 import java.util.List;
@@ -26,6 +25,7 @@ import org.opendaylight.controller.config.manager.impl.util.OsgiRegistrationUtil
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.opendaylight.yangtools.sal.binding.generator.impl.GeneratedClassLoadingStrategy;
 import org.opendaylight.yangtools.sal.binding.generator.impl.ModuleInfoBackedContext;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
 import org.osgi.util.tracker.ServiceTracker;
@@ -53,8 +53,7 @@ public class ConfigManagerActivator implements BundleActivator {
             ModuleInfoBundleTracker moduleInfoBundleTracker = new ModuleInfoBundleTracker(moduleInfoRegistryWrapper);
 
             // start config registry
-            BundleContextBackedModuleFactoriesResolver bundleContextBackedModuleFactoriesResolver = new BundleContextBackedModuleFactoriesResolver(
-                    context);
+            BundleContextBackedModuleFactoriesResolver bundleContextBackedModuleFactoriesResolver = new BundleContextBackedModuleFactoriesResolver();
             ConfigRegistryImpl configRegistry = new ConfigRegistryImpl(bundleContextBackedModuleFactoriesResolver, configMBeanServer,
                     bindingContextProvider);
 
@@ -63,10 +62,12 @@ public class ConfigManagerActivator implements BundleActivator {
                     configRegistry);
             ModuleFactoryBundleTracker primaryModuleFactoryBundleTracker = new ModuleFactoryBundleTracker(
                     blankTransactionServiceTracker);
+            bundleContextBackedModuleFactoriesResolver.setModuleFactoryBundleTracker(primaryModuleFactoryBundleTracker);
 
             // start extensible tracker
             ExtensibleBundleTracker<?> bundleTracker = new ExtensibleBundleTracker<>(context,
-                    primaryModuleFactoryBundleTracker, moduleInfoBundleTracker);
+                Bundle.RESOLVED | Bundle.STARTING | Bundle.STOPPING | Bundle.ACTIVE,
+                primaryModuleFactoryBundleTracker, moduleInfoBundleTracker);
             bundleTracker.open();
 
             // Wrap config registry with JMX notification publishing adapter
index eb3c502..61c0505 100644 (file)
@@ -7,13 +7,11 @@
  */
 package org.opendaylight.controller.config.manager.impl.osgi;
 
-import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import com.google.common.util.concurrent.MoreExecutors;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import java.util.concurrent.ThreadFactory;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
@@ -50,9 +48,6 @@ import org.slf4j.LoggerFactory;
  */
 public final class ExtensibleBundleTracker<T> extends BundleTracker<Future<T>> {
 
-    private static final ThreadFactory THREAD_FACTORY = new ThreadFactoryBuilder()
-        .setNameFormat("config-bundle-tracker-%d")
-        .build();
     private final ExecutorService eventExecutor;
     private final BundleTrackerCustomizer<T> primaryTracker;
     private final BundleTrackerCustomizer<?>[] additionalTrackers;
@@ -70,7 +65,7 @@ public final class ExtensibleBundleTracker<T> extends BundleTracker<Future<T>> {
         super(context, bundleState, null);
         this.primaryTracker = primaryBundleTrackerCustomizer;
         this.additionalTrackers = additionalBundleTrackerCustomizers;
-        eventExecutor = Executors.newSingleThreadExecutor(THREAD_FACTORY);
+        eventExecutor = MoreExecutors.newDirectExecutorService();
         LOG.trace("Registered as extender with context {} and bundle state {}", context, bundleState);
     }
 
index cd72a73..51443d0 100644 (file)
@@ -10,13 +10,24 @@ package org.opendaylight.controller.config.manager.impl.osgi;
 import static java.lang.String.format;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
 import com.google.common.io.Resources;
+import com.google.common.util.concurrent.Uninterruptibles;
 import java.io.IOException;
 import java.net.URL;
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.TimeUnit;
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
-import org.osgi.framework.ServiceRegistration;
 import org.osgi.util.tracker.BundleTrackerCustomizer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,8 +42,13 @@ import org.slf4j.LoggerFactory;
  * Code based on http://www.toedter.com/blog/?p=236
  */
 public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Object> {
-    private final BlankTransactionServiceTracker blankTransactionServiceTracker;
     private static final Logger LOG = LoggerFactory.getLogger(ModuleFactoryBundleTracker.class);
+    private static final long BUNDLE_CONTEXT_TIMEOUT = TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS);
+
+    private final BlankTransactionServiceTracker blankTransactionServiceTracker;
+
+    @GuardedBy(value = "bundleModuleFactoryMap")
+    private final Multimap<BundleKey, ModuleFactory> bundleModuleFactoryMap = HashMultimap.create();
 
     public ModuleFactoryBundleTracker(BlankTransactionServiceTracker blankTransactionServiceTracker) {
         this.blankTransactionServiceTracker = blankTransactionServiceTracker;
@@ -46,7 +62,11 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
         if (resource != null) {
             try {
                 for (String factoryClassName : Resources.readLines(resource, Charsets.UTF_8)) {
-                    registerFactory(factoryClassName, bundle);
+                    Entry<ModuleFactory, Bundle> moduleFactoryEntry = registerFactory(factoryClassName, bundle);
+                    synchronized (bundleModuleFactoryMap) {
+                        bundleModuleFactoryMap.put(new BundleKey(moduleFactoryEntry.getValue()),
+                                moduleFactoryEntry.getKey());
+                    }
                 }
             } catch (IOException e) {
                 LOG.error("Error while reading {}", resource, e);
@@ -63,23 +83,42 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
 
     @Override
     public void removedBundle(Bundle bundle, BundleEvent event, Object object) {
+        bundleModuleFactoryMap.removeAll(new BundleKey(bundle));
+
         // workaround for service tracker not getting removed service event
         blankTransactionServiceTracker.blankTransaction();
     }
 
+    public Collection<Map.Entry<ModuleFactory, BundleContext>> getModuleFactoryEntries() {
+        Collection<Entry<BundleKey, ModuleFactory>> entries;
+        synchronized (bundleModuleFactoryMap) {
+            entries = new ArrayList<>(bundleModuleFactoryMap.entries());
+        }
+
+        Collection<Map.Entry<ModuleFactory, BundleContext>> result = new ArrayList<>(entries.size());
+        for(Entry<BundleKey, ModuleFactory> entry: entries) {
+            BundleContext context = entry.getKey().getBundleContext();
+            if(context == null) {
+                LOG.warn("Bundle context for {} ModuleFactory not found", entry.getValue());
+            } else {
+                result.add(new AbstractMap.SimpleImmutableEntry<>(entry.getValue(), context));
+            }
+        }
+
+        return result;
+    }
+
     @VisibleForTesting
-    protected static ServiceRegistration<?> registerFactory(String factoryClassName, Bundle bundle) {
+    protected static Map.Entry<ModuleFactory, Bundle> registerFactory(String factoryClassName, Bundle bundle) {
         String errorMessage;
         Exception ex = null;
         try {
             Class<?> clazz = bundle.loadClass(factoryClassName);
             if (ModuleFactory.class.isAssignableFrom(clazz)) {
                 try {
-                    LOG.debug("Registering {} in bundle {}",
-                            clazz.getName(), bundle);
-                    return bundle.getBundleContext().registerService(
-                            ModuleFactory.class.getName(), clazz.newInstance(),
-                            null);
+                    LOG.debug("Registering {} in bundle {}", clazz.getName(), bundle);
+
+                    return new AbstractMap.SimpleImmutableEntry<>((ModuleFactory)clazz.newInstance(), bundle);
                 } catch (InstantiationException e) {
                     errorMessage = logMessage(
                             "Could not instantiate {} in bundle {}, reason {}",
@@ -90,6 +129,11 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
                             "Illegal access during instantiation of class {} in bundle {}, reason {}",
                             factoryClassName, bundle, e);
                     ex = e;
+                } catch (RuntimeException e) {
+                    errorMessage = logMessage(
+                            "Unexpected exception during instantiation of class {} in bundle {}, reason {}",
+                            clazz, bundle.getBundleContext(), e);
+                    ex = e;
                 }
             } else {
                 errorMessage = logMessage(
@@ -111,4 +155,46 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
         String formatMessage = slfMessage.replaceAll("\\{\\}", "%s");
         return format(formatMessage, params);
     }
+
+    private static class BundleKey {
+        Bundle bundle;
+        BundleContext bundleContext;
+
+        public BundleKey(Bundle bundle) {
+            this.bundle = bundle;
+        }
+
+        BundleContext getBundleContext() {
+            if(bundleContext != null) {
+                return bundleContext;
+            }
+
+            // If the bundle isn't activated yet, it may not have a BundleContext yet so busy wait for it.
+            Stopwatch timer = Stopwatch.createStarted();
+            while(timer.elapsed(TimeUnit.MILLISECONDS) <= BUNDLE_CONTEXT_TIMEOUT) {
+                bundleContext = bundle.getBundleContext();
+                if(bundleContext != null) {
+                    return bundleContext;
+                }
+
+                Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS);
+            }
+
+            return null;
+        }
+
+        @Override
+        public int hashCode() {
+            return (int) bundle.getBundleId();
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (getClass() != obj.getClass()) {
+                return false;
+            }
+            BundleKey other = (BundleKey) obj;
+            return bundle.getBundleId() == other.bundle.getBundleId();
+        }
+    }
 }
index bbd8e04..dee52e4 100644 (file)
@@ -13,41 +13,58 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
-
-import com.google.common.collect.Lists;
+import com.google.common.util.concurrent.Uninterruptibles;
+import java.util.AbstractMap;
+import java.util.Arrays;
 import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceReference;
 
 public class BundleContextBackedModuleFactoriesResolverTest {
 
     @Mock
-    private BundleContext bundleContext;
+    private ModuleFactoryBundleTracker mockBundleTracker;
+
+    @Mock
+    private BundleContext mockBundleContext1;
+
+    @Mock
+    private BundleContext mockBundleContext2;
+
+    @Mock
+    private Bundle mockBundle1;
+
+    @Mock
+    private Bundle mockBundle2;
+
     private BundleContextBackedModuleFactoriesResolver resolver;
-    private ServiceReference<?> s1;
-    private ServiceReference<?> s2;
     private ModuleFactory f1;
     private ModuleFactory f2;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
-        s1 = getServiceRef();
-        s2 = getServiceRef();
-        doReturn(Lists.newArrayList(s1, s2)).when(bundleContext).getServiceReferences(ModuleFactory.class, null);
+        doReturn(mockBundleContext1).when(mockBundle1).getBundleContext();
+        doReturn(mockBundleContext2).when(mockBundle2).getBundleContext();
+
         f1 = getMockFactory("f1");
-        doReturn(f1).when(bundleContext).getService(s1);
         f2 = getMockFactory("f2");
-        doReturn(f2).when(bundleContext).getService(s2);
-        resolver = new BundleContextBackedModuleFactoriesResolver(bundleContext);
+
+        resolver = new BundleContextBackedModuleFactoriesResolver();
+        resolver.setModuleFactoryBundleTracker(mockBundleTracker);
     }
 
     private ModuleFactory getMockFactory(final String name) {
@@ -57,30 +74,28 @@ public class BundleContextBackedModuleFactoriesResolverTest {
         return mock;
     }
 
-    private ServiceReference<?> getServiceRef() {
-        ServiceReference<?> mock = mock(ServiceReference.class);
-        doReturn("serviceRef").when(mock).toString();
-        final Bundle bundle = mock(Bundle.class);
-        doReturn(bundleContext).when(bundle).getBundleContext();
-        doReturn(bundle).when(mock).getBundle();
-        return mock;
-    }
-
     @Test
     public void testGetAllFactories() throws Exception {
+        doReturn(Arrays.asList(new AbstractMap.SimpleImmutableEntry<>(f1, mockBundleContext1),
+                new AbstractMap.SimpleImmutableEntry<>(f2, mockBundleContext2))).
+                        when(mockBundleTracker).getModuleFactoryEntries();
+
         Map<String, Map.Entry<ModuleFactory, BundleContext>> allFactories = resolver.getAllFactories();
         assertEquals(2, allFactories.size());
         assertTrue(allFactories.containsKey(f1.getImplementationName()));
         assertEquals(f1, allFactories.get(f1.getImplementationName()).getKey());
-        assertEquals(bundleContext, allFactories.get(f1.getImplementationName()).getValue());
+        assertEquals(mockBundleContext1, allFactories.get(f1.getImplementationName()).getValue());
         assertTrue(allFactories.containsKey(f2.getImplementationName()));
         assertEquals(f2, allFactories.get(f2.getImplementationName()).getKey());
-        assertEquals(bundleContext, allFactories.get(f2.getImplementationName()).getValue());
+        assertEquals(mockBundleContext2, allFactories.get(f2.getImplementationName()).getValue());
     }
 
     @Test
     public void testDuplicateFactories() throws Exception {
-        doReturn(f1).when(bundleContext).getService(s2);
+        doReturn(Arrays.asList(new AbstractMap.SimpleImmutableEntry<>(f1, mockBundleContext1),
+                new AbstractMap.SimpleImmutableEntry<>(f1, mockBundleContext2))).
+                        when(mockBundleTracker).getModuleFactoryEntries();
+
         try {
             resolver.getAllFactories();
         } catch (Exception e) {
@@ -92,21 +107,54 @@ public class BundleContextBackedModuleFactoriesResolverTest {
         fail("Should fail with duplicate factory name");
     }
 
-    @Test(expected = NullPointerException.class)
-    public void testNullFactory() throws Exception {
-        doReturn(null).when(bundleContext).getService(s2);
-        resolver.getAllFactories();
-    }
-
     @Test(expected = IllegalStateException.class)
     public void testNullFactoryName() throws Exception {
+        doReturn(Arrays.asList(new AbstractMap.SimpleImmutableEntry<>(f1, mockBundleContext1))).
+                when(mockBundleTracker).getModuleFactoryEntries();
+
         doReturn(null).when(f1).getImplementationName();
         resolver.getAllFactories();
     }
 
-    @Test(expected = NullPointerException.class)
-    public void testNullBundleName() throws Exception {
-        doReturn(null).when(s1).getBundle();
-        resolver.getAllFactories();
+    @Test
+    public void testBundleContextInitiallyNull() throws Exception {
+        final AtomicReference<BundleContext> bundleContext = new AtomicReference<>();
+        Answer<BundleContext> answer = new Answer<BundleContext>() {
+            @Override
+            public BundleContext answer(InvocationOnMock invocation) throws Throwable {
+                return bundleContext.get();
+            }
+        };
+
+        doAnswer(answer).when(mockBundle1).getBundleContext();
+        doReturn(Arrays.asList(new AbstractMap.SimpleImmutableEntry<>(f1, mockBundleContext1))).
+                when(mockBundleTracker).getModuleFactoryEntries();
+
+        final AtomicReference<Map<String, Map.Entry<ModuleFactory, BundleContext>>> allFactories = new AtomicReference<>();
+        final AtomicReference<Exception> caughtEx = new AtomicReference<>();
+        final CountDownLatch doneLatch = new CountDownLatch(1);
+        new Thread() {
+            @Override
+            public void run() {
+                try {
+                    allFactories.set(resolver.getAllFactories());
+                } catch (Exception e) {
+                    caughtEx.set(e);
+                } finally {
+                    doneLatch.countDown();
+                }
+            }
+        }.start();
+
+        Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
+        bundleContext.set(mockBundleContext1);
+
+        assertEquals(true, doneLatch.await(5, TimeUnit.SECONDS));
+        if(caughtEx.get() != null) {
+            throw caughtEx.get();
+        }
+
+        assertEquals(1, allFactories.get().size());
+        assertTrue(allFactories.get().containsKey(f1.getImplementationName()));
     }
 }
index 66063c9..9ac570e 100644 (file)
@@ -15,12 +15,14 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
-
+import java.util.Collection;
 import java.util.Dictionary;
+import java.util.Map.Entry;
 import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
@@ -47,6 +49,8 @@ public class ModuleFactoryBundleTrackerTest {
     private BundleContext context;
     @Mock
     private ServiceRegistration<?> reg;
+    @Mock
+    private BlankTransactionServiceTracker blankTxTracker;
 
     @Before
     public void setUp() throws Exception {
@@ -58,14 +62,18 @@ public class ModuleFactoryBundleTrackerTest {
             }
         }).when(bundle).loadClass(anyString());
         doReturn("mockBundle").when(bundle).toString();
+        doReturn("mockBundleContext").when(context).toString();
         doReturn(context).when(bundle).getBundleContext();
+        doReturn(100L).when(bundle).getBundleId();
         doReturn(reg).when(context).registerService(anyString(), anyObject(), any(Dictionary.class));
     }
 
     @Test
     public void testRegisterFactory() throws Exception {
-        ModuleFactoryBundleTracker.registerFactory(TestingFactory.class.getName(), bundle);
-        verify(context).registerService(ModuleFactory.class.getName(), TestingFactory.currentInstance, null);
+        Entry<ModuleFactory, Bundle> entry = ModuleFactoryBundleTracker.registerFactory(
+                TestingFactory.class.getName(), bundle);
+        assertEquals(TestingFactory.currentInstance, entry.getKey());
+        assertEquals(bundle, entry.getValue());
     }
 
     @Test
@@ -122,15 +130,28 @@ public class ModuleFactoryBundleTrackerTest {
         fail("Cannot register without extend");
     }
 
-    @Mock
-    private BlankTransactionServiceTracker blankTxTracker;
-
     @Test
-    public void testAddingBundle() throws Exception {
+    public void testBundleAddAndRemove() throws Exception {
         final ModuleFactoryBundleTracker tracker = new ModuleFactoryBundleTracker(blankTxTracker);
         doReturn(getClass().getResource("/module-factories/module-factory-ok")).when(bundle).getEntry(anyString());
         tracker.addingBundle(bundle, mock(BundleEvent.class));
-        verify(context).registerService(ModuleFactory.class.getName(), TestingFactory.currentInstance, null);
+
+        Collection<Entry<ModuleFactory, BundleContext>> entries = tracker.getModuleFactoryEntries();
+        assertNotNull(entries);
+        assertEquals(1, entries.size());
+        Entry<ModuleFactory, BundleContext> entry = entries.iterator().next();
+        assertEquals(TestingFactory.currentInstance, entry.getKey());
+        assertEquals(context, entry.getValue());
+
+        doNothing().when(blankTxTracker).blankTransaction();;
+
+        tracker.removedBundle(bundle, mock(BundleEvent.class), bundle);
+
+        entries = tracker.getModuleFactoryEntries();
+        assertNotNull(entries);
+        assertEquals(0, entries.size());
+
+        verify(blankTxTracker).blankTransaction();;
     }
 
     @Test