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 e8639d588197c146483b434fd83b4fb653a32c49..e9af46819f9147323acaff3bd9ddd14cda8a756e 100644 (file)
@@ -8,14 +8,12 @@
 package org.opendaylight.controller.config.manager.impl.osgi;
 
 import java.util.AbstractMap;
 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.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.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;
 
 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);
         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() {
     }
 
     @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()) {
             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) {
 
             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 {
                 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;
     }
 }
         return result;
     }
 }
index 79788c7d6107e9315991dbbd467376ce3015001f..7fbd488eb39a4b4e52bda7d67777ade4901da7a8 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 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;
 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.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;
 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
             ModuleInfoBundleTracker moduleInfoBundleTracker = new ModuleInfoBundleTracker(moduleInfoRegistryWrapper);
 
             // start config registry
-            BundleContextBackedModuleFactoriesResolver bundleContextBackedModuleFactoriesResolver = new BundleContextBackedModuleFactoriesResolver(
-                    context);
+            BundleContextBackedModuleFactoriesResolver bundleContextBackedModuleFactoriesResolver = new BundleContextBackedModuleFactoriesResolver();
             ConfigRegistryImpl configRegistry = new ConfigRegistryImpl(bundleContextBackedModuleFactoriesResolver, configMBeanServer,
                     bindingContextProvider);
 
             ConfigRegistryImpl configRegistry = new ConfigRegistryImpl(bundleContextBackedModuleFactoriesResolver, configMBeanServer,
                     bindingContextProvider);
 
@@ -63,10 +62,12 @@ public class ConfigManagerActivator implements BundleActivator {
                     configRegistry);
             ModuleFactoryBundleTracker primaryModuleFactoryBundleTracker = new ModuleFactoryBundleTracker(
                     blankTransactionServiceTracker);
                     configRegistry);
             ModuleFactoryBundleTracker primaryModuleFactoryBundleTracker = new ModuleFactoryBundleTracker(
                     blankTransactionServiceTracker);
+            bundleContextBackedModuleFactoriesResolver.setModuleFactoryBundleTracker(primaryModuleFactoryBundleTracker);
 
             // start extensible tracker
             ExtensibleBundleTracker<?> bundleTracker = new ExtensibleBundleTracker<>(context,
 
             // 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
             bundleTracker.open();
 
             // Wrap config registry with JMX notification publishing adapter
index eb3c502cfb9f4765349d0a0e996b3d82d9bca9b9..61c050506e4130e0937b898ea26003e721ce40ef 100644 (file)
@@ -7,13 +7,11 @@
  */
 package org.opendaylight.controller.config.manager.impl.osgi;
 
  */
 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.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.Future;
-import java.util.concurrent.ThreadFactory;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
 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>> {
 
  */
 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;
     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;
         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);
     }
 
         LOG.trace("Registered as extender with context {} and bundle state {}", context, bundleState);
     }
 
index cd72a73ecf3455365c35ae1a74b71d62654f9763..51443d01e8b7b9b2cbecd878b68fa35dd46827a9 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 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.io.Resources;
+import com.google.common.util.concurrent.Uninterruptibles;
 import java.io.IOException;
 import java.net.URL;
 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.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleEvent;
 import org.osgi.framework.BundleEvent;
-import org.osgi.framework.ServiceRegistration;
 import org.osgi.util.tracker.BundleTrackerCustomizer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 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> {
  * 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 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;
 
     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)) {
         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);
                 }
             } 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) {
 
     @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();
     }
 
         // 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
     @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 {
         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 {}",
                 } 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;
                             "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(
                 }
             } else {
                 errorMessage = logMessage(
@@ -111,4 +155,46 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
         String formatMessage = slfMessage.replaceAll("\\{\\}", "%s");
         return format(formatMessage, params);
     }
         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 bbd8e04eddfaff1b19eba9ca430a4451ca5806f8..dee52e488dde2f758caadef22acfdff2e98709a5 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.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 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.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.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.opendaylight.controller.config.spi.ModuleFactory;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceReference;
 
 public class BundleContextBackedModuleFactoriesResolverTest {
 
     @Mock
 
 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 BundleContextBackedModuleFactoriesResolver resolver;
-    private ServiceReference<?> s1;
-    private ServiceReference<?> s2;
     private ModuleFactory f1;
     private ModuleFactory f2;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
     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");
         f1 = getMockFactory("f1");
-        doReturn(f1).when(bundleContext).getService(s1);
         f2 = getMockFactory("f2");
         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) {
     }
 
     private ModuleFactory getMockFactory(final String name) {
@@ -57,30 +74,28 @@ public class BundleContextBackedModuleFactoriesResolverTest {
         return mock;
     }
 
         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 {
     @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());
         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());
         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 {
     }
 
     @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) {
         try {
             resolver.getAllFactories();
         } catch (Exception e) {
@@ -92,21 +107,54 @@ public class BundleContextBackedModuleFactoriesResolverTest {
         fail("Should fail with duplicate factory name");
     }
 
         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 {
     @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();
     }
 
         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 66063c98b36ed2df8611f5e52401d52e0353357c..9ac570e2eed166045ac7930425b77932b5229263 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.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 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.Dictionary;
+import java.util.Map.Entry;
 import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
 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;
     private BundleContext context;
     @Mock
     private ServiceRegistration<?> reg;
+    @Mock
+    private BlankTransactionServiceTracker blankTxTracker;
 
     @Before
     public void setUp() throws Exception {
 
     @Before
     public void setUp() throws Exception {
@@ -58,14 +62,18 @@ public class ModuleFactoryBundleTrackerTest {
             }
         }).when(bundle).loadClass(anyString());
         doReturn("mockBundle").when(bundle).toString();
             }
         }).when(bundle).loadClass(anyString());
         doReturn("mockBundle").when(bundle).toString();
+        doReturn("mockBundleContext").when(context).toString();
         doReturn(context).when(bundle).getBundleContext();
         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 {
         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
     }
 
     @Test
@@ -122,15 +130,28 @@ public class ModuleFactoryBundleTrackerTest {
         fail("Cannot register without extend");
     }
 
         fail("Cannot register without extend");
     }
 
-    @Mock
-    private BlankTransactionServiceTracker blankTxTracker;
-
     @Test
     @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));
         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
     }
 
     @Test