Skip to content

Commit

Permalink
Reduce InstanceIdentifier proliferation
Browse files Browse the repository at this point in the history 
Use DataObjectReference.firstKeyOf() instead of converting to
InstanceIdentifier. Also update some code to work on
DataObjectIdentifiers.

Change-Id: I901cd3dff2ee688ec904c911083285e0b06d47b7
Signed-off-by: Robert Varga <[email protected]>
  • Loading branch information
@rovarga
rovarga committed Feb 11, 2025
1 parent 74431e8 commit b2c9bb2
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 60 deletions.
2 changes: 1 addition & 1 deletion ...pl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/OpenConfigMappingUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private OpenConfigMappingUtil() {
}

static String getRibInstanceName(final DataObjectIdentifier<Bgp> rootIdentifier) {
return rootIdentifier.toLegacy().firstKeyOf(Protocol.class).getName();
return rootIdentifier.firstKeyOf(Protocol.class).getName();
}

static KeyMapping getNeighborKey(final Neighbor neighbor) {
Expand Down
4 changes: 2 additions & 2 deletions ...src/main/java/org/opendaylight/bgpcep/bgp/topology/provider/LinkstateTopologyBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ protected void addSrAwareTopologyType(final WriteTransaction trans) {
}
final var iid = getInstanceIdentifier();
LOG.debug("Adding SR-aware topology-type for topology {}",
iid.toLegacy().firstKeyOf(Topology.class).getTopologyId().getValue());
iid.firstKeyOf(Topology.class).getTopologyId().getValue());
trans.put(LogicalDatastoreType.OPERATIONAL, iid.toBuilder().child(TopologyTypes.class).build(),
SR_AWARE_LINKSTATE_TOPOLOGY_TYPE);
srAwareTopologyTypeAdded = true;
Expand All @@ -935,7 +935,7 @@ protected void removeSrAwareTopologyTypeIfRequired(final WriteTransaction trans)

final var iid = getInstanceIdentifier();
LOG.debug("Removing SR-aware topology-type from topology {}",
iid.toLegacy().firstKeyOf(Topology.class).getTopologyId().getValue());
iid.firstKeyOf(Topology.class).getTopologyId().getValue());
trans.put(LogicalDatastoreType.OPERATIONAL, iid.toBuilder()
.child(TopologyTypes.class)
.build(), LINKSTATE_TOPOLOGY_TYPE);
Expand Down
2 changes: 1 addition & 1 deletion ...endaylight/bgpcep/bgp/topology/provider/config/TopologyReferenceSingletonServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public FluentFuture<? extends CommitInfo> closeServiceInstance() {
@Override
public ServiceGroupIdentifier getIdentifier() {
return new ServiceGroupIdentifier(
getInstanceIdentifier().toLegacy().firstKeyOf(Topology.class).getTopologyId().getValue());
getInstanceIdentifier().firstKeyOf(Topology.class).getTopologyId().getValue());
}

