From de1171755a19d99aedffda9e8beeb8708ba42ba9 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 25 Feb 2024 08:43:27 +0100 Subject: [PATCH] Clean up NetconfNodeActor fields Do not retain NetconfTopologySetup, as we are using only actor system, which is a constant for the actor anyway. Retain SchemaResourcesDTO without extracting things out of it, as we will be evolving it a bit -- and update them when RefreshSetupMasterActorData comes in. JIRA: NETCONF-840 Change-Id: I2b07aa84ae1b51153cc4144bc739ef8cae0480f6 Signed-off-by: Robert Varga --- .../impl/actors/NetconfNodeActor.java | 66 +++++++--------- .../singleton/messages/RefreshSlaveActor.java | 22 ++---- .../singleton/impl/NetconfNodeActorTest.java | 76 ++++++++----------- 3 files changed, 66 insertions(+), 98 deletions(-) diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java index 906799fae0..21bf46a8a4 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/actors/NetconfNodeActor.java @@ -23,7 +23,6 @@ import java.time.Duration; import java.util.List; import java.util.stream.Collectors; import org.opendaylight.controller.cluster.common.actor.AbstractUntypedActor; -import org.opendaylight.controller.cluster.schema.provider.RemoteYangTextSourceProvider; import org.opendaylight.controller.cluster.schema.provider.impl.RemoteSchemaProvider; import org.opendaylight.controller.cluster.schema.provider.impl.YangTextSchemaSourceSerializationProxy; import org.opendaylight.mdsal.dom.api.DOMActionResult; @@ -36,6 +35,7 @@ import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteTransaction; import org.opendaylight.mdsal.dom.api.DOMMountPointService; import org.opendaylight.mdsal.dom.api.DOMRpcResult; import org.opendaylight.mdsal.dom.api.DOMRpcService; +import org.opendaylight.netconf.client.mdsal.NetconfDevice.SchemaResourcesDTO; import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceId; import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceServices; import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceServices.Actions; @@ -75,19 +75,15 @@ import org.opendaylight.yangtools.yang.model.api.source.SourceIdentifier; import org.opendaylight.yangtools.yang.model.api.source.YangTextSource; import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute; import org.opendaylight.yangtools.yang.model.repo.api.EffectiveModelContextFactory; -import org.opendaylight.yangtools.yang.model.repo.api.SchemaRepository; import org.opendaylight.yangtools.yang.model.repo.spi.PotentialSchemaSource; -import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistry; public class NetconfNodeActor extends AbstractUntypedActor { private final Duration writeTxIdleTimeout; private final DOMMountPointService mountPointService; - private SchemaSourceRegistry schemaRegistry; - private SchemaRepository schemaRepository; + private SchemaResourcesDTO schemaResources; private Timeout actorResponseWaitTime; private RemoteDeviceId id; - private NetconfTopologySetup setup; private List sourceIdentifiers = null; private DOMRpcService deviceRpc = null; private DOMActionService deviceAction = null; @@ -98,24 +94,21 @@ public class NetconfNodeActor extends AbstractUntypedActor { private ActorRef readTxActor; private List registeredSchemas; - public static Props props(final NetconfTopologySetup setup, final RemoteDeviceId id, + protected NetconfNodeActor(final NetconfTopologySetup setup, final RemoteDeviceId id, final Timeout actorResponseWaitTime, final DOMMountPointService mountPointService) { - return Props.create(NetconfNodeActor.class, () -> - new NetconfNodeActor(setup, id, actorResponseWaitTime, mountPointService)); - } - - protected NetconfNodeActor(final NetconfTopologySetup setup, - final RemoteDeviceId id, final Timeout actorResponseWaitTime, - final DOMMountPointService mountPointService) { - this.setup = setup; this.id = id; - schemaRegistry = setup.getSchemaResourcesDTO().getSchemaRegistry(); - schemaRepository = setup.getSchemaResourcesDTO().getSchemaRepository(); + schemaResources = setup.getSchemaResourcesDTO(); this.actorResponseWaitTime = actorResponseWaitTime; writeTxIdleTimeout = setup.getIdleTimeout(); this.mountPointService = mountPointService; } + public static Props props(final NetconfTopologySetup setup, final RemoteDeviceId id, + final Timeout actorResponseWaitTime, final DOMMountPointService mountPointService) { + return Props.create(NetconfNodeActor.class, () -> + new NetconfNodeActor(setup, id, actorResponseWaitTime, mountPointService)); + } + @SuppressWarnings("checkstyle:IllegalCatch") @Override public void handleReceive(final Object message) { @@ -134,9 +127,9 @@ public class NetconfNodeActor extends AbstractUntypedActor { sender().tell(new MasterActorDataInitialized(), self()); LOG.debug("{}: Master is ready.", id); - } else if (message instanceof RefreshSetupMasterActorData) { - setup = ((RefreshSetupMasterActorData) message).getNetconfTopologyDeviceSetup(); - id = ((RefreshSetupMasterActorData) message).getRemoteDeviceId(); + } else if (message instanceof RefreshSetupMasterActorData masterActorData) { + id = masterActorData.getRemoteDeviceId(); + schemaResources = masterActorData.getNetconfTopologyDeviceSetup().getSchemaResourcesDTO(); sender().tell(new MasterActorDataInitialized(), self()); } else if (message instanceof AskForMasterMountPoint askForMasterMountPoint) { // master // only master contains reference to deviceDataBroker @@ -184,12 +177,10 @@ public class NetconfNodeActor extends AbstractUntypedActor { } else if (message instanceof RefreshSlaveActor refreshSlave) { //slave actorResponseWaitTime = refreshSlave.getActorResponseWaitTime(); id = refreshSlave.getId(); - schemaRegistry = refreshSlave.getSchemaRegistry(); - setup = refreshSlave.getSetup(); - schemaRepository = refreshSlave.getSchemaRepository(); + schemaResources = refreshSlave.getSetup().getSchemaResourcesDTO(); } else if (message instanceof NetconfDataTreeServiceRequest) { - ActorRef netconfActor = context() - .actorOf(NetconfDataTreeServiceActor.props(netconfService, writeTxIdleTimeout)); + final var netconfActor = context().actorOf(NetconfDataTreeServiceActor.props(netconfService, + writeTxIdleTimeout)); sender().tell(new Success(netconfActor), self()); } } @@ -213,7 +204,8 @@ public class NetconfNodeActor extends AbstractUntypedActor { } private void sendYangTextSchemaSourceProxy(final SourceIdentifier sourceIdentifier, final ActorRef sender) { - Futures.addCallback(schemaRepository.getSchemaSource(sourceIdentifier, YangTextSource.class), + Futures.addCallback( + schemaResources.getSchemaRepository().getSchemaSource(sourceIdentifier, YangTextSource.class), new FutureCallback<>() { @Override public void onSuccess(final YangTextSource yangTextSchemaSource) { @@ -306,24 +298,24 @@ public class NetconfNodeActor extends AbstractUntypedActor { private void registerSlaveMountPoint(final ActorRef masterReference) { unregisterSlaveMountPoint(); - slaveSalManager = new SlaveSalFacade(id, setup.getActorSystem(), actorResponseWaitTime, mountPointService); + slaveSalManager = new SlaveSalFacade(id, context().system(), actorResponseWaitTime, mountPointService); resolveSchemaContext(createSchemaContextFactory(masterReference), slaveSalManager, masterReference, 1); } private EffectiveModelContextFactory createSchemaContextFactory(final ActorRef masterReference) { - final RemoteYangTextSourceProvider remoteYangTextSourceProvider = - new ProxyYangTextSourceProvider(masterReference, getContext().dispatcher(), actorResponseWaitTime); - final RemoteSchemaProvider remoteProvider = new RemoteSchemaProvider(remoteYangTextSourceProvider, - getContext().dispatcher()); + final var dispatcher = getContext().dispatcher(); + final var remoteYangTextSourceProvider = new ProxyYangTextSourceProvider(masterReference, dispatcher, + actorResponseWaitTime); + final var remoteProvider = new RemoteSchemaProvider(remoteYangTextSourceProvider, dispatcher); + final var schemaRegistry = schemaResources.getSchemaRegistry(); registeredSchemas = sourceIdentifiers.stream() - .map(sourceId -> - schemaRegistry.registerSchemaSource(remoteProvider, PotentialSchemaSource.create(sourceId, - YangTextSource.class, PotentialSchemaSource.Costs.REMOTE_IO.getValue()))) - .collect(Collectors.toList()); + .map(sourceId -> schemaRegistry.registerSchemaSource(remoteProvider, PotentialSchemaSource.create(sourceId, + YangTextSource.class, PotentialSchemaSource.Costs.REMOTE_IO.getValue()))) + .collect(Collectors.toList()); - return schemaRepository.createEffectiveModelContextFactory(); + return schemaResources.getSchemaRepository().createEffectiveModelContextFactory(); } private void resolveSchemaContext(final EffectiveModelContextFactory schemaContextFactory, @@ -338,7 +330,7 @@ public class NetconfNodeActor extends AbstractUntypedActor { if (slaveSalManager == localSlaveSalManager) { LOG.info("{}: Schema context resolved: {} - registering slave mount point", id, result.getModules()); - final var actorSystem = setup.getActorSystem(); + final var actorSystem = context().system(); final var rpcProxy = new ProxyDOMRpcService(actorSystem, masterReference, id, actorResponseWaitTime); slaveSalManager.registerSlaveMountPoint(result, masterReference, new RemoteDeviceServices( diff --git a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/messages/RefreshSlaveActor.java b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/messages/RefreshSlaveActor.java index 3151a00891..91ef359150 100644 --- a/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/messages/RefreshSlaveActor.java +++ b/apps/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/messages/RefreshSlaveActor.java @@ -7,25 +7,21 @@ */ package org.opendaylight.netconf.topology.singleton.messages; +import static java.util.Objects.requireNonNull; + import akka.util.Timeout; import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceId; import org.opendaylight.netconf.topology.singleton.impl.utils.NetconfTopologySetup; -import org.opendaylight.yangtools.yang.model.repo.api.SchemaRepository; -import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceRegistry; public class RefreshSlaveActor { - private final SchemaRepository schemaRepository; private final RemoteDeviceId id; - private final SchemaSourceRegistry schemaRegistry; private final NetconfTopologySetup setup; private final Timeout actorResponseWaitTime; public RefreshSlaveActor(final NetconfTopologySetup setup, final RemoteDeviceId id, - final Timeout actorResponseWaitTime) { - this.setup = setup; - this.id = id; - schemaRegistry = setup.getSchemaResourcesDTO().getSchemaRegistry(); - schemaRepository = setup.getSchemaResourcesDTO().getSchemaRepository(); + final Timeout actorResponseWaitTime) { + this.setup = requireNonNull(setup); + this.id = requireNonNull(id); this.actorResponseWaitTime = actorResponseWaitTime; } @@ -33,18 +29,10 @@ public class RefreshSlaveActor { return actorResponseWaitTime; } - public SchemaRepository getSchemaRepository() { - return schemaRepository; - } - public RemoteDeviceId getId() { return id; } - public SchemaSourceRegistry getSchemaRegistry() { - return schemaRegistry; - } - public NetconfTopologySetup getSetup() { return setup; } diff --git a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java index 656028af8a..a2f3f12cb8 100644 --- a/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java +++ b/apps/netconf-topology-singleton/src/test/java/org/opendaylight/netconf/topology/singleton/impl/NetconfNodeActorTest.java @@ -63,7 +63,6 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.opendaylight.controller.cluster.schema.provider.impl.YangTextSchemaSourceSerializationProxy; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.dom.api.DOMActionException; import org.opendaylight.mdsal.dom.api.DOMActionResult; @@ -109,8 +108,7 @@ import org.opendaylight.yangtools.yang.common.RpcResultBuilder; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; -import org.opendaylight.yangtools.yang.data.impl.schema.Builders; -import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.data.spi.node.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.source.SourceIdentifier; @@ -181,8 +179,6 @@ public class NetconfNodeActorTest extends AbstractBaseSchemasTest { masterSchemaRepository.registerSchemaSourceListener( TextToIRTransformer.create(masterSchemaRepository, masterSchemaRepository)); - doReturn(masterSchemaRepository).when(schemaResourceDTO).getSchemaRepository(); - doReturn(mockRegistry).when(schemaResourceDTO).getSchemaRegistry(); final NetconfTopologySetup setup = NetconfTopologySetup.builder() .setActorSystem(system) .setIdleTimeout(Duration.ofSeconds(1)) @@ -413,66 +409,58 @@ public class NetconfNodeActorTest extends AbstractBaseSchemasTest { verify(mockSchemaRepository, times(2)).createEffectiveModelContextFactory(); } - @Test(expected = MissingSchemaSourceException.class) + @Test public void testMissingSchemaSourceOnMissingProvider() throws Exception { - final SharedSchemaRepository repository = new SharedSchemaRepository("test"); + final var repository = new SharedSchemaRepository("test"); - SchemaResourcesDTO schemaResourceDTO2 = mock(SchemaResourcesDTO.class); - doReturn(repository).when(schemaResourceDTO2).getSchemaRegistry(); + final var schemaResourceDTO2 = mock(SchemaResourcesDTO.class); doReturn(repository).when(schemaResourceDTO2).getSchemaRepository(); - final NetconfTopologySetup setup = NetconfTopologySetup.builder() + final var setup = NetconfTopologySetup.builder() .setActorSystem(system) .setSchemaResourceDTO(schemaResourceDTO2) .setIdleTimeout(Duration.ofSeconds(1)) .setBaseSchemaProvider(BASE_SCHEMAS) .build(); - final Props props = NetconfNodeActor.props(setup, remoteDeviceId, TIMEOUT, mockMountPointService); - ActorRef actor = TestActorRef.create(system, props, "master_messages_2"); + final var props = NetconfNodeActor.props(setup, remoteDeviceId, TIMEOUT, mockMountPointService); + final var actor = TestActorRef.create(system, props, "master_messages_2"); - final SourceIdentifier sourceIdentifier = new SourceIdentifier("testID"); + final var sourceIdentifier = new SourceIdentifier("testID"); - final ProxyYangTextSourceProvider proxyYangProvider = - new ProxyYangTextSourceProvider(actor, system.dispatcher(), TIMEOUT); + final var proxyYangProvider = new ProxyYangTextSourceProvider(actor, system.dispatcher(), TIMEOUT); - final Future resolvedSchemaFuture = - proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); - Await.result(resolvedSchemaFuture, TIMEOUT.duration()); + final var resolvedSchemaFuture = proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); + final var ex = assertThrows(MissingSchemaSourceException.class, + () -> Await.result(resolvedSchemaFuture, TIMEOUT.duration())); + assertEquals("No providers registered for source SourceIdentifier [testID]", ex.getMessage()); } @Test public void testYangTextSchemaSourceRequest() throws Exception { - final SourceIdentifier sourceIdentifier = new SourceIdentifier("testID"); - - final ProxyYangTextSourceProvider proxyYangProvider = - new ProxyYangTextSourceProvider(masterRef, system.dispatcher(), TIMEOUT); + doReturn(masterSchemaRepository).when(schemaResourceDTO).getSchemaRepository(); - final YangTextSource yangTextSchemaSource = new DelegatedYangTextSource(sourceIdentifier, - CharSource.wrap("YANG")); + final var sourceIdentifier = new SourceIdentifier("testID"); - // Test success. + final var proxyYangProvider = new ProxyYangTextSourceProvider(masterRef, system.dispatcher(), TIMEOUT); - final var schemaSourceReg = masterSchemaRepository.registerSchemaSource( - id -> Futures.immediateFuture(yangTextSchemaSource), - PotentialSchemaSource.create(sourceIdentifier, YangTextSource.class, 1)); + final var yangTextSchemaSource = new DelegatedYangTextSource(sourceIdentifier, CharSource.wrap("YANG")); - final Future resolvedSchemaFuture = - proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); + // Test success. - final YangTextSchemaSourceSerializationProxy success = Await.result(resolvedSchemaFuture, TIMEOUT.duration()); + try (var schemaSourceReg = masterSchemaRepository.registerSchemaSource( + id -> Futures.immediateFuture(yangTextSchemaSource), + PotentialSchemaSource.create(sourceIdentifier, YangTextSource.class, 1))) { + final var resolvedSchemaFuture = proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); + final var success = Await.result(resolvedSchemaFuture, TIMEOUT.duration()); - assertEquals(sourceIdentifier, success.getRepresentation().sourceId()); - assertEquals("YANG", success.getRepresentation().read()); + assertEquals(sourceIdentifier, success.getRepresentation().sourceId()); + assertEquals("YANG", success.getRepresentation().read()); + } // Test missing source failure. - schemaSourceReg.close(); - - final MissingSchemaSourceException ex = assertThrows(MissingSchemaSourceException.class, - () -> { - final Future failedSchemaFuture = - proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); - Await.result(failedSchemaFuture, TIMEOUT.duration()); - }); + final var failedSchemaFuture = proxyYangProvider.getYangTextSchemaSource(sourceIdentifier); + final var ex = assertThrows(MissingSchemaSourceException.class, + () -> Await.result(failedSchemaFuture, TIMEOUT.duration())); assertThat(ex.getMessage(), startsWith("No providers registered for source")); assertThat(ex.getMessage(), containsString(sourceIdentifier.toString())); } @@ -490,7 +478,7 @@ public class NetconfNodeActorTest extends AbstractBaseSchemasTest { assertTrue(slaveDomRPCService instanceof ProxyDOMRpcService); final QName testQName = QName.create("", "TestQname"); - final ContainerNode outputNode = Builders.containerBuilder() + final ContainerNode outputNode = ImmutableNodes.newContainerBuilder() .withNodeIdentifier(new NodeIdentifier(testQName)) .withChild(ImmutableNodes.leafNode(testQName, "foo")).build(); final RpcError rpcError = RpcResultBuilder.newError(ErrorType.RPC, null, "Rpc invocation failed."); @@ -564,7 +552,7 @@ public class NetconfNodeActorTest extends AbstractBaseSchemasTest { final DOMDataTreeIdentifier domDataTreeIdentifier = new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, yangIIdPath); - final ContainerNode outputNode = Builders.containerBuilder() + final ContainerNode outputNode = ImmutableNodes.newContainerBuilder() .withNodeIdentifier(new NodeIdentifier(testQName)) .withChild(ImmutableNodes.leafNode(testQName, "foo")).build(); @@ -634,7 +622,7 @@ public class NetconfNodeActorTest extends AbstractBaseSchemasTest { final YangInstanceIdentifier PATH = YangInstanceIdentifier.of(); final LogicalDatastoreType STORE = LogicalDatastoreType.CONFIGURATION; - final ContainerNode NODE = Builders.containerBuilder() + final ContainerNode NODE = ImmutableNodes.newContainerBuilder() .withNodeIdentifier(new NodeIdentifier(QName.create("", "cont"))) .build(); -- 2.36.6