BUG-2283 Fix close order when reconfiguring config modules. 43/13343/13
authorMaros Marsalek <mmarsale@cisco.com>
Wed, 3 Dec 2014 15:23:24 +0000 (16:23 +0100)
committerMaros Marsalek <mmarsale@cisco.com>
Tue, 20 Jan 2015 10:15:54 +0000 (10:15 +0000)
Close is called from top(dependency source) to bottom(dependency target) while createInstance is called in opposite direction.

This commit changes API and SPI of config subsystem and thus breaks runtime compatibility.

Change-Id: I5b03f0673c5ecb95efbfccad7fa6ed7a490ff61b
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
33 files changed:
opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/DependencyResolver.java
opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java [new file with mode: 0644]
opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/Module.java
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/dependencyresolver/DependencyResolverImpl.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/ModuleInternalTransactionalInfo.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/AbstractMockedModule.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
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/TestingParallelAPSPModule.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/parallelapsp/test/MockedDependenciesTest.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/scheduledthreadpool/TestingScheduledThreadPoolModule.java
opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/testingservices/threadpool/TestingFixedThreadPoolModule.java
opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/FixedThreadPoolConfigBeanTest.java
opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/fixed/TestingFixedThreadPoolModule.java
opendaylight/config/threadpool-config-impl/src/test/java/org/opendaylight/controller/config/threadpool/naming/TestingNamingThreadPoolFactoryModule.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/AbstractModuleTemplate.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/GeneralClassTemplate.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/ftl/TemplateFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/gofactory/AbsModuleGeneratedObjectFactory.java
opendaylight/config/yang-jmx-generator-plugin/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/plugin/JMXGeneratorTest.java
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/.gitignore
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java [new file with mode: 0644]
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/DepTestImplModuleStub.txt
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/IdentityTestModuleStub.txt
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleStub.txt
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/NetconfTestImplModuleStub.txt
opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/TestImplModuleStub.txt
opendaylight/config/yang-test/src/main/yang/config-test-impl.yang
opendaylight/config/yang-test/src/main/yang/config-test.yang
opendaylight/config/yang-test/src/test/java/org/opendaylight/controller/config/yang/test/impl/MultipleDependenciesModuleTest.java

index 40ff7e17030bc254026574cd34a7a1ca48970819..466f1ed60cafdb1b40b1e4595e4aae57d76f42af 100644 (file)
@@ -51,6 +51,7 @@ public interface DependencyResolver extends Identifiable<ModuleIdentifier> {
     <T> T resolveInstance(Class<T> expectedType, ObjectName objectName,
                           JmxAttribute jmxAttribute);
 
     <T> T resolveInstance(Class<T> expectedType, ObjectName objectName,
                           JmxAttribute jmxAttribute);
 
+
     /**
      * To be used during commit phase to resolve identity-ref config attributes.
      *
     /**
      * To be used during commit phase to resolve identity-ref config attributes.
      *
@@ -86,4 +87,12 @@ public interface DependencyResolver extends Identifiable<ModuleIdentifier> {
      */
     <T> T newMXBeanProxy(ObjectName objectName, Class<T> interfaceClass);
 
      */
     <T> T newMXBeanProxy(ObjectName objectName, Class<T> interfaceClass);
 
+    /**
+     * Check whether a dependency will be reused or (re)created. Useful when deciding if current module could be also reused.
+     *
+     * @param objectName ObjectName ID of a dependency
+     * @param jmxAttribute JMXAttribute ID of a dependency
+     * @return true if the dependency will be reused false otherwise
+     */
+    boolean canReuseDependency(ObjectName objectName, JmxAttribute jmxAttribute);
 }
 }
diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java
new file mode 100644 (file)
index 0000000..772bacc
--- /dev/null
@@ -0,0 +1,119 @@
+package org.opendaylight.controller.config.spi;
+
+import org.opendaylight.controller.config.api.DependencyResolver;
+import org.opendaylight.controller.config.api.ModuleIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base implementation of Module. This implementation contains base logic for Module reconfiguration with associated fields.
+ * @param <M> Type of module implementation. Enables easier implementation for the {@link #isSame} method
+ */
+public abstract class AbstractModule<M extends AbstractModule<M>> implements org.opendaylight.controller.config.spi.Module {
+
+    private static final Logger LOG = LoggerFactory.getLogger(AbstractModule.class);
+
+    private final M oldModule;
+    private final AutoCloseable oldInstance;
+    protected final ModuleIdentifier identifier;
+    private AutoCloseable instance;
+    protected final DependencyResolver dependencyResolver;
+
+    /**
+     * Called when module is configured.
+     *
+     * @param identifier id of current instance.
+     * @param dependencyResolver resolver used in dependency injection and validation.
+     */
+    public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver) {
+        this(identifier, dependencyResolver, null, null);
+    }
+
+    /**
+     * Called when module is reconfigured.
+     *
+     * @param identifier id of current instance.
+     * @param dependencyResolver resolver used in dependency injection and validation.
+     * @param oldModule old instance of module that is being reconfigred(replaced) by current instance. The old instance can be examined for reuse.
+     * @param oldInstance old instance wrapped by the old module. This is the resource that is actually being reused if possible or closed otherwise.
+     */
+    public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver, M oldModule, AutoCloseable oldInstance) {
+        this.identifier = identifier;
+        this.dependencyResolver = dependencyResolver;
+        this.oldModule = oldModule;
+        this.oldInstance = oldInstance;
+    }
+
+    @Override
+    public ModuleIdentifier getIdentifier() {
+        return identifier;
+    }
+
+    /**
+     *
+     * General algorithm for spawning/closing and reusing wrapped instances.
+     *
+     * @return current instance of wrapped resource either by reusing the old one (if present) or constructing a brand new.
+     */
+    @Override
+    public final AutoCloseable getInstance() {
+        if(instance==null) {
+            if(oldInstance!=null && canReuseInstance(oldModule)) {
+                resolveDependencies();
+                instance = reuseInstance(oldInstance);
+            } else {
+                if(oldInstance!=null) {
+                    try {
+                        oldInstance.close();
+                    } catch(Exception e) {
+                        LOG.error("An error occurred while closing old instance {} for module {}", oldInstance, getIdentifier(), e);
+                    }
+                }
+                resolveDependencies();
+                instance = createInstance();
+                if (instance == null) {
+                    throw new IllegalStateException("Error in createInstance - null is not allowed as return value. Module: " + getIdentifier());
+                }
+            }
+        }
+        return instance;
+    }
+
+    /**
+     * @return Brand new instance of wrapped class in case no previous instance is present or reconfiguration is impossible.
+     */
+    protected abstract AutoCloseable createInstance();
+
+    @Override
+    public final boolean canReuse(Module oldModule) {
+        // Just cast into a specific instance
+        // TODO unify this method with canReuseInstance (required Module interface to be generic which requires quite a lot of changes)
+        return getClass().isInstance(oldModule) ? canReuseInstance((M) oldModule) : false;
+    }
+
+    /**
+     *
+     * Users are welcome to override this method to provide custom logic for advanced reusability detection.
+     *
+     * @param oldModule old instance of a Module
+     * @return true if the old instance is reusable false if a new one should be spawned
+     */
+    protected abstract boolean canReuseInstance(final M oldModule);
+
+    /**
+     * By default the oldInstance is returned since this method is by default called only if the oldModule had the same configuration and dependencies configured.
+     * Users are welcome to override this method to provide custom logic for advanced reusability.
+     *
+     * @param oldInstance old instance of a class wrapped by the module
+     * @return reused instance
+     */
+    protected AutoCloseable reuseInstance(AutoCloseable oldInstance) {
+        // implement if instance reuse should be supported. Override canReuseInstance to change the criteria.
+        return oldInstance;
+    }
+
+    /**
+     * Inject all the dependencies using dependency resolver instance.
+     */
+    protected abstract void resolveDependencies();
+}
index b557108d4d00a39b988d7f82e5e4bb3bc41957b3..53f03a2398ef9cf8b854fd0bcc7e6959b8304ba5 100644 (file)
@@ -54,4 +54,17 @@ public interface Module extends Identifiable<ModuleIdentifier>{
      */
     AutoCloseable getInstance();
 
      */
     AutoCloseable getInstance();
 
+
+    /**
+     * Compare current module with oldModule and if the instance/live object
+     * produced by the old module can be reused in this module as well return true.
+     * Typically true should be returned if the old module had the same configuration.
+     *
+     *
+     * @param oldModule old instance of Module
+     * @return true if the instance produced by oldModule can be reused with current instance as well.
+     */
+    public boolean canReuse(Module oldModule);
+
+
 }
 }
