From af45b16ab668b74b6853b46c46fb20f5784f0c67 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 1 Jan 2023 20:17:58 +0100 Subject: [PATCH] Remove RemoteDeviceId.getTopologyBindingPath() This path is only used transiently by netconf-topology components, move the responsibility back to them, instantiating the path as needed. JIRA: NETCONF-913 Change-Id: Iee8a62b9b181324a7ccc51cf3d9e70d37bf3c66f Signed-off-by: Robert Varga --- .../singleton/impl/MasterSalFacade.java | 2 +- .../spi/NetconfDeviceTopologyAdapter.java | 34 ++++++++++++++----- .../spi/NetconfTopologyDeviceSalFacade.java | 2 +- ...fDeficeTopologyAdapterIntegrationTest.java | 17 +++++++--- .../spi/NetconfDeviceTopologyAdapterTest.java | 12 +++++-- .../sal/connect/util/RemoteDeviceId.java | 14 ++------ 6 files changed, 52 insertions(+), 29 deletions(-) diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/MasterSalFacade.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/MasterSalFacade.java index 3ff56c9f8d..6360b71ebc 100644 --- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/MasterSalFacade.java +++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/MasterSalFacade.java @@ -70,7 +70,7 @@ class MasterSalFacade implements RemoteDeviceHandler, AutoCloseable { this.actorResponseWaitTime = actorResponseWaitTime; this.lockDatastore = lockDatastore; - datastoreAdapter = new NetconfDeviceTopologyAdapter(dataBroker, id); + datastoreAdapter = new NetconfDeviceTopologyAdapter(dataBroker, RemoteDeviceId.DEFAULT_TOPOLOGY_IID, id); } @Override diff --git a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapter.java b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapter.java index 55fffb3b1e..1e7235b06f 100644 --- a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapter.java +++ b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapter.java @@ -15,6 +15,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.Transaction; import org.opendaylight.mdsal.binding.api.TransactionChain; @@ -33,7 +34,14 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev221225.co import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev221225.connection.oper.unavailable.capabilities.UnavailableCapabilityBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev221225.NetconfNode; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev221225.NetconfNodeBuilder; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeBuilder; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; import org.opendaylight.yangtools.yang.common.Empty; import org.opendaylight.yangtools.yang.common.Uint16; import org.slf4j.Logger; @@ -43,20 +51,23 @@ public final class NetconfDeviceTopologyAdapter implements TransactionChainListe private static final Logger LOG = LoggerFactory.getLogger(NetconfDeviceTopologyAdapter.class); private final SettableFuture closeFuture = SettableFuture.create(); + private final @NonNull KeyedInstanceIdentifier topologyPath; private final DataBroker dataBroker; private final RemoteDeviceId id; private TransactionChain txChain; - public NetconfDeviceTopologyAdapter(final DataBroker dataBroker, final RemoteDeviceId id) { + public NetconfDeviceTopologyAdapter(final DataBroker dataBroker, + final KeyedInstanceIdentifier topologyPath, final RemoteDeviceId id) { this.dataBroker = requireNonNull(dataBroker); + this.topologyPath = requireNonNull(topologyPath); this.id = requireNonNull(id); txChain = dataBroker.createMergingTransactionChain(this); final WriteTransaction writeTx = txChain.newWriteOnlyTransaction(); LOG.trace("{}: Init device state transaction {} putting if absent operational data started.", id, writeTx.getIdentifier()); - final var nodePath = id.getTopologyBindingPath(); + final var nodePath = nodePath(); writeTx.put(LogicalDatastoreType.OPERATIONAL, nodePath, new NodeBuilder() .withKey(nodePath.getKey()) .addAugmentation(new NetconfNodeBuilder() @@ -69,14 +80,21 @@ public final class NetconfDeviceTopologyAdapter implements TransactionChainListe commitTransaction(writeTx, "init"); } + private @NonNull KeyedInstanceIdentifier nodePath() { + return topologyPath.child(Node.class, new NodeKey(new NodeId(id.getName()))); + } + + private @NonNull InstanceIdentifier netconfNodePath() { + return nodePath().augmentation(NetconfNode.class); + } + public void updateDeviceData(final boolean up, final NetconfDeviceCapabilities capabilities) { final WriteTransaction writeTx = txChain.newWriteOnlyTransaction(); LOG.trace("{}: Update device state transaction {} merging operational data started.", id, writeTx.getIdentifier()); // FIXME: this needs to be tied together with node's operational existence - writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, - id.getTopologyBindingPath().augmentation(NetconfNode.class), + writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, netconfNodePath(), newNetconfNodeBuilder(up, capabilities).build()); LOG.trace("{}: Update device state transaction {} merging operational data ended.", id, writeTx.getIdentifier()); @@ -89,8 +107,7 @@ public final class NetconfDeviceTopologyAdapter implements TransactionChainListe final WriteTransaction writeTx = txChain.newWriteOnlyTransaction(); LOG.trace("{}: Update device state transaction {} merging operational data started.", id, writeTx.getIdentifier()); - writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, - id.getTopologyBindingPath().augmentation(NetconfNode.class), + writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, netconfNodePath(), newNetconfNodeBuilder(up, capabilities) .setClusteredConnectionStatus(new ClusteredConnectionStatusBuilder() .setNetconfMasterNode(masterAddress) @@ -131,8 +148,7 @@ public final class NetconfDeviceTopologyAdapter implements TransactionChainListe LOG.trace( "{}: Setting device state as failed {} putting operational data started.", id, writeTx.getIdentifier()); - writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, - id.getTopologyBindingPath().augmentation(NetconfNode.class), data); + writeTx.mergeParentStructurePut(LogicalDatastoreType.OPERATIONAL, netconfNodePath(), data); LOG.trace( "{}: Setting device state as failed {} putting operational data ended.", id, writeTx.getIdentifier()); @@ -182,7 +198,7 @@ public final class NetconfDeviceTopologyAdapter implements TransactionChainListe public void close() { final WriteTransaction writeTx = txChain.newWriteOnlyTransaction(); LOG.trace("{}: Close device state transaction {} removing all data started.", id, writeTx.getIdentifier()); - writeTx.delete(LogicalDatastoreType.OPERATIONAL, id.getTopologyBindingPath()); + writeTx.delete(LogicalDatastoreType.OPERATIONAL, nodePath()); LOG.trace("{}: Close device state transaction {} removing all data ended.", id, writeTx.getIdentifier()); commitTransaction(writeTx, "close"); diff --git a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfTopologyDeviceSalFacade.java b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfTopologyDeviceSalFacade.java index fbaa8ac14a..e89dcc57d0 100644 --- a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfTopologyDeviceSalFacade.java +++ b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/NetconfTopologyDeviceSalFacade.java @@ -25,7 +25,7 @@ public class NetconfTopologyDeviceSalFacade extends NetconfDeviceSalFacade { public NetconfTopologyDeviceSalFacade(final RemoteDeviceId id, final DOMMountPointService mountPointService, final boolean lockDatastore, final DataBroker dataBroker) { super(id, mountPointService, lockDatastore); - datastoreAdapter = new NetconfDeviceTopologyAdapter(dataBroker, id); + datastoreAdapter = new NetconfDeviceTopologyAdapter(dataBroker, RemoteDeviceId.DEFAULT_TOPOLOGY_IID, id); } @Override diff --git a/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeficeTopologyAdapterIntegrationTest.java b/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeficeTopologyAdapterIntegrationTest.java index db98f76f57..dd5c718f32 100644 --- a/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeficeTopologyAdapterIntegrationTest.java +++ b/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeficeTopologyAdapterIntegrationTest.java @@ -30,9 +30,13 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev221225.Co import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev221225.NetconfNode; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.augment.test.rev160808.Node1; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyBuilder; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey; +import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @@ -41,6 +45,9 @@ import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; // FIXME: base on AbstractDataBrokerTest test? public class NetconfDeficeTopologyAdapterIntegrationTest { private static final RemoteDeviceId ID = new RemoteDeviceId("test", new InetSocketAddress("localhost", 22)); + private static final KeyedInstanceIdentifier TEST_TOPOLOGY_ID = + // FIXME: do not use this constant + RemoteDeviceId.DEFAULT_TOPOLOGY_IID; private static BindingRuntimeContext RUNTIME_CONTEXT; @@ -67,12 +74,12 @@ public class NetconfDeficeTopologyAdapterIntegrationTest { customizer.updateSchema(RUNTIME_CONTEXT); final var tx = dataBroker.newWriteOnlyTransaction(); - tx.put(LogicalDatastoreType.OPERATIONAL, RemoteDeviceId.DEFAULT_TOPOLOGY_IID, new TopologyBuilder() - .withKey(RemoteDeviceId.DEFAULT_TOPOLOGY_IID.getKey()) + tx.put(LogicalDatastoreType.OPERATIONAL, TEST_TOPOLOGY_ID, new TopologyBuilder() + .withKey(TEST_TOPOLOGY_ID.getKey()) .build()); tx.commit().get(2, TimeUnit.SECONDS); - adapter = new NetconfDeviceTopologyAdapter(dataBroker, ID); + adapter = new NetconfDeviceTopologyAdapter(dataBroker, TEST_TOPOLOGY_ID, ID); } @Test @@ -80,7 +87,9 @@ public class NetconfDeficeTopologyAdapterIntegrationTest { adapter.setDeviceAsFailed(null); Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> dataBroker.newReadWriteTransaction() - .read(LogicalDatastoreType.OPERATIONAL, ID.getTopologyBindingPath().augmentation(NetconfNode.class)) + .read(LogicalDatastoreType.OPERATIONAL, TEST_TOPOLOGY_ID + .child(Node.class, new NodeKey(new NodeId(ID.getName()))) + .augmentation(NetconfNode.class)) .get(5, TimeUnit.SECONDS) .filter(conn -> conn.getConnectionStatus() == ConnectionStatus.UnableToConnect) .isPresent()); diff --git a/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapterTest.java b/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapterTest.java index 804b4df563..8fd871ab9c 100644 --- a/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapterTest.java +++ b/netconf/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/NetconfDeviceTopologyAdapterTest.java @@ -29,11 +29,18 @@ import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.netconf.sal.connect.netconf.listener.NetconfDeviceCapabilities; import org.opendaylight.netconf.sal.connect.util.RemoteDeviceId; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; +import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; @RunWith(MockitoJUnitRunner.StrictStubs.class) public class NetconfDeviceTopologyAdapterTest { + private static final KeyedInstanceIdentifier TEST_TOPOLOGY_ID = + RemoteDeviceId.DEFAULT_TOPOLOGY_IID; private final RemoteDeviceId id = new RemoteDeviceId("test", new InetSocketAddress("localhost", 22)); @Mock @@ -56,7 +63,7 @@ public class NetconfDeviceTopologyAdapterTest { doReturn(CommitInfo.emptyFluentFuture()).when(mockTx).commit(); doReturn(mockChain).when(mockBroker).createMergingTransactionChain(listeners.capture()); - adapter = new NetconfDeviceTopologyAdapter(mockBroker, id); + adapter = new NetconfDeviceTopologyAdapter(mockBroker, TEST_TOPOLOGY_ID, id); } @Test @@ -90,7 +97,8 @@ public class NetconfDeviceTopologyAdapterTest { adapter.close(); verify(mockChain, times(2)).newWriteOnlyTransaction(); - verify(mockTx).delete(LogicalDatastoreType.OPERATIONAL, id.getTopologyBindingPath()); + verify(mockTx).delete(LogicalDatastoreType.OPERATIONAL, + TEST_TOPOLOGY_ID.child(Node.class, new NodeKey(new NodeId(id.getName())))); verify(mockTx, times(2)).commit(); } } diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/RemoteDeviceId.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/RemoteDeviceId.java index f5295fc6bc..13aaa60b8f 100644 --- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/RemoteDeviceId.java +++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/RemoteDeviceId.java @@ -18,12 +18,10 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types. import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev221225.network.topology.topology.topology.types.TopologyNetconf; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology; -import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; -import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeKey; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; import org.opendaylight.yangtools.yang.common.QName; @@ -31,6 +29,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; public final class RemoteDeviceId { + // FIXME: extract all of this to users, as they are in control of topology-id private static final String DEFAULT_TOPOLOGY_NAME = TopologyNetconf.QNAME.getLocalName(); private static final YangInstanceIdentifier DEFAULT_TOPOLOGY_NODE = YangInstanceIdentifier.builder() .node(NetworkTopology.QNAME).node(Topology.QNAME) @@ -47,7 +46,6 @@ public final class RemoteDeviceId { private final @NonNull String name; private final @NonNull YangInstanceIdentifier topologyPath; - private final @NonNull KeyedInstanceIdentifier topologyBindingPath; private InetSocketAddress address; private Host host; @@ -55,18 +53,14 @@ public final class RemoteDeviceId { private RemoteDeviceId(final String name) { this.name = requireNonNull(name); topologyPath = DEFAULT_TOPOLOGY_NODE.node(NodeIdentifierWithPredicates.of(Node.QNAME, NODE_ID_QNAME, name)); - topologyBindingPath = DEFAULT_TOPOLOGY_IID.child(Node.class, new NodeKey(new NodeId(name))); } public RemoteDeviceId(final String name, final InetSocketAddress address) { this(name); this.address = address; - host = buildHost(); - } - private Host buildHost() { final var addr = address.getAddress(); - return addr != null ? new Host(IetfInetUtil.INSTANCE.ipAddressFor(addr)) + host = addr != null ? new Host(IetfInetUtil.INSTANCE.ipAddressFor(addr)) : new Host(new DomainName(address.getHostString())); } @@ -74,10 +68,6 @@ public final class RemoteDeviceId { return name; } - public @NonNull KeyedInstanceIdentifier getTopologyBindingPath() { - return topologyBindingPath; - } - public @NonNull YangInstanceIdentifier getTopologyPath() { return topologyPath; } -- 2.36.6