From 1d802172c8b176cc51674f5dbf570ee1bd32fcd6 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 16 Nov 2023 13:40:03 +0100 Subject: [PATCH] Use Timer in NetconfNodeHandler We are using ScheduledExecutorService to scheduler timers. Since we are already passing down Timer to other timeouts, also migrate NetconfNodeHandler to use it. JIRA: NETCONF-1106 Change-Id: I7b231cd4c91b67eff8c8937124a04338ffc8bbe0 Signed-off-by: Robert Varga --- .../callhome/mount/CallHomeMountFactory.java | 25 ++++------ .../callhome/mount/CallHomeTopology.java | 13 +++--- .../mount/CallHomeMountFactoryTest.java | 7 +-- .../topology/impl/NetconfTopologyImpl.java | 37 +++++++-------- .../impl/NetconfTopologyImplTest.java | 15 +++--- .../singleton/impl/NetconfNodeContext.java | 7 ++- .../impl/NetconfTopologyManager.java | 35 ++++++-------- .../impl/utils/NetconfTopologySetup.java | 17 ------- .../impl/MountPointEndToEndTest.java | 14 ++---- .../impl/NetconfTopologyManagerTest.java | 40 ++++++++-------- .../topology/spi/AbstractNetconfTopology.java | 14 ++---- .../topology/spi/NetconfNodeHandler.java | 46 ++++++++++--------- .../topology/spi/NetconfNodeHandlerTest.java | 30 +++++------- 13 files changed, 123 insertions(+), 177 deletions(-) diff --git a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactory.java b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactory.java index b24810e2e4..4efdb4bbf2 100644 --- a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactory.java +++ b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactory.java @@ -15,8 +15,6 @@ import com.google.common.util.concurrent.ListenableFuture; import io.netty.util.Timer; import java.net.InetSocketAddress; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; -import org.opendaylight.controller.config.threadpool.ScheduledThreadPool; import org.opendaylight.controller.config.threadpool.ThreadPool; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.dom.api.DOMMountPointService; @@ -46,7 +44,6 @@ public class CallHomeMountFactory implements NetconfClientFactory, CallHomeNetco private final CallHomeMountSessionManager sessionManager = new CallHomeMountSessionManager(); private final String topologyId; private final Timer timer; - private final ScheduledExecutorService scheduledExecutor; private final Executor processingExecutor; private final SchemaResourceManager schemaRepositoryProvider; private final DataBroker dataBroker; @@ -58,39 +55,33 @@ public class CallHomeMountFactory implements NetconfClientFactory, CallHomeNetco private final DeviceActionFactory deviceActionFactory; private final BaseNetconfSchemas baseSchemas; - public CallHomeMountFactory(final String topologyId, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, + public CallHomeMountFactory(final String topologyId, final Timer timer, final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, final BaseNetconfSchemas baseSchemas, final DataBroker dataBroker, final DOMMountPointService mountService, final NetconfClientConfigurationBuilderFactory builderFactory) { - this(topologyId, timer, scheduledExecutor, processingExecutor, schemaRepositoryProvider, baseSchemas, - dataBroker, mountService, builderFactory, null); + this(topologyId, timer, processingExecutor, schemaRepositoryProvider, baseSchemas, dataBroker, mountService, + builderFactory, null); } @Activate public CallHomeMountFactory(@Reference(target = "(type=global-timer)") final Timer timer, - @Reference(target = "(type=global-netconf-ssh-scheduled-executor)") - final ScheduledThreadPool scheduledThreadPool, @Reference(target = "(type=global-netconf-processing-executor)") final ThreadPool processingThreadPool, @Reference final SchemaResourceManager schemaRepositoryProvider, @Reference final BaseNetconfSchemas baseSchemas, @Reference final DataBroker dataBroker, @Reference final DOMMountPointService mountService, @Reference(target = "(type=legacy)") final NetconfClientConfigurationBuilderFactory builderFactory, @Reference final DeviceActionFactory deviceActionFactory) { - this(NetconfNodeUtils.DEFAULT_TOPOLOGY_NAME, timer, scheduledThreadPool.getExecutor(), - processingThreadPool.getExecutor(), schemaRepositoryProvider, baseSchemas, dataBroker, mountService, - builderFactory, deviceActionFactory); + this(NetconfNodeUtils.DEFAULT_TOPOLOGY_NAME, timer, processingThreadPool.getExecutor(), + schemaRepositoryProvider, baseSchemas, dataBroker, mountService, builderFactory, deviceActionFactory); } - public CallHomeMountFactory(final String topologyId, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, + public CallHomeMountFactory(final String topologyId, final Timer timer, final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, final BaseNetconfSchemas baseSchemas, final DataBroker dataBroker, final DOMMountPointService mountService, final NetconfClientConfigurationBuilderFactory builderFactory, final DeviceActionFactory deviceActionFactory) { this.topologyId = topologyId; this.timer = requireNonNull(timer); - this.scheduledExecutor = scheduledExecutor; this.processingExecutor = processingExecutor; this.schemaRepositoryProvider = schemaRepositoryProvider; this.deviceActionFactory = deviceActionFactory; @@ -131,8 +122,8 @@ public class CallHomeMountFactory implements NetconfClientFactory, CallHomeNetco @VisibleForTesting void createTopology() { - topology = new CallHomeTopology(topologyId, this, timer, scheduledExecutor, processingExecutor, - schemaRepositoryProvider, dataBroker, mountService, builderFactory, baseSchemas, deviceActionFactory); + topology = new CallHomeTopology(topologyId, this, timer, processingExecutor, schemaRepositoryProvider, + dataBroker, mountService, builderFactory, baseSchemas, deviceActionFactory); } @VisibleForTesting diff --git a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeTopology.java b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeTopology.java index bf3626b037..934f964a8d 100644 --- a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeTopology.java +++ b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeTopology.java @@ -10,7 +10,6 @@ package org.opendaylight.netconf.callhome.mount; import com.google.common.annotations.VisibleForTesting; import io.netty.util.Timer; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.dom.api.DOMMountPointService; import org.opendaylight.netconf.client.NetconfClientFactory; @@ -25,12 +24,12 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology. // Non-final for mocking public class CallHomeTopology extends AbstractNetconfTopology { public CallHomeTopology(final String topologyId, final NetconfClientFactory clientFactory, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, - final SchemaResourceManager schemaRepositoryProvider, final DataBroker dataBroker, - final DOMMountPointService mountPointService, final NetconfClientConfigurationBuilderFactory builderFactory, - final BaseNetconfSchemas baseSchemas, final DeviceActionFactory deviceActionFactory) { - super(topologyId, clientFactory, timer, scheduledExecutor, processingExecutor, schemaRepositoryProvider, - dataBroker, mountPointService, builderFactory, deviceActionFactory, baseSchemas); + final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, + final DataBroker dataBroker, final DOMMountPointService mountPointService, + final NetconfClientConfigurationBuilderFactory builderFactory, final BaseNetconfSchemas baseSchemas, + final DeviceActionFactory deviceActionFactory) { + super(topologyId, clientFactory, timer, processingExecutor, schemaRepositoryProvider, dataBroker, + mountPointService, builderFactory, deviceActionFactory, baseSchemas); } void disconnectNode(final NodeId nodeId) { diff --git a/apps/callhome-provider/src/test/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactoryTest.java b/apps/callhome-provider/src/test/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactoryTest.java index 866afcaba8..833440989d 100644 --- a/apps/callhome-provider/src/test/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactoryTest.java +++ b/apps/callhome-provider/src/test/java/org/opendaylight/netconf/callhome/mount/CallHomeMountFactoryTest.java @@ -21,7 +21,6 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,8 +45,6 @@ public class CallHomeMountFactoryTest { @Mock private Timer mockTimer; @Mock - private ScheduledExecutorService mockScheduledExecutor; - @Mock private Executor mockProcessingExecutor; @Mock private SchemaResourceManager mockSchemaRepoProvider; @@ -73,8 +70,8 @@ public class CallHomeMountFactoryTest { public void setup() { topologyId = ""; - instance = new CallHomeMountFactory(topologyId, mockTimer, mockScheduledExecutor, mockProcessingExecutor, - mockSchemaRepoProvider, mockBaseSchemas, mockDataBroker, mockMount, mockBuilderFactory) { + instance = new CallHomeMountFactory(topologyId, mockTimer, mockProcessingExecutor, mockSchemaRepoProvider, + mockBaseSchemas, mockDataBroker, mockMount, mockBuilderFactory) { @Override CallHomeMountSessionManager sessionManager() { return mockSessMgr; diff --git a/apps/netconf-topology-impl/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java b/apps/netconf-topology-impl/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java index 8cd0821a25..6ae27746d9 100644 --- a/apps/netconf-topology-impl/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java +++ b/apps/netconf-topology-impl/src/main/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImpl.java @@ -11,12 +11,10 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.netty.util.Timer; import java.util.Collection; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.aaa.encrypt.AAAEncryptionService; -import org.opendaylight.controller.config.threadpool.ScheduledThreadPool; import org.opendaylight.controller.config.threadpool.ThreadPool; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; @@ -62,8 +60,6 @@ public class NetconfTopologyImpl extends AbstractNetconfTopology public NetconfTopologyImpl( @Reference(target = "(type=netconf-client-factory)") final NetconfClientFactory clientFactory, @Reference(target = "(type=global-timer)") final Timer timer, - @Reference(target = "(type=global-netconf-ssh-scheduled-executor)") - final ScheduledThreadPool scheduledThreadPool, @Reference(target = "(type=global-netconf-processing-executor)") final ThreadPool processingThreadPool, @Reference final SchemaResourceManager schemaRepositoryProvider, @Reference final DataBroker dataBroker, @Reference final DOMMountPointService mountPointService, @@ -71,31 +67,30 @@ public class NetconfTopologyImpl extends AbstractNetconfTopology @Reference(target = "(type=default)") final NetconfClientConfigurationBuilderFactory builderFactory, @Reference final RpcProviderService rpcProviderService, @Reference final BaseNetconfSchemas baseSchemas, @Reference final DeviceActionFactory deviceActionFactory) { - this(NetconfNodeUtils.DEFAULT_TOPOLOGY_NAME, clientFactory, timer, scheduledThreadPool.getExecutor(), - processingThreadPool.getExecutor(), schemaRepositoryProvider, dataBroker, mountPointService, - encryptionService, builderFactory, rpcProviderService, baseSchemas, deviceActionFactory); + this(NetconfNodeUtils.DEFAULT_TOPOLOGY_NAME, clientFactory, timer, processingThreadPool.getExecutor(), + schemaRepositoryProvider, dataBroker, mountPointService, encryptionService, builderFactory, + rpcProviderService, baseSchemas, deviceActionFactory); } public NetconfTopologyImpl(final String topologyId, final NetconfClientFactory clientclientFactory, - final Timer timer, final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, - final SchemaResourceManager schemaRepositoryProvider, final DataBroker dataBroker, - final DOMMountPointService mountPointService, final AAAEncryptionService encryptionService, - final NetconfClientConfigurationBuilderFactory builderFactory, final RpcProviderService rpcProviderService, - final BaseNetconfSchemas baseSchemas) { - this(topologyId, clientclientFactory, timer, scheduledExecutor, processingExecutor, schemaRepositoryProvider, - dataBroker, mountPointService, encryptionService, builderFactory, rpcProviderService, baseSchemas, null); + final Timer timer, final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, + final DataBroker dataBroker, final DOMMountPointService mountPointService, + final AAAEncryptionService encryptionService, final NetconfClientConfigurationBuilderFactory builderFactory, + final RpcProviderService rpcProviderService, final BaseNetconfSchemas baseSchemas) { + this(topologyId, clientclientFactory, timer, processingExecutor, schemaRepositoryProvider, dataBroker, + mountPointService, encryptionService, builderFactory, rpcProviderService, baseSchemas, null); } @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", justification = "DTCL registration of 'this'") public NetconfTopologyImpl(final String topologyId, final NetconfClientFactory clientFactory, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, - final SchemaResourceManager schemaRepositoryProvider, final DataBroker dataBroker, - final DOMMountPointService mountPointService, final AAAEncryptionService encryptionService, - final NetconfClientConfigurationBuilderFactory builderFactory, final RpcProviderService rpcProviderService, - final BaseNetconfSchemas baseSchemas, final DeviceActionFactory deviceActionFactory) { - super(topologyId, clientFactory, timer, scheduledExecutor, processingExecutor, - schemaRepositoryProvider, dataBroker, mountPointService, builderFactory, deviceActionFactory, baseSchemas); + final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, + final DataBroker dataBroker, final DOMMountPointService mountPointService, + final AAAEncryptionService encryptionService, final NetconfClientConfigurationBuilderFactory builderFactory, + final RpcProviderService rpcProviderService, final BaseNetconfSchemas baseSchemas, + final DeviceActionFactory deviceActionFactory) { + super(topologyId, clientFactory, timer, processingExecutor, schemaRepositoryProvider, dataBroker, + mountPointService, builderFactory, deviceActionFactory, baseSchemas); LOG.debug("Registering datastore listener"); dtclReg = dataBroker.registerDataTreeChangeListener(DataTreeIdentifier.create( diff --git a/apps/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java b/apps/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java index 4783e53bc6..530b3c935c 100644 --- a/apps/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java +++ b/apps/netconf-topology-impl/src/test/java/org/opendaylight/netconf/topology/impl/NetconfTopologyImplTest.java @@ -16,7 +16,6 @@ import com.google.common.util.concurrent.MoreExecutors; import io.netty.util.Timer; import java.util.List; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -69,8 +68,6 @@ class NetconfTopologyImplTest { @Mock private Timer mockedTimer; @Mock - private ScheduledExecutorService mockedScheduledExecutor; - @Mock private SchemaResourceManager mockedResourceManager; @Mock private DataBroker dataBroker; @@ -98,8 +95,8 @@ class NetconfTopologyImplTest { doReturn(CommitInfo.emptyFluentFuture()).when(wtx).commit(); topology = new TestingNetconfTopologyImpl(TOPOLOGY_KEY.getTopologyId().getValue(), mockedClientFactory, - mockedTimer, mockedScheduledExecutor, MoreExecutors.directExecutor(), mockedResourceManager, dataBroker, - mountPointService, encryptionService, builderFactory, rpcProviderService, + mockedTimer, MoreExecutors.directExecutor(), mockedResourceManager, dataBroker, mountPointService, + encryptionService, builderFactory, rpcProviderService, new DefaultBaseNetconfSchemas(new DefaultYangParserFactory())); //verify initialization of topology verify(wtx).merge(LogicalDatastoreType.OPERATIONAL, TOPOLOGY_PATH, @@ -152,12 +149,12 @@ class NetconfTopologyImplTest { private static class TestingNetconfTopologyImpl extends NetconfTopologyImpl { TestingNetconfTopologyImpl(final String topologyId, final NetconfClientFactory clientFactory, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, - final SchemaResourceManager schemaRepositoryProvider, final DataBroker dataBroker, - final DOMMountPointService mountPointService, final AAAEncryptionService encryptionService, + final Executor processingExecutor, final SchemaResourceManager schemaRepositoryProvider, + final DataBroker dataBroker, final DOMMountPointService mountPointService, + final AAAEncryptionService encryptionService, final NetconfClientConfigurationBuilderFactory builderFactory, final RpcProviderService rpcProviderService, final BaseNetconfSchemas baseSchemas) { - super(topologyId, clientFactory, timer, scheduledExecutor, processingExecutor, schemaRepositoryProvider, + super(topologyId, clientFactory, timer, processingExecutor, schemaRepositoryProvider, dataBroker, mountPointService, encryptionService, builderFactory, rpcProviderService, baseSchemas); } diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeContext.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeContext.java index 1b4a6bc1b1..368030ad69 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeContext.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeContext.java @@ -141,10 +141,9 @@ final class NetconfNodeContext implements AutoCloseable { // Instantiate the handler ... masterSalFacade = createSalFacade(netconfNode.requireLockDatastore()); - nodeHandler = new NetconfNodeHandler(setup.getNetconfClientFactory(), setup.getTimer(), - setup.getKeepaliveExecutor(), setup.getBaseSchemas(), schemaManager, setup.getProcessingExecutor(), - builderFactory, deviceActionFactory, masterSalFacade, remoteDeviceId, configNode.getNodeId(), netconfNode, - nodeOptional); + nodeHandler = new NetconfNodeHandler(setup.getNetconfClientFactory(), setup.getTimer(), setup.getBaseSchemas(), + schemaManager, setup.getProcessingExecutor(), builderFactory, deviceActionFactory, masterSalFacade, + remoteDeviceId, configNode.getNodeId(), netconfNode, nodeOptional); nodeHandler.connect(); } diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java index c4bfe1d5fb..88f465ca1f 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManager.java @@ -21,13 +21,11 @@ import java.util.Collection; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.aaa.encrypt.AAAEncryptionService; import org.opendaylight.controller.cluster.ActorSystemProvider; -import org.opendaylight.controller.config.threadpool.ScheduledThreadPool; import org.opendaylight.controller.config.threadpool.ThreadPool; import org.opendaylight.mdsal.binding.api.ClusteredDataTreeChangeListener; import org.opendaylight.mdsal.binding.api.DataBroker; @@ -99,7 +97,6 @@ public class NetconfTopologyManager implements ClusteredDataTreeChangeListener instanceIdentifier; private final Node node; private final Timer timer; - private final ScheduledExecutorService keepaliveExecutor; private final Executor processingExecutor; private final ActorSystem actorSystem; private final NetconfClientFactory netconfClientFactory; @@ -44,7 +42,6 @@ public final class NetconfTopologySetup { instanceIdentifier = builder.getInstanceIdentifier(); node = builder.getNode(); timer = builder.getTimer(); - keepaliveExecutor = builder.getKeepaliveExecutor(); processingExecutor = builder.getProcessingExecutor(); actorSystem = builder.getActorSystem(); netconfClientFactory = builder.getNetconfClientFactory(); @@ -74,10 +71,6 @@ public final class NetconfTopologySetup { return processingExecutor; } - public ScheduledExecutorService getKeepaliveExecutor() { - return keepaliveExecutor; - } - public Timer getTimer() { return timer; } @@ -116,7 +109,6 @@ public final class NetconfTopologySetup { private InstanceIdentifier instanceIdentifier; private Node node; private Timer timer; - private ScheduledExecutorService keepaliveExecutor; private Executor processingExecutor; private ActorSystem actorSystem; private String topologyId; @@ -188,15 +180,6 @@ public final class NetconfTopologySetup { return this; } - ScheduledExecutorService getKeepaliveExecutor() { - return keepaliveExecutor; - } - - public Builder setKeepaliveExecutor(final ScheduledExecutorService keepaliveExecutor) { - this.keepaliveExecutor = keepaliveExecutor; - return this; - } - Executor getProcessingExecutor() { return processingExecutor; } diff --git a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java index 5f10f89e92..fde7661b12 100644 --- a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java +++ b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/MountPointEndToEndTest.java @@ -42,7 +42,6 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.apache.commons.io.FileUtils; @@ -193,8 +192,6 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest { @Mock private Timer mockTimer; @Mock - private ScheduledExecutorService mockKeepaliveExecutor; - @Mock private DeviceActionFactory deviceActionFactory; @Mock private CredentialProvider credentialProvider; @@ -317,8 +314,8 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest { YangTextSchemaSource.class, 1)); masterNetconfTopologyManager = new NetconfTopologyManager(BASE_SCHEMAS, masterDataBroker, - masterClusterSingletonServiceProvider, mockTimer, mockKeepaliveExecutor, MoreExecutors.directExecutor(), - masterSystem, mockClientFactory, masterMountPointService, mockEncryptionService, mockRpcProviderService, + masterClusterSingletonServiceProvider, mockTimer, MoreExecutors.directExecutor(), masterSystem, + mockClientFactory, masterMountPointService, mockEncryptionService, mockRpcProviderService, deviceActionFactory, resourceManager, builderFactory, TOPOLOGY_ID, Uint16.ZERO) { @Override protected NetconfTopologyContext newNetconfTopologyContext(final NetconfTopologySetup setup, @@ -353,10 +350,9 @@ public class MountPointEndToEndTest extends AbstractBaseSchemasTest { .registerClusterSingletonService(any()); slaveNetconfTopologyManager = new NetconfTopologyManager(BASE_SCHEMAS, slaveDataBroker, - mockSlaveClusterSingletonServiceProvider, mockTimer, mockKeepaliveExecutor, - MoreExecutors.directExecutor(), slaveSystem, mockClientFactory, slaveMountPointService, - mockEncryptionService, mockRpcProviderService, deviceActionFactory, resourceManager, builderFactory, - TOPOLOGY_ID, Uint16.ZERO) { + mockSlaveClusterSingletonServiceProvider, mockTimer, MoreExecutors.directExecutor(), slaveSystem, + mockClientFactory, slaveMountPointService, mockEncryptionService, mockRpcProviderService, + deviceActionFactory, resourceManager, builderFactory, TOPOLOGY_ID, Uint16.ZERO) { @Override protected NetconfTopologyContext newNetconfTopologyContext(final NetconfTopologySetup setup, final ServiceGroupIdentifier serviceGroupIdent, final Timeout actorResponseWaitTime, diff --git a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java index a71db25604..65dc68394c 100644 --- a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java +++ b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfTopologyManagerTest.java @@ -32,7 +32,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.function.Function; import org.junit.Before; @@ -81,7 +80,7 @@ import org.opendaylight.yangtools.yang.parser.impl.DefaultYangParserFactory; @RunWith(MockitoJUnitRunner.StrictStubs.class) public class NetconfTopologyManagerTest extends AbstractBaseSchemasTest { - private static final Uint16 ACTOR_RESPONSE_WAIT_TIME = Uint16.valueOf(10); + private static final Uint16 ACTOR_RESPONSE_WAIT_TIME = Uint16.TEN; private static final String TOPOLOGY_ID = "topologyID"; private NetconfTopologyManager netconfTopologyManager; @@ -92,6 +91,24 @@ public class NetconfTopologyManagerTest extends AbstractBaseSchemasTest { private ListenerRegistration mockListenerReg; @Mock private Registration mockRpcReg; + @Mock + private Timer timer; + @Mock + private ExecutorService processingService; + @Mock + private ActorSystem actorSystem; + @Mock + private NetconfClientFactory clientFactory; + @Mock + private DOMMountPointService mountPointService; + @Mock + private AAAEncryptionService encryptionService; + @Mock + private DeviceActionFactory actionFactory; + @Mock + private RpcProviderService rpcProviderService; + @Mock + private NetconfClientConfigurationBuilderFactory builderFactory; private DataBroker dataBroker; @@ -110,27 +127,14 @@ public class NetconfTopologyManagerTest extends AbstractBaseSchemasTest { dataBrokerTest.setup(); dataBroker = spy(dataBrokerTest.getDataBroker()); - final Timer timer = mock(Timer.class); - final ScheduledExecutorService keepaliveExecutor = mock(ScheduledExecutorService.class); - final ExecutorService processingService = mock(ExecutorService.class); - final ActorSystem actorSystem = mock(ActorSystem.class); - final NetconfClientFactory clientFactory = mock(NetconfClientFactory.class); - final DOMMountPointService mountPointService = mock(DOMMountPointService.class); - final AAAEncryptionService encryptionService = mock(AAAEncryptionService.class); - final DeviceActionFactory deviceActionFactory = mock(DeviceActionFactory.class); - final RpcProviderService rpcProviderService = mock(RpcProviderService.class); - final NetconfClientConfigurationBuilderFactory builderFactory = - mock(NetconfClientConfigurationBuilderFactory.class); - doNothing().when(mockListenerReg).close(); doReturn(mockListenerReg).when(dataBroker).registerDataTreeChangeListener(any(), any()); doReturn(mockRpcReg).when(rpcProviderService).registerRpcImplementations(any()); netconfTopologyManager = new NetconfTopologyManager(BASE_SCHEMAS, dataBroker, clusterSingletonServiceProvider, - timer, keepaliveExecutor, processingService, actorSystem, clientFactory, mountPointService, - encryptionService, rpcProviderService, deviceActionFactory, - new DefaultSchemaResourceManager(new DefaultYangParserFactory()), builderFactory, - TOPOLOGY_ID, Uint16.ZERO) { + timer, processingService, actorSystem, clientFactory, mountPointService, encryptionService, + rpcProviderService, actionFactory, new DefaultSchemaResourceManager(new DefaultYangParserFactory()), + builderFactory, TOPOLOGY_ID, Uint16.ZERO) { @Override protected NetconfTopologyContext newNetconfTopologyContext(final NetconfTopologySetup setup, final ServiceGroupIdentifier serviceGroupIdent, final Timeout actorResponseWaitTime, diff --git a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java index 6dcd83fefa..859bb02bf8 100644 --- a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java +++ b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java @@ -15,7 +15,6 @@ import java.util.HashMap; import java.util.NoSuchElementException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.dom.api.DOMMountPointService; @@ -49,21 +48,19 @@ public abstract class AbstractNetconfTopology { private final NetconfClientConfigurationBuilderFactory builderFactory; private final Timer timer; - protected final ScheduledExecutorService scheduledExecutor; protected final Executor processingExecutor; protected final DataBroker dataBroker; protected final DOMMountPointService mountPointService; protected final String topologyId; protected AbstractNetconfTopology(final String topologyId, final NetconfClientFactory clientFactory, - final Timer timer, final ScheduledExecutorService scheduledExecutor, final Executor processingExecutor, - final SchemaResourceManager schemaManager, final DataBroker dataBroker, - final DOMMountPointService mountPointService, final NetconfClientConfigurationBuilderFactory builderFactory, + final Timer timer, final Executor processingExecutor, final SchemaResourceManager schemaManager, + final DataBroker dataBroker, final DOMMountPointService mountPointService, + final NetconfClientConfigurationBuilderFactory builderFactory, final DeviceActionFactory deviceActionFactory, final BaseNetconfSchemas baseSchemas) { this.topologyId = requireNonNull(topologyId); this.clientFactory = requireNonNull(clientFactory); this.timer = requireNonNull(timer); - this.scheduledExecutor = requireNonNull(scheduledExecutor); this.processingExecutor = requireNonNull(processingExecutor); this.schemaManager = requireNonNull(schemaManager); this.deviceActionFactory = deviceActionFactory; @@ -122,9 +119,8 @@ public abstract class AbstractNetconfTopology { final NetconfNodeHandler nodeHandler; try { - nodeHandler = new NetconfNodeHandler(clientFactory, timer, scheduledExecutor, baseSchemas, - schemaManager, processingExecutor, builderFactory, deviceActionFactory, deviceSalFacade, - deviceId, nodeId, netconfNode, nodeOptional); + nodeHandler = new NetconfNodeHandler(clientFactory, timer, baseSchemas, schemaManager, processingExecutor, + builderFactory, deviceActionFactory, deviceSalFacade, deviceId, nodeId, netconfNode, nodeOptional); } catch (IllegalArgumentException e) { // This is a workaround for NETCONF-1114 where the encrypted password's lexical structure is not enforced // in the datastore and it ends up surfacing when we decrypt the password. diff --git a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java index ea27e19da6..e084352201 100644 --- a/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java +++ b/apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandler.java @@ -14,14 +14,12 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; +import io.netty.util.Timeout; import io.netty.util.Timer; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.Executor; -import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import org.checkerframework.checker.lock.qual.GuardedBy; import org.checkerframework.checker.lock.qual.Holding; @@ -63,20 +61,20 @@ import org.slf4j.LoggerFactory; */ public final class NetconfNodeHandler extends AbstractRegistration implements RemoteDeviceHandler { private abstract static sealed class Task { - private final Future future; - Task(final Future future) { - this.future = requireNonNull(future); - } - - final void cancel() { - future.cancel(false); - } + abstract void cancel(); } private final class ConnectingTask extends Task implements FutureCallback { + private final ListenableFuture future; + ConnectingTask(final ListenableFuture future) { - super(future); + this.future = requireNonNull(future); + } + + @Override + void cancel() { + future.cancel(false); } @Override @@ -95,8 +93,15 @@ public final class NetconfNodeHandler extends AbstractRegistration implements Re } private static final class SleepingTask extends Task { - SleepingTask(final ScheduledFuture future) { - super(future); + private final Timeout timeout; + + SleepingTask(final Timeout timeout) { + this.timeout = requireNonNull(timeout); + } + + @Override + void cancel() { + timeout.cancel(); } } @@ -107,7 +112,7 @@ public final class NetconfNodeHandler extends AbstractRegistration implements Re private final @NonNull NetconfClientConfiguration clientConfig; private final @NonNull NetconfDeviceCommunicator communicator; private final @NonNull RemoteDeviceHandler delegate; - private final @NonNull ScheduledExecutorService scheduledExecutor; + private final @NonNull Timer timer; private final @NonNull RemoteDeviceId deviceId; private final long maxAttempts; @@ -122,14 +127,13 @@ public final class NetconfNodeHandler extends AbstractRegistration implements Re private Task currentTask; public NetconfNodeHandler(final NetconfClientFactory clientFactory, final Timer timer, - final ScheduledExecutorService scheduledExecutor, final BaseNetconfSchemas baseSchemas, - final SchemaResourceManager schemaManager, final Executor processingExecutor, - final NetconfClientConfigurationBuilderFactory builderFactory, + final BaseNetconfSchemas baseSchemas, final SchemaResourceManager schemaManager, + final Executor processingExecutor, final NetconfClientConfigurationBuilderFactory builderFactory, final DeviceActionFactory deviceActionFactory, final RemoteDeviceHandler delegate, final RemoteDeviceId deviceId, final NodeId nodeId, final NetconfNode node, final NetconfNodeAugmentedOptional nodeOptional) { this.clientFactory = requireNonNull(clientFactory); - this.scheduledExecutor = requireNonNull(scheduledExecutor); + this.timer = requireNonNull(timer); this.delegate = requireNonNull(delegate); this.deviceId = requireNonNull(deviceId); @@ -318,11 +322,11 @@ public final class NetconfNodeHandler extends AbstractRegistration implements Re // immediate reconnect. While we could check and got to lockedConnect(), it makes for a rare special case. // That special case makes for more code paths to test and introduces additional uncertainty as to whether // the attempt was executed on this thread or not. - currentTask = new SleepingTask(scheduledExecutor.schedule(this::reconnect, delayMillis, TimeUnit.MILLISECONDS)); + currentTask = new SleepingTask(timer.newTimeout(this::reconnect, delayMillis, TimeUnit.MILLISECONDS)); return null; } - private synchronized void reconnect() { + private synchronized void reconnect(final Timeout timeout) { currentTask = null; if (notClosed()) { lockedConnect(); diff --git a/apps/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandlerTest.java b/apps/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandlerTest.java index 81c2193f4c..c14f62c6eb 100644 --- a/apps/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandlerTest.java +++ b/apps/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfNodeHandlerTest.java @@ -23,12 +23,12 @@ import static org.mockito.Mockito.verifyNoInteractions; import com.google.common.net.InetAddresses; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.SettableFuture; +import io.netty.util.Timeout; import io.netty.util.Timer; +import io.netty.util.TimerTask; import java.net.InetSocketAddress; import java.util.List; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.BeforeClass; @@ -82,8 +82,6 @@ public class NetconfNodeHandlerTest { @Mock private Timer timer; @Mock - private ScheduledExecutorService scheduledExecutor; - @Mock private SchemaResourceManager schemaManager; @Mock private Executor processingExecutor; @@ -112,11 +110,11 @@ public class NetconfNodeHandlerTest { @Captor private ArgumentCaptor servicesCaptor; - // Mock eventExecutor-related things + // Mock Timer-related things @Mock - private ScheduledFuture scheduleFuture; + private Timeout timeout; @Captor - private ArgumentCaptor scheduleCaptor; + private ArgumentCaptor timerCaptor; @Mock private EffectiveModelContext schemaContext; @@ -135,8 +133,7 @@ public class NetconfNodeHandlerTest { @Before public void before() { // Instantiate the handler - handler = new NetconfNodeHandler(clientFactory, timer, scheduledExecutor, BASE_SCHEMAS, - schemaManager, processingExecutor, + handler = new NetconfNodeHandler(clientFactory, timer, BASE_SCHEMAS, schemaManager, processingExecutor, new DefaultNetconfClientConfigurationBuilderFactory(encryptionService, credentialProvider, sslHandlerFactoryProvider), deviceActionFactory, delegate, DEVICE_ID, NODE_ID, new NetconfNodeBuilder() @@ -189,15 +186,14 @@ public class NetconfNodeHandlerTest { assertEquals(1, handler.attempts()); // Note: this will count as a second attempt - doReturn(scheduleFuture).when(scheduledExecutor) - .schedule(scheduleCaptor.capture(), anyLong(), any(TimeUnit.class)); + doReturn(timeout).when(timer).newTimeout(timerCaptor.capture(), anyLong(), any()); handler.onDeviceFailed(new AssertionError("schema failure")); assertEquals(2, handler.attempts()); // and when we run the task, we get a clientDispatcher invocation, but attempts are still the same - scheduleCaptor.getValue().run(); + timerCaptor.getValue().run(timeout); verify(clientFactory, times(2)).createClient(any()); assertEquals(2, handler.attempts()); } @@ -209,14 +205,13 @@ public class NetconfNodeHandlerTest { // when the device is connected, we propagate the information and initiate reconnect doNothing().when(delegate).onDeviceDisconnected(); - doReturn(scheduleFuture).when(scheduledExecutor).schedule(scheduleCaptor.capture(), eq(100L), - eq(TimeUnit.MILLISECONDS)); + doReturn(timeout).when(timer).newTimeout(timerCaptor.capture(), eq(100L), eq(TimeUnit.MILLISECONDS)); handler.onDeviceDisconnected(); assertEquals(1, handler.attempts()); // and when we run the task, we get a clientDispatcher invocation, but attempts are still the same - scheduleCaptor.getValue().run(); + timerCaptor.getValue().run(timeout); verify(clientFactory, times(2)).createClient(any()); assertEquals(1, handler.attempts()); } @@ -229,14 +224,13 @@ public class NetconfNodeHandlerTest { handler.connect(); assertEquals(1, handler.attempts()); - doReturn(scheduleFuture).when(scheduledExecutor).schedule(scheduleCaptor.capture(), eq(150L), - eq(TimeUnit.MILLISECONDS)); + doReturn(timeout).when(timer).newTimeout(timerCaptor.capture(), eq(150L), eq(TimeUnit.MILLISECONDS)); firstFuture.setException(new AssertionError("first")); assertEquals(2, handler.attempts()); // and when we run the task, we get a clientDispatcher invocation, but attempts are still the same - scheduleCaptor.getValue().run(); + timerCaptor.getValue().run(timeout); verify(clientFactory, times(2)).createClient(any()); assertEquals(2, handler.attempts()); -- 2.36.6