@Override
Expand Down
3 changes: 1 addition & 2 deletions graph/graph-impl/src/main/java/org/opendaylight/graph/impl/GraphListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private static void parseSubTree(final ConnectedGraph cgraph,
public void onDataTreeChanged(final List<DataTreeModification<Graph>> changes) {
for (var change : changes) {
final var root = change.getRootNode();
final var key = change.path().toLegacy().firstKeyOf(Graph.class);
final var key = change.path().firstKeyOf(Graph.class);
switch (root.modificationType()) {
case DELETE:
graphProvider.deleteGraph(key);
Expand All @@ -152,6 +152,5 @@ public void onDataTreeChanged(final List<DataTreeModification<Graph>> changes) {
}
}
}

}

2 changes: 1 addition & 1 deletion ...rc/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPStatefulPeerProposal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ final void update(final DataTreeModification<?> modification, final V value) {
}

private static @NonNull NodeId extractNodeId(final DataTreeModification<?> modification) {
return verifyNotNull(modification.path().toLegacy().firstKeyOf(Node.class)).getNodeId();
return verifyNotNull(modification.path().firstKeyOf(Node.class)).getNodeId();
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...der/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologyTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ void finishDestroy(final TopologyKey topology, final PCEPTopologySingleton insta
}

private static @NonNull TopologyKey extractTopologyKey(final DataTreeModification<?> change) {
final var path = change.path().toLegacy();
final var path = change.path();
return verifyNotNull(path.firstKeyOf(Topology.class), "No topology key in %s", path);
}
}
2 changes: 0 additions & 2 deletions ...ovider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyStatsRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,13 @@ private static PcepSessionState transformStatefulAugmentation(final PcepSessionS

private List<TopologyId> getAvailableTopologyIds() {
return sessionStateMap.keySet().stream()
.map(DataObjectIdentifier::toLegacy)
.map(iid -> iid.firstKeyOf(Topology.class).getTopologyId())
.distinct()
.collect(Collectors.toList());
}

private List<NodeId> getAvailableNodeIds(final TopologyId topologyId) {
return sessionStateMap.keySet().stream()
.map(DataObjectIdentifier::toLegacy)
.filter(iid -> iid.firstKeyOf(Topology.class).getTopologyId().equals(topologyId))
.map(iid -> iid.firstKeyOf(Node.class).getNodeId())
.collect(Collectors.toList());
Expand Down
106 changes: 56 additions & 50 deletions ...vider/src/main/java/org/opendaylight/bgpcep/pcep/tunnel/provider/NodeChangedListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.nt.l3.unicast.igp.topology.rev131021.igp.termination.point.attributes.igp.termination.point.attributes.termination.point.type.Ip;
import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.nt.l3.unicast.igp.topology.rev131021.igp.termination.point.attributes.igp.termination.point.attributes.termination.point.type.IpBuilder;
import org.opendaylight.yangtools.binding.DataObject;
import org.opendaylight.yangtools.binding.DataObjectIdentifier;
import org.opendaylight.yangtools.binding.DataObjectIdentifier.WithKey;
import org.opendaylight.yangtools.binding.DataObjectStep;
import org.opendaylight.yangtools.binding.util.BindingMap;
Expand All @@ -89,24 +90,26 @@ public final class NodeChangedListener implements DataTreeChangeListener<Node> {
this.source = requireNonNull(source);
}

private static void categorizeIdentifier(final InstanceIdentifier<?> identifier,
final Set<InstanceIdentifier<ReportedLsp>> changedLsps,
final Set<InstanceIdentifier<Node>> changedNodes) {
final InstanceIdentifier<ReportedLsp> li = identifier.firstIdentifierOf(ReportedLsp.class);
private static void categorizeIdentifier(final DataObjectIdentifier<?> identifier,
final Set<DataObjectIdentifier<ReportedLsp>> changedLsps,
final Set<DataObjectIdentifier<Node>> changedNodes) {
final var legacy = identifier.toLegacy();

final var li = legacy.firstIdentifierOf(ReportedLsp.class);
if (li == null) {
final InstanceIdentifier<Node> ni = identifier.firstIdentifierOf(Node.class);
final var ni = legacy.firstIdentifierOf(Node.class);
if (ni == null) {
LOG.warn("Ignoring uncategorized identifier {}", identifier);
} else {
changedNodes.add(ni);
changedNodes.add(ni.toIdentifier());
}
} else {
changedLsps.add(li);
changedLsps.add(li.toIdentifier());
}
}

private static void enumerateLsps(final InstanceIdentifier<Node> id, final Node node,
final Set<InstanceIdentifier<ReportedLsp>> lsps) {
private static void enumerateLsps(final DataObjectIdentifier<Node> id, final Node node,
final Set<DataObjectIdentifier<ReportedLsp>> lsps) {
if (node == null) {
LOG.trace("Skipping null node {}", id);
return;
Expand All @@ -117,13 +120,16 @@ private static void enumerateLsps(final InstanceIdentifier<Node> id, final Node
return;
}

for (final ReportedLsp l : pccnode.getPathComputationClient().nonnullReportedLsp().values()) {
lsps.add(id.builder().augmentation(Node1.class).child(PathComputationClient.class)
.child(ReportedLsp.class, l.key()).build());
for (var lsp : pccnode.getPathComputationClient().nonnullReportedLsp().values()) {
lsps.add(id.toBuilder()
.augmentation(Node1.class)
.child(PathComputationClient.class)
.child(ReportedLsp.class, lsp.key())
.build());
}
}

private static LinkId linkIdForLsp(final InstanceIdentifier<ReportedLsp> identifier, final ReportedLsp lsp) {
private static LinkId linkIdForLsp(final DataObjectIdentifier<ReportedLsp> identifier, final ReportedLsp lsp) {
return new LinkId(identifier.firstKeyOf(Node.class).getNodeId().getValue() + "/lsps/" + lsp.getName());
}

Expand All @@ -145,10 +151,10 @@ private SupportingNode createSupportingNode(final NodeId sni, final Boolean inCo
.build();
}

private void handleSni(final InstanceIdentifier<Node> sni, final Node node, final Boolean inControl,
private void handleSni(final DataObjectIdentifier<Node> sni, final Node node, final Boolean inControl,
final ReadWriteTransaction trans) {
if (sni != null) {
final NodeKey k = InstanceIdentifier.keyOf(sni);
final NodeKey k = InstanceIdentifier.keyOf(sni.toLegacy());
boolean have = false;
/*
* We may have found a termination point which has been created as a destination,
Expand All @@ -172,7 +178,7 @@ private void handleSni(final InstanceIdentifier<Node> sni, final Node node, fina
}

private WithKey<TerminationPoint, TerminationPointKey> getIpTerminationPoint(final ReadWriteTransaction trans,
final IpAddress addr, final InstanceIdentifier<Node> sni, final Boolean inControl)
final IpAddress addr, final DataObjectIdentifier<Node> sni, final Boolean inControl)
throws ExecutionException, InterruptedException {
final Topology topo = trans.read(LogicalDatastoreType.OPERATIONAL, target).get().orElseThrow();
for (final Node n : topo.nonnullNode().values()) {
Expand Down Expand Up @@ -202,7 +208,7 @@ private WithKey<TerminationPoint, TerminationPointKey> getIpTerminationPoint(fin
}

private WithKey<TerminationPoint, TerminationPointKey> createTP(final IpAddress addr,
final InstanceIdentifier<Node> sni, final Boolean inControl, final ReadWriteTransaction trans) {
final DataObjectIdentifier<Node> sni, final Boolean inControl, final ReadWriteTransaction trans) {
final String url = "ip://" + addr.toString();
final TerminationPointKey tpk = new TerminationPointKey(new TpId(url));
final TerminationPointBuilder tpb = new TerminationPointBuilder();
Expand All @@ -218,17 +224,17 @@ private WithKey<TerminationPoint, TerminationPointKey> createTP(final IpAddress
nb.withKey(nk).setNodeId(nk.getNodeId());
nb.setTerminationPoint(BindingMap.of(tpb.build()));
if (sni != null) {
nb.setSupportingNode(BindingMap.of(createSupportingNode(InstanceIdentifier.keyOf(sni).getNodeId(),
inControl)));
nb.setSupportingNode(BindingMap.of(
createSupportingNode(InstanceIdentifier.keyOf(sni.toLegacy()).getNodeId(), inControl)));
}
final var nid = target.toBuilder().child(Node.class, nb.key()).build();
trans.put(LogicalDatastoreType.OPERATIONAL, nid, nb.build());
return nid.toBuilder().child(TerminationPoint.class, tpb.key()).build();
}

private void create(final ReadWriteTransaction trans, final InstanceIdentifier<ReportedLsp> identifier,
private void create(final ReadWriteTransaction trans, final DataObjectIdentifier<ReportedLsp> identifier,
final ReportedLsp value) throws ExecutionException, InterruptedException {
final InstanceIdentifier<Node> ni = identifier.firstIdentifierOf(Node.class);
final var ni = identifier.toLegacy().firstIdentifierOf(Node.class).toIdentifier();

final Path1 rl = value.nonnullPath().values().iterator().next().augmentation(Path1.class);

Expand Down Expand Up @@ -261,8 +267,8 @@ private void create(final ReadWriteTransaction trans, final InstanceIdentifier<R
}
lab.setSymbolicPathName(value.getName());

final var dst = getIpTerminationPoint(trans, dstIp, null, Boolean.FALSE).toLegacy();
final var src = getIpTerminationPoint(trans, srcIp, ni, rl.getLsp().getDelegate()).toLegacy();
final var dst = getIpTerminationPoint(trans, dstIp, null, Boolean.FALSE);
final var src = getIpTerminationPoint(trans, srcIp, ni, rl.getLsp().getDelegate());

final var slab = new org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.ietf.stateful
.rev200720.Link1Builder()
Expand Down Expand Up @@ -295,7 +301,7 @@ private void create(final ReadWriteTransaction trans, final InstanceIdentifier<R
return target.toBuilder().child(Node.class, new NodeKey(node)).build();
}

private void remove(final ReadWriteTransaction trans, final InstanceIdentifier<ReportedLsp> identifier,
private void remove(final ReadWriteTransaction trans, final DataObjectIdentifier<ReportedLsp> identifier,
final ReportedLsp value) throws ExecutionException, InterruptedException {
final var li = linkForLsp(linkIdForLsp(identifier, value));

Expand Down Expand Up @@ -403,16 +409,15 @@ private void remove(final ReadWriteTransaction trans, final InstanceIdentifier<R
public void onDataTreeChanged(final List<DataTreeModification<Node>> changes) {
final var trans = dataProvider.newReadWriteTransaction();

final var lsps = new HashSet<InstanceIdentifier<ReportedLsp>>();
final var nodes = new HashSet<InstanceIdentifier<Node>>();
final var lsps = new HashSet<DataObjectIdentifier<ReportedLsp>>();
final var nodes = new HashSet<DataObjectIdentifier<Node>>();

final var original = new HashMap<InstanceIdentifier<?>, DataObject>();
final var updated = new HashMap<InstanceIdentifier<?>, DataObject>();
final var created = new HashMap<InstanceIdentifier<?>, DataObject>();
final var original = new HashMap<DataObjectIdentifier<?>, DataObject>();
final var updated = new HashMap<DataObjectIdentifier<?>, DataObject>();
final var created = new HashMap<DataObjectIdentifier<?>, DataObject>();

for (final var change : changes) {
handleChangedNode(change.getRootNode(), change.getRootPath().path(), lsps, nodes, original, updated,
created);
handleChangedNode(change.getRootNode(), change.path(), lsps, nodes, original, updated, created);
}

// Now walk all nodes, check for removals/additions and cascade them to LSPs
Expand All @@ -438,10 +443,11 @@ public void onFailure(final Throwable throwable) {
}, MoreExecutors.directExecutor());
}

private void handleChangedNode(final DataObjectModification<?> changedNode, final InstanceIdentifier<?> iid,
final Set<InstanceIdentifier<ReportedLsp>> lsps, final Set<InstanceIdentifier<Node>> nodes,
final Map<InstanceIdentifier<?>, DataObject> original, final Map<InstanceIdentifier<?>, DataObject> updated,
final Map<InstanceIdentifier<?>, DataObject> created) {
private void handleChangedNode(final DataObjectModification<?> changedNode, final DataObjectIdentifier<?> iid,
final Set<DataObjectIdentifier<ReportedLsp>> lsps, final Set<DataObjectIdentifier<Node>> nodes,
final Map<DataObjectIdentifier<?>, DataObject> original,
final Map<DataObjectIdentifier<?>, DataObject> updated,
final Map<DataObjectIdentifier<?>, DataObject> created) {

// Categorize reported identifiers
categorizeIdentifier(iid, lsps, nodes);
Expand All @@ -466,37 +472,37 @@ private void handleChangedNode(final DataObjectModification<?> changedNode, fina
final var pathArguments = new ArrayList<DataObjectStep<?>>();
iid.getPathArguments().forEach(pathArguments::add);
pathArguments.add(child.step());
final var childIID = InstanceIdentifier.unsafeOf(pathArguments);
final var childIID = InstanceIdentifier.unsafeOf(pathArguments).toIdentifier();
handleChangedNode(child, childIID, lsps, nodes, original, updated, created);
}
}

private void updateTransaction(final ReadWriteTransaction trans,
final Set<InstanceIdentifier<ReportedLsp>> lsps,
final Map<InstanceIdentifier<?>, ? extends DataObject> old,
final Map<InstanceIdentifier<?>, DataObject> updated,
final Map<InstanceIdentifier<?>, DataObject> created) {

for (final InstanceIdentifier<ReportedLsp> i : lsps) {
final ReportedLsp oldValue = (ReportedLsp) old.get(i);
ReportedLsp newValue = (ReportedLsp) updated.get(i);
final Set<DataObjectIdentifier<ReportedLsp>> lsps,
final Map<DataObjectIdentifier<?>, ? extends DataObject> old,
final Map<DataObjectIdentifier<?>, DataObject> updated,
final Map<DataObjectIdentifier<?>, DataObject> created) {

for (var lsp : lsps) {
final ReportedLsp oldValue = (ReportedLsp) old.get(lsp);
ReportedLsp newValue = (ReportedLsp) updated.get(lsp);
if (newValue == null) {
newValue = (ReportedLsp) created.get(i);
newValue = (ReportedLsp) created.get(lsp);
}

LOG.debug("Updating lsp {} value {} -> {}", i, oldValue, newValue);
LOG.debug("Updating lsp {} value {} -> {}", lsp, oldValue, newValue);
if (oldValue != null) {
try {
remove(trans, i, oldValue);
remove(trans, lsp, oldValue);
} catch (final ExecutionException | InterruptedException e) {
LOG.warn("Failed to remove LSP {}", i, e);
LOG.warn("Failed to remove LSP {}", lsp, e);
}
}
if (newValue != null) {
try {
create(trans, i, newValue);
create(trans, lsp, newValue);
} catch (final ExecutionException | InterruptedException e) {
LOG.warn("Failed to add LSP {}", i, e);
LOG.warn("Failed to add LSP {}", lsp, e);
}
}
}
Expand Down

0 comments on commit b2c9bb2

Please sign in to comment.