Add warning when user destroys default module. 45/3345/2
authorTomas Olvecky <tolvecky@cisco.com>
Mon, 2 Dec 2013 15:39:12 +0000 (16:39 +0100)
committerGerrit Code Review <gerrit@opendaylight.org>
Mon, 2 Dec 2013 18:50:50 +0000 (18:50 +0000)
Change-Id: I0a2b27fa8bac7e00b138847935cf803bb1ee8156
Signed-off-by: Tomas Olvecky <tolvecky@cisco.com>
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalInfo.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ModuleInternalTransactionalInfo.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManager.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModulesHolder.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/TransactionHolder.java
opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/BlankTransactionServiceTracker.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImplTest.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/DependencyResolverManagerTest.java

index 0fd6c52..1b695a9 100644 (file)
@@ -129,10 +129,18 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
      */
     @Override
     public synchronized ObjectName beginConfig() {
-        return beginConfigInternal().getControllerObjectName();
+        return beginConfig(false);
     }
 
-    private synchronized ConfigTransactionControllerInternal beginConfigInternal() {
+    /**
+     * @param blankTransaction true if this transaction is created automatically by
+     *                         org.opendaylight.controller.config.manager.impl.osgi.BlankTransactionServiceTracker
+     */
+    public synchronized ObjectName beginConfig(boolean blankTransaction) {
+        return beginConfigInternal(blankTransaction).getControllerObjectName();
+    }
+
+    private synchronized ConfigTransactionControllerInternal beginConfigInternal(boolean blankTransaction) {
         versionCounter++;
         String transactionName = "ConfigTransaction-" + version + "-" + versionCounter;
         TransactionJMXRegistrator transactionRegistrator = baseJMXRegistrator
@@ -140,7 +148,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
         Map<String, Map.Entry<ModuleFactory, BundleContext>> allCurrentFactories = Collections.unmodifiableMap(resolver.getAllFactories());
         ConfigTransactionControllerInternal transactionController = new ConfigTransactionControllerImpl(
                 transactionName, transactionRegistrator, version,
-                versionCounter, allCurrentFactories, transactionsMBeanServer, configMBeanServer, bundleContext);
+                versionCounter, allCurrentFactories, transactionsMBeanServer, configMBeanServer, blankTransaction);
         try {
             transactionRegistrator.registerMBean(transactionController, transactionController.getControllerObjectName());
         } catch (InstanceAlreadyExistsException e) {
@@ -223,7 +231,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
             RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator;
             if (entry.hasOldModule() == false) {
                 runtimeBeanRegistrator = baseJMXRegistrator
-                        .createRuntimeBeanRegistrator(entry.getName());
+                        .createRuntimeBeanRegistrator(entry.getIdentifier());
             } else {
                 // reuse old JMX registrator
                 runtimeBeanRegistrator = entry.getOldInternalInfo()
@@ -236,7 +244,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
                         .setRuntimeBeanRegistrator(runtimeBeanRegistrator);
             }
             // save it to info so it is accessible afterwards
-            runtimeRegistrators.put(entry.getName(), runtimeBeanRegistrator);
+            runtimeRegistrators.put(entry.getIdentifier(), runtimeBeanRegistrator);
         }
 
         // can register runtime beans
@@ -264,8 +272,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
             // determine if current instance was recreated or reused or is new
 
             // rules for closing resources:
-            // osgi registration - will be (re)created every time, so it needs
-            // to be closed here
+            // osgi registration - will be reused if possible.
             // module jmx registration - will be (re)created every time, needs
             // to be closed here
             // runtime jmx registration - should be taken care of by module
@@ -279,7 +286,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
                 ModuleInternalInfo oldInternalInfo = entry.getOldInternalInfo();
                 DynamicReadableWrapper oldReadableConfigBean = oldInternalInfo
                         .getReadableModule();
-                currentConfig.remove(entry.getName());
+                currentConfig.remove(entry.getIdentifier());
 
                 // test if old instance == new instance
                 if (oldReadableConfigBean.getInstance().equals(module.getInstance())) {
@@ -324,7 +331,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
                     BundleContext bc = configTransactionController.
                             getModuleFactoryBundleContext(moduleFactory.getImplementationName());
                     osgiRegistration = beanToOsgiServiceManager.registerToOsgi(module.getClass(),
-                            newReadableConfigBean.getInstance(), entry.getName(), bc);
+                            newReadableConfigBean.getInstance(), entry.getIdentifier(), bc);
                 } else {
                     throw new NullPointerException(entry.getIdentifier().getFactoryName() + " ModuleFactory not found.");
                 }
