From 089bfc0f3f32322b54dba2bf9402769638c09e06 Mon Sep 17 00:00:00 2001 From: Milos Fabian Date: Mon, 25 Nov 2013 13:52:19 +0100 Subject: [PATCH] Fixes - config-registry registered to OSGi, used modules's factory bundle context for module instance registration. Change-Id: Iaae34d47c7a8515ea68f2d8d3629edd0b073152e Signed-off-by: Milos Fabian --- .../manager/impl/ConfigRegistryImpl.java | 17 +++-- .../impl/ConfigTransactionControllerImpl.java | 33 +++++++--- .../ConfigTransactionControllerInternal.java | 3 + ...ierarchicalConfigMBeanFactoriesHolder.java | 65 +++++++------------ .../ModuleFactoriesResolver.java | 7 +- .../impl/osgi/BeanToOsgiServiceManager.java | 18 ++--- ...eContextBackedModuleFactoriesResolver.java | 51 ++++++++++++--- .../impl/osgi/ConfigManagerActivator.java | 10 +++ .../manager/impl/util/InterfacesHelper.java | 1 + .../manager/ConfigRegistryImplTest.java | 13 ++-- .../manager/impl/AbstractConfigTest.java | 33 +++++----- .../ConfigTransactionControllerImplTest.java | 7 +- .../HardcodedModuleFactoriesResolver.java | 65 +++++++++++++++++-- 13 files changed, 213 insertions(+), 110 deletions(-) diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java index 84c2c6dd4d..0fd6c52794 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java @@ -114,8 +114,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe BundleContext bundleContext, MBeanServer configMBeanServer, BaseJMXRegistrator baseJMXRegistrator) { this.resolver = resolver; - this.beanToOsgiServiceManager = new BeanToOsgiServiceManager( - bundleContext); + this.beanToOsgiServiceManager = new BeanToOsgiServiceManager(); this.bundleContext = bundleContext; this.configMBeanServer = configMBeanServer; this.baseJMXRegistrator = baseJMXRegistrator; @@ -138,7 +137,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe String transactionName = "ConfigTransaction-" + version + "-" + versionCounter; TransactionJMXRegistrator transactionRegistrator = baseJMXRegistrator .createTransactionJMXRegistrator(transactionName); - List allCurrentFactories = Collections.unmodifiableList(resolver.getAllFactories()); + Map> allCurrentFactories = Collections.unmodifiableMap(resolver.getAllFactories()); ConfigTransactionControllerInternal transactionController = new ConfigTransactionControllerImpl( transactionName, transactionRegistrator, version, versionCounter, allCurrentFactories, transactionsMBeanServer, configMBeanServer, bundleContext); @@ -320,8 +319,16 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe // register to OSGi if (osgiRegistration == null) { - osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(), - newReadableConfigBean.getInstance(), entry.getName()); + ModuleFactory moduleFactory = entry.getModuleFactory(); + if(moduleFactory != null) { + BundleContext bc = configTransactionController. + getModuleFactoryBundleContext(moduleFactory.getImplementationName()); + osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(), + newReadableConfigBean.getInstance(), entry.getName(), bc); + } else { + throw new NullPointerException(entry.getIdentifier().getFactoryName() + " ModuleFactory not found."); + } + } RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = runtimeRegistrators diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java index 3f569ae324..a9ab664fd6 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java @@ -34,12 +34,13 @@ import javax.management.InstanceAlreadyExistsException; import javax.management.InstanceNotFoundException; import javax.management.MBeanServer; import javax.management.ObjectName; -import java.util.ArrayList; -import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.ArrayList; +import java.util.Collection; import java.util.concurrent.atomic.AtomicBoolean; import static java.lang.String.format; @@ -64,7 +65,7 @@ class ConfigTransactionControllerImpl implements private final DependencyResolverManager dependencyResolverManager; private final TransactionStatus transactionStatus; private final MBeanServer transactionsMBeanServer; - private final List currentlyRegisteredFactories; + private final Map> currentlyRegisteredFactories; /** * Disables ability of {@link DynamicWritableWrapper} to change attributes @@ -82,7 +83,7 @@ class ConfigTransactionControllerImpl implements public ConfigTransactionControllerImpl(String transactionName, TransactionJMXRegistrator transactionRegistrator, long parentVersion, long currentVersion, - List currentlyRegisteredFactories, + Map> currentlyRegisteredFactories, MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer, BundleContext bundleContext) { this.transactionIdentifier = new TransactionIdentifier(transactionName); @@ -120,11 +121,11 @@ class ConfigTransactionControllerImpl implements transactionStatus.checkNotAborted(); Set oldSet = new HashSet<>(lastListOfFactories); - Set newSet = new HashSet<>(currentlyRegisteredFactories); + Set newSet = new HashSet<>(factoriesHolder.getModuleFactories()); List toBeAdded = new ArrayList<>(); List toBeRemoved = new ArrayList<>(); - for(ModuleFactory moduleFactory: currentlyRegisteredFactories) { + for(ModuleFactory moduleFactory: factoriesHolder.getModuleFactories()) { if (oldSet.contains(moduleFactory) == false){ toBeAdded.add(moduleFactory); } @@ -136,7 +137,8 @@ class ConfigTransactionControllerImpl implements } // add default modules for (ModuleFactory moduleFactory : toBeAdded) { - Set defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager, bundleContext); + Set defaultModules = moduleFactory.getDefaultModules(dependencyResolverManager, + getModuleFactoryBundleContext(moduleFactory.getImplementationName())); for (Module module : defaultModules) { // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier()); @@ -173,9 +175,10 @@ class ConfigTransactionControllerImpl implements DependencyResolver dependencyResolver = dependencyResolverManager .getOrCreate(moduleIdentifier); try { + BundleContext bc = getModuleFactoryBundleContext(moduleFactory.getImplementationName()); module = moduleFactory.createModule( moduleIdentifier.getInstanceName(), dependencyResolver, - oldConfigBeanInfo.getReadableModule(), bundleContext); + oldConfigBeanInfo.getReadableModule(), bc); } catch (Exception e) { throw new IllegalStateException(format( "Error while copying old configuration from %s to %s", @@ -196,7 +199,8 @@ class ConfigTransactionControllerImpl implements // find factory ModuleFactory moduleFactory = factoriesHolder.findByModuleName(factoryName); DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier); - Module module = moduleFactory.createModule(instanceName, dependencyResolver, bundleContext); + Module module = moduleFactory.createModule(instanceName, dependencyResolver, + getModuleFactoryBundleContext(moduleFactory.getImplementationName())); return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, null, dependencyResolver); } @@ -466,11 +470,20 @@ class ConfigTransactionControllerImpl implements @Override public List getCurrentlyRegisteredFactories() { - return currentlyRegisteredFactories; + return new ArrayList<>(factoriesHolder.getModuleFactories()); } @Override public TransactionIdentifier getIdentifier() { return transactionIdentifier; } + + @Override + public BundleContext getModuleFactoryBundleContext(String factoryName) { + Map.Entry factoryBundleContextEntry = this.currentlyRegisteredFactories.get(factoryName); + if (factoryBundleContextEntry == null || factoryBundleContextEntry.getValue() == null) { + throw new NullPointerException("Bundle context of " + factoryName + " ModuleFactory not found."); + } + return factoryBundleContextEntry.getValue(); + } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java index 556639c3ab..4dc877c62b 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerInternal.java @@ -15,6 +15,7 @@ import javax.management.ObjectName; import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.ValidationException; import org.opendaylight.controller.config.spi.ModuleFactory; +import org.osgi.framework.BundleContext; /** * Defines contract between {@link ConfigTransactionControllerImpl} (producer) @@ -68,4 +69,6 @@ interface ConfigTransactionControllerInternal extends boolean isClosed(); List getCurrentlyRegisteredFactories(); + + BundleContext getModuleFactoryBundleContext(String factoryName); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java index 8f1c69eee3..f82a7295aa 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HierarchicalConfigMBeanFactoriesHolder.java @@ -7,17 +7,20 @@ */ package org.opendaylight.controller.config.manager.impl.factoriesresolver; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeSet; import org.opendaylight.controller.config.spi.ModuleFactory; +import org.osgi.framework.BundleContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Set; +import java.util.List; +import java.util.Map; +import java.util.Collections; +import java.util.TreeSet; +import java.util.Collection; +import java.util.ArrayList; + /** * Hold sorted ConfigMBeanFactories by their module names. Check that module * names are globally unique. @@ -26,8 +29,9 @@ public class HierarchicalConfigMBeanFactoriesHolder { private static final Logger logger = LoggerFactory .getLogger(HierarchicalConfigMBeanFactoriesHolder.class); - private final Map moduleNamesToConfigBeanFactories; + private final Map> moduleNamesToConfigBeanFactories; private final Set moduleNames; + private final List moduleFactories; /** * Create instance. @@ -36,40 +40,17 @@ public class HierarchicalConfigMBeanFactoriesHolder { * if unique constraint on module names is violated */ public HierarchicalConfigMBeanFactoriesHolder( - List list) { - Map moduleNamesToConfigBeanFactories = new HashMap<>(); - StringBuffer errors = new StringBuffer(); - for (ModuleFactory factory : list) { - String moduleName = factory.getImplementationName(); - if (moduleName == null || moduleName.isEmpty()) { - throw new IllegalStateException( - "Invalid implementation name for " + factory); - } - logger.debug("Reading factory {} {}", moduleName, factory); - String error = null; - ModuleFactory conflicting = moduleNamesToConfigBeanFactories - .get(moduleName); - if (conflicting != null) { - error = String - .format("Module name is not unique. Found two conflicting factories with same name '%s': " + - "\n\t%s\n\t%s\n", moduleName, conflicting, factory); - - } - - if (error == null) { - moduleNamesToConfigBeanFactories.put(moduleName, factory); - } else { - errors.append(error); - } - - } - if (errors.length() > 0) { - throw new IllegalArgumentException(errors.toString()); - } + Map> factoriesMap) { this.moduleNamesToConfigBeanFactories = Collections - .unmodifiableMap(moduleNamesToConfigBeanFactories); + .unmodifiableMap(factoriesMap); moduleNames = Collections.unmodifiableSet(new TreeSet<>( moduleNamesToConfigBeanFactories.keySet())); + List factories = new ArrayList<>(this.moduleNamesToConfigBeanFactories.size()); + Collection> entryCollection = this.moduleNamesToConfigBeanFactories.values(); + for (Map.Entry entry : entryCollection) { + factories.add(entry.getKey()); + } + this.moduleFactories = Collections.unmodifiableList(factories); } /** @@ -79,16 +60,20 @@ public class HierarchicalConfigMBeanFactoriesHolder { * if factory is not found */ public ModuleFactory findByModuleName(String moduleName) { - ModuleFactory result = moduleNamesToConfigBeanFactories.get(moduleName); + Map.Entry result = moduleNamesToConfigBeanFactories.get(moduleName); if (result == null) { throw new IllegalArgumentException( "ModuleFactory not found with module name: " + moduleName); } - return result; + return result.getKey(); } public Set getModuleNames() { return moduleNames; } + public List getModuleFactories() { + return moduleFactories; + } + } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/ModuleFactoriesResolver.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/ModuleFactoriesResolver.java index 4c6697e6b2..fc39fab1fb 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/ModuleFactoriesResolver.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/ModuleFactoriesResolver.java @@ -7,9 +7,10 @@ */ package org.opendaylight.controller.config.manager.impl.factoriesresolver; -import java.util.List; - import org.opendaylight.controller.config.spi.ModuleFactory; +import org.osgi.framework.BundleContext; + +import java.util.Map; /** * {@link org.opendaylight.controller.config.manager.impl.ConfigTransactionControllerImpl} @@ -20,6 +21,6 @@ import org.opendaylight.controller.config.spi.ModuleFactory; */ public interface ModuleFactoriesResolver { - List getAllFactories(); + Map> getAllFactories(); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java index 42f8b8790b..41f577844a 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BeanToOsgiServiceManager.java @@ -7,11 +7,6 @@ */ package org.opendaylight.controller.config.manager.impl.osgi; -import java.util.Dictionary; -import java.util.HashSet; -import java.util.Hashtable; -import java.util.Set; - import org.opendaylight.controller.config.api.ModuleIdentifier; import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation; import org.opendaylight.controller.config.manager.impl.util.InterfacesHelper; @@ -19,6 +14,11 @@ import org.opendaylight.controller.config.spi.Module; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; +import java.util.Dictionary; +import java.util.HashSet; +import java.util.Hashtable; +import java.util.Set; + /** * Registers instantiated beans as OSGi services and unregisters these services * if beans are destroyed. @@ -28,12 +28,6 @@ public class BeanToOsgiServiceManager { static final String INSTANCE_NAME_OSGI_PROP = "instanceName"; static final String IMPLEMENTATION_NAME_OSGI_PROP = "implementationName"; - private final BundleContext bundleContext; - - public BeanToOsgiServiceManager(BundleContext context) { - this.bundleContext = context; - } - /** * To be called for every created, reconfigured and recreated config bean. * It is expected that before using this method OSGi service registry will @@ -41,7 +35,7 @@ public class BeanToOsgiServiceManager { */ public OsgiRegistration registerToOsgi( Class configBeanClass, AutoCloseable instance, - ModuleIdentifier moduleIdentifier) { + ModuleIdentifier moduleIdentifier, BundleContext bundleContext) { try { final Set> configuresInterfaces = InterfacesHelper .getOsgiRegistrationTypes(configBeanClass); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java index 7c8c4e6008..2a533ab9a3 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BundleContextBackedModuleFactoriesResolver.java @@ -7,21 +7,26 @@ */ package org.opendaylight.controller.config.manager.impl.osgi; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - 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 java.util.AbstractMap; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; /** * Retrieves list of currently registered Module Factories using bundlecontext. */ public class BundleContextBackedModuleFactoriesResolver implements ModuleFactoriesResolver { + private static final Logger logger = LoggerFactory + .getLogger(BundleContextBackedModuleFactoriesResolver.class); private final BundleContext bundleContext; public BundleContextBackedModuleFactoriesResolver( @@ -30,7 +35,7 @@ public class BundleContextBackedModuleFactoriesResolver implements } @Override - public List getAllFactories() { + public Map> getAllFactories() { Collection> serviceReferences; try { serviceReferences = bundleContext.getServiceReferences( @@ -38,15 +43,43 @@ public class BundleContextBackedModuleFactoriesResolver implements } catch (InvalidSyntaxException e) { throw new IllegalStateException(e); } - List result = new ArrayList<>(serviceReferences.size()); + Map> result = new HashMap<>(serviceReferences.size()); for (ServiceReference serviceReference : serviceReferences) { - ModuleFactory service = bundleContext.getService(serviceReference); + 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 (service != null) { - result.add(service); + if(factory == null) { + throw new NullPointerException("ServiceReference of class" + serviceReference.getClass() + "not found."); + } + StringBuffer errors = new StringBuffer(); + 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."); + } + logger.debug("Reading factory {} {}", moduleName, factory); + String error = null; + Map.Entry conflicting = result.get(moduleName); + if (conflicting != null) { + error = String + .format("Module name is not unique. Found two conflicting factories with same name '%s': " + + "\n\t%s\n\t%s\n", moduleName, conflicting.getKey(), factory); + + } + + if (error == null) { + result.put(moduleName, new AbstractMap.SimpleImmutableEntry<>(factory, + serviceReference.getBundle().getBundleContext())); + } else { + errors.append(error); + } + if (errors.length() > 0) { + throw new IllegalArgumentException(errors.toString()); } } return result; diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java index f567f1b31e..ab81143170 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java @@ -16,6 +16,7 @@ import org.opendaylight.controller.config.manager.impl.jmx.ConfigRegistryJMXRegi import org.opendaylight.controller.config.spi.ModuleFactory; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; import org.osgi.util.tracker.ServiceTracker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,6 +28,7 @@ public class ConfigManagerActivator implements BundleActivator { private ExtenderBundleTracker extenderBundleTracker; private ConfigRegistryImpl configRegistry; private ConfigRegistryJMXRegistrator configRegistryJMXRegistrator; + private ServiceRegistration configRegistryServiceRegistration; @Override public void start(BundleContext context) throws Exception { @@ -37,6 +39,9 @@ public class ConfigManagerActivator implements BundleActivator { bundleContextBackedModuleFactoriesResolver, context, configMBeanServer); + // register config registry to OSGi + configRegistryServiceRegistration = context.registerService(ConfigRegistryImpl.class, configRegistry, null); + // register config registry to jmx configRegistryJMXRegistrator = new ConfigRegistryJMXRegistrator(configMBeanServer); configRegistryJMXRegistrator.registerToJMX(configRegistry); @@ -69,5 +74,10 @@ public class ConfigManagerActivator implements BundleActivator { "Exception while closing config registry jmx registrator", e); } + try { + configRegistryServiceRegistration.unregister(); + } catch (Exception e) { + logger.warn("Exception while unregistering config registry", e); + } } } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java index 6cd855450d..76cb64cf93 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java @@ -87,6 +87,7 @@ public class InterfacesHelper { */ public static Set> getOsgiRegistrationTypes( Class configBeanClass) { + // TODO test with service interface hierarchy Set> serviceInterfaces = getServiceInterfaces(configBeanClass); Set> result = new HashSet<>(); for (Class clazz : serviceInterfaces) { diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/ConfigRegistryImplTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/ConfigRegistryImplTest.java index 928f2c1879..ccb67d371e 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/ConfigRegistryImplTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/ConfigRegistryImplTest.java @@ -33,14 +33,15 @@ public class ConfigRegistryImplTest extends @Test public void testFailOnTwoFactoriesExportingSameImpl() { ModuleFactory factory = new TestingFixedThreadPoolModuleFactory(); - ModuleFactoriesResolver resolver = new HardcodedModuleFactoriesResolver( - factory, factory); - BundleContext context = mock(BundleContext.class); - - ConfigRegistryImpl configRegistry = new ConfigRegistryImpl(resolver, - context, ManagementFactory.getPlatformMBeanServer()); + ConfigRegistryImpl configRegistry = null; try { + ModuleFactoriesResolver resolver = new HardcodedModuleFactoriesResolver( + factory, factory); + + configRegistry = new ConfigRegistryImpl(resolver, + context, ManagementFactory.getPlatformMBeanServer()); + configRegistry.beginConfig(); fail(); } catch (IllegalArgumentException e) { diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java index 18a22bb26f..5ed56bd2bf 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractConfigTest.java @@ -7,21 +7,6 @@ */ package org.opendaylight.controller.config.manager.impl; -import static org.junit.Assert.assertEquals; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; - -import java.io.Closeable; -import java.lang.management.ManagementFactory; -import java.util.Dictionary; -import java.util.Set; - -import javax.management.InstanceAlreadyExistsException; -import javax.management.MBeanServer; -import javax.management.ObjectName; - import org.junit.After; import org.mockito.Matchers; import org.opendaylight.controller.config.api.jmx.CommitStatus; @@ -35,6 +20,20 @@ import org.opendaylight.controller.config.util.ConfigTransactionJMXClient; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; +import javax.management.InstanceAlreadyExistsException; +import javax.management.MBeanServer; +import javax.management.ObjectName; +import java.io.Closeable; +import java.lang.management.ManagementFactory; +import java.util.Dictionary; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + /** * Each test that relies on * {@link org.opendaylight.controller.config.manager.impl.ConfigRegistryImpl} @@ -51,7 +50,7 @@ public abstract class AbstractConfigTest extends protected ConfigRegistryJMXClient configRegistryClient; protected BaseJMXRegistrator baseJmxRegistrator; protected InternalJMXRegistrator internalJmxRegistrator; - protected BundleContext mockedContext; + protected BundleContext mockedContext = mock(BundleContext.class); protected ServiceRegistration mockedServiceRegistration; // this method should be called in @Before @@ -62,12 +61,12 @@ public abstract class AbstractConfigTest extends configRegistryJMXRegistrator = new ConfigRegistryJMXRegistrator( platformMBeanServer); - this.mockedContext = mock(BundleContext.class); this.mockedServiceRegistration = mock(ServiceRegistration.class); doNothing().when(mockedServiceRegistration).unregister(); doReturn(mockedServiceRegistration).when(mockedContext).registerService( Matchers.any(String[].class), any(Closeable.class), any(Dictionary.class)); + internalJmxRegistrator = new InternalJMXRegistrator(platformMBeanServer); baseJmxRegistrator = new BaseJMXRegistrator(internalJmxRegistrator); diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java index c77c97b8f0..3cb0f447b4 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java @@ -17,13 +17,14 @@ import org.opendaylight.controller.config.manager.impl.jmx.TransactionJMXRegistr import org.opendaylight.controller.config.manager.impl.jmx.TransactionModuleJMXRegistrator; import org.opendaylight.controller.config.manager.impl.runtimembean.TestingRuntimeBean; import org.opendaylight.controller.config.spi.ModuleFactory; +import org.osgi.framework.BundleContext; import javax.management.MBeanServer; import javax.management.MBeanServerFactory; import javax.management.ObjectName; import java.lang.management.ManagementFactory; -import java.util.ArrayList; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import static org.junit.Assert.assertEquals; @@ -59,7 +60,7 @@ public class ConfigTransactionControllerImplTest extends baseJMXRegistrator = new BaseJMXRegistrator( ManagementFactory.getPlatformMBeanServer()); transactionsMBeanServer = MBeanServerFactory.createMBeanServer(); - List currentlyRegisteredFactories = new ArrayList<>(); + Map> currentlyRegisteredFactories = new HashMap<>(); TransactionJMXRegistrator jmxRegistrator123 = baseJMXRegistrator .createTransactionJMXRegistrator(transactionName123); diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HardcodedModuleFactoriesResolver.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HardcodedModuleFactoriesResolver.java index 04f651f6d3..3c2230735e 100644 --- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HardcodedModuleFactoriesResolver.java +++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/factoriesresolver/HardcodedModuleFactoriesResolver.java @@ -7,21 +7,76 @@ */ package org.opendaylight.controller.config.manager.impl.factoriesresolver; +import org.mockito.Matchers; +import org.mockito.Mockito; +import org.opendaylight.controller.config.spi.ModuleFactory; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; + +import java.io.Closeable; import java.util.Arrays; +import java.util.Dictionary; import java.util.List; +import java.util.Map; +import java.util.HashMap; +import java.util.AbstractMap; -import org.opendaylight.controller.config.spi.ModuleFactory; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; public class HardcodedModuleFactoriesResolver implements ModuleFactoriesResolver { - private final List list; + private Map> factories; + + public HardcodedModuleFactoriesResolver(BundleContext bundleContext, ModuleFactory... list) { + List factoryList = Arrays.asList(list); + this.factories = new HashMap<>(factoryList.size()); + for (ModuleFactory moduleFactory : list) { + StringBuffer errors = new StringBuffer(); + String moduleName = moduleFactory.getImplementationName(); + if (moduleName == null || moduleName.isEmpty()) { + throw new IllegalStateException( + "Invalid implementation name for " + moduleFactory); + } + String error = null; + Map.Entry conflicting = factories.get(moduleName); + if (conflicting != null) { + error = String + .format("Module name is not unique. Found two conflicting factories with same name '%s': " + + "\n\t%s\n\t%s\n", moduleName, conflicting.getKey(), moduleFactory); + + } + + if (error == null) { + factories.put(moduleName, new AbstractMap.SimpleEntry<>(moduleFactory, + bundleContext)); + } else { + errors.append(error); + } + if (errors.length() > 0) { + throw new IllegalArgumentException(errors.toString()); + } + } + } public HardcodedModuleFactoriesResolver(ModuleFactory... list) { - this.list = Arrays.asList(list); + this(mockBundleContext(),list); + } + + private static BundleContext mockBundleContext() { + BundleContext bundleContext = Mockito.mock(BundleContext.class); + ServiceRegistration serviceRegistration = mock(ServiceRegistration.class); + doNothing().when(serviceRegistration).unregister(); + doReturn(serviceRegistration).when(bundleContext).registerService( + Matchers.any(String[].class), any(Closeable.class), + any(Dictionary.class)); + return bundleContext; } @Override - public List getAllFactories() { - return list; + public Map> getAllFactories() { + return factories; } } -- 2.36.6