index 96739fb822c72ac97c97637a07d9d9620f45c81f..91c67b8b02317681953287fd8a764b6c0694a4f0 100644 (file)
@@ -430,8 +430,7 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
             }
         }
 
             }
         }
 
-        // destroy all live objects one after another in order of the dependency
-        // hierarchy
+        // destroy all live objects one after another in order of the dependency hierarchy, from top to bottom
         List<DestroyedModule> destroyedModules = currentConfig
                 .getModulesToBeDestroyed();
         for (DestroyedModule destroyedModule : destroyedModules) {
         List<DestroyedModule> destroyedModules = currentConfig
                 .getModulesToBeDestroyed();
         for (DestroyedModule destroyedModule : destroyedModules) {
index bb61c4acc70aa6506b6d3093748a519d5fa036be..37c2e2d777d83004b43e9dbd32cb3e9793c61f36 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.controller.config.manager.impl;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.lang.String.format;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.lang.String.format;
 
+import com.google.common.collect.Lists;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
@@ -383,32 +384,34 @@ class ConfigTransactionControllerImpl implements
 
         LOG.trace("Committing transaction {}", getTransactionIdentifier());
 
 
         LOG.trace("Committing transaction {}", getTransactionIdentifier());
 
-        // call getInstance()
-        for (Entry<ModuleIdentifier, Module> entry : dependencyResolverManager
-                .getAllModules().entrySet()) {
-            Module module = entry.getValue();
-            ModuleIdentifier name = entry.getKey();
+        Map<ModuleIdentifier, Module> allModules = dependencyResolverManager.getAllModules();
+
+        // call getInstance() on all Modules from top to bottom (from source to target of the dependency relation)
+        // The source of a dependency closes itself and calls getInstance recursively on the dependencies (in case of reconfiguration)
+        // This makes close() calls from top to bottom while createInstance() calls are performed bottom to top
+        List<ModuleIdentifier> sortedModuleIdentifiers = Lists.reverse(dependencyResolverManager.getSortedModuleIdentifiers());
+        for (ModuleIdentifier moduleIdentifier : sortedModuleIdentifiers) {
+            Module module = allModules.get(moduleIdentifier);
+
             try {
                 LOG.debug("About to commit {} in transaction {}",
             try {
                 LOG.debug("About to commit {} in transaction {}",
-                        name, getTransactionIdentifier());
+                        moduleIdentifier, getTransactionIdentifier());
                 AutoCloseable instance = module.getInstance();
                 AutoCloseable instance = module.getInstance();
-                checkNotNull(instance, "Instance is null:{} in transaction {}", name, getTransactionIdentifier());
+                checkNotNull(instance, "Instance is null:{} in transaction {}", moduleIdentifier, getTransactionIdentifier());
             } catch (Exception e) {
             } catch (Exception e) {
-                LOG.error("Commit failed on {} in transaction {}", name,
+                LOG.error("Commit failed on {} in transaction {}", moduleIdentifier,
                         getTransactionIdentifier(), e);
                 internalAbort();
                 throw new IllegalStateException(
                         format("Error - getInstance() failed for %s in transaction %s",
                         getTransactionIdentifier(), e);
                 internalAbort();
                 throw new IllegalStateException(
                         format("Error - getInstance() failed for %s in transaction %s",
-                                name, getTransactionIdentifier()), e);
+                                moduleIdentifier, getTransactionIdentifier()), e);
             }
         }
 
             }
         }
 
-        // count dependency order
-
         LOG.trace("Committed configuration {}", getTransactionIdentifier());
         transactionStatus.setCommitted();
 
         LOG.trace("Committed configuration {}", getTransactionIdentifier());
         transactionStatus.setCommitted();
 
-        return dependencyResolverManager.getSortedModuleIdentifiers();
+        return sortedModuleIdentifiers;
     }
 
     @Override
     }
 
     @Override
index 0741dab69d8c783459baf12530bcf6153a6a2632..024518ca98f43a544ec2f3f87673033914de3113 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import static java.lang.String.format;
 
 
 import static java.lang.String.format;
 
+import com.google.common.base.Preconditions;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
@@ -151,28 +152,17 @@ final class DependencyResolverImpl implements DependencyResolver,
     @Override
     public <T> T resolveInstance(Class<T> expectedType, ObjectName dependentReadOnlyON,
                                  JmxAttribute jmxAttribute) {
     @Override
     public <T> T resolveInstance(Class<T> expectedType, ObjectName dependentReadOnlyON,
                                  JmxAttribute jmxAttribute) {
-        if (expectedType == null || dependentReadOnlyON == null || jmxAttribute == null) {
-            throw new IllegalArgumentException(format(
-                    "Null parameters not allowed, got %s %s %s", expectedType,
-                    dependentReadOnlyON, jmxAttribute));
-        }
-        ObjectName translatedDependentReadOnlyON = translateServiceRefIfPossible(dependentReadOnlyON);
-        transactionStatus.checkCommitStarted();
-        transactionStatus.checkNotCommitted();
+        Module module = resolveModuleInstance(dependentReadOnlyON, jmxAttribute);
 
 
-        ModuleIdentifier dependentModuleIdentifier = ObjectNameUtil.fromON(
-                translatedDependentReadOnlyON, ObjectNameUtil.TYPE_MODULE);
-        Module module = modulesHolder.findModule(dependentModuleIdentifier,
-                jmxAttribute);
         synchronized (this) {
         synchronized (this) {
-            dependencies.add(dependentModuleIdentifier);
+            dependencies.add(module.getIdentifier());
         }
         AutoCloseable instance = module.getInstance();
         if (instance == null) {
             String message = format(
                     "Error while %s resolving instance %s. getInstance() returned null. "
                             + "Expected type %s , attribute %s", name,
         }
         AutoCloseable instance = module.getInstance();
         if (instance == null) {
             String message = format(
                     "Error while %s resolving instance %s. getInstance() returned null. "
                             + "Expected type %s , attribute %s", name,
-                    dependentModuleIdentifier, expectedType, jmxAttribute
+                    module.getIdentifier(), expectedType, jmxAttribute
             );
             throw new JmxAttributeValidationException(message, jmxAttribute);
         }
             );
             throw new JmxAttributeValidationException(message, jmxAttribute);
         }
@@ -188,6 +178,36 @@ final class DependencyResolverImpl implements DependencyResolver,
         }
     }
 
         }
     }
 
