From 47f9449f977820bc605bafa23e81ee2efe6d9f37 Mon Sep 17 00:00:00 2001 From: Michael Vorburger Date: Fri, 14 Jul 2017 02:47:24 +0530 Subject: [PATCH] Bug 8163: getDataTreeChangeListenerExecutor() & DataBrokerTestModule Adjust AbstractDataBrokerTestCustomizer with a getDataTreeChangeListenerExecutor() instead of a setDataTreeChangeListenerExecutor(), just for more consistency with the existing getCommitCoordinatorExecutor() method. Also less confusing (to me) than seeing the private Executor set by default which may get changed by the setter later. Adjust ConcurrentDataBrokerTestCustomizer with the useMTDataTreeChangeListenerExecutor as a constructor argument, instead of an useMTDataTreeChangeListenerExecutor() method, just for consistency for how you already have it in AbstractConcurrentDataBrokerTest. Extend ConstantSchemaAbstractDataBrokerTest and DataBrokerTestModule to allow passing through this new opt-in tweak flag, so that tests in downstream projects such as genius and netvirt can staring exploring enabling this. Change-Id: I4ad85ac48163d2f4bac865f46a3b047d5b7d333a Signed-off-by: Michael Vorburger --- .../test/AbstractConcurrentDataBrokerTest.java | 7 +------ .../test/AbstractDataBrokerTestCustomizer.java | 13 ++++++------- .../test/ConcurrentDataBrokerTestCustomizer.java | 16 ++++++++++++++-- .../ConstantSchemaAbstractDataBrokerTest.java | 8 ++++++++ .../sal/binding/test/DataBrokerTestModule.java | 16 +++++++++++++--- 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractConcurrentDataBrokerTest.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractConcurrentDataBrokerTest.java index 77c0c3f83f..de4d0ec13a 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractConcurrentDataBrokerTest.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractConcurrentDataBrokerTest.java @@ -31,12 +31,7 @@ public abstract class AbstractConcurrentDataBrokerTest extends AbstractBaseDataB @Override protected AbstractDataBrokerTestCustomizer createDataBrokerTestCustomizer() { - ConcurrentDataBrokerTestCustomizer customizer = new ConcurrentDataBrokerTestCustomizer(); - if (useMTDataTreeChangeListenerExecutor) { - customizer.useMTDataTreeChangeListenerExecutor(); - } - - return customizer; + return new ConcurrentDataBrokerTestCustomizer(useMTDataTreeChangeListenerExecutor); } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractDataBrokerTestCustomizer.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractDataBrokerTestCustomizer.java index 8d968b0a01..375a9f119c 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractDataBrokerTestCustomizer.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/AbstractDataBrokerTestCustomizer.java @@ -40,7 +40,6 @@ public abstract class AbstractDataBrokerTestCustomizer { private final MockSchemaService schemaService; private ImmutableMap datastores; private final BindingToNormalizedNodeCodec bindingToNormalized; - private ListeningExecutorService dataTreeChangeListenerExecutor = MoreExecutors.newDirectExecutorService(); public ImmutableMap createDatastores() { return ImmutableMap.builder() @@ -61,13 +60,13 @@ public abstract class AbstractDataBrokerTestCustomizer { } public DOMStore createConfigurationDatastore() { - final InMemoryDOMDataStore store = new InMemoryDOMDataStore("CFG", dataTreeChangeListenerExecutor); + final InMemoryDOMDataStore store = new InMemoryDOMDataStore("CFG", getDataTreeChangeListenerExecutor()); this.schemaService.registerSchemaContextListener(store); return store; } public DOMStore createOperationalDatastore() { - final InMemoryDOMDataStore store = new InMemoryDOMDataStore("OPER", dataTreeChangeListenerExecutor); + final InMemoryDOMDataStore store = new InMemoryDOMDataStore("OPER", getDataTreeChangeListenerExecutor()); this.schemaService.registerSchemaContextListener(store); return store; } @@ -86,6 +85,10 @@ public abstract class AbstractDataBrokerTestCustomizer { public abstract ListeningExecutorService getCommitCoordinatorExecutor(); + public ListeningExecutorService getDataTreeChangeListenerExecutor() { + return MoreExecutors.newDirectExecutorService(); + } + public DataBroker createDataBroker() { return new BindingDOMDataBrokerAdapter(getDOMDataBroker(), this.bindingToNormalized); } @@ -112,10 +115,6 @@ public abstract class AbstractDataBrokerTestCustomizer { return this.datastores; } - public void setDataTreeChangeListenerExecutor(ListeningExecutorService executor) { - this.dataTreeChangeListenerExecutor = executor; - } - public void updateSchema(final SchemaContext ctx) { this.schemaService.changeSchema(ctx); } diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConcurrentDataBrokerTestCustomizer.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConcurrentDataBrokerTestCustomizer.java index 0b2de070f8..ee761f4919 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConcurrentDataBrokerTestCustomizer.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConcurrentDataBrokerTestCustomizer.java @@ -21,12 +21,24 @@ import java.util.concurrent.Executors; */ public class ConcurrentDataBrokerTestCustomizer extends AbstractDataBrokerTestCustomizer { + private final ListeningExecutorService dataTreeChangeListenerExecutorSingleton; + + public ConcurrentDataBrokerTestCustomizer(boolean useMTDataTreeChangeListenerExecutor) { + if (useMTDataTreeChangeListenerExecutor) { + dataTreeChangeListenerExecutorSingleton = MoreExecutors.listeningDecorator(Executors.newCachedThreadPool()); + } else { + dataTreeChangeListenerExecutorSingleton = MoreExecutors.newDirectExecutorService(); + } + } + @Override public ListeningExecutorService getCommitCoordinatorExecutor() { return MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()); } - public void useMTDataTreeChangeListenerExecutor() { - setDataTreeChangeListenerExecutor(MoreExecutors.listeningDecorator(Executors.newCachedThreadPool())); + @Override + public ListeningExecutorService getDataTreeChangeListenerExecutor() { + return dataTreeChangeListenerExecutorSingleton; } + } diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConstantSchemaAbstractDataBrokerTest.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConstantSchemaAbstractDataBrokerTest.java index 5c09a4f863..ba646c4858 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConstantSchemaAbstractDataBrokerTest.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/ConstantSchemaAbstractDataBrokerTest.java @@ -18,6 +18,14 @@ import org.opendaylight.yangtools.yang.model.api.SchemaContext; */ public class ConstantSchemaAbstractDataBrokerTest extends AbstractConcurrentDataBrokerTest { + public ConstantSchemaAbstractDataBrokerTest() { + super(); + } + + public ConstantSchemaAbstractDataBrokerTest(final boolean useMTDataTreeChangeListenerExecutor) { + super(useMTDataTreeChangeListenerExecutor); + } + @Override protected SchemaContext getSchemaContext() throws Exception { return SchemaContextSingleton.getSchemaContext(super::getSchemaContext); diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/DataBrokerTestModule.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/DataBrokerTestModule.java index c71049e444..990d75a917 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/DataBrokerTestModule.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/test/DataBrokerTestModule.java @@ -9,19 +9,29 @@ package org.opendaylight.controller.md.sal.binding.test; import org.opendaylight.controller.md.sal.binding.api.DataBroker; -// @Module public class DataBrokerTestModule { + public static DataBroker dataBroker() { + return new DataBrokerTestModule(false).getDataBroker(); + } + + private final boolean useMTDataTreeChangeListenerExecutor; + + public DataBrokerTestModule(boolean useMTDataTreeChangeListenerExecutor) { + this.useMTDataTreeChangeListenerExecutor = useMTDataTreeChangeListenerExecutor; + } + // Suppress IllegalCatch because of AbstractDataBrokerTest (change later) @SuppressWarnings({ "checkstyle:IllegalCatch", "checkstyle:IllegalThrows" }) - public static /* @Provides @Singleton */ DataBroker dataBroker() throws RuntimeException { + public DataBroker getDataBroker() throws RuntimeException { try { // This is a little bit "upside down" - in the future, // we should probably put what is in AbstractDataBrokerTest // into this DataBrokerTestModule, and make AbstractDataBrokerTest // use it, instead of the way around it currently is (the opposite); // this is just for historical reasons... and works for now. - ConstantSchemaAbstractDataBrokerTest dataBrokerTest = new ConstantSchemaAbstractDataBrokerTest(); + ConstantSchemaAbstractDataBrokerTest dataBrokerTest + = new ConstantSchemaAbstractDataBrokerTest(useMTDataTreeChangeListenerExecutor); dataBrokerTest.setup(); return dataBrokerTest.getDataBroker(); } catch (Exception e) { -- 2.36.6