From: Tomas Slusny Date: Thu, 24 Mar 2016 13:08:02 +0000 (+0100) Subject: Fix "Bug 5589 - Deprecate PortNumberCache" X-Git-Tag: release/boron~277^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=0cd77f8ef3c8045fba03dde20044a08ec44e29f5;p=openflowplugin.git 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 --- 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 68340a73c0..83655b42b2 100644 --- 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 2688f9fa8d..d4a650d324 100644 --- 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 identifier = + (KeyedInstanceIdentifier) 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 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 2c87f25221..2293b8bcbd 100644 --- 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 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 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 index 0000000000..008486a735 --- /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 nodeInstanceIdentifier; + @Mock + GetFeaturesOutput getFeaturesOutput; + @Mock + List phyPorts; + @Mock + Iterator 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 nodePath = KeyedInstanceIdentifier + .create(Nodes.class) + .child(Node.class, new NodeKey(new NodeId(ID_VALUE))); + + // Mock first device state + final List 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 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