Merge topic 'archetype'
authorTony Tkacik <ttkacik@cisco.com>
Tue, 20 Jan 2015 15:05:37 +0000 (15:05 +0000)
committerGerrit Code Review <gerrit@opendaylight.org>
Tue, 20 Jan 2015 15:05:38 +0000 (15:05 +0000)
* changes:
  Fixed missing uses of ${artifactId} in urn
  Fixed copyrights in opendaylight-startup archetype

45 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-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryBuilder.java
opendaylight/config/yang-jmx-generator/src/main/java/org/opendaylight/controller/config/yangjmxgenerator/RuntimeBeanEntry.java
opendaylight/config/yang-jmx-generator/src/test/java/org/opendaylight/controller/config/yangjmxgenerator/ModuleMXBeanEntryTest.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
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorTest.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java
opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/ConfigPusherImpl.java
opendaylight/netconf/config-persister-impl/src/main/java/org/opendaylight/controller/netconf/persist/impl/osgi/ConfigPersisterActivator.java
opendaylight/netconf/netconf-netty-util/pom.xml

index 40ff7e1..466f1ed 100644 (file)
@@ -51,6 +51,7 @@ public interface DependencyResolver extends Identifiable<ModuleIdentifier> {
     <T> T resolveInstance(Class<T> expectedType, ObjectName objectName,
                           JmxAttribute jmxAttribute);
 
+
     /**
      * 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);
 
+    /**
+     * 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 b557108..53f03a2 100644 (file)
@@ -54,4 +54,17 @@ public interface Module extends Identifiable<ModuleIdentifier>{
      */
     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 96739fb..91c67b8 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) {
index bb61c4a..37c2e2d 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 com.google.common.collect.Lists;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
@@ -383,32 +384,34 @@ class ConfigTransactionControllerImpl implements
 
         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 {}",
-                        name, getTransactionIdentifier());
+                        moduleIdentifier, getTransactionIdentifier());
                 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) {
-                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",
-                                name, getTransactionIdentifier()), e);
+                                moduleIdentifier, getTransactionIdentifier()), e);
             }
         }
 
-        // count dependency order
-
         LOG.trace("Committed configuration {}", getTransactionIdentifier());
         transactionStatus.setCommitted();
 
-        return dependencyResolverManager.getSortedModuleIdentifiers();
+        return sortedModuleIdentifiers;
     }
 
     @Override
index 0741dab..024518c 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import static java.lang.String.format;
 
+import com.google.common.base.Preconditions;
 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) {
-        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) {
-            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,
-                    dependentModuleIdentifier, expectedType, jmxAttribute
+                    module.getIdentifier(), expectedType, 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());
@@ -217,7 +237,7 @@ final class DependencyResolverImpl implements DependencyResolver,
 
     @Override
     public int compareTo(DependencyResolverImpl o) {
-        transactionStatus.checkCommitted();
+        transactionStatus.checkCommitStarted();
         return Integer.compare(getMaxDependencyDepth(),
                 o.getMaxDependencyDepth());
     }
@@ -232,7 +252,11 @@ final class DependencyResolverImpl implements DependencyResolver,
     }
 
     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>());
index da6349a..0014a59 100644 (file)
@@ -88,7 +88,7 @@ public class DependencyResolverManager implements DependencyResolverFactory, Aut
      * things?
      */
     private List<DependencyResolverImpl> getAllSorted() {
-        transactionStatus.checkCommitted();
+        transactionStatus.checkCommitStarted();
         List<DependencyResolverImpl> sorted = new ArrayList<>(
                 moduleIdentifiersToDependencyResolverMap.values());
         for (DependencyResolverImpl dri : sorted) {
index 26d8c74..210b6d2 100644 (file)
@@ -7,6 +7,7 @@
  */
 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;
@@ -70,10 +71,7 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
 
     @Nullable
     public ModuleInternalInfo getOldInternalInfo() {
-        if (maybeOldInternalInfo == null) {
-            throw new NullPointerException();
-        }
-        return maybeOldInternalInfo;
+        return Preconditions.checkNotNull(maybeOldInternalInfo);
     }
 
     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
index 837ae2b..a6e24e9 100644 (file)
@@ -31,6 +31,12 @@ public abstract class AbstractMockedModule implements Module {
         this.id = id==null ? new ModuleIdentifier(getClass().getCanonicalName(), "mock") : id;
     }
 
+
+    @Override
+    public boolean canReuse(Module oldModule) {
+        return instance!=null;
+    }
+
     @Override
     public void validate() {
     }
index 0029f26..63a33a3 100644 (file)
@@ -69,7 +69,10 @@ public class DependencyResolverManagerTest extends AbstractLockedPlatformMBeanSe
 
         // switch to second phase committed
         reset(transactionStatus);
+        doNothing().when(transactionStatus).checkCommitStarted();
         doNothing().when(transactionStatus).checkCommitted();
+        doNothing().when(transactionStatus).checkNotCommitted();
+
         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();
+        doReturn(new ModuleIdentifier("fact", "instance")).when(mockedModule).getIdentifier();
         return mockedModule;
     }
 
index 7deade5..95de90d 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);
index 1811e7d..e35509d 100644 (file)
@@ -61,6 +61,11 @@ public class TestingScheduledThreadPoolModule implements Module,
                 "Parameter 'ThreadCount' must be greater than 0");
     }
 
+    @Override
+    public boolean canReuse(final Module oldModule) {
+        return false;
+    }
+
     @Override
     public int getThreadCount() {
         return threadCount;
index 464f786..c773dd2 100644 (file)
@@ -69,6 +69,11 @@ public class TestingFixedThreadPoolModule implements
                 "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) {
index 64e83a5..5bf4ac2 100644 (file)
@@ -40,6 +40,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
 
     private FixedThreadPoolModuleFactory factory;
     private final String nameInstance = "fixedInstance";
+    private ObjectName threadFactoryON;
 
     @Before
     public void setUp() {
@@ -73,10 +74,12 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
         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());
-        assertStatus(status, 0, 0, 2);
+        assertStatus(status, 0, 2, 0);
     }
 
     @Test
@@ -161,7 +164,7 @@ public class FixedThreadPoolConfigBeanTest extends AbstractConfigTest {
         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);