+    private Module resolveModuleInstance(ObjectName dependentReadOnlyON,
+                                 JmxAttribute jmxAttribute) {
+        Preconditions.checkArgument(dependentReadOnlyON != null ,"dependentReadOnlyON");
+        Preconditions.checkArgument(jmxAttribute != null, "jmxAttribute");
+        ObjectName translatedDependentReadOnlyON = translateServiceRefIfPossible(dependentReadOnlyON);
+        transactionStatus.checkCommitStarted();
+        transactionStatus.checkNotCommitted();
+
+        ModuleIdentifier dependentModuleIdentifier = ObjectNameUtil.fromON(
+                translatedDependentReadOnlyON, ObjectNameUtil.TYPE_MODULE);
+
+        return Preconditions.checkNotNull(modulesHolder.findModule(dependentModuleIdentifier, jmxAttribute));
+    }
+
+    @Override
+    public boolean canReuseDependency(ObjectName objectName, JmxAttribute jmxAttribute) {
+        Preconditions.checkNotNull(objectName);
+        Preconditions.checkNotNull(jmxAttribute);
+
+        Module currentModule = resolveModuleInstance(objectName, jmxAttribute);
+        ModuleIdentifier identifier = currentModule.getIdentifier();
+        ModuleInternalTransactionalInfo moduleInternalTransactionalInfo = modulesHolder.findModuleInternalTransactionalInfo(identifier);
+
+        if(moduleInternalTransactionalInfo.hasOldModule()) {
+            Module oldModule = moduleInternalTransactionalInfo.getOldInternalInfo().getReadableModule().getModule();
+            return currentModule.canReuse(oldModule);
+        }
+        return false;
+    }
+
     @Override
     public <T extends BaseIdentity> Class<? extends T> resolveIdentity(IdentityAttributeRef identityRef, Class<T> expectedBaseClass) {
         final QName qName = QName.create(identityRef.getqNameOfIdentity());
     @Override
     public <T extends BaseIdentity> Class<? extends T> resolveIdentity(IdentityAttributeRef identityRef, Class<T> expectedBaseClass) {
         final QName qName = QName.create(identityRef.getqNameOfIdentity());
@@ -217,7 +237,7 @@ final class DependencyResolverImpl implements DependencyResolver,
 
     @Override
     public int compareTo(DependencyResolverImpl o) {
 
     @Override
     public int compareTo(DependencyResolverImpl o) {
-        transactionStatus.checkCommitted();
+        transactionStatus.checkCommitStarted();
         return Integer.compare(getMaxDependencyDepth(),
                 o.getMaxDependencyDepth());
     }
         return Integer.compare(getMaxDependencyDepth(),
                 o.getMaxDependencyDepth());
     }
@@ -232,7 +252,11 @@ final class DependencyResolverImpl implements DependencyResolver,
     }
 
     void countMaxDependencyDepth(DependencyResolverManager manager) {
     }
 
     void countMaxDependencyDepth(DependencyResolverManager manager) {
-        transactionStatus.checkCommitted();
+        // We can calculate the dependency after second phase commit was started
+        // Second phase commit starts after validation and validation adds the dependencies into the dependency resolver, which are necessary for the calculation
+        // FIXME generated code for abstract module declares validate method as non-final
+        // Overriding the validate would cause recreate every time instead of reuse + also possibly wrong close order if there is another module depending
+        transactionStatus.checkCommitStarted();
         if (maxDependencyDepth == null) {
             maxDependencyDepth = getMaxDepth(this, manager,
                     new LinkedHashSet<ModuleIdentifier>());
         if (maxDependencyDepth == null) {
             maxDependencyDepth = getMaxDepth(this, manager,
                     new LinkedHashSet<ModuleIdentifier>());
index da6349ac5386c75ef17a8120a8c5a10b16726367..0014a5924d3268bd72182f756868b448389bcd84 100644 (file)
@@ -88,7 +88,7 @@ public class DependencyResolverManager implements DependencyResolverFactory, Aut
      * things?
      */
     private List<DependencyResolverImpl> getAllSorted() {
      * things?
      */
     private List<DependencyResolverImpl> getAllSorted() {
-        transactionStatus.checkCommitted();
+        transactionStatus.checkCommitStarted();
         List<DependencyResolverImpl> sorted = new ArrayList<>(
                 moduleIdentifiersToDependencyResolverMap.values());
         for (DependencyResolverImpl dri : sorted) {
         List<DependencyResolverImpl> sorted = new ArrayList<>(
                 moduleIdentifiersToDependencyResolverMap.values());
         for (DependencyResolverImpl dri : sorted) {
index 26d8c74fb3384858446ca3e5f4ee147f34e212b2..210b6d2aa702793fddb1d6f25f2c12c45074f6dc 100644 (file)
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
  */
 package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
+import com.google.common.base.Preconditions;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo;
@@ -70,10 +71,7 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
 
     @Nullable
     public ModuleInternalInfo getOldInternalInfo() {
 
     @Nullable
     public ModuleInternalInfo getOldInternalInfo() {
-        if (maybeOldInternalInfo == null) {
-            throw new NullPointerException();
-        }
-        return maybeOldInternalInfo;
+        return Preconditions.checkNotNull(maybeOldInternalInfo);
     }
 
     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
     }
 
     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
index 837ae2b3c86d7ee977648407b142f50dec17c76e..a6e24e9a495fe0df079952144ee440e02732cea0 100644 (file)
@@ -31,6 +31,12 @@ public abstract class AbstractMockedModule implements Module {
         this.id = id==null ? new ModuleIdentifier(getClass().getCanonicalName(), "mock") : id;
     }
 
         this.id = id==null ? new ModuleIdentifier(getClass().getCanonicalName(), "mock") : id;
     }
 
+
+    @Override
+    public boolean canReuse(Module oldModule) {
+        return instance!=null;
+    }
+
     @Override
     public void validate() {
     }
     @Override
     public void validate() {
     }
index 5b2f7ad1e70ea79e515dd6c55ab65cacef593c58..c7b98a14d009b7a5299a1c3f9159acc22f064b34 100644 (file)
@@ -120,5 +120,4 @@ public class ConfigTransactionControllerImplTest extends
                 moduleName124, instanceName134);
         assertEquals(Sets.newHashSet(name1), beans);
     }
                 moduleName124, instanceName134);
         assertEquals(Sets.newHashSet(name1), beans);
     }
-
-}
+}
\ No newline at end of file
index 0029f26308d080ed12670b2725250669e28fdbc6..63a33a30315b55a3ad1ac65c9bc99a6691dec609 100644 (file)
@@ -69,7 +69,10 @@ public class DependencyResolverManagerTest extends AbstractLockedPlatformMBeanSe
 
         // switch to second phase committed
         reset(transactionStatus);
 
         // switch to second phase committed
         reset(transactionStatus);
+        doNothing().when(transactionStatus).checkCommitStarted();
         doNothing().when(transactionStatus).checkCommitted();
         doNothing().when(transactionStatus).checkCommitted();
+        doNothing().when(transactionStatus).checkNotCommitted();
+
         List<ModuleIdentifier> sortedModuleIdentifiers = tested
                 .getSortedModuleIdentifiers();
         assertEquals(
         List<ModuleIdentifier> sortedModuleIdentifiers = tested
                 .getSortedModuleIdentifiers();
         assertEquals(
@@ -109,6 +112,7 @@ public class DependencyResolverManagerTest extends AbstractLockedPlatformMBeanSe
     private static Module mockedModule() {
         Module mockedModule = mock(Module.class);
         doReturn(mock(AutoCloseable.class)).when(mockedModule).getInstance();
     private static Module mockedModule() {
         Module mockedModule = mock(Module.class);
         doReturn(mock(AutoCloseable.class)).when(mockedModule).getInstance();
+        doReturn(new ModuleIdentifier("fact", "instance")).when(mockedModule).getIdentifier();
         return mockedModule;
     }
 
         return mockedModule;
     }
 
index d25956b774b85ac965d68b4ce74a5cbcb6457859..1857aaa22f70fbaa5acd7e807b8d9cefc14a1a15 100644 (file)
@@ -146,6 +146,11 @@ public class TestingParallelAPSPModule implements Module,
         return instance;
     }
 
         return instance;
     }
 
+    @Override
+    public boolean canReuse(final Module oldModule) {
+        return false;
+    }
+
     @Override
     public ModuleIdentifier getIdentifier() {
         return identifier;
     @Override
     public ModuleIdentifier getIdentifier() {
         return identifier;
index 7deade5de25b79d97cbf79b4c0635e8cc3b97f6c..95de90d3af59f92656b9256bb4b62d7e1fb9dda9 100644 (file)
@@ -73,6 +73,11 @@ public class MockedDependenciesTest extends AbstractParallelAPSPTest {
 
         }
 
 
         }
 
+        @Override
+        public boolean canReuse(Module oldModule) {
+            return false;
+        }
+
         @Override
         public Closeable getInstance() {
             return new MockedThreadPool(threadCount);
         @Override
         public Closeable getInstance() {
             return new MockedThreadPool(threadCount);
index 1811e7dd949625c8454d5dd6856bd39da4c40481..e35509d09025854e6f205b65f0ba05d6778cd754 100644 (file)
@@ -61,6 +61,11 @@ public class TestingScheduledThreadPoolModule implements Module,
                 "Parameter 'ThreadCount' must be greater than 0");
     }
 
                 "Parameter 'ThreadCount' must be greater than 0");
     }
 
+    @Override
+    public boolean canReuse(final Module oldModule) {
+        return false;
+    }
+
     @Override
     public int getThreadCount() {
         return threadCount;
     @Override
     public int getThreadCount() {
         return threadCount;
index 464f786e6f61c55ade0b3047b58e95fc43720515..c773dd232bb0e12341286e0a762420935a8fb333 100644 (file)
@@ -69,6 +69,11 @@ public class TestingFixedThreadPoolModule implements
                 "Parameter 'threadCount' must be greater than 0");
     }
 
                 "Parameter 'threadCount' must be greater than 0");
     }
 
