Remove dead code from DeviceFlowRegistryImpl
[openflowplugin.git] / openflowplugin-impl / src / main / java / org / opendaylight / openflowplugin / impl / registry / flow / DeviceFlowRegistryImpl.java
index 5009323f93c76aa89aef1a84634578c7d5cdaa3b..bd628ff9407383fd0b2faa75f0dff2db323d6ee6 100644 (file)
@@ -8,28 +8,29 @@
 package org.opendaylight.openflowplugin.impl.registry.flow;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
 import com.google.common.collect.Maps;
-import com.google.common.util.concurrent.CheckedFuture;
+import com.google.common.util.concurrent.FluentFuture;
 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 java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 import javax.annotation.Nonnull;
 import javax.annotation.concurrent.ThreadSafe;
-import org.opendaylight.controller.md.sal.binding.api.DataBroker;
-import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
-import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
+import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.mdsal.binding.api.ReadTransaction;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.DeviceFlowRegistry;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowDescriptor;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowRegistryKey;
@@ -38,6 +39,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.Fl
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchNodesNodeTableFlow;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
 import org.slf4j.Logger;
@@ -55,7 +57,9 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
     private final List<ListenableFuture<List<Optional<FlowCapableNode>>>> lastFillFutures = new ArrayList<>();
     private final Consumer<Flow> flowConsumer;
 
