Fix "Bug 5589 - Deprecate PortNumberCache"

- New class that can convert DeviceState to NodeConnectorRef and NodeConnectorRef to port number.
- 1 test that will verify if conversion between DeviceState and NodeConnectorRef is valid.
- Made methods in PortNumberCache deprecated and replaced use of them in PacketReceivedTranslator with new methods.
- Modified tests for PacketReceivedTranslator to work with new methods.

Change-Id: I4960fd701ce0bf905208806c94e6ba1b7684a0cd
Signed-off-by: Tomas Slusny <tomas.slusny@pantheon.sk>

diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/PortNumberCache.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/PortNumberCache.java
index 68340a73c0d19dcee25e8b507b751dc7834bef79..83655b42b211074e2a5f5cab7155bf4dacff8e45 100644 (file)
--- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/PortNumberCache.java
+++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/PortNumberCache.java
@@ -22,11 +22,13 @@ public interface PortNumberCache {
      * @return corresponding nodeConnectorRef if present
      */
     @Nullable
+    @Deprecated
     NodeConnectorRef lookupNodeConnectorRef(Long portNumber);
 
     /**
      * @param portNumber       protocol port number
      * @param nodeConnectorRef corresponding value of {@link NodeConnectorRef}
      */
+    @Deprecated
     void storeNodeConnectorRef(@Nonnull Long portNumber, @Nonnull NodeConnectorRef nodeConnectorRef);
 }

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslator.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslator.java
index 2688f9fa8d411a14f7131e1d06f1192e5963284e..d4a650d324753a498274f7dc3af79c60186caac4 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslator.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslator.java
@@ -11,15 +11,14 @@ package org.opendaylight.openflowplugin.impl.translator;
 import com.google.common.annotations.VisibleForTesting;
 import java.math.BigInteger;
 import java.util.List;
-import org.opendaylight.openflowplugin.api.OFConstants;
 import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext;
 import org.opendaylight.openflowplugin.api.openflow.device.MessageTranslator;
 import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
 import org.opendaylight.openflowplugin.extension.api.AugmentTuple;
 import org.opendaylight.openflowplugin.extension.api.path.MatchPath;
+import org.opendaylight.openflowplugin.impl.util.NodeConnectorRefToPortTranslator;
 import org.opendaylight.openflowplugin.openflow.md.core.extension.MatchExtensionHelper;
 import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.match.MatchConvertorImpl;
-import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil;
 import org.opendaylight.openflowplugin.openflow.md.util.PacketInUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.FlowCookie;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRef;
@@ -42,30 +41,19 @@ public class PacketReceivedTranslator implements MessageTranslator<PacketInMessa
         PacketReceivedBuilder packetReceivedBuilder = new PacketReceivedBuilder();
         BigInteger datapathId = deviceContext.getPrimaryConnectionContext().getFeatures().getDatapathId();
 
-        // extract the port number
-        Long port = null;
-        if (input.getVersion() == OFConstants.OFP_VERSION_1_0 && input.getInPort() != null) {
-            port = input.getInPort().longValue();
-        } else if (input.getVersion() == OFConstants.OFP_VERSION_1_3) {
-            if (input.getMatch() != null && input.getMatch().getMatchEntry() != null) {
-                port = getPortNumberFromMatch(input.getMatch().getMatchEntry());
-            }
-        }
+        // TODO: connection cookie from connection distinguisher
+        // packetReceivedBuilder.setConnectionCookie(new ConnectionCookie(input.getCookie().longValue()));
 
-        //TODO connection cookie from connection distinguisher
-//        packetReceivedBuilder.setConnectionCookie(new ConnectionCookie(input.getCookie().longValue()));
         packetReceivedBuilder.setPayload(input.getData());
+
         // get the Cookie if it exists
         if (input.getCookie() != null) {
             packetReceivedBuilder.setFlowCookie(new FlowCookie(input.getCookie()));
         }