+    @Override
+    public boolean canReuse(final Module oldModule) {
+        return isReusable() && triggerNewInstanceCreation == false;
+    }
+
     @Override
     public Closeable getInstance() {
         if (instance == null) {
     @Override
     public Closeable getInstance() {
         if (instance == null) {
index 64e83a544f7455116a2be6f8145af00ffd9218a2..5bf4ac2d7bc33742d9fc6eb009b4f7f190cd7198 100644 (file)
@@ -40,6 +40,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
 
     private FixedThreadPoolModuleFactory factory;
     private final String nameInstance = "fixedInstance";
 
     private FixedThreadPoolModuleFactory factory;
     private final String nameInstance = "fixedInstance";
+    private ObjectName threadFactoryON;
 
     @Before
     public void setUp() {
 
     @Before
     public void setUp() {
@@ -73,10 +74,12 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
         assertBeanCount(1, factory.getImplementationName());
 
         transaction = configRegistryClient.createTransaction();
         assertBeanCount(1, factory.getImplementationName());
 
         transaction = configRegistryClient.createTransaction();
+        NamingThreadFactoryModuleMXBean namingThreadFactoryModuleMXBean = transaction.newMXBeanProxy(threadFactoryON, NamingThreadFactoryModuleMXBean.class);
+        namingThreadFactoryModuleMXBean.setNamePrefix("newPrefix");
         CommitStatus status = transaction.commit();
 
         assertBeanCount(1, factory.getImplementationName());
         CommitStatus status = transaction.commit();
 
         assertBeanCount(1, factory.getImplementationName());
-        assertStatus(status, 0, 0, 2);
+        assertStatus(status, 0, 2, 0);
     }
 
     @Test
     }
 
     @Test
@@ -161,7 +164,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
         FixedThreadPoolModuleMXBean mxBean = transaction.newMXBeanProxy(nameCreated, FixedThreadPoolModuleMXBean.class);
         mxBean.setMaxThreadCount(numberOfThreads);
 
         FixedThreadPoolModuleMXBean mxBean = transaction.newMXBeanProxy(nameCreated, FixedThreadPoolModuleMXBean.class);
         mxBean.setMaxThreadCount(numberOfThreads);
 
-        ObjectName threadFactoryON = transaction.createModule(NamingThreadFactoryModuleFactory.NAME, "naming");
+        threadFactoryON = transaction.createModule(NamingThreadFactoryModuleFactory.NAME, "naming");
         NamingThreadFactoryModuleMXBean namingThreadFactoryModuleMXBean = transaction.newMXBeanProxy(threadFactoryON,
                 NamingThreadFactoryModuleMXBean.class);
         namingThreadFactoryModuleMXBean.setNamePrefix(prefix);
         NamingThreadFactoryModuleMXBean namingThreadFactoryModuleMXBean = transaction.newMXBeanProxy(threadFactoryON,
                 NamingThreadFactoryModuleMXBean.class);
         namingThreadFactoryModuleMXBean.setNamePrefix(prefix);
index d6171263af78dec29db330149c9e324fee74ed85..bd57fe98f6efa976f43aef4140af64429db763b8 100644 (file)
@@ -32,5 +32,4 @@ public class TestingFixedThreadPoolModule extends AbstractMockedModule implement
         doReturn(mock(ExecutorService.class)).when(pool).getExecutor();
         return pool;
     }
         doReturn(mock(ExecutorService.class)).when(pool).getExecutor();
         return pool;
     }
-
 }
 }
index 37fb83907036749ff7f95e38ca90ce2fd9440e64..f91ec47ad911f1c2d12bce912fa467864d55fb59 100644 (file)
@@ -62,4 +62,9 @@ public class TestingNamingThreadPoolFactoryModule implements Module, ThreadFacto
         return fact;
     }
 
         return fact;
     }
 
+    @Override
+    public boolean canReuse(Module oldModule) {
+        return false;
+    }
+
 }
 }
