Cleanup PCEPTopologyDeployerImpl

General cleanup of the logic flow in the class, documenting locking
rules.

Change-Id: Ife8eee1eb7190a2f8355c4a909ba2030cf1162ad
JIRA: BGPCEP-983
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
index 98f4124065218a8db6a00708cb8bbe282543369d..cb3371a7cb4964cb1d10428f3acb37d683f42050 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyDeployerImpl.java
@@ -8,28 +8,28 @@
 package org.opendaylight.bgpcep.pcep.topology.provider.config;
 
 import static java.util.Objects.requireNonNull;
-import static org.opendaylight.bgpcep.pcep.topology.provider.config.PCEPTopologyProviderUtil.filterPcepTopologies;
-import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATION;
 
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
 import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.checkerframework.checker.lock.qual.Holding;
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.bgpcep.programming.spi.InstructionScheduler;
 import org.opendaylight.bgpcep.programming.spi.InstructionSchedulerFactory;
 import org.opendaylight.mdsal.binding.api.ClusteredDataTreeChangeListener;
 import org.opendaylight.mdsal.binding.api.DataBroker;
-import org.opendaylight.mdsal.binding.api.DataObjectModification;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.binding.api.DataTreeModification;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.config.rev200120.pcep.config.SessionConfig;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.config.rev181109.PcepTopologyTypeConfig;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.TopologyTypes1;
 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.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.topology.TopologyTypes;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.osgi.service.blueprint.container.BlueprintContainer;
@@ -39,10 +39,9 @@ import org.slf4j.LoggerFactory;
 public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener<Topology>, AutoCloseable {
 
     private static final Logger LOG = LoggerFactory.getLogger(PCEPTopologyDeployerImpl.class);
-
     private static final long TIMEOUT_NS = TimeUnit.SECONDS.toNanos(5);
+
     private final BlueprintContainer container;
-    private final InstanceIdentifier<NetworkTopology> networTopology;
     private final DataBroker dataBroker;
     private final InstructionSchedulerFactory instructionSchedulerFactory;
     @GuardedBy("this")
@@ -54,51 +53,53 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
         this.container = requireNonNull(container);
         this.dataBroker = requireNonNull(dataBroker);
         this.instructionSchedulerFactory = requireNonNull(instructionSchedulerFactory);
-        this.networTopology = InstanceIdentifier.builder(NetworkTopology.class).build();
     }
 
     public synchronized void init() {
         LOG.info("PCEP Topology Deployer initialized");
-        this.listenerRegistration = this.dataBroker.registerDataTreeChangeListener(
-                DataTreeIdentifier.create(CONFIGURATION, this.networTopology.child(Topology.class)), this);
+        listenerRegistration = dataBroker.registerDataTreeChangeListener(
+            DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION,
+                InstanceIdentifier.builder(NetworkTopology.class).child(Topology.class).build()), this);
     }
 
     @Override
     public synchronized void onDataTreeChanged(final Collection<DataTreeModification<Topology>> changes) {
-        final List<DataObjectModification<Topology>> topoChanges = changes.stream()
-                .map(DataTreeModification::getRootNode)
-                .collect(Collectors.toList());
-        topoChanges.stream().iterator().forEachRemaining(topo -> {
+        for (var change : changes) {
+            final var topo = change.getRootNode();
             switch (topo.getModificationType()) {
                 case SUBTREE_MODIFIED:
                 case WRITE:
+                    // FIXME: BGPCEP-983: propagate all modifications
                     updateTopologyProvider(topo.getDataAfter());
                     break;
                 case DELETE:
                     removeTopologyProvider(topo.getDataBefore());
                     break;
                 default:
+                    throw new IllegalStateException("Unhandled modification type " + topo.getModificationType());
             }
-        });
+        }
     }
 