index 2402014..487bf63 100644 (file)
@@ -22,13 +22,12 @@ public class AbstractModuleTemplate extends GeneralClassTemplate {
     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) {
-        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;
index 3f4917f..4adb2b0 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(
-                    "Class cannot have more than one super " + "class");
+                    "Class cannot have more than one super class, found: " + extendedClass);
         }
         return extendedClass;
     }
index 53ab4ef..19e875f 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.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;
@@ -196,7 +196,6 @@ public class TemplateFactory {
 
         List<ModuleField> moduleFields = attrProcessor.getModuleFields();
         List<String> implementedIfcs = Lists.newArrayList(
-                Module.class.getCanonicalName(),
                 mbe.getFullyQualifiedName(mbe.getMXBeanInterfaceName()));
 
         for (String implementedService : mbe.getProvidedServices().keySet()) {
@@ -218,9 +217,11 @@ public class TemplateFactory {
                     .getCanonicalName());
         }
 
+        List<String> extendedClasses = Collections.singletonList(AbstractModule.class.getCanonicalName() + "<" + mbe.getAbstractModuleName() + ">");
+
         AbstractModuleTemplate abstractModuleTemplate = new AbstractModuleTemplate(
                 getHeaderFromEntry(mbe), mbe.getPackageName(),
-                mbe.getAbstractModuleName(), implementedIfcs, moduleFields,
+                mbe.getAbstractModuleName(), extendedClasses, implementedIfcs, moduleFields,
                 attrProcessor.getMethods(), generateRuntime,
                 registratorFullyQualifiedName);
 
index 64a13d3..92a0eec 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 com.google.common.base.Function;
 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;
@@ -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.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.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;
@@ -39,16 +45,26 @@ import org.slf4j.LoggerFactory;
 
 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();
 
-        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()));
@@ -56,7 +72,7 @@ public class AbsModuleGeneratedObjectFactory {
             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());
     }
@@ -65,6 +81,7 @@ public class AbsModuleGeneratedObjectFactory {
                                              Optional<String> copyright,
                                              Optional<String> header,
                                              Optional<String> classJavaDoc,
+                                             Optional<FullyQualifiedName> extended,
                                              List<FullyQualifiedName> implementedInterfaces,
                                              List<ModuleField> moduleFields,
                                              Optional<FullyQualifiedName> maybeRegistratorType,
@@ -84,6 +101,9 @@ public class AbsModuleGeneratedObjectFactory {
         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()));
         }
@@ -99,9 +119,6 @@ public class AbsModuleGeneratedObjectFactory {
         b.addToBody("//attributes end");
 
 
-        b.addToBody(getCommonFields(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(getGetInstance(moduleFields));
+        b.addToBody(getResolveDependencies(moduleFields));
         b.addToBody(getReuseLogic(moduleFields, abstractFQN));
         b.addToBody(getEqualsAndHashCode(abstractFQN));
 
         b.addToBody(getMethods(methods));
+        b.addToBody(getGetLogger());
 
         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) {
+            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 (%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(
-                    "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";
@@ -192,13 +213,7 @@ public class AbsModuleGeneratedObjectFactory {
         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) {
@@ -207,19 +222,21 @@ public class AbsModuleGeneratedObjectFactory {
                 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(
-                        "%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);
             }
         }
 
+        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) {
@@ -236,9 +253,9 @@ public class AbsModuleGeneratedObjectFactory {
                 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());
                 }
@@ -256,44 +273,10 @@ public class AbsModuleGeneratedObjectFactory {
                 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;
     }
 
-    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) {
@@ -355,7 +338,7 @@ public class AbsModuleGeneratedObjectFactory {
             "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()) +
@@ -363,8 +346,14 @@ public class AbsModuleGeneratedObjectFactory {
                                     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());
+                    if(moduleField.getDependency().isMandatory() == false) {
+                        result += "}\n";
+                    }
                 }
             }
         }
@@ -378,7 +367,7 @@ public class AbsModuleGeneratedObjectFactory {
     }
 
     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);
     }
 
@@ -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);
-        String setters = "";
-        for (String paramName : parameters.values()) {
-            setters += format("this.%s = %1$s;\n", paramName);
-        }
         return format("public %s(", fqn.getTypeName()) +
                 paramString +
                 ") {\n" +
-                setters +
                 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");