index 2402014fd34509a32169d3efabb9d9722f76a091..487bf63e62f9b1806b01c0402cfa6c158035ca75 100644 (file)
@@ -22,13 +22,12 @@ public class AbstractModuleTemplate extends GeneralClassTemplate {
     private final String registratorType;
 
     public AbstractModuleTemplate(Header header, String packageName,
     private final String registratorType;
 
     public AbstractModuleTemplate(Header header, String packageName,
-            String abstractModuleName, List<String> implementedIfcs,
-            List<ModuleField> moduleFields, List<MethodDefinition> methods,
+            String abstractModuleName, List<String> extendedClasses,
+            List<String> implementedIfcs, List<ModuleField> moduleFields, List<MethodDefinition> methods,
             boolean isRuntime, String registratorType) {
             boolean isRuntime, String registratorType) {
-        super(header, packageName, abstractModuleName, Collections
-                .<String> emptyList(), implementedIfcs, Collections
-                .<Field> emptyList(), methods, true, false, Collections
-                .<Constructor> emptyList());
+        super(header, packageName, abstractModuleName, extendedClasses,
+                implementedIfcs, Collections.<Field> emptyList(), methods,
+                true, false, Collections.<Constructor> emptyList());
         this.moduleFields = moduleFields;
         this.runtime = isRuntime;
         this.registratorType = registratorType;
         this.moduleFields = moduleFields;
         this.runtime = isRuntime;
         this.registratorType = registratorType;
index 3f4917f2ffcc3bab9fb7014d763aa9b0ccdb87da..4adb2b0cf590b1d5a7b8ccba9707e00d72c2ee4e 100644 (file)
@@ -40,7 +40,7 @@ public class GeneralClassTemplate extends AbstractFtlTemplate {
     static List<String> checkCardinality(List<String> extendedClass) {
         if (extendedClass.size() > 1) {
             throw new IllegalArgumentException(
     static List<String> checkCardinality(List<String> extendedClass) {
         if (extendedClass.size() > 1) {
             throw new IllegalArgumentException(
-                    "Class cannot have more than one super " + "class");
+                    "Class cannot have more than one super class, found: " + extendedClass);
         }
         return extendedClass;
     }
         }
         return extendedClass;
     }
index 53ab4ef335973029568f0820968c830d59112f17..19e875f9b1805f851322e82ebb565752c9d710cc 100644 (file)
@@ -23,7 +23,7 @@ import org.opendaylight.controller.config.api.IdentityAttributeRef;
 import org.opendaylight.controller.config.api.RuntimeBeanRegistratorAwareModule;
 import org.opendaylight.controller.config.api.annotations.AbstractServiceInterface;
 import org.opendaylight.controller.config.api.runtime.RuntimeBean;
 import org.opendaylight.controller.config.api.RuntimeBeanRegistratorAwareModule;
 import org.opendaylight.controller.config.api.annotations.AbstractServiceInterface;
 import org.opendaylight.controller.config.api.runtime.RuntimeBean;
-import org.opendaylight.controller.config.spi.Module;
+import org.opendaylight.controller.config.spi.AbstractModule;
 import org.opendaylight.controller.config.yangjmxgenerator.AbstractEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.RuntimeBeanEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.AbstractEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.RuntimeBeanEntry;
@@ -196,7 +196,6 @@ public class TemplateFactory {
 
         List<ModuleField> moduleFields = attrProcessor.getModuleFields();
         List<String> implementedIfcs = Lists.newArrayList(
 
         List<ModuleField> moduleFields = attrProcessor.getModuleFields();
         List<String> implementedIfcs = Lists.newArrayList(
-                Module.class.getCanonicalName(),
                 mbe.getFullyQualifiedName(mbe.getMXBeanInterfaceName()));
 
         for (String implementedService : mbe.getProvidedServices().keySet()) {
                 mbe.getFullyQualifiedName(mbe.getMXBeanInterfaceName()));
 
         for (String implementedService : mbe.getProvidedServices().keySet()) {
@@ -218,9 +217,11 @@ public class TemplateFactory {
                     .getCanonicalName());
         }
 
                     .getCanonicalName());
         }
 
+        List<String> extendedClasses = Collections.singletonList(AbstractModule.class.getCanonicalName() + "<" + mbe.getAbstractModuleName() + ">");
+
         AbstractModuleTemplate abstractModuleTemplate = new AbstractModuleTemplate(
                 getHeaderFromEntry(mbe), mbe.getPackageName(),
         AbstractModuleTemplate abstractModuleTemplate = new AbstractModuleTemplate(
                 getHeaderFromEntry(mbe), mbe.getPackageName(),
-                mbe.getAbstractModuleName(), implementedIfcs, moduleFields,
+                mbe.getAbstractModuleName(), extendedClasses, implementedIfcs, moduleFields,
                 attrProcessor.getMethods(), generateRuntime,
                 registratorFullyQualifiedName);
 
                 attrProcessor.getMethods(), generateRuntime,
                 registratorFullyQualifiedName);
 
index 64a13d3f80e9e0e4b33468512647a0636400d690..92a0eec5866877834f4fef563e9d3a3e06acc3aa 100644 (file)
@@ -10,9 +10,13 @@ package org.opendaylight.controller.config.yangjmxgenerator.plugin.gofactory;
 import static com.google.common.base.Preconditions.checkState;
 import static java.lang.String.format;
 
 import static com.google.common.base.Preconditions.checkState;
 import static java.lang.String.format;
 
+import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
-import java.util.ArrayList;
+import com.google.common.collect.Collections2;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -25,8 +29,10 @@ import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.AbstractModuleTemplate;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.TemplateFactory;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Annotation;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.AbstractModuleTemplate;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.TemplateFactory;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Annotation;
+import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Field;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.IdentityRefModuleField;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Method;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.IdentityRefModuleField;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.Method;
+import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.MethodDefinition;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.ModuleField;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.FullyQualifiedName;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.GeneratedObject;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.ftl.model.ModuleField;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.FullyQualifiedName;
 import org.opendaylight.controller.config.yangjmxgenerator.plugin.java.GeneratedObject;
@@ -39,16 +45,26 @@ import org.slf4j.LoggerFactory;
 
 public class AbsModuleGeneratedObjectFactory {
 
 
 public class AbsModuleGeneratedObjectFactory {
 
+    private static final Function<String, FullyQualifiedName> FULLY_QUALIFIED_NAME_FUNCTION = new Function<String, FullyQualifiedName>() {
+        @Override
+        public FullyQualifiedName apply(final String input) {
+            return FullyQualifiedName.fromString(input);
+        }
+    };
+
     public GeneratedObject toGeneratedObject(ModuleMXBeanEntry mbe, Optional<String> copyright) {
         FullyQualifiedName abstractFQN = new FullyQualifiedName(mbe.getPackageName(), mbe.getAbstractModuleName());
         Optional<String> classJavaDoc = Optional.fromNullable(mbe.getNullableDescription());
         AbstractModuleTemplate abstractModuleTemplate = TemplateFactory.abstractModuleTemplateFromMbe(mbe);
         Optional<String> header = abstractModuleTemplate.getHeaderString();
 
     public GeneratedObject toGeneratedObject(ModuleMXBeanEntry mbe, Optional<String> copyright) {
         FullyQualifiedName abstractFQN = new FullyQualifiedName(mbe.getPackageName(), mbe.getAbstractModuleName());
         Optional<String> classJavaDoc = Optional.fromNullable(mbe.getNullableDescription());
         AbstractModuleTemplate abstractModuleTemplate = TemplateFactory.abstractModuleTemplateFromMbe(mbe);
         Optional<String> header = abstractModuleTemplate.getHeaderString();
 
-        List<FullyQualifiedName> implementedInterfaces = new ArrayList<>();
-        for(String implemented: abstractModuleTemplate.getTypeDeclaration().getImplemented()) {
-            implementedInterfaces.add(FullyQualifiedName.fromString(implemented));
-        }
+        List<FullyQualifiedName> implementedInterfaces = Lists.transform(abstractModuleTemplate.getTypeDeclaration().getImplemented(), FULLY_QUALIFIED_NAME_FUNCTION);
+
+        Optional<FullyQualifiedName> extended =
+                Optional.fromNullable(
+                        Iterables.getFirst(
+                                Collections2.transform(abstractModuleTemplate.getTypeDeclaration().getExtended(), FULLY_QUALIFIED_NAME_FUNCTION), null));
+
         Optional<FullyQualifiedName> maybeRegistratorType;
         if (abstractModuleTemplate.isRuntime()) {
             maybeRegistratorType = Optional.of(FullyQualifiedName.fromString(abstractModuleTemplate.getRegistratorType()));
         Optional<FullyQualifiedName> maybeRegistratorType;
         if (abstractModuleTemplate.isRuntime()) {
             maybeRegistratorType = Optional.of(FullyQualifiedName.fromString(abstractModuleTemplate.getRegistratorType()));
@@ -56,7 +72,7 @@ public class AbsModuleGeneratedObjectFactory {
             maybeRegistratorType = Optional.absent();
         }
 
             maybeRegistratorType = Optional.absent();
         }
 
-        return toGeneratedObject(abstractFQN, copyright, header, classJavaDoc, implementedInterfaces,
+        return toGeneratedObject(abstractFQN, copyright, header, classJavaDoc, extended, implementedInterfaces,
                 abstractModuleTemplate.getModuleFields(), maybeRegistratorType, abstractModuleTemplate.getMethods(),
                 mbe.getYangModuleQName());
     }
                 abstractModuleTemplate.getModuleFields(), maybeRegistratorType, abstractModuleTemplate.getMethods(),
                 mbe.getYangModuleQName());
     }
@@ -65,6 +81,7 @@ public class AbsModuleGeneratedObjectFactory {
                                              Optional<String> copyright,
                                              Optional<String> header,
                                              Optional<String> classJavaDoc,
                                              Optional<String> copyright,
                                              Optional<String> header,
                                              Optional<String> classJavaDoc,
+                                             Optional<FullyQualifiedName> extended,
                                              List<FullyQualifiedName> implementedInterfaces,
                                              List<ModuleField> moduleFields,
                                              Optional<FullyQualifiedName> maybeRegistratorType,
                                              List<FullyQualifiedName> implementedInterfaces,
                                              List<ModuleField> moduleFields,
                                              Optional<FullyQualifiedName> maybeRegistratorType,
@@ -84,6 +101,9 @@ public class AbsModuleGeneratedObjectFactory {
         for(FullyQualifiedName implemented: implementedInterfaces) {
             b.addImplementsFQN(implemented);
         }
         for(FullyQualifiedName implemented: implementedInterfaces) {
             b.addImplementsFQN(implemented);
         }
+        if(extended.isPresent()) {
+            b.addExtendsFQN(extended.get());
+        }
         if (classJavaDoc.isPresent()) {
             b.addClassAnnotation(format("@%s(value=\"%s\")", Description.class.getCanonicalName(), classJavaDoc.get()));
         }
         if (classJavaDoc.isPresent()) {
             b.addClassAnnotation(format("@%s(value=\"%s\")", Description.class.getCanonicalName(), classJavaDoc.get()));
         }
@@ -99,9 +119,6 @@ public class AbsModuleGeneratedObjectFactory {
         b.addToBody("//attributes end");
 
 
         b.addToBody("//attributes end");
 
 
-        b.addToBody(getCommonFields(abstractFQN));
-
-
         b.addToBody(getNewConstructor(abstractFQN));
         b.addToBody(getCopyFromOldConstructor(abstractFQN));
 
         b.addToBody(getNewConstructor(abstractFQN));
         b.addToBody(getCopyFromOldConstructor(abstractFQN));
 
@@ -110,11 +127,12 @@ public class AbsModuleGeneratedObjectFactory {
 
         b.addToBody(getCachesOfResolvedDependencies(moduleFields));
         b.addToBody(getCachesOfResolvedIdentityRefs(moduleFields));
 
         b.addToBody(getCachesOfResolvedDependencies(moduleFields));
         b.addToBody(getCachesOfResolvedIdentityRefs(moduleFields));
-        b.addToBody(getGetInstance(moduleFields));
+        b.addToBody(getResolveDependencies(moduleFields));
         b.addToBody(getReuseLogic(moduleFields, abstractFQN));
         b.addToBody(getEqualsAndHashCode(abstractFQN));
 
         b.addToBody(getMethods(methods));
         b.addToBody(getReuseLogic(moduleFields, abstractFQN));
         b.addToBody(getEqualsAndHashCode(abstractFQN));
 
         b.addToBody(getMethods(methods));
+        b.addToBody(getGetLogger());
 
         return new GeneratedObjectBuilder(b.build()).toGeneratedObject();
     }
 
         return new GeneratedObjectBuilder(b.build()).toGeneratedObject();
     }
@@ -163,28 +181,31 @@ public class AbsModuleGeneratedObjectFactory {
         // loop through fields, do deep equals on each field
 
         for (ModuleField moduleField : moduleFields) {
         // loop through fields, do deep equals on each field
 
         for (ModuleField moduleField : moduleFields) {
+            result += format(
+                "if (java.util.Objects.deepEquals(%s, other.%1$s) == false) {\n"+
+                    "return false;\n"+
+                "}\n", moduleField.getName());
+
             if (moduleField.isListOfDependencies()) {
                 result += format(
             if (moduleField.isListOfDependencies()) {
                 result += format(
-                    "if (%1$sDependency.equals(other.%1$sDependency) == false) {\n"+
-                        "return false;\n"+
-                    "}\n"+
-                    "for (int idx = 0; idx < %1$sDependency.size(); idx++) {\n"+
-                        "if (%1$sDependency.get(idx) != other.%1$sDependency.get(idx)) {\n"+
-                            "return false;\n"+
-                        "}\n"+
-                    "}\n" ,moduleField.getName());
+                        "for (int idx = 0; idx < %1$s.size(); idx++) {\n"+
+                            "if (!dependencyResolver.canReuseDependency(%1$s.get(idx), %1$sJmxAttribute)) {\n"+
+                                "return false;\n"+
+                            "}\n"+
+                        "}\n" , moduleField.getName());
             } else if (moduleField.isDependent()) {
                 result += format(
             } else if (moduleField.isDependent()) {
                 result += format(
-                    "if (%sDependency != other.%1$sDependency) { // reference to dependency must be same\n"+
-                        "return false;\n"+
-                    "}\n",moduleField.getName());
-            } else {
-                result += format(
-                    "if (java.util.Objects.deepEquals(%s, other.%1$s) == false) {\n"+
-                        "return false;\n"+
-                    "}\n", moduleField.getName());
+                        // If a reference is null (ie optional reference) it makes no sens to call canReuse on it
+                        // In such case we continue in the isSame method because if we have null here, the previous value was null as well
+                        // If the previous value was not null and current is or vice verse, the deepEquals comparison would return false
+                        "if(%1$s!= null) {\n" +
+                            "if (!dependencyResolver.canReuseDependency(%1$s, %1$sJmxAttribute)) { // reference to dependency must be reusable as well\n" +
+                                "return false;\n" +
+                            "}\n" +
+                        "}\n", moduleField.getName());
             }
         }
             }
         }
+
         result += "\n"+
                 "return true;\n"+
             "}\n";
         result += "\n"+
                 "return true;\n"+
             "}\n";
@@ -192,13 +213,7 @@ public class AbsModuleGeneratedObjectFactory {
         return result;
     }
 
         return result;
     }
 
-    private static String getGetInstance(List<ModuleField> moduleFields) {
-        String result = "\n"+
-            "@Override\n"+
-            format("public final %s getInstance() {\n", AutoCloseable.class.getCanonicalName())+
-                "if(instance==null) {\n";
-        // create instance start
-
+    private static String getResolveDependencies(final List<ModuleField> moduleFields) {
         // loop through dependent fields, use dependency resolver to instantiate dependencies. Do it in loop in case field represents list of dependencies.
         Map<ModuleField, String> resolveDependenciesMap = new HashMap<>();
         for(ModuleField moduleField: moduleFields) {
         // loop through dependent fields, use dependency resolver to instantiate dependencies. Do it in loop in case field represents list of dependencies.
         Map<ModuleField, String> resolveDependenciesMap = new HashMap<>();
         for(ModuleField moduleField: moduleFields) {
@@ -207,19 +222,21 @@ public class AbsModuleGeneratedObjectFactory {
                 String osgi = moduleField.getDependency().getSie().getExportedOsgiClassName();
                 if (moduleField.isList()) {
                     str = format(
                 String osgi = moduleField.getDependency().getSie().getExportedOsgiClassName();
                 if (moduleField.isList()) {
                     str = format(
-                        "%sDependency = new java.util.ArrayList<%s>();\n"+
-                        "for(javax.management.ObjectName dep : %1$s) {\n"+
-                            "%1$sDependency.add(dependencyResolver.resolveInstance(%2$s.class, dep, %1$sJmxAttribute));\n"+
-                        "}\n", moduleField.getName(), osgi);
+                            "%sDependency = new java.util.ArrayList<%s>();\n"+
+                                    "for(javax.management.ObjectName dep : %1$s) {\n"+
+                                    "%1$sDependency.add(dependencyResolver.resolveInstance(%2$s.class, dep, %1$sJmxAttribute));\n"+
+                                    "}\n", moduleField.getName(), osgi);
                 } else {
                     str = format(
                 } else {
                     str = format(
-                        "%1$sDependency = dependencyResolver.resolveInstance(%2$s.class, %1$s, %1$sJmxAttribute);\n",
-                        moduleField.getName(), osgi);
+                            "%1$sDependency = dependencyResolver.resolveInstance(%2$s.class, %1$s, %1$sJmxAttribute);\n",
+                            moduleField.getName(), osgi);
                 }
                 resolveDependenciesMap.put(moduleField, str);
             }
         }
 
                 }
                 resolveDependenciesMap.put(moduleField, str);
             }
         }
 
+        String result = "\n"
+                + "protected final void resolveDependencies() {\n";
         // wrap each field resolvation statement with if !=null when dependency is not mandatory
         for (Map.Entry<ModuleField, String> entry : resolveDependenciesMap.entrySet()) {
             if (entry.getKey().getDependency().isMandatory() == false) {
         // wrap each field resolvation statement with if !=null when dependency is not mandatory
         for (Map.Entry<ModuleField, String> entry : resolveDependenciesMap.entrySet()) {
             if (entry.getKey().getDependency().isMandatory() == false) {
@@ -236,9 +253,9 @@ public class AbsModuleGeneratedObjectFactory {
                 result += format("if (%s!=null){\n", moduleField.getName());
                 if (moduleField.isList()) {
                     result += format(
                 result += format("if (%s!=null){\n", moduleField.getName());
                 if (moduleField.isList()) {
                     result += format(
-                        "for(%s candidate : %s) {\n"+
-                            "candidate.injectDependencyResolver(dependencyResolver);\n"+
-                        "}\n", moduleField.getGenericInnerType(), moduleField.getName());
+                            "for(%s candidate : %s) {\n"+
+                                    "candidate.injectDependencyResolver(dependencyResolver);\n"+
+                                    "}\n", moduleField.getGenericInnerType(), moduleField.getName());
                 } else {
                     result += format("%s.injectDependencyResolver(dependencyResolver);\n", moduleField.getName());
                 }
                 } else {
                     result += format("%s.injectDependencyResolver(dependencyResolver);\n", moduleField.getName());
                 }
@@ -256,44 +273,10 @@ public class AbsModuleGeneratedObjectFactory {
                 result += "}\n";
             }
         }
                 result += "}\n";
             }
         }
-
-        // create instance end: reuse and recreate logic
-        result +=   "if(oldInstance!=null && canReuseInstance(oldModule)) {\n"+
-                        "instance = reuseInstance(oldInstance);\n"+
-                    "} else {\n"+
-                        "if(oldInstance!=null) {\n"+
-                           "try {\n"+
-                                "oldInstance.close();\n"+
-                            "} catch(Exception e) {\n"+
-                                "logger.error(\"An error occurred while closing old instance \" + oldInstance, e);\n"+
-                            "}\n"+
-                        "}\n"+
-                        "instance = createInstance();\n"+
-                        "if (instance == null) {\n"+
-                            "throw new IllegalStateException(\"Error in createInstance - null is not allowed as return value\");\n"+
-                        "}\n"+
-                    "}\n"+
-                "}\n"+
-                "return instance;\n"+
-            "}\n"+
-            format("public abstract %s createInstance();\n", AutoCloseable.class.getCanonicalName());
-
+        result += "}\n";
         return result;
     }
 
         return result;
     }
 
-    private static String getCommonFields(FullyQualifiedName abstractFQN) {
-        return "\n"+
-            format("private final %s oldModule;\n", abstractFQN.getTypeName())+
-            format("private final %s oldInstance;\n", AutoCloseable.class.getCanonicalName())+
-            format("private %s instance;\n", AutoCloseable.class.getCanonicalName())+
-            format("protected final %s dependencyResolver;\n", DependencyResolver.class.getCanonicalName())+
-            format("private final %s identifier;\n", ModuleIdentifier.class.getCanonicalName())+
-            "@Override\n"+
-            format("public %s getIdentifier() {\n", ModuleIdentifier.class.getCanonicalName())+
-                "return identifier;\n"+
-            "}\n";
-    }
-
     private static String getCachesOfResolvedIdentityRefs(List<ModuleField> moduleFields) {
         StringBuilder result = new StringBuilder();
         for (ModuleField moduleField : moduleFields) {
     private static String getCachesOfResolvedIdentityRefs(List<ModuleField> moduleFields) {
         StringBuilder result = new StringBuilder();
         for (ModuleField moduleField : moduleFields) {
@@ -355,7 +338,7 @@ public class AbsModuleGeneratedObjectFactory {
             "public void validate() {\n";
         // validate each mandatory dependency
         for(ModuleField moduleField: moduleFields) {
             "public void validate() {\n";
         // validate each mandatory dependency
         for(ModuleField moduleField: moduleFields) {
-            if (moduleField.isDependent() && moduleField.getDependency().isMandatory()) {
+            if (moduleField.isDependent()) {
                 if (moduleField.isList()) {
                     result += "" +
                             format("for(javax.management.ObjectName dep : %s) {\n", moduleField.getName()) +
                 if (moduleField.isList()) {
                     result += "" +
                             format("for(javax.management.ObjectName dep : %s) {\n", moduleField.getName()) +
@@ -363,8 +346,14 @@ public class AbsModuleGeneratedObjectFactory {
                                     moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName()) +
                             "}\n";
                 } else {
                                     moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName()) +
                             "}\n";
                 } else {
-                    result += format("dependencyResolver.validateDependency(%s.class, %s, %sJmxAttribute);",
+                    if(moduleField.getDependency().isMandatory() == false) {
+                        result += format("if(%s != null) {\n", moduleField.getName());
+                    }
+                    result += format("dependencyResolver.validateDependency(%s.class, %s, %sJmxAttribute);\n",
                             moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName(), moduleField.getName());
                             moduleField.getDependency().getSie().getFullyQualifiedName(), moduleField.getName(), moduleField.getName());
+                    if(moduleField.getDependency().isMandatory() == false) {
+                        result += "}\n";
+                    }
                 }
             }
         }
                 }
             }
         }
@@ -378,7 +367,7 @@ public class AbsModuleGeneratedObjectFactory {
     }
 
     private static String getLoggerDefinition(FullyQualifiedName fqn) {
     }
 
     private static String getLoggerDefinition(FullyQualifiedName fqn) {
-        return format("private static final %s logger = %s.getLogger(%s.class);",
+        return format("private static final %s LOGGER = %s.getLogger(%s.class);",
                 Logger.class.getCanonicalName(), LoggerFactory.class.getCanonicalName(), fqn);
     }
 
                 Logger.class.getCanonicalName(), LoggerFactory.class.getCanonicalName(), fqn);
     }
 
@@ -386,14 +375,9 @@ public class AbsModuleGeneratedObjectFactory {
     private static String getConstructorStart(FullyQualifiedName fqn,
                                               LinkedHashMap<String, String> parameters, String after) {
         String paramString = Joiner.on(",").withKeyValueSeparator(" ").join(parameters);
     private static String getConstructorStart(FullyQualifiedName fqn,
                                               LinkedHashMap<String, String> parameters, String after) {
         String paramString = Joiner.on(",").withKeyValueSeparator(" ").join(parameters);
-        String setters = "";
-        for (String paramName : parameters.values()) {
-            setters += format("this.%s = %1$s;\n", paramName);
-        }
         return format("public %s(", fqn.getTypeName()) +
                 paramString +
                 ") {\n" +
         return format("public %s(", fqn.getTypeName()) +
                 paramString +
                 ") {\n" +
-                setters +
                 after +
                 "}\n";
     }
                 after +
                 "}\n";
     }
@@ -402,10 +386,8 @@ public class AbsModuleGeneratedObjectFactory {
         LinkedHashMap<String, String> parameters = new LinkedHashMap<>();
         parameters.put(ModuleIdentifier.class.getCanonicalName(), "identifier");
         parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver");
         LinkedHashMap<String, String> parameters = new LinkedHashMap<>();
         parameters.put(ModuleIdentifier.class.getCanonicalName(), "identifier");
         parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver");
-
-        String setToNulls = "this.oldInstance=null;\n" +
-                "this.oldModule=null;\n";
-        return getConstructorStart(abstractFQN, parameters, setToNulls);
+        String init = "super(identifier, dependencyResolver);\n";
+        return getConstructorStart(abstractFQN, parameters, init);
     }
 
     private static String getCopyFromOldConstructor(FullyQualifiedName abstractFQN) {
     }
 
     private static String getCopyFromOldConstructor(FullyQualifiedName abstractFQN) {
@@ -414,6 +396,11 @@ public class AbsModuleGeneratedObjectFactory {
         parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver");
         parameters.put(abstractFQN.getTypeName(), "oldModule");
         parameters.put(AutoCloseable.class.getCanonicalName(), "oldInstance");
         parameters.put(DependencyResolver.class.getCanonicalName(), "dependencyResolver");
         parameters.put(abstractFQN.getTypeName(), "oldModule");
         parameters.put(AutoCloseable.class.getCanonicalName(), "oldInstance");
-        return getConstructorStart(abstractFQN, parameters, "");
+        String init = "super(identifier, dependencyResolver, oldModule, oldInstance);\n";
+        return getConstructorStart(abstractFQN, parameters, init);
+    }
+
+    public String getGetLogger() {
+        return new MethodDefinition(Logger.class.getCanonicalName(), "getLogger", Collections.<Field>emptyList(), "return LOGGER;").toString();
     }
 }
     }
 }
index 8c2b5d69ebdbd9c722ebc381b69368ccead924a6..a6cfc58c34e96a06626393c49774dcf504c5e5b8 100644 (file)
@@ -63,6 +63,7 @@ import org.opendaylight.controller.config.api.DynamicMBeanWithInstance;
 import org.opendaylight.controller.config.api.annotations.Description;
 import org.opendaylight.controller.config.api.annotations.RequireInterface;
 import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation;
 import org.opendaylight.controller.config.api.annotations.Description;
 import org.opendaylight.controller.config.api.annotations.RequireInterface;
 import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnotation;
+import org.opendaylight.controller.config.spi.AbstractModule;
 import org.opendaylight.controller.config.spi.Module;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.opendaylight.controller.config.yangjmxgenerator.ConfigConstants;
 import org.opendaylight.controller.config.spi.Module;
 import org.opendaylight.controller.config.spi.ModuleFactory;
 import org.opendaylight.controller.config.yangjmxgenerator.ConfigConstants;
@@ -520,11 +521,11 @@ public class JMXGeneratorTest extends AbstractGeneratorTest {
         assertContains(visitor.implmts,
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.java.DynamicThreadPoolModuleMXBean",
         assertContains(visitor.implmts,
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.java.DynamicThreadPoolModuleMXBean",
-                Module.class.getCanonicalName(),
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.ScheduledThreadPoolServiceInterface",
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.ThreadPoolServiceInterface");
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.ScheduledThreadPoolServiceInterface",
                 PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                         + ".threads.ThreadPoolServiceInterface");
+        assertContains(visitor.extnds, AbstractModule.class.getCanonicalName());
         assertEquals(2, visitor.constructors.size());
         Set<String> fieldDeclarations = visitor.fieldDeclarations;
         assertDeclaredField(fieldDeclarations,
         assertEquals(2, visitor.constructors.size());
         Set<String> fieldDeclarations = visitor.fieldDeclarations;
         assertDeclaredField(fieldDeclarations,
@@ -538,7 +539,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest {
         assertDeclaredField(fieldDeclarations,
                 "private java.lang.Long coreSize");
         assertDeclaredField(fieldDeclarations, "private byte[] binary");
         assertDeclaredField(fieldDeclarations,
                 "private java.lang.Long coreSize");
         assertDeclaredField(fieldDeclarations, "private byte[] binary");
-        assertEquals(22, fieldDeclarations.size());
+        assertEquals(17, fieldDeclarations.size());
 
         assertEquals(1, visitor.requireIfc.size());
         String reqIfc = visitor.requireIfc.get("setThreadfactory");
 
         assertEquals(1, visitor.requireIfc.size());
         String reqIfc = visitor.requireIfc.get("setThreadfactory");
@@ -546,7 +547,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest {
         assertContains(reqIfc, PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                 + ".threads.ThreadFactoryServiceInterface");
 
         assertContains(reqIfc, PackageTranslatorTest.EXPECTED_PACKAGE_PREFIX
                 + ".threads.ThreadFactoryServiceInterface");
 
-        assertEquals("Incorrenct number of generated methods", 27,
+        assertEquals("Incorrenct number of generated methods", 26,
                 visitor.methods.size());
         assertEquals("Incorrenct number of generated method descriptions", 3,
                 visitor.methodDescriptions.size());
                 visitor.methods.size());
         assertEquals("Incorrenct number of generated method descriptions", 3,
                 visitor.methodDescriptions.size());
diff --git a/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java b/opendaylight/config/yang-test/src/main/java/org/opendaylight/controller/config/yang/test/impl/CheckedAutoCloseable.java
new file mode 100644 (file)
index 0000000..74cf617
--- /dev/null
@@ -0,0 +1,17 @@
+package org.opendaylight.controller.config.yang.test.impl;
+
+import com.google.common.base.Preconditions;
+
+public class CheckedAutoCloseable implements AutoCloseable {
+    private boolean closed = false;
+
+    @Override
+    public synchronized void close() throws Exception {
+        Preconditions.checkState(closed == false);
+        this.closed = true;
+    }
+
+    public synchronized boolean isClosed() {
+        return this.closed;
+    }
+}
index 80c1e54a15e528d0b93708dfe76d506fbf03ad54..2359b50e0a46a5595cab18fa81b78261e0cf2520 100644 (file)
@@ -1,5 +1 @@
-        return new AutoCloseable() {
-            @Override
-            public void close() throws Exception {
-            }
-        };
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable();
index 7fa0c10eb2c570bd91c47f924ecc26087d4e360e..458ec01165273c92c4aac7a15baeb0d30dbe262d 100644 (file)
@@ -9,8 +9,4 @@
         }
         logger.info("IdentityContainer Afi class: {}", getIdentitiesContainer().resolveAfi());
 
         }
         logger.info("IdentityContainer Afi class: {}", getIdentitiesContainer().resolveAfi());
 
-        return new AutoCloseable() {
-            @Override
-            public void close() throws Exception {
-            }
-        };
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable();
index 80c1e54a15e528d0b93708dfe76d506fbf03ad54..bbf2a71faa475cb9b0e3edb8c43b605135d5acb9 100644 (file)
@@ -1,5 +1,16 @@
-        return new AutoCloseable() {
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable() {
             @Override
             @Override
-            public void close() throws Exception {
+            public synchronized void close() throws Exception {
+                if(getSingleDependency() != null && getSingleDependency().isClosed() == true) {
+                    // Simulate a cleanup on dependencies that should not be closed yet
+                    throw new java.lang.Error("Dependency was closed first");
+                }
+                for (org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable autoCloseable : getTestingDepsDependency()) {
+                    if(autoCloseable.isClosed() == true) {
+                        // Simulate a cleanup on dependencies that should not be closed yet
+                        throw new java.lang.Error("Dependency was closed first");
+                    }
+                }
+                super.close();
             }
             }
-        };
+        };
\ No newline at end of file
index 4b5b78f70a5602232a4ad4b6dc46d9c6319b2076..7e155e05c71ee5e8eb9c0507733c1da6c41b0683 100644 (file)
@@ -1 +1,7 @@
-        return org.opendaylight.controller.config.yang.test.util.NetconfTestImplModuleUtil.registerRuntimeBeans(this);
+        final AutoCloseable runtime = org.opendaylight.controller.config.yang.test.util.NetconfTestImplModuleUtil.registerRuntimeBeans(this);
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable() {
+            @Override
+            public synchronized void close() throws Exception {
+                runtime.close();
+            }
+        };
\ No newline at end of file
index 80c1e54a15e528d0b93708dfe76d506fbf03ad54..dc933b4f3b07e178b3794e4f21ac16cefec2e287 100644 (file)
@@ -1,5 +1 @@
-        return new AutoCloseable() {
-            @Override
-            public void close() throws Exception {
-            }
-        };
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable();
\ No newline at end of file
index 093d7b3f13673f0f0799030fe35c70597db7ba1e..f772a26898b1765ee833866a61688055c76128a2 100644 (file)
@@ -468,7 +468,9 @@ module config-test-impl {
                         }
                     }
                 }
                         }
                     }
                 }
+
             }
             }
+
         }
     }
 
         }
     }
 
@@ -485,6 +487,19 @@ module config-test-impl {
                         }
                     }
                 }
                         }
                     }
                 }