@@ -332,11 +339,11 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
             }
 
             RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator = runtimeRegistrators
-                    .get(entry.getName());
+                    .get(entry.getIdentifier());
             ModuleInternalInfo newInfo = new ModuleInternalInfo(
-                    entry.getName(), newReadableConfigBean, osgiRegistration,
+                    entry.getIdentifier(), newReadableConfigBean, osgiRegistration,
                     runtimeBeanRegistrator, newModuleJMXRegistrator,
-                    orderingIdx);
+                    orderingIdx, entry.isDefaultBean());
 
             newConfigEntries.put(module, newInfo);
             orderingIdx++;
@@ -367,7 +374,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
     /**
      * Abort open transactions and unregister read only modules. Since this
      * class is not responsible for registering itself under
-     * {@link ConfigRegistryMXBean#OBJECT_NAME}, it will not unregister itself
+     * {@link org.opendaylight.controller.config.api.ConfigRegistry#OBJECT_NAME}, it will not unregister itself
      * here.
      */
     @Override
index a9ab664..3e53a7a 100644 (file)
@@ -78,13 +78,14 @@ class ConfigTransactionControllerImpl implements
             configBeanModificationDisabled);
     private final MBeanServer configMBeanServer;
 
-    private final BundleContext bundleContext;
+    private final boolean blankTransaction;
 
     public ConfigTransactionControllerImpl(String transactionName,
                                            TransactionJMXRegistrator transactionRegistrator,
                                            long parentVersion, long currentVersion,
                                            Map<String, Map.Entry<ModuleFactory, BundleContext>> currentlyRegisteredFactories,
-                                           MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer, BundleContext bundleContext) {
+                                           MBeanServer transactionsMBeanServer, MBeanServer configMBeanServer,
+                                           boolean blankTransaction) {
 
         this.transactionIdentifier = new TransactionIdentifier(transactionName);
         this.controllerON = ObjectNameUtil
@@ -100,7 +101,7 @@ class ConfigTransactionControllerImpl implements
         this.dependencyResolverManager = new DependencyResolverManager(transactionName, transactionStatus);
         this.transactionsMBeanServer = transactionsMBeanServer;
         this.configMBeanServer = configMBeanServer;
-        this.bundleContext = bundleContext;
+        this.blankTransaction = blankTransaction;
     }
 
     @Override
@@ -143,7 +144,8 @@ class ConfigTransactionControllerImpl implements
                 // ensure default module to be registered to jmx even if its module factory does not use dependencyResolverFactory
                 DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(module.getIdentifier());
                 try {
-                    putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver);
+                    boolean defaultBean = true;
+                    putConfigBeanToJMXAndInternalMaps(module.getIdentifier(), module, moduleFactory, null, dependencyResolver, defaultBean);
                 } catch (InstanceAlreadyExistsException e) {
                     throw new IllegalStateException(e);
                 }
@@ -184,7 +186,8 @@ class ConfigTransactionControllerImpl implements
                     "Error while copying old configuration from %s to %s",
                     oldConfigBeanInfo, moduleFactory), e);
         }
-        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver);
+        putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module, moduleFactory, oldConfigBeanInfo, dependencyResolver,
+                oldConfigBeanInfo.isDefaultBean());
     }
 
     @Override
@@ -201,14 +204,15 @@ class ConfigTransactionControllerImpl implements
         DependencyResolver dependencyResolver = dependencyResolverManager.getOrCreate(moduleIdentifier);
         Module module = moduleFactory.createModule(instanceName, dependencyResolver,
                 getModuleFactoryBundleContext(moduleFactory.getImplementationName()));
+        boolean defaultBean = false;
         return putConfigBeanToJMXAndInternalMaps(moduleIdentifier, module,
-                moduleFactory, null, dependencyResolver);
+                moduleFactory, null, dependencyResolver, defaultBean);
     }
 
     private synchronized ObjectName putConfigBeanToJMXAndInternalMaps(
             ModuleIdentifier moduleIdentifier, Module module,
             ModuleFactory moduleFactory,
-            @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver)
+            @Nullable ModuleInternalInfo maybeOldConfigBeanInfo, DependencyResolver dependencyResolver, boolean isDefaultBean)
             throws InstanceAlreadyExistsException {
 
         logger.debug("Adding module {} to transaction {}", moduleIdentifier, this);
@@ -232,14 +236,14 @@ class ConfigTransactionControllerImpl implements
                 .registerMBean(writableDynamicWrapper, writableON);
         ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = new ModuleInternalTransactionalInfo(
                 moduleIdentifier, module, moduleFactory,
-                maybeOldConfigBeanInfo, transactionModuleJMXRegistration);
+                maybeOldConfigBeanInfo, transactionModuleJMXRegistration, isDefaultBean);
 
         dependencyResolverManager.put(moduleInternalTransactionalInfo);
         return writableON;
     }
 
     @Override