-        if (port != null) {
-            NodeConnectorRef nodeConnectorRef = deviceContext.lookupNodeConnectorRef(port);
-            if (nodeConnectorRef == null) {
-                nodeConnectorRef = InventoryDataServiceUtil.nodeConnectorRefFromDatapathIdPortno(
-                        datapathId, port, OpenflowVersion.get(input.getVersion()), deviceContext.getDeviceState().getNodeInstanceIdentifier());
-                deviceContext.storeNodeConnectorRef(port, nodeConnectorRef);
-            }
+
+        NodeConnectorRef nodeConnectorRef = NodeConnectorRefToPortTranslator.toNodeConnectorRef(deviceContext.getDeviceState());
+
+        if (nodeConnectorRef != null) {
             packetReceivedBuilder.setIngress(nodeConnectorRef);
         }
 

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslator.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslator.java
new file mode 100644 (file)
index 0000000..e85e930
--- /dev/null
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslator.java
@@ -0,0 +1,84 @@
+/**
+ * Copyright (c) 2015 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.openflowplugin.impl.util;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.opendaylight.openflowplugin.api.openflow.device.DeviceState;
+import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
+import org.opendaylight.openflowplugin.openflow.md.util.InventoryDataServiceUtil;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRef;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.node.NodeConnector;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.node.NodeConnectorKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.features.reply.PhyPort;
+import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.math.BigInteger;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * Created by Tomas Slusny on 23.3.2016.
+ */
+public class NodeConnectorRefToPortTranslator {
+    /**
+     * Converts {@link DeviceState} to {@link NodeConnectorRef}
+     * @param deviceState Device state to be converted
+     * @return Device state converted to node connector reference
+     */
+    @Nullable
+    public static NodeConnectorRef toNodeConnectorRef(@Nonnull DeviceState deviceState) {
+        Preconditions.checkNotNull(deviceState);
+
+        Long port = getPortNoFromDeviceState(deviceState);
+        OpenflowVersion version = OpenflowVersion.get(deviceState.getVersion());
+        BigInteger dataPathId = deviceState.getFeatures().getDatapathId();
+
+        return InventoryDataServiceUtil.nodeConnectorRefFromDatapathIdPortno(dataPathId, port, version);
+    }
+
+    /**
+     * Gets port number from {@link NodeConnectorRef}. If it is null, it will try to get the port from
+     * {@link DeviceState}
+     * @param deviceState Device state fallback if there is any problem with node connector reference
+     * @param nodeConnectorRef Node connector reference
+     * @return port number
+     */
+    @SuppressWarnings("unchecked")
+    @Nullable
+    public static Long fromNodeConnectorRef(@Nonnull DeviceState deviceState, NodeConnectorRef nodeConnectorRef) {
+        Preconditions.checkNotNull(deviceState);
+
+        if (nodeConnectorRef != null && nodeConnectorRef.getValue() instanceof KeyedInstanceIdentifier) {
+            KeyedInstanceIdentifier<NodeConnector, NodeConnectorKey> identifier =
+                    (KeyedInstanceIdentifier<NodeConnector, NodeConnectorKey>) nodeConnectorRef.getValue();
+
+            OpenflowVersion version = OpenflowVersion.get(deviceState.getVersion());
+            String nodeConnectorId = identifier.getKey().getId().getValue();
+
+            return InventoryDataServiceUtil.portNumberfromNodeConnectorId(version, nodeConnectorId);
+        } else {
+            return getPortNoFromDeviceState(deviceState);
+        }
+    }
+
+    @VisibleForTesting
+    @Nullable
+    static Long getPortNoFromDeviceState(@Nonnull DeviceState deviceState) {
+        Preconditions.checkNotNull(deviceState);
+
+        List<PhyPort> ports = deviceState.getFeatures().getPhyPort();
+
+        return ports != null ?
+                ports.stream().filter(Objects::nonNull).map(PhyPort::getPortNo).filter(Objects::nonNull).findFirst().orElse(null) :
+                null;
+    }
+}

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslatorTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslatorTest.java
index 2c87f2522137c2c88dee281df83f07c00d157840..2293b8bcbd53f551b928bbc60ad0f0bfc8102366 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslatorTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/translator/PacketReceivedTranslatorTest.java
@@ -2,6 +2,9 @@ package org.opendaylight.openflowplugin.impl.translator;
 
 import com.google.common.collect.Lists;
 import java.math.BigInteger;
+import java.util.Arrays;
+import java.util.List;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -32,8 +35,10 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.matc
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.match.entry.value.grouping.match.entry.value.in.port._case.InPortBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.match.grouping.MatchBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.FeaturesReply;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.GetFeaturesOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.PacketInMessage;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.PacketInMessageBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.features.reply.PhyPort;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketReceived;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.packet.received.Match;
 import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
@@ -50,13 +55,21 @@ public class PacketReceivedTranslatorTest {
     @Mock
     FeaturesReply featuresReply;
     @Mock
+    GetFeaturesOutput getFeaturesOutput;
+    @Mock
     DeviceState deviceState;
     @Mock
     DataBroker dataBroker;
     @Mock
     DeviceContext deviceContext;
+    @Mock
+    List<PhyPort> phyPorts;
+    @Mock
+    PhyPort phyPort;
 
-    final String data = "Test_Data";
+    static final Long PORT_NO = 5l;
+    static final String DATA = "Test_Data";
+    static final Long PORT_NUM_VALUE = 11l;
 
     public PacketReceivedTranslatorTest() {
         OpenflowPortsUtil.init();
@@ -64,10 +77,17 @@ public class PacketReceivedTranslatorTest {
 
     @Before
     public void setUp() throws Exception {
+        final List<PhyPort> phyPorts = Arrays.asList(phyPort);
+
         Mockito.when(deviceContext.getPrimaryConnectionContext()).thenReturn(connectionContext);
         Mockito.when(connectionContext.getFeatures()).thenReturn(featuresReply);
         Mockito.when(featuresReply.getDatapathId()).thenReturn(BigInteger.TEN);
         Mockito.when(deviceContext.getDeviceState()).thenReturn(deviceState);
+        Mockito.when(deviceState.getVersion()).thenReturn(OFConstants.OFP_VERSION_1_3);
+        Mockito.when(deviceState.getFeatures()).thenReturn(getFeaturesOutput);
+        Mockito.when(getFeaturesOutput.getDatapathId()).thenReturn(BigInteger.TEN);
+        Mockito.when(getFeaturesOutput.getPhyPort()).thenReturn(phyPorts);
+        Mockito.when(phyPort.getPortNo()).thenReturn(PORT_NO);
     }
 
     @Test
@@ -76,7 +96,7 @@ public class PacketReceivedTranslatorTest {
                 .create(Nodes.class)
                 .child(Node.class, new NodeKey(new NodeId("openflow:10")));
         final PacketReceivedTranslator packetReceivedTranslator = new PacketReceivedTranslator();
-        final PacketInMessage packetInMessage = createPacketInMessage(data.getBytes(), 5L);
+        final PacketInMessage packetInMessage = createPacketInMessage(DATA.getBytes(), PORT_NO);
         Mockito.when(deviceState.getNodeInstanceIdentifier()).thenReturn(nodePath);
 
         final PacketReceived packetReceived = packetReceivedTranslator.translate(packetInMessage, deviceContext, null);
@@ -84,7 +104,7 @@ public class PacketReceivedTranslatorTest {
         Assert.assertArrayEquals(packetInMessage.getData(), packetReceived.getPayload());
         Assert.assertEquals("org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.SendToController",
                 packetReceived.getPacketInReason().getName());
-        Assert.assertEquals("openflow:10:5",
+        Assert.assertEquals("openflow:10:" + PORT_NO,
                 packetReceived.getIngress().getValue().firstKeyOf(NodeConnector.class, NodeConnectorKey.class)
                         .getId().getValue());
         Assert.assertEquals(0L, packetReceived.getFlowCookie().getValue().longValue());
@@ -112,9 +132,7 @@ public class PacketReceivedTranslatorTest {
 
     @Test
     public void testGetPacketInMatch() throws Exception {
-        final long portNumValue = 11L;
-
-        MatchEntryBuilder matchEntryBuilder = assembleMatchEntryBld(portNumValue);
+        MatchEntryBuilder matchEntryBuilder = assembleMatchEntryBld(PORT_NUM_VALUE);
         MatchBuilder packetInMatchBld = new MatchBuilder()
                 .setMatchEntry(Lists.newArrayList(matchEntryBuilder.build()));
         PacketInMessageBuilder inputBld = new PacketInMessageBuilder()
@@ -125,7 +143,7 @@ public class PacketReceivedTranslatorTest {
         final Match packetInMatch = PacketReceivedTranslator.getPacketInMatch(inputBld.build(), dpid);
 
         Assert.assertNotNull(packetInMatch.getInPort());
-        Assert.assertEquals("openflow:10:11", packetInMatch.getInPort().getValue());
+        Assert.assertEquals("openflow:10:" + PORT_NUM_VALUE, packetInMatch.getInPort().getValue());
     }
 
     private static MatchEntryBuilder assembleMatchEntryBld(long portNumValue) {

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslatorTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslatorTest.java
new file mode 100644 (file)
index 0000000..008486a
--- /dev/null
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/util/NodeConnectorRefToPortTranslatorTest.java
@@ -0,0 +1,135 @@
+package org.opendaylight.openflowplugin.impl.util;
+
+import junit.framework.TestCase;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.opendaylight.openflowplugin.api.OFConstants;
+import org.opendaylight.openflowplugin.api.openflow.device.DeviceState;
+import org.opendaylight.openflowplugin.openflow.md.util.OpenflowPortsUtil;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRef;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
+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.openflow.protocol.rev130731.GetFeaturesOutput;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.features.reply.PhyPort;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.features.reply.PhyPortBuilder;
+import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
+
+import static org.mockito.Mockito.*;
+
+import java.math.BigInteger;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Created by Tomas Slusny on 24.3.2016.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class NodeConnectorRefToPortTranslatorTest extends TestCase {
+
+    @Mock
+    DeviceState deviceState;
+    @Mock
+    KeyedInstanceIdentifier<Node, NodeKey> nodeInstanceIdentifier;
+    @Mock
+    GetFeaturesOutput getFeaturesOutput;
+    @Mock
+    List<PhyPort> phyPorts;
+    @Mock
+    Iterator<PhyPort> phyPortsIterator;
+    @Mock
+    PhyPort phyPort;
+    @Mock
+    PhyPort phyPort2;
+
+
+    @Mock
+    DeviceState secondDeviceState;
+    @Mock
+    GetFeaturesOutput secondGetFeaturesOutput;
+    @Mock
+    PhyPort secondPhyPort;
+
+    static final Long PORT_NO = 5l;
+    static final Long SECOND_PORT_NO = 6l;
+    static final String ID_VALUE = "openflow:10";
+
+    @Before
+    public void setUp() throws Exception {
+        // Create nodePath (we cannot mock it in regular way because KeyedInstanceIdentifier.getKey() is final)
+        final KeyedInstanceIdentifier<Node, NodeKey> nodePath = KeyedInstanceIdentifier
+                .create(Nodes.class)
+                .child(Node.class, new NodeKey(new NodeId(ID_VALUE)));
+
+        // Mock first device state
+        final List<PhyPort> phyPorts = Arrays.asList(null, phyPort2, phyPort);
+        when(deviceState.getVersion()).thenReturn(OFConstants.OFP_VERSION_1_3);
+        when(deviceState.getNodeInstanceIdentifier()).thenReturn(nodePath);
+        when(deviceState.getFeatures()).thenReturn(getFeaturesOutput);
+        when(getFeaturesOutput.getDatapathId()).thenReturn(BigInteger.TEN);
+        when(getFeaturesOutput.getPhyPort()).thenReturn(phyPorts);
+        when(phyPort.getPortNo()).thenReturn(PORT_NO);
+        when(phyPort2.getPortNo()).thenReturn(null);
+
+        // Mock second device state
+        final List<PhyPort> secondPhyPorts = Arrays.asList(phyPort);
+        when(secondDeviceState.getVersion()).thenReturn(OFConstants.OFP_VERSION_1_3);
+        when(secondDeviceState.getNodeInstanceIdentifier()).thenReturn(nodePath);
+        when(secondDeviceState.getFeatures()).thenReturn(secondGetFeaturesOutput);
+        when(secondGetFeaturesOutput.getDatapathId()).thenReturn(BigInteger.TEN);
+        when(secondGetFeaturesOutput.getPhyPort()).thenReturn(secondPhyPorts);
+        when(secondPhyPort.getPortNo()).thenReturn(SECOND_PORT_NO);
+
+
+        OpenflowPortsUtil.init();
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testForNotNullableDeviceStateInGetPortNo() throws Exception {
+        NodeConnectorRefToPortTranslator.getPortNoFromDeviceState(null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testForNotNullableDeviceStateInToNodeConnectorRef() throws Exception {
+        NodeConnectorRefToPortTranslator.toNodeConnectorRef(null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testForNotNullableDeviceStateInFromNodeConnectorRef() throws Exception {
+        NodeConnectorRefToPortTranslator.fromNodeConnectorRef(null, null);
+    }
+
+    @Test
+    public void testGetPortNoFromDeviceState() throws Exception {
+        Long portNo = NodeConnectorRefToPortTranslator.getPortNoFromDeviceState(deviceState);
+        assertEquals(portNo, PORT_NO);
+    }
+
+    @Test
+    public void testNodeConnectorConversion() throws Exception {
+        // Convert DeviceState to NodeConnectorRef
+        NodeConnectorRef ref = NodeConnectorRefToPortTranslator.toNodeConnectorRef(deviceState);
+
+        // Test getting port from NodeConnectorRef
+        Long refPort = NodeConnectorRefToPortTranslator.fromNodeConnectorRef(deviceState, ref);
+        assertEquals(PORT_NO, refPort);
+
+        // Test for getting same port, even when we used different DeviceState as fallback
+        Long secondPort = NodeConnectorRefToPortTranslator.fromNodeConnectorRef(secondDeviceState, ref);
+        assertEquals(refPort, secondPort);
+
+        // Test fallback to device state if there is any problem with NodeConnectorRef
+        refPort = NodeConnectorRefToPortTranslator.fromNodeConnectorRef(deviceState, null);
+        assertEquals(PORT_NO, refPort);
+
+        // Check if 2 NodeConnectorRef created from same DeviceState have same value
+        assertEquals(ref, NodeConnectorRefToPortTranslator.toNodeConnectorRef(deviceState));
+    }
+}
\ No newline at end of file