+                container single {
+                        uses config:service-ref {
+                            refine type {
+                                mandatory false;
+                                config:required-identity test:testing;
+                            }
+                        }
+                }
+
+                leaf simple {
+                    type boolean;
+                    default false;
+                }
             }
         }
     }
             }
         }
     }
index 2daf4055db646e19346cb95419393c83342c99bb..4843ebc895cedd16efe23e1019c40a02268bda87 100644 (file)
@@ -18,6 +18,6 @@ module config-test {
             "Test api";
 
         base "config:service-type";
             "Test api";
 
         base "config:service-type";
-        config:java-class "java.lang.AutoCloseable";
+        config:java-class "org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable";
     }
 }
     }
 }
index 2df15d0dc1ac9eb456ea70773e732a02f1d57b7a..997e4e3196f87949a54249c36de1ef1bb2fe5c91 100644 (file)
@@ -11,13 +11,17 @@ package org.opendaylight.controller.config.yang.test.impl;
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getInstanceName;
 import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getTransactionName;
 
 import java.util.List;
 import javax.management.ObjectName;
 import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getInstanceName;
 import static org.opendaylight.controller.config.api.jmx.ObjectNameUtil.getTransactionName;
 
 import java.util.List;
 import javax.management.ObjectName;
+import org.hamcrest.CoreMatchers;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Before;
 import org.junit.Test;