-    private synchronized void updateTopologyProvider(final Topology topology) {
+    @Holding("this")
+    private void updateTopologyProvider(final Topology topology) {
         if (!filterPcepTopologies(topology.getTopologyTypes())) {
             return;
         }
         final TopologyId topologyId = topology.getTopologyId();
         LOG.info("Updating Topology {}", topologyId);
-        final PCEPTopologyProviderBean previous = this.pcepTopologyServices.remove(topology.getTopologyId());
+        final PCEPTopologyProviderBean previous = pcepTopologyServices.remove(topologyId);
         closeTopology(previous, topologyId);
         createTopologyProvider(topology);
     }
 
-    private synchronized void createTopologyProvider(final Topology topology) {
+    @Holding("this")
+    private void createTopologyProvider(final Topology topology) {
         if (!filterPcepTopologies(topology.getTopologyTypes())) {
             return;
         }
         final TopologyId topologyId = topology.getTopologyId();
-        if (this.pcepTopologyServices.containsKey(topology.getTopologyId())) {
+        if (pcepTopologyServices.containsKey(topologyId)) {
             LOG.warn("Topology Provider {} already exist. New instance won't be created", topologyId);
             return;
         }
@@ -106,42 +107,42 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
         LOG.trace("Topology {}.", topology);
 
         final SessionConfig config = topology.augmentation(PcepTopologyTypeConfig.class).getSessionConfig();
-        final InstructionScheduler instructionScheduler = this.instructionSchedulerFactory
+        final InstructionScheduler instructionScheduler = instructionSchedulerFactory
                 .createInstructionScheduler(topologyId.getValue());
 
         final PCEPTopologyConfiguration dependencies = new PCEPTopologyConfiguration(config, topology);
 
-        final PCEPTopologyProviderBean pcepTopologyProviderBean = (PCEPTopologyProviderBean) this.container
+        final PCEPTopologyProviderBean pcepTopologyProviderBean = (PCEPTopologyProviderBean) container
                 .getComponentInstance(PCEPTopologyProviderBean.class.getSimpleName());
-        this.pcepTopologyServices.put(topologyId, pcepTopologyProviderBean);
+        pcepTopologyServices.put(topologyId, pcepTopologyProviderBean);
         pcepTopologyProviderBean.start(dependencies, instructionScheduler);
     }
 
+    @Holding("this")
     private synchronized void removeTopologyProvider(final Topology topology) {
         if (!filterPcepTopologies(topology.getTopologyTypes())) {
             return;
         }
         final TopologyId topologyId = topology.getTopologyId();
-        closeTopology(this.pcepTopologyServices.remove(topologyId), topologyId);
+        closeTopology(pcepTopologyServices.remove(topologyId), topologyId);
     }
 
     @Override
     public synchronized void close() {
         LOG.info("PCEP Topology Deployer closing");
-        if (this.listenerRegistration != null) {
-            this.listenerRegistration.close();
-            this.listenerRegistration = null;
+        if (listenerRegistration != null) {
+            listenerRegistration.close();
+            listenerRegistration = null;
         }
-        for (Map.Entry<TopologyId, PCEPTopologyProviderBean> entry:this.pcepTopologyServices.entrySet()) {
+        for (Map.Entry<TopologyId, PCEPTopologyProviderBean> entry : pcepTopologyServices.entrySet()) {
             closeTopology(entry.getValue(), entry.getKey());
         }
-        this.pcepTopologyServices.clear();
+        pcepTopologyServices.clear();
         LOG.info("PCEP Topology Deployer closed");
     }
 
-
     @SuppressWarnings("checkstyle:IllegalCatch")
-    private synchronized void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) {
+    private static void closeTopology(final PCEPTopologyProviderBean topology, final TopologyId topologyId) {
         if (topology == null) {
             return;
         }
@@ -153,4 +154,12 @@ public class PCEPTopologyDeployerImpl implements ClusteredDataTreeChangeListener
             LOG.error("Topology {} instance failed to close service instance", topologyId, e);
         }
     }
+
+    private static boolean filterPcepTopologies(final @Nullable TopologyTypes topologyTypes) {
+        if (topologyTypes == null) {
+            return false;
+        }
+        final TopologyTypes1 aug = topologyTypes.augmentation(TopologyTypes1.class);
+        return aug != null && aug.getTopologyPcep() != null;
+    }
 }
\ No newline at end of file

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
index 4ea817b97bf763b27c234af8f087648d4c8b5cfe..f29cbfd3586a8bb06ebfd58f5a7bda8fb56e63a9 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/config/PCEPTopologyProviderUtil.java
@@ -13,7 +13,6 @@ import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 import org.eclipse.jdt.annotation.NonNull;
-import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.protocol.concepts.KeyMapping;
 import org.opendaylight.protocol.pcep.SpeakerIdMapping;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IetfInetUtil;
@@ -21,10 +20,8 @@ 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.PortNumber;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.rfc2385.cfg.rev160324.Rfc2385Key;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.config.rev181109.PcepNodeConfig;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.TopologyTypes1;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.sync.optimizations.config.rev181109.PcepNodeSyncConfig;
 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.topology.TopologyTypes;
 
 final class PCEPTopologyProviderUtil {
     private PCEPTopologyProviderUtil() {
@@ -59,16 +56,6 @@ final class PCEPTopologyProviderUtil {
         return new InetSocketAddress(IetfInetUtil.INSTANCE.inetAddressForNoZone(address), port.getValue().toJava());
     }
 
-    static boolean filterPcepTopologies(final @Nullable TopologyTypes topologyTypes) {
-        if (topologyTypes == null) {
-            return false;
-        }
-        final TopologyTypes1 aug = topologyTypes.augmentation(TopologyTypes1.class);
-
-        return aug != null && aug.getTopologyPcep() != null;
-    }
-
-
     static SpeakerIdMapping contructSpeakersId(final Topology topology) {
         final SpeakerIdMapping ret = SpeakerIdMapping.getSpeakerIdMap();
         if (topology.getNode() == null) {