-
-        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) {
@@ -414,6 +396,11 @@ public class AbsModuleGeneratedObjectFactory {
         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 8c2b5d6..a6cfc58 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.spi.AbstractModule;
 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",
-                Module.class.getCanonicalName(),
                 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,
@@ -538,7 +539,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest {
         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");
@@ -546,7 +547,7 @@ public class JMXGeneratorTest extends AbstractGeneratorTest {
         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());
index ed727c9..0199e8c 100644 (file)
@@ -11,7 +11,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static java.lang.String.format;
 import static org.opendaylight.controller.config.yangjmxgenerator.ConfigConstants.createConfigQName;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
@@ -250,15 +249,10 @@ final class ModuleMXBeanEntryBuilder {
         String moduleLocalNameFromXPath = matcher.group(1);
         IdentitySchemaNode moduleIdentity = moduleIdentities.get(moduleLocalNameFromXPath);
         unaugmentedModuleIdentities.remove(moduleLocalNameFromXPath);
-        checkState(moduleIdentity != null, "Cannot find identity " + moduleLocalNameFromXPath
-                + " matching augmentation " + augmentation);
+        checkState(moduleIdentity != null, "Cannot find identity %s matching augmentation %s", moduleLocalNameFromXPath, augmentation);
         Map<String, QName> providedServices = findProvidedServices(moduleIdentity, currentModule, qNamesToSIEs,
                 schemaContext);
 
-        if (moduleIdentity == null) {
-            throw new IllegalStateException("Cannot find identity specified by augmentation xpath constraint: "
-                    + moduleLocalNameFromXPath + " of " + augmentation);
-        }
         String javaNamePrefix = TypeProviderWrapper.findJavaNamePrefix(moduleIdentity);
 
         Map<String, AttributeIfc> yangToAttributes = null;
@@ -346,7 +340,7 @@ final class ModuleMXBeanEntryBuilder {
         }
     }
 
-    private void checkUniqueAttributesWithGeneratedClass(final Map<String, QName> uniqueGeneratedClassNames,
+    private static void checkUniqueAttributesWithGeneratedClass(final Map<String, QName> uniqueGeneratedClassNames,
             final QName parentQName, final Map<String, AttributeIfc> yangToAttributes) {
         for (Map.Entry<String, AttributeIfc> attr : yangToAttributes.entrySet()) {
             if (attr.getValue() instanceof TOAttribute) {
@@ -359,7 +353,7 @@ final class ModuleMXBeanEntryBuilder {
         }
     }
 
-    private void checkUniqueTOAttr(final Map<String, QName> uniqueGeneratedClassNames, final QName parentQName, final TOAttribute attr) {
+    private static void checkUniqueTOAttr(final Map<String, QName> uniqueGeneratedClassNames, final QName parentQName, final TOAttribute attr) {
         final String upperCaseCamelCase = attr.getUpperCaseCammelCase();
         if (uniqueGeneratedClassNames.containsKey(upperCaseCamelCase)) {
             QName firstDefinedQName = uniqueGeneratedClassNames.get(upperCaseCamelCase);
index 74981a9..34e3c2e 100644 (file)
@@ -9,7 +9,6 @@ package org.opendaylight.controller.config.yangjmxgenerator;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
@@ -95,7 +94,7 @@ public class RuntimeBeanEntry {
             final List<RuntimeBeanEntry> children, final Set<Rpc> rpcs) {
 
         checkArgument(isRoot == false || keyYangName.isPresent() == false,
-                "Root RuntimeBeanEntry must not have key " + "set");
+                "Root RuntimeBeanEntry must not have key set");
         this.packageName = packageName;
         this.isRoot = isRoot;
         this.yangName = yangName;
@@ -108,16 +107,14 @@ public class RuntimeBeanEntry {
 
         for (AttributeIfc a : attributes) {
             checkState(map.containsKey(a.getAttributeYangName()) == false,
-                    "Attribute already defined: " + a.getAttributeYangName()
-                    + " in " + nodeForReporting);
+                    "Attribute already defined: %s in %s", a.getAttributeYangName(), nodeForReporting);
             map.put(a.getAttributeYangName(), a);
         }
 
         if (keyYangName.isPresent()) {
             AttributeIfc keyJavaName = map.get(keyYangName.get());
-            checkArgument(keyJavaName != null, "Key " + keyYangName.get()
-                    + " not found in attribute " + "list " + attributes
-                    + " in " + nodeForReporting);
+            checkArgument(keyJavaName != null, "Key %s not found in attribute list %s in %s", keyYangName.get(),
+                    attributes, nodeForReporting);
             this.keyJavaName = Optional
                     .of(keyJavaName.getUpperCaseCammelCase());
         } else {
index 17d4d9a..e116f48 100644 (file)
@@ -8,6 +8,7 @@
 package org.opendaylight.controller.config.yangjmxgenerator;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.isA;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -16,7 +17,6 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
-
 import com.google.common.collect.Sets;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -113,7 +113,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest {
                 is("threadFactory"));
         assertThat(threadFactoryAttribute.getUpperCaseCammelCase(),
                 is("ThreadFactory"));
-        assertThat(threadFactoryAttribute.getOpenType(), is(SimpleType.class));
+        assertThat(threadFactoryAttribute.getOpenType(), isA(SimpleType.class));
         assertNull(threadFactoryAttribute.getNullableDefault());
         assertNull(threadFactoryAttribute.getNullableDescription());
         assertThat(threadFactoryAttribute.getType().getName(), is("ObjectName"));
@@ -261,7 +261,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest {
                 assertThat(toAttr.getAttributeYangName(), is("peer"));
                 assertThat(toAttr.getLowerCaseCammelCase(), is("peer"));
                 assertThat(toAttr.getUpperCaseCammelCase(), is("Peer"));
-                assertThat(toAttr.getOpenType(), is(CompositeType.class));
+                assertThat(toAttr.getOpenType(), isA(CompositeType.class));
                 Set<String> propsExpected = new HashSet<String>(2);
                 propsExpected.add("port");
                 propsExpected.add("core-size");
@@ -296,7 +296,7 @@ public class ModuleMXBeanEntryTest extends AbstractYangTest {
                         is("innerStreamList"));
                 assertThat(innerStream.getUpperCaseCammelCase(),
                         is("InnerStreamList"));
-                assertThat(innerStream.getOpenType(), is(ArrayType.class));
+                assertThat(innerStream.getOpenType(), isA(ArrayType.class));
 
             }
 
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 80c1e54..2359b50 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 7fa0c10..458ec01 100644 (file)
@@ -9,8 +9,4 @@
         }
         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 80c1e54..bbf2a71 100644 (file)
@@ -1,5 +1,16 @@
-        return new AutoCloseable() {
+        return new org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable() {
             @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 4b5b78f..7e155e0 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 80c1e54..dc933b4 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 093d7b3..f772a26 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 2daf405..4843ebc 100644 (file)
@@ -18,6 +18,6 @@ module config-test {
             "Test api";
 
         base "config:service-type";
-        config:java-class "java.lang.AutoCloseable";
+        config:java-class "org.opendaylight.controller.config.yang.test.impl.CheckedAutoCloseable";
     }
 }
index 2df15d0..997e4e3 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 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 org.hamcrest.CoreMatchers;
 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;
@@ -49,4 +53,53 @@ public class MultipleDependenciesModuleTest extends AbstractConfigTest {
         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"));
+        }
+    }
 }
index 3b84692..132e09d 100644 (file)
@@ -113,6 +113,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
 
     private int currentRecoveryBatchCount;
 
+
+
     public RaftActor(String id, Map<String, String> peerAddresses) {
         this(id, peerAddresses, Optional.<ConfigParams>absent());
     }
@@ -377,15 +379,18 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         if (oldBehavior != currentBehavior){
             onStateChanged();
         }
-        if (oldBehavior != null) {
-            // it can happen that the state has not changed but the leader has changed.
-            onLeaderChanged(oldBehavior.getLeaderId(), currentBehavior.getLeaderId());
 
-            if (getRoleChangeNotifier().isPresent() && oldBehavior.state() != currentBehavior.state()) {
-                // we do not want to notify when the behavior/role is set for the first time (i.e follower)
-                getRoleChangeNotifier().get().tell(new RoleChanged(getId(), oldBehavior.state().name(),
-                    currentBehavior.state().name()), getSelf());
-            }
+        String oldBehaviorLeaderId = oldBehavior == null? null : oldBehavior.getLeaderId();
+        String oldBehaviorState = oldBehavior == null? null : oldBehavior.state().name();
+
+        // it can happen that the state has not changed but the leader has changed.
+        onLeaderChanged(oldBehaviorLeaderId, currentBehavior.getLeaderId());
+
+        if (getRoleChangeNotifier().isPresent() &&
+                (oldBehavior == null || (oldBehavior.state() != currentBehavior.state()))) {
+            getRoleChangeNotifier().get().tell(
+                    new RoleChanged(getId(), oldBehaviorState , currentBehavior.state().name()),
+                    getSelf());
         }
     }
 
@@ -397,8 +402,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
      * @param identifier
      * @param data
      */
-    protected void persistData(ActorRef clientActor, String identifier,
-        Payload data) {
+    protected void persistData(final ActorRef clientActor, final String identifier,
+        final Payload data) {
 
         ReplicatedLogEntry replicatedLogEntry = new ReplicatedLogImplEntry(
             context.getReplicatedLog().lastIndex() + 1,
@@ -408,9 +413,42 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
             LOG.debug("Persist data {}", replicatedLogEntry);
         }
 
+        final RaftActorContext raftContext = getRaftActorContext();
+
         replicatedLog
-            .appendAndPersist(clientActor, identifier, replicatedLogEntry);
-    }
+                .appendAndPersist(replicatedLogEntry, new Procedure<ReplicatedLogEntry>() {
+                    @Override
+                    public void apply(ReplicatedLogEntry replicatedLogEntry) throws Exception {
+                        if(!hasFollowers()){
+                            // Increment the Commit Index and the Last Applied values
+                            raftContext.setCommitIndex(replicatedLogEntry.getIndex());
+                            raftContext.setLastApplied(replicatedLogEntry.getIndex());
+
+                            // Apply the state immediately
+                            applyState(clientActor, identifier, data);
+
+                            // Send a ApplyLogEntries message so that we write the fact that we applied
+                            // the state to durable storage
+                            self().tell(new ApplyLogEntries((int) replicatedLogEntry.getIndex()), self());
+
+                            // Check if the "real" snapshot capture has been initiated. If no then do the fake snapshot
+                            if(!hasSnapshotCaptureInitiated){
+                                raftContext.getReplicatedLog().snapshotPreCommit(raftContext.getLastApplied(),
+                                        raftContext.getTermInformation().getCurrentTerm());
+                                raftContext.getReplicatedLog().snapshotCommit();
+                            } else {
+                                LOG.debug("Skipping fake snapshotting for {} because real snapshotting is in progress", getId());
+                            }
+                        } else if (clientActor != null) {
+                            // Send message for replication
+                            currentBehavior.handleMessage(getSelf(),
+                                    new Replicate(clientActor, identifier,
+                                            replicatedLogEntry)
+                            );
+                        }
+
+                    }
+                });    }
 
     protected String getId() {
         return context.getId();
@@ -650,8 +688,15 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         hasSnapshotCaptureInitiated = false;
     }
 
+    protected boolean hasFollowers(){
+        return getRaftActorContext().getPeerAddresses().keySet().size() > 0;
+    }
+
     private class ReplicatedLogImpl extends AbstractReplicatedLogImpl {
 
+        private static final int DATA_SIZE_DIVIDER = 5;
+        private long dataSizeSinceLastSnapshot = 0;
+
         public ReplicatedLogImpl(Snapshot snapshot) {
             super(snapshot.getLastAppliedIndex(), snapshot.getLastAppliedTerm(),
                 snapshot.getUnAppliedEntries());
@@ -686,7 +731,7 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
 
         @Override public void appendAndPersist(
             final ReplicatedLogEntry replicatedLogEntry) {
-            appendAndPersist(null, null, replicatedLogEntry);
+            appendAndPersist(replicatedLogEntry, null);
         }
 
         @Override
@@ -694,9 +739,9 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
             return dataSize;
         }
 
-        public void appendAndPersist(final ActorRef clientActor,
-            final String identifier,
-            final ReplicatedLogEntry replicatedLogEntry) {
+        public void appendAndPersist(
+            final ReplicatedLogEntry replicatedLogEntry,
+            final Procedure<ReplicatedLogEntry> callback)  {
 
             if(LOG.isDebugEnabled()) {
                 LOG.debug("Append log entry and persist {} ", replicatedLogEntry);
@@ -714,22 +759,48 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
                 new Procedure<ReplicatedLogEntry>() {
                     @Override
                     public void apply(ReplicatedLogEntry evt) throws Exception {
-                        dataSize += replicatedLogEntry.size();
+                        int logEntrySize = replicatedLogEntry.size();
+
+                        dataSize += logEntrySize;
+                        long dataSizeForCheck = dataSize;
+
+                        dataSizeSinceLastSnapshot += logEntrySize;
+                        long journalSize = lastIndex()+1;
+
+                        if(!hasFollowers()) {
+                            // When we do not have followers we do not maintain an in-memory log
+                            // due to this the journalSize will never become anything close to the
+                            // snapshot batch count. In fact will mostly be 1.
+                            // Similarly since the journal's dataSize depends on the entries in the
+                            // journal the journal's dataSize will never reach a value close to the
+                            // memory threshold.
+                            // By maintaining the dataSize outside the journal we are tracking essentially
+                            // what we have written to the disk however since we no longer are in
+                            // need of doing a snapshot just for the sake of freeing up memory we adjust
+                            // the real size of data by the DATA_SIZE_DIVIDER so that we do not snapshot as often
+                            // as if we were maintaining a real snapshot
+                            dataSizeForCheck = dataSizeSinceLastSnapshot / DATA_SIZE_DIVIDER;
+                        }
 
                         long dataThreshold = Runtime.getRuntime().totalMemory() *
                                 getRaftActorContext().getConfigParams().getSnapshotDataThresholdPercentage() / 100;
 
                         // when a snaphsot is being taken, captureSnapshot != null
                         if (hasSnapshotCaptureInitiated == false &&
-                                ( journal.size() % context.getConfigParams().getSnapshotBatchCount() == 0 ||
-                                        dataSize > dataThreshold)) {
+                                ( journalSize % context.getConfigParams().getSnapshotBatchCount() == 0 ||
+                                        dataSizeForCheck > dataThreshold)) {
+
+                            dataSizeSinceLastSnapshot = 0;
 
                             LOG.info("Initiating Snapshot Capture..");
                             long lastAppliedIndex = -1;
                             long lastAppliedTerm = -1;
 
                             ReplicatedLogEntry lastAppliedEntry = get(context.getLastApplied());
-                            if (lastAppliedEntry != null) {
+                            if (!hasFollowers()) {
+                                lastAppliedIndex = replicatedLogEntry.getIndex();
+                                lastAppliedTerm = replicatedLogEntry.getTerm();
+                            } else if (lastAppliedEntry != null) {
                                 lastAppliedIndex = lastAppliedEntry.getIndex();
                                 lastAppliedTerm = lastAppliedEntry.getTerm();
                             }
@@ -748,12 +819,8 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
                                 null);
                             hasSnapshotCaptureInitiated = true;
                         }
-                        // Send message for replication
-                        if (clientActor != null) {
-                            currentBehavior.handleMessage(getSelf(),
-                                new Replicate(clientActor, identifier,
-                                    replicatedLogEntry)
-                            );
+                        if(callback != null){
+                            callback.apply(replicatedLogEntry);
                         }
                     }
                 }
index c833a86..d999bb2 100644 (file)
@@ -883,24 +883,27 @@ public class RaftActorTest extends AbstractActorTest {
             TestActorRef<MockRaftActor> mockActorRef = TestActorRef.create(getSystem(), MockRaftActor.props(id,
                 Collections.<String,String>emptyMap(), Optional.<ConfigParams>of(config), notifierActor), id);
 
-            MockRaftActor mockRaftActor = mockActorRef.underlyingActor();
-            mockRaftActor.setCurrentBehavior(new Follower(mockRaftActor.getRaftActorContext()));
-
             // sleeping for a minimum of 2 seconds, if it spans more its fine.
             Uninterruptibles.sleepUninterruptibly(2, TimeUnit.SECONDS);
 
             List<Object> matches =  MessageCollectorActor.getAllMatching(notifierActor, RoleChanged.class);
             assertNotNull(matches);
-            assertEquals(2, matches.size());
+            assertEquals(3, matches.size());
 
-            // check if the notifier got a role change from Follower to Candidate
+            // check if the notifier got a role change from null to Follower
             RoleChanged raftRoleChanged = (RoleChanged) matches.get(0);
             assertEquals(id, raftRoleChanged.getMemberId());
+            assertNull(raftRoleChanged.getOldRole());
+            assertEquals(RaftState.Follower.name(), raftRoleChanged.getNewRole());
+
+            // check if the notifier got a role change from Follower to Candidate
+            raftRoleChanged = (RoleChanged) matches.get(1);
+            assertEquals(id, raftRoleChanged.getMemberId());
             assertEquals(RaftState.Follower.name(), raftRoleChanged.getOldRole());
             assertEquals(RaftState.Candidate.name(), raftRoleChanged.getNewRole());
 
             // check if the notifier got a role change from Candidate to Leader
-            raftRoleChanged = (RoleChanged) matches.get(1);
+            raftRoleChanged = (RoleChanged) matches.get(2);
             assertEquals(id, raftRoleChanged.getMemberId());
             assertEquals(RaftState.Candidate.name(), raftRoleChanged.getOldRole());
             assertEquals(RaftState.Leader.name(), raftRoleChanged.getNewRole());
index 7d6dde9..7ef6e04 100644 (file)
@@ -133,7 +133,7 @@ public class Shard extends RaftActor {
 
     private Cancellable txCommitTimeoutCheckSchedule;
 
-    private Optional<ActorRef> roleChangeNotifier;
+    private final Optional<ActorRef> roleChangeNotifier;
 
     /**
      * Coordinates persistence recovery on startup.
@@ -321,8 +321,14 @@ public class Shard extends RaftActor {
             // currently uses a same thread executor anyway.
             cohortEntry.getCohort().preCommit().get();
 
-            Shard.this.persistData(getSender(), transactionID,
-                    new CompositeModificationByteStringPayload(cohortEntry.getModification().toSerializable()));
+            // If we do not have any followers and we are not using persistence we can
+            // apply modification to the state immediately
+            if(!hasFollowers() && !persistence().isRecoveryApplicable()){
+                applyModificationToState(getSender(), transactionID, cohortEntry.getModification());
+            } else {
+                Shard.this.persistData(getSender(), transactionID,
+                        new CompositeModificationByteStringPayload(cohortEntry.getModification().toSerializable()));
+            }
         } catch (InterruptedException | ExecutionException e) {
             LOG.error(e, "An exception occurred while preCommitting transaction {}",
                     cohortEntry.getTransactionID());
@@ -422,7 +428,7 @@ public class Shard extends RaftActor {
         doAbortTransaction(abort.getTransactionID(), getSender());
     }
 
-    private void doAbortTransaction(final String transactionID, final ActorRef sender) {
+    void doAbortTransaction(final String transactionID, final ActorRef sender) {
         final CohortEntry cohortEntry = commitCoordinator.getCohortEntryIfCurrent(transactionID);
         if(cohortEntry != null) {
             LOG.debug("Aborting transaction {}", transactionID);
index e70d79c..8c56efd 100644 (file)
@@ -110,8 +110,6 @@ public class ShardManagerTest extends AbstractActorTest {
         new JavaTestKit(getSystem()) {{
             final ActorRef shardManager = getSystem().actorOf(newShardMgrProps());
 
-            shardManager.tell(new UpdateSchemaContext(TestModel.createTestContext()), getRef());
-
             shardManager.tell(new FindPrimary(Shard.DEFAULT_NAME, false).toSerializable(), getRef());
 
             expectMsgClass(duration("5 seconds"), ActorNotInitialized.class);
@@ -174,8 +172,6 @@ public class ShardManagerTest extends AbstractActorTest {
         new JavaTestKit(getSystem()) {{
             final ActorRef shardManager = getSystem().actorOf(newShardMgrProps());
 
-            shardManager.tell(new UpdateSchemaContext(TestModel.createTestContext()), getRef());
-
             shardManager.tell(new FindLocalShard(Shard.DEFAULT_NAME, false), getRef());
 
             expectMsgClass(duration("5 seconds"), ActorNotInitialized.class);
index 2792342..ed842b2 100644 (file)
@@ -43,6 +43,7 @@ import org.junit.Test;
 import org.mockito.InOrder;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.opendaylight.controller.cluster.datastore.DatastoreContext.Builder;
 import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier;
 import org.opendaylight.controller.cluster.datastore.messages.AbortTransaction;
 import org.opendaylight.controller.cluster.datastore.messages.AbortTransactionReply;
@@ -111,12 +112,13 @@ public class ShardTest extends AbstractActorTest {
     private final ShardIdentifier shardID = ShardIdentifier.builder().memberName("member-1")
             .shardName("inventory").type("config" + NEXT_SHARD_NUM.getAndIncrement()).build();
 
-    private DatastoreContext dataStoreContext = DatastoreContext.newBuilder().
+    private final Builder dataStoreContextBuilder = DatastoreContext.newBuilder().
             shardJournalRecoveryLogBatchSize(3).shardSnapshotBatchCount(5000).
-            shardHeartbeatIntervalInMillis(100).build();
+            shardHeartbeatIntervalInMillis(100);
 
     @Before
     public void setUp() {
+        Builder newBuilder = DatastoreContext.newBuilder();
         InMemorySnapshotStore.clear();
         InMemoryJournal.clear();
     }
@@ -127,9 +129,13 @@ public class ShardTest extends AbstractActorTest {
         InMemoryJournal.clear();
     }
 
+    private DatastoreContext newDatastoreContext() {
+        return dataStoreContextBuilder.build();
+    }
+
     private Props newShardProps() {
         return Shard.props(shardID, Collections.<ShardIdentifier,String>emptyMap(),
-                dataStoreContext, SCHEMA_CONTEXT);
+                newDatastoreContext(), SCHEMA_CONTEXT);
     }
 
     @Test
@@ -186,7 +192,7 @@ public class ShardTest extends AbstractActorTest {
                 @Override
                 public Shard create() throws Exception {
                     return new Shard(shardID, Collections.<ShardIdentifier,String>emptyMap(),
-                            dataStoreContext, SCHEMA_CONTEXT) {
+                            newDatastoreContext(), SCHEMA_CONTEXT) {
                         @Override
                         public void onReceiveCommand(final Object message) throws Exception {
                             if(message instanceof ElectionTimeout && firstElectionTimeout) {
@@ -314,7 +320,7 @@ public class ShardTest extends AbstractActorTest {
             class TestShard extends Shard {
                 TestShard() {
                     super(shardID, Collections.<ShardIdentifier, String>singletonMap(shardID, null),
-                            dataStoreContext, SCHEMA_CONTEXT);
+                            newDatastoreContext(), SCHEMA_CONTEXT);
                 }
 
                 Map<String, String> getPeerAddresses() {
@@ -470,7 +476,7 @@ public class ShardTest extends AbstractActorTest {
             @Override
             public Shard create() throws Exception {
                 return new Shard(shardID, Collections.<ShardIdentifier,String>emptyMap(),
-                        dataStoreContext, SCHEMA_CONTEXT) {
+                        newDatastoreContext(), SCHEMA_CONTEXT) {
                     @Override
                     protected void onRecoveryComplete() {
                         try {
@@ -774,15 +780,72 @@ public class ShardTest extends AbstractActorTest {
             assertTrue("Missing leaf " + TestModel.ID_QNAME.getLocalName(), idLeaf.isPresent());
             assertEquals(TestModel.ID_QNAME.getLocalName() + " value", 1, idLeaf.get().getValue());
 
-            for(int i = 0; i < 20 * 5; i++) {
-                long lastLogIndex = shard.underlyingActor().getShardMBean().getLastLogIndex();
-                if(lastLogIndex == 2) {
-                    break;
-                }
-                Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS);
+            verifyLastLogIndex(shard, 2);
+
+            shard.tell(PoisonPill.getInstance(), ActorRef.noSender());
+        }};
+    }
+
+    private void verifyLastLogIndex(TestActorRef<Shard> shard, long expectedValue) {
+        for(int i = 0; i < 20 * 5; i++) {
+            long lastLogIndex = shard.underlyingActor().getShardMBean().getLastLogIndex();
+            if(lastLogIndex == expectedValue) {
+                break;
             }
+            Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS);
+        }
+
+        assertEquals("Last log index", expectedValue, shard.underlyingActor().getShardMBean().getLastLogIndex());
+    }
+
+    @Test
+    public void testCommitWithPersistenceDisabled() throws Throwable {
+        dataStoreContextBuilder.persistent(false);
+        new ShardTestKit(getSystem()) {{
+            final TestActorRef<Shard> shard = TestActorRef.create(getSystem(),
+                    newShardProps().withDispatcher(Dispatchers.DefaultDispatcherId()),
+                    "testCommitPhaseFailure");
 
-            assertEquals("Last log index", 2, shard.underlyingActor().getShardMBean().getLastLogIndex());
+            waitUntilLeader(shard);
+
+            InMemoryDOMDataStore dataStore = shard.underlyingActor().getDataStore();
+
+            // Setup a simulated transactions with a mock cohort.
+
+            String transactionID = "tx";
+            MutableCompositeModification modification = new MutableCompositeModification();
+            NormalizedNode<?, ?> containerNode = ImmutableNodes.containerNode(TestModel.TEST_QNAME);
+            DOMStoreThreePhaseCommitCohort cohort = setupMockWriteTransaction("cohort", dataStore,
+                    TestModel.TEST_PATH, containerNode, modification);
+
+            FiniteDuration duration = duration("5 seconds");
+
+            // Simulate the ForwardedReadyTransaction messages that would be sent
+            // by the ShardTransaction.
+
+            shard.tell(new ForwardedReadyTransaction(transactionID, CURRENT_VERSION,
+                    cohort, modification, true), getRef());
+            expectMsgClass(duration, ReadyTransactionReply.SERIALIZABLE_CLASS);
+
+            // Send the CanCommitTransaction message.
+
+            shard.tell(new CanCommitTransaction(transactionID).toSerializable(), getRef());
+            CanCommitTransactionReply canCommitReply = CanCommitTransactionReply.fromSerializable(
+                    expectMsgClass(duration, CanCommitTransactionReply.SERIALIZABLE_CLASS));
+            assertEquals("Can commit", true, canCommitReply.getCanCommit());
+
+            // Send the CanCommitTransaction message.
+
+            shard.tell(new CommitTransaction(transactionID).toSerializable(), getRef());
+            expectMsgClass(duration, CommitTransactionReply.SERIALIZABLE_CLASS);
+
+            InOrder inOrder = inOrder(cohort);
+            inOrder.verify(cohort).canCommit();
+            inOrder.verify(cohort).preCommit();
+            inOrder.verify(cohort).commit();
+
+            NormalizedNode<?, ?> actualNode = readStore(shard, TestModel.TEST_PATH);
+            assertEquals(TestModel.TEST_QNAME.getLocalName(), containerNode, actualNode);
 
             shard.tell(PoisonPill.getInstance(), ActorRef.noSender());
         }};
@@ -954,26 +1017,24 @@ public class ShardTest extends AbstractActorTest {
             waitUntilLeader(shard);
 
             final FiniteDuration duration = duration("5 seconds");
-            final Timeout timeout = new Timeout(duration);
-
             InMemoryDOMDataStore dataStore = shard.underlyingActor().getDataStore();
 
             final String transactionID = "tx1";
-            final CountDownLatch abortComplete = new CountDownLatch(1);
             Function<DOMStoreThreePhaseCommitCohort,ListenableFuture<Void>> preCommit =
                           new Function<DOMStoreThreePhaseCommitCohort,ListenableFuture<Void>>() {
                 @Override
                 public ListenableFuture<Void> apply(final DOMStoreThreePhaseCommitCohort cohort) {
                     ListenableFuture<Void> preCommitFuture = cohort.preCommit();
 
-                    Future<Object> abortFuture = Patterns.ask(shard,
-                            new AbortTransaction(transactionID).toSerializable(), timeout);
-                    abortFuture.onComplete(new OnComplete<Object>() {
-                        @Override
-                        public void onComplete(final Throwable e, final Object resp) {
-                            abortComplete.countDown();
-                        }
-                    }, getSystem().dispatcher());
+                    // Simulate an AbortTransaction message occurring during replication, after
+                    // persisting and before finishing the commit to the in-memory store.
+                    // We have no followers so due to optimizations in the RaftActor, it does not
+                    // attempt replication and thus we can't send an AbortTransaction message b/c
+                    // it would be processed too late after CommitTransaction completes. So we'll
+                    // simulate an AbortTransaction message occurring during replication by calling
+                    // the shard directly.
+                    //
+                    shard.underlyingActor().doAbortTransaction(transactionID, null);
 
                     return preCommitFuture;
                 }
@@ -993,14 +1054,14 @@ public class ShardTest extends AbstractActorTest {
                     expectMsgClass(duration, CanCommitTransactionReply.SERIALIZABLE_CLASS));
             assertEquals("Can commit", true, canCommitReply.getCanCommit());
 
-            Future<Object> commitFuture = Patterns.ask(shard,
-                    new CommitTransaction(transactionID).toSerializable(), timeout);
-
-            assertEquals("Abort complete", true, abortComplete.await(5, TimeUnit.SECONDS));
-
-            Await.result(commitFuture, duration);
+            shard.tell(new CommitTransaction(transactionID).toSerializable(), getRef());
+            expectMsgClass(duration, CommitTransactionReply.SERIALIZABLE_CLASS);
 
             NormalizedNode<?, ?> node = readStore(shard, TestModel.TEST_PATH);
+
+            // Since we're simulating an abort occurring during replication and before finish commit,
+            // the data should still get written to the in-memory store since we've gotten past
+            // canCommit and preCommit and persisted the data.
             assertNotNull(TestModel.TEST_QNAME.getLocalName() + " not found", node);
 
             shard.tell(PoisonPill.getInstance(), ActorRef.noSender());
@@ -1009,7 +1070,7 @@ public class ShardTest extends AbstractActorTest {
 
     @Test
     public void testTransactionCommitTimeout() throws Throwable {
-        dataStoreContext = DatastoreContext.newBuilder().shardTransactionCommitTimeoutInSeconds(1).build();
+        dataStoreContextBuilder.shardTransactionCommitTimeoutInSeconds(1);
 
         new ShardTestKit(getSystem()) {{
             final TestActorRef<Shard> shard = TestActorRef.create(getSystem(),
@@ -1081,7 +1142,7 @@ public class ShardTest extends AbstractActorTest {
 
     @Test
     public void testTransactionCommitQueueCapacityExceeded() throws Throwable {
-        dataStoreContext = DatastoreContext.newBuilder().shardTransactionCommitQueueCapacity(1).build();
+        dataStoreContextBuilder.shardTransactionCommitQueueCapacity(1);
 
         new ShardTestKit(getSystem()) {{
             final TestActorRef<Shard> shard = TestActorRef.create(getSystem(),
@@ -1214,15 +1275,7 @@ public class ShardTest extends AbstractActorTest {
 
             // Wait for the 2nd Tx to complete the canCommit phase.
 
-            final CountDownLatch latch = new CountDownLatch(1);
-            canCommitFuture.onComplete(new OnComplete<Object>() {
-                @Override
-                public void onComplete(final Throwable t, final Object resp) {
-                    latch.countDown();
-                }
-            }, getSystem().dispatcher());
-
-            assertEquals("2nd CanCommit complete", true, latch.await(5, TimeUnit.SECONDS));
+            Await.ready(canCommitFuture, duration);
 
             InOrder inOrder = inOrder(cohort1, cohort2);
             inOrder.verify(cohort1).canCommit();
@@ -1253,7 +1306,7 @@ public class ShardTest extends AbstractActorTest {
                 @Override
                 public Shard create() throws Exception {
                     return new Shard(shardID, Collections.<ShardIdentifier,String>emptyMap(),
-                            dataStoreContext, SCHEMA_CONTEXT) {
+                            newDatastoreContext(), SCHEMA_CONTEXT) {
                         @Override
                         protected void commitSnapshot(final long sequenceNumber) {
                             super.commitSnapshot(sequenceNumber);
index e24500a..dd51246 100644 (file)
@@ -891,10 +891,6 @@ public class RestconfImpl implements RestconfService {
             final DataNodeContainer parentSchema = (DataNodeContainer) incompleteInstIdWithData.getSchemaNode();
             DOMMountPoint mountPoint = incompleteInstIdWithData.getMountPoint();
             final Module module = findModule(mountPoint, payload);
-            if (module == null) {
-                throw new RestconfDocumentedException("Module was not found for \"" + payloadNS + "\"",
-                        ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ELEMENT);
-            }
 
             String payloadName = this.getName(payload);
             final DataSchemaNode schemaNode = ControllerContext.findInstanceDataChildByNameAndNamespace(
@@ -941,11 +937,6 @@ public class RestconfImpl implements RestconfService {
         }
 
         final Module module = this.findModule(null, payload);
-        if (module == null) {
-            throw new RestconfDocumentedException(
-                    "Data has bad format. Root element node has incorrect namespace (XML format) or module name(JSON format)",
-                    ErrorType.PROTOCOL, ErrorTag.UNKNOWN_NAMESPACE);
-        }
 
         String payloadName = this.getName(payload);
         final DataSchemaNode schemaNode = ControllerContext.findInstanceDataChildByNameAndNamespace(module,
@@ -1112,19 +1103,23 @@ public class RestconfImpl implements RestconfService {
     }
 
     private Module findModule(final DOMMountPoint mountPoint, final Node<?> data) {
+        Module module = null;
         if (data instanceof NodeWrapper) {
-            return findModule(mountPoint, (NodeWrapper<?>) data);
+            module = findModule(mountPoint, (NodeWrapper<?>) data);
         } else if (data != null) {
             URI namespace = data.getNodeType().getNamespace();
             if (mountPoint != null) {
-                return this.controllerContext.findModuleByNamespace(mountPoint, namespace);
+                module = this.controllerContext.findModuleByNamespace(mountPoint, namespace);
             } else {
-                return this.controllerContext.findModuleByNamespace(namespace);
+                module = this.controllerContext.findModuleByNamespace(namespace);
             }
-        } else {
-            throw new IllegalArgumentException("Unhandled parameter types: "
-                    + Arrays.<Object> asList(mountPoint, data).toString());
         }
+        if (module != null) {
+            return module;
+        }
+        throw new RestconfDocumentedException(
+                "Data has bad format. Root element node has incorrect namespace (XML format) or module name(JSON format)",
+                ErrorType.PROTOCOL, ErrorTag.UNKNOWN_NAMESPACE);
     }
 
     private Module findModule(final DOMMountPoint mountPoint, final NodeWrapper<?> data) {
index c20007d..cf9958e 100644 (file)
@@ -115,17 +115,20 @@ public class ConfigPusherImpl implements ConfigPusher {
      */
     private synchronized EditAndCommitResponse pushConfigWithConflictingVersionRetries(ConfigSnapshotHolder configSnapshotHolder) throws NetconfDocumentedException {
         ConflictingVersionException lastException;
-        Stopwatch stopwatch = new Stopwatch().start();
+        Stopwatch stopwatch = new Stopwatch();
         do {
             String idForReporting = configSnapshotHolder.toString();
             SortedSet<String> expectedCapabilities = checkNotNull(configSnapshotHolder.getCapabilities(),
                     "Expected capabilities must not be null - %s, check %s", idForReporting,
                     configSnapshotHolder.getClass().getName());
             try (NetconfOperationService operationService = getOperationServiceWithRetries(expectedCapabilities, idForReporting)) {
+                if(!stopwatch.isRunning()) {
+                    stopwatch.start();
+                }
                 return pushConfig(configSnapshotHolder, operationService);
             } catch (ConflictingVersionException e) {
                 lastException = e;
-                LOG.debug("Conflicting version detected, will retry after timeout");
+                LOG.info("Conflicting version detected, will retry after timeout");
                 sleep();
             }
         } while (stopwatch.elapsed(TimeUnit.MILLISECONDS) < conflictingVersionTimeoutMillis);
index 135d5ff..787f8b1 100644 (file)
@@ -41,7 +41,7 @@ public class ConfigPersisterActivator implements BundleActivator {
     public static final String MAX_WAIT_FOR_CAPABILITIES_MILLIS_PROPERTY = "maxWaitForCapabilitiesMillis";
     private static final long MAX_WAIT_FOR_CAPABILITIES_MILLIS_DEFAULT = TimeUnit.MINUTES.toMillis(2);
     public static final String CONFLICTING_VERSION_TIMEOUT_MILLIS_PROPERTY = "conflictingVersionTimeoutMillis";
-    private static final long CONFLICTING_VERSION_TIMEOUT_MILLIS_DEFAULT = TimeUnit.SECONDS.toMillis(30);
+    private static final long CONFLICTING_VERSION_TIMEOUT_MILLIS_DEFAULT = TimeUnit.MINUTES.toMillis(1);
 
     public static final String NETCONF_CONFIG_PERSISTER = "netconf.config.persister";
 
index f1f7375..45088f6 100644 (file)
         <artifactId>maven-bundle-plugin</artifactId>
         <configuration>
           <instructions>
-            <Import-Package>org.apache.sshd.*, com.google.common.base, com.google.common.collect, io.netty.buffer,
-              io.netty.channel, io.netty.channel.socket, io.netty.handler.codec, io.netty.handler.ssl, io.netty.util,
-              io.netty.util.concurrent, javax.xml.transform, javax.xml.transform.dom, javax.xml.transform.sax,
-              javax.xml.transform.stream, org.opendaylight.controller.netconf.api,
-              org.opendaylight.controller.netconf.util.messages, org.opendaylight.controller.netconf.util.xml,
-              org.opendaylight.protocol.framework, org.openexi.proc, org.openexi.proc.common, org.openexi.proc.grammars,
-              org.openexi.sax, org.openexi.schema, org.slf4j, org.w3c.dom, org.xml.sax</Import-Package>
+            <Import-Package>*</Import-Package>
             <Export-Package>org.opendaylight.controller.netconf.nettyutil,
               org.opendaylight.controller.netconf.nettyutil.handler,
               org.opendaylight.controller.netconf.nettyutil.handler.exi,