+import org.opendaylight.controller.config.api.ValidationException;
 import org.opendaylight.controller.config.manager.impl.AbstractConfigTest;
 import org.opendaylight.controller.config.manager.impl.factoriesresolver.HardcodedModuleFactoriesResolver;
 import org.opendaylight.controller.config.util.ConfigTransactionJMXClient;
 import org.opendaylight.controller.config.manager.impl.AbstractConfigTest;
 import org.opendaylight.controller.config.manager.impl.factoriesresolver.HardcodedModuleFactoriesResolver;
 import org.opendaylight.controller.config.util.ConfigTransactionJMXClient;
@@ -49,4 +53,53 @@ public class MultipleDependenciesModuleTest extends AbstractConfigTest {
         assertNull(getTransactionName(d1WithoutTxName));
         transaction.commit();
     }
         assertNull(getTransactionName(d1WithoutTxName));
         transaction.commit();
     }
+
+    @Test
+    public void testCloseOrdering() throws Exception {
+        // Tests whether close is called in correct order on the module instances on following graph
+        // Each module tests whether its dependencies were closed before it (to simulate resource clean up failure)
+        // R1
+        // | \
+        // M1 M2
+        // |
+        // L1
+        ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction();
+        ObjectName r1 = transaction.createModule(factory.getImplementationName(), "root1");
+        ObjectName m1 = transaction.createModule(factory.getImplementationName(), "middle1");
+        ObjectName m2 = transaction.createModule(factory.getImplementationName(), "middle2");
+        ObjectName l1 = transaction.createModule(factory.getImplementationName(), "leaf1");
+
+        MultipleDependenciesModuleMXBean r1Proxy = transaction.newMXBeanProxy(r1, MultipleDependenciesModuleMXBean.class);
+        MultipleDependenciesModuleMXBean i1Proxy = transaction.newMXBeanProxy(m1, MultipleDependenciesModuleMXBean.class);
+        r1Proxy.setSingle(m1);
+        i1Proxy.setSingle(l1);
+        r1Proxy.setTestingDeps(asList(m2));
+        transaction.commit();
+
+        configRegistryClient.createTransaction().commit();
+        transaction = configRegistryClient.createTransaction();
+        MultipleDependenciesModuleMXBean l1Proxy = transaction.newMXBeanProxy(l1, MultipleDependenciesModuleMXBean.class);
+        l1Proxy.setSimple(true);
+        transaction.commit();
+    }
+
+    @Test
+    public void testDestroyModuleDependency() throws Exception {
+        ConfigTransactionJMXClient transaction = configRegistryClient.createTransaction();
+        ObjectName r1 = transaction.createModule(factory.getImplementationName(), "root1");
+        ObjectName m1 = transaction.createModule(factory.getImplementationName(), "middle1");
+
+        MultipleDependenciesModuleMXBean r1Proxy = transaction.newMXBeanProxy(r1, MultipleDependenciesModuleMXBean.class);
+        r1Proxy.setSingle(m1);
+        transaction.commit();
+
+        transaction = configRegistryClient.createTransaction();
+        transaction.destroyModule(factory.getImplementationName(), "middle1");
+        try {
+            transaction.commit();
+            fail("Validation exception expected");
+        } catch (ValidationException e) {
+            assertThat(e.getFailedValidations().keySet(), CoreMatchers.hasItem("multiple-dependencies"));
+        }
+    }
 }
 }