-    public DeviceFlowRegistryImpl(final short version, final DataBroker dataBroker, final KeyedInstanceIdentifier<Node, NodeKey> instanceIdentifier) {
+    public DeviceFlowRegistryImpl(final short version,
+                                  final DataBroker dataBroker,
+                                  final KeyedInstanceIdentifier<Node, NodeKey> instanceIdentifier) {
         this.dataBroker = dataBroker;
         this.instanceIdentifier = instanceIdentifier;
 
@@ -63,7 +67,7 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
         flowConsumer = flow -> {
             final FlowRegistryKey flowRegistryKey = FlowRegistryKeyFactory.create(version, flow);
 
-            if (!flowRegistry.containsKey(flowRegistryKey)) {
+            if (getExistingKey(flowRegistryKey) == null) {
                 // Now, we will update the registry
                 storeDescriptor(flowRegistryKey, FlowDescriptorFactory.create(flow.getTableId(), flow.getId()));
             }
@@ -81,65 +85,52 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
         final InstanceIdentifier<FlowCapableNode> path = instanceIdentifier.augmentation(FlowCapableNode.class);
 
         // First, try to fill registry with flows from DS/Configuration
-        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> configFuture = fillFromDatastore(LogicalDatastoreType.CONFIGURATION, path);
+        final FluentFuture<Optional<FlowCapableNode>> configFuture =
+                fillFromDatastore(LogicalDatastoreType.CONFIGURATION, path);
 
         // Now, try to fill registry with flows from DS/Operational
         // in case of cluster fail over, when clients are not using DS/Configuration
         // for adding flows, but only RPCs
-        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> operationalFuture = fillFromDatastore(LogicalDatastoreType.OPERATIONAL, path);
+        final FluentFuture<Optional<FlowCapableNode>> operationalFuture =
+                fillFromDatastore(LogicalDatastoreType.OPERATIONAL, path);
 
         // And at last, chain and return futures created above.
         // Also, cache this future, so call to DeviceFlowRegistry.close() will be able
         // to cancel this future immediately if it will be still in progress
-        final ListenableFuture<List<Optional<FlowCapableNode>>> lastFillFuture = Futures.allAsList(Arrays.asList(configFuture, operationalFuture));
+        final ListenableFuture<List<Optional<FlowCapableNode>>> lastFillFuture =
+                Futures.allAsList(Arrays.asList(configFuture, operationalFuture));
         lastFillFutures.add(lastFillFuture);
         return lastFillFuture;
     }
 
-    private CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> fillFromDatastore(final LogicalDatastoreType logicalDatastoreType, final InstanceIdentifier<FlowCapableNode> path) {
-        // Create new read-only transaction
-        final ReadOnlyTransaction transaction = dataBroker.newReadOnlyTransaction();
-
-        // Bail out early if transaction is null
-        if (transaction == null) {
-            return Futures.immediateFailedCheckedFuture(
-                    new ReadFailedException("Read transaction is null"));
-        }
-
+    private FluentFuture<Optional<FlowCapableNode>> fillFromDatastore(final LogicalDatastoreType logicalDatastoreType,
+                              final InstanceIdentifier<FlowCapableNode> path) {
         // Prepare read operation from datastore for path
-        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> future =
-                transaction.read(logicalDatastoreType, path);
-
-        // Bail out early if future is null
-        if (future == null) {
-            return Futures.immediateFailedCheckedFuture(
-                    new ReadFailedException("Future from read transaction is null"));
+        final FluentFuture<Optional<FlowCapableNode>> future;
+        try (ReadTransaction transaction = dataBroker.newReadOnlyTransaction()) {
+            future = transaction.read(logicalDatastoreType, path);
         }
 
-        Futures.addCallback(future, new FutureCallback<Optional<FlowCapableNode>>() {
+        future.addCallback(new FutureCallback<Optional<FlowCapableNode>>() {
             @Override
             public void onSuccess(Optional<FlowCapableNode> result) {
-                result.asSet().stream()
+                result.map(Collections::singleton).orElse(Collections.emptySet()).stream()
                         .filter(Objects::nonNull)
-                        .filter(flowCapableNode -> Objects.nonNull(flowCapableNode.getTable()))
+                        .filter(flowCapableNode -> flowCapableNode.getTable() != null)
                         .flatMap(flowCapableNode -> flowCapableNode.getTable().stream())
                         .filter(Objects::nonNull)
-                        .filter(table -> Objects.nonNull(table.getFlow()))
+                        .filter(table -> table.getFlow() != null)
                         .flatMap(table -> table.getFlow().stream())
                         .filter(Objects::nonNull)
-                        .filter(flow -> Objects.nonNull(flow.getId()))
+                        .filter(flow -> flow.getId() != null)
                         .forEach(flowConsumer);
-
-                // After we are done with reading from datastore, close the transaction
-                transaction.close();
             }
 
             @Override
-            public void onFailure(Throwable t) {
-                // Even when read operation failed, close the transaction
-                transaction.close();
+            public void onFailure(Throwable throwable) {
+                LOG.debug("Failed to read {} path {}", logicalDatastoreType, path, throwable);
             }
-        });
+        }, MoreExecutors.directExecutor());
 
         return future;
     }
@@ -147,10 +138,14 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
     @Override
     public FlowDescriptor retrieveDescriptor(@Nonnull final FlowRegistryKey flowRegistryKey) {
         if (LOG.isTraceEnabled()) {
-            LOG.trace("Retrieving flow descriptor for flow hash : {}", flowRegistryKey.hashCode());
+            LOG.trace("Retrieving flow descriptor for flow registry : {}", flowRegistryKey.toString());
         }
 
-        return flowRegistry.get(flowRegistryKey);
+        FlowRegistryKey existingFlowRegistryKey = getExistingKey(flowRegistryKey);
+        if (existingFlowRegistryKey != null) {
+            return flowRegistry.get(existingFlowRegistryKey);
+        }
+        return null;
     }
 
     @Override
@@ -159,19 +154,22 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
         try {
             if (LOG.isTraceEnabled()) {
                 LOG.trace("Storing flowDescriptor with table ID : {} and flow ID : {} for flow hash : {}",
-                        flowDescriptor.getTableKey().getId(), flowDescriptor.getFlowId().getValue(), flowRegistryKey.hashCode());
+                        flowDescriptor.getTableKey().getId(),
+                        flowDescriptor.getFlowId().getValue(),
+                        flowRegistryKey.toString());
             }
 
-            flowRegistry.put(flowRegistryKey, flowDescriptor);
+            addToFlowRegistry(flowRegistryKey, flowDescriptor);
         } catch (IllegalArgumentException ex) {
             if (LOG.isWarnEnabled()) {
-                LOG.warn("Flow with flow ID {} already exists in table {}, generating alien flow ID", flowDescriptor.getFlowId().getValue(),
+                LOG.warn("Flow with flow ID {} already exists in table {}, generating alien flow ID",
+                        flowDescriptor.getFlowId().getValue(),
                         flowDescriptor.getTableKey().getId());
             }
 
             // We are trying to store new flow to flow registry, but we already have different flow with same flow ID
             // stored in registry, so we need to create alien ID for this new flow here.
-            flowRegistry.put(
+            addToFlowRegistry(
                     flowRegistryKey,
                     FlowDescriptorFactory.create(
                             flowDescriptor.getTableKey().getId(),
@@ -181,10 +179,8 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
 
     @Override
     public void store(final FlowRegistryKey flowRegistryKey) {
-        if (Objects.isNull(retrieveDescriptor(flowRegistryKey))) {
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Flow descriptor for flow hash : {} not found, generating alien flow ID", flowRegistryKey.hashCode());
-            }
+        if (retrieveDescriptor(flowRegistryKey) == null) {
+            LOG.debug("Flow descriptor for flow hash : {} not found, generating alien flow ID", flowRegistryKey);
 
             // We do not found flow in flow registry, that means it do not have any ID already assigned, so we need
             // to generate new alien flow ID here.
@@ -199,10 +195,10 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
     @Override
     public void addMark(final FlowRegistryKey flowRegistryKey) {
         if (LOG.isTraceEnabled()) {
-            LOG.trace("Removing flow descriptor for flow hash : {}", flowRegistryKey.hashCode());
+            LOG.trace("Removing flow descriptor for flow hash : {}", flowRegistryKey.toString());
         }
 
-        flowRegistry.remove(flowRegistryKey);
+        removeFromFlowRegistry(flowRegistryKey);
     }
 
     @Override
@@ -244,6 +240,46 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
         return new FlowId(alienId);
     }
 
+    //Hashcode generation of the extension augmentation can differ for the same object received from the datastore and
+    // the one received after deserialization of switch message. OpenFlowplugin extensions are list, and the order in
+    // which it can receive the extensions back from switch can differ and that lead to a different hashcode. In that
+    // scenario, hashcode won't match and flowRegistry return the  related key. To overcome this issue, these methods
+    // make sure that key is stored only if it doesn't equals to any existing key.
+    private void addToFlowRegistry(final FlowRegistryKey flowRegistryKey, final FlowDescriptor flowDescriptor) {
+        FlowRegistryKey existingFlowRegistryKey = getExistingKey(flowRegistryKey);
+        if (existingFlowRegistryKey == null) {
+            flowRegistry.put(flowRegistryKey, flowDescriptor);
+        } else {
+            flowRegistry.put(existingFlowRegistryKey, flowDescriptor);
+        }
+    }
+
+    private void removeFromFlowRegistry(final FlowRegistryKey flowRegistryKey) {
+        FlowRegistryKey existingFlowRegistryKey = getExistingKey(flowRegistryKey);
+        if (existingFlowRegistryKey != null) {
+            flowRegistry.remove(existingFlowRegistryKey);
+        } else {
+            flowRegistry.remove(flowRegistryKey);
+        }
+    }
+
+    private FlowRegistryKey getExistingKey(final FlowRegistryKey flowRegistryKey) {
+        if (flowRegistryKey.getMatch().augmentation(GeneralAugMatchNodesNodeTableFlow.class) == null) {
+            if (flowRegistry.containsKey(flowRegistryKey)) {
+                return flowRegistryKey;
+            }
+        } else {
+            synchronized (flowRegistry) {
+                for (Map.Entry<FlowRegistryKey, FlowDescriptor> keyValueSet : flowRegistry.entrySet()) {
+                    if (keyValueSet.getKey().equals(flowRegistryKey)) {
+                        return keyValueSet.getKey();
+                    }
+                }
+            }
+        }
+        return null;
+    }
+
     @VisibleForTesting
     Map<FlowRegistryKey, FlowDescriptor> getAllFlowDescriptors() {
         return flowRegistry;