-    public void destroyModule(ObjectName objectName)
+    public synchronized void destroyModule(ObjectName objectName)
             throws InstanceNotFoundException {
         String foundTransactionName = ObjectNameUtil
                 .getTransactionName(objectName);
@@ -253,9 +257,17 @@ class ConfigTransactionControllerImpl implements
         destroyModule(moduleIdentifier);
     }
 
-    private void destroyModule(ModuleIdentifier moduleIdentifier) {
+    private synchronized void destroyModule(ModuleIdentifier moduleIdentifier) {
         logger.debug("Destroying module {} in transaction {}", moduleIdentifier, this);
         transactionStatus.checkNotAborted();
+
+        if (blankTransaction == false) {
+            ModuleInternalTransactionalInfo found =
+                    dependencyResolverManager.findModuleInternalTransactionalInfo(moduleIdentifier);
+            if (found.isDefaultBean()) {
+                logger.warn("Warning: removing default bean. This will be forbidden in next version of config-subsystem");
+            }
+        }
         ModuleInternalTransactionalInfo removedTInfo = dependencyResolverManager.destroyModule(moduleIdentifier);
         // remove from jmx
         removedTInfo.getTransactionModuleJMXRegistration().close();
index c675db5..14a706d 100644 (file)
@@ -35,12 +35,14 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
     private final OsgiRegistration osgiRegistration;
     private final ModuleJMXRegistrator moduleJMXRegistrator;
     private final int orderingIdx;
+    private final boolean isDefaultBean;
 
     public ModuleInternalInfo(ModuleIdentifier name,
             @Nullable DynamicReadableWrapper readableModule,
             OsgiRegistration osgiRegistration,
             RootRuntimeBeanRegistratorImpl runtimeBeanRegistrator,
-            ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx) {
+            ModuleJMXRegistrator moduleJMXRegistrator, int orderingIdx,
+            boolean isDefaultBean) {
 
         if (osgiRegistration == null) {
             throw new IllegalArgumentException(
@@ -56,6 +58,7 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
         this.name = name;
         this.moduleJMXRegistrator = moduleJMXRegistrator;
         this.orderingIdx = orderingIdx;
+        this.isDefaultBean = isDefaultBean;
     }
 
     public DynamicReadableWrapper getReadableModule() {
@@ -117,4 +120,8 @@ public class ModuleInternalInfo implements Comparable<ModuleInternalInfo>,
     public ModuleIdentifier getIdentifier() {
         return name;
     }
+
+    public boolean isDefaultBean() {
+        return isDefaultBean;
+    }
 }
index c4f40fb..0a4ceac 100644 (file)
@@ -24,27 +24,22 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
     @Nullable
     private final ModuleInternalInfo maybeOldInternalInfo;
     private final TransactionModuleJMXRegistration transactionModuleJMXRegistration;
+    private final boolean isDefaultBean;
 
     ModuleInternalTransactionalInfo(ModuleIdentifier name, Module module,
             ModuleFactory moduleFactory,
             ModuleInternalInfo maybeOldInternalInfo,
-            TransactionModuleJMXRegistration transactionModuleJMXRegistration) {
+            TransactionModuleJMXRegistration transactionModuleJMXRegistration,
+            boolean isDefaultBean) {
         this.name = name;
         this.module = module;
         this.moduleFactory = moduleFactory;
         this.maybeOldInternalInfo = maybeOldInternalInfo;
         this.transactionModuleJMXRegistration = transactionModuleJMXRegistration;
+        this.isDefaultBean = isDefaultBean;
     }
 
 
-    /**
-     * Use {@link #getIdentifier()} instead.
-     */
-    @Deprecated
-    public ModuleIdentifier getName() {
-        return name;
-    }
-
     public boolean hasOldModule() {
         return maybeOldInternalInfo != null;
     }
@@ -85,4 +80,8 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
     public ModuleIdentifier getIdentifier() {
         return name;
     }
+
+    public boolean isDefaultBean() {
+        return isDefaultBean;
+    }
 }
index dea78c8..645aab3 100644 (file)
@@ -117,6 +117,11 @@ public class DependencyResolverManager implements TransactionHolder, DependencyR
                 jmxAttributeForReporting);
     }
 
+    @Override
+    public ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier) {
+        return modulesHolder.findModuleInternalTransactionalInfo(moduleIdentifier);
+    }
+
     @Override
     public ModuleFactory findModuleFactory(ModuleIdentifier moduleIdentifier,
             JmxAttribute jmxAttributeForReporting) {
@@ -139,7 +144,7 @@ public class DependencyResolverManager implements TransactionHolder, DependencyR
         List<ModuleIdentifier> result = new ArrayList<>();
         for( ModuleInternalTransactionalInfo  info : modulesHolder.getAllInfos()) {
             if (factory.equals(info.getModuleFactory())) {
-                result.add(info.getName());
+                result.add(info.getIdentifier());
             }
         }
         return result;
index 6bbd787..7c7d9f9 100644 (file)
@@ -85,7 +85,7 @@ class ModulesHolder implements TransactionHolder {
     public Map<ModuleIdentifier, Module> getAllModules() {
         Map<ModuleIdentifier, Module> result = new HashMap<>();
         for (ModuleInternalTransactionalInfo entry : commitMap.values()) {
-            ModuleIdentifier name = entry.getName();
+            ModuleIdentifier name = entry.getIdentifier();
             result.put(name, entry.getModule());
         }
         return result;
@@ -94,17 +94,17 @@ class ModulesHolder implements TransactionHolder {
     @Override
     public void put(
             ModuleInternalTransactionalInfo moduleInternalTransactionalInfo) {
-        commitMap.put(moduleInternalTransactionalInfo.getName(),
+        commitMap.put(moduleInternalTransactionalInfo.getIdentifier(),
                 moduleInternalTransactionalInfo);
     }
 
     @Override
     public ModuleInternalTransactionalInfo destroyModule(
             ModuleIdentifier moduleIdentifier) {
-        ModuleInternalTransactionalInfo found = commitMap
-                .remove(moduleIdentifier);
-        if (found == null)
+        ModuleInternalTransactionalInfo found = commitMap.remove(moduleIdentifier);
+        if (found == null) {
             throw new IllegalStateException("Not found:" + moduleIdentifier);
+        }
         if (found.hasOldModule()) {
             unorderedDestroyedFromPreviousTransactions.add(found);
         }
@@ -123,4 +123,13 @@ class ModulesHolder implements TransactionHolder {
     public Collection<ModuleInternalTransactionalInfo> getAllInfos(){
         return commitMap.values();
     }
+
+    @Override
+    public ModuleInternalTransactionalInfo findModuleInternalTransactionalInfo(ModuleIdentifier moduleIdentifier) {
+        ModuleInternalTransactionalInfo found = commitMap.get(moduleIdentifier);
+        if (found == null) {
+            throw new IllegalStateException("Not found:" + moduleIdentifier);
+        }
+        return found;
+    }
 }
index 3d0decb..ec1a290 100644 (file)
@@ -44,7 +44,8 @@ public class BlankTransactionServiceTracker implements ServiceTrackerCustomizer<
         for (int i = 0; i < 10; i++) {
             try {
                 // create transaction
-                ObjectName tx = configRegistry.beginConfig();
+                boolean blankTransaction = true;
+                ObjectName tx = configRegistry.beginConfig(blankTransaction);
                 CommitStatus commitStatus = configRegistry.commitConfig(tx);
                 logger.debug("Committed blank transaction with status {}", commitStatus);
                 return;
index 3cb0f44..22a9590 100644 (file)
@@ -67,7 +67,7 @@ public class ConfigTransactionControllerImplTest extends
         testedTxController = new ConfigTransactionControllerImpl(
                 transactionName123, jmxRegistrator123, 1, 1,
                 currentlyRegisteredFactories, transactionsMBeanServer,
-                ManagementFactory.getPlatformMBeanServer(), null);
+                ManagementFactory.getPlatformMBeanServer(), false);
         TransactionModuleJMXRegistrator transactionModuleJMXRegistrator123 = testedTxController
                 .getTxModuleJMXRegistrator();
         transactionModuleJMXRegistrator123.registerMBean(
index a79cb83..65b0105 100644 (file)
@@ -86,7 +86,7 @@ public class DependencyResolverManagerTest {
     private static void mockGetInstance(DependencyResolverManager tested,
             ModuleIdentifier moduleIdentifier) {
         ModuleInternalTransactionalInfo mock = mock(ModuleInternalTransactionalInfo.class);
-        doReturn(moduleIdentifier).when(mock).getName();
+        doReturn(moduleIdentifier).when(mock).getIdentifier();
         doReturn(mockedModule()).when(mock).getModule();
         tested.put(mock);
     }