Bugfix: OpticalPortOperator should be able to overwrite port number.
- bug introduced in ONOS-3503.
- added exact equality comparison method to PortNumber
- removed null PortNumber case handling test
Change-Id: I6d1f191b5a64b79426de9d80cffadd6c9de96d56
(cherry picked from commit fdb82fa496ba319fbff49e56956377dcc1c71d5f)
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java b/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
index 0bf45b3..bd56751 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/OpticalPortOperator.java
@@ -102,50 +102,64 @@
}
// updates a port description whose port type has not changed.
- private static PortDescription updateDescription(
- PortNumber port, SparseAnnotations sa, PortDescription descr) {
+ /**
+ * Updates {@link PortDescription} using specified number and annotations.
+ *
+ * @param port {@link PortNumber} to use in updated description
+ * @param sa annotations to use in updated description
+ * @param descr base {@link PortDescription}
+ * @return updated {@link PortDescription}
+ */
+ private static PortDescription updateDescription(PortNumber port,
+ SparseAnnotations sa,
+ PortDescription descr) {
+
+ // TODO This switch can go away once deprecation is complete.
switch (descr.type()) {
case OMS:
if (descr instanceof OmsPortDescription) {
- // TODO This block can go away once deprecation is complete.
OmsPortDescription oms = (OmsPortDescription) descr;
return omsPortDescription(port, oms.isEnabled(), oms.minFrequency(),
oms.maxFrequency(), oms.grid(), sa);
}
- return descr;
+ break;
case OCH:
// We might need to update lambda below with STATIC_LAMBDA.
if (descr instanceof OchPortDescription) {
- // TODO This block can go away once deprecation is complete.
OchPortDescription och = (OchPortDescription) descr;
return ochPortDescription(port, och.isEnabled(), och.signalType(),
och.isTunable(), och.lambda(), sa);
}
- return descr;
+ break;
case ODUCLT:
if (descr instanceof OduCltPortDescription) {
- // TODO This block can go away once deprecation is complete.
OduCltPortDescription odu = (OduCltPortDescription) descr;
return oduCltPortDescription(port, odu.isEnabled(), odu.signalType(), sa);
}
- return descr;
+ break;
case PACKET:
case FIBER:
case COPPER:
- // TODO: it should be safe to just return descr. confirm and fix
- return new DefaultPortDescription(port, descr.isEnabled(), descr.type(),
- descr.portSpeed(), sa);
+ break;
case OTU:
if (descr instanceof OtuPortDescription) {
- // TODO This block can go away once deprecation is complete.
OtuPortDescription otu = (OtuPortDescription) descr;
return otuPortDescription(port, otu.isEnabled(), otu.signalType(), sa);
}
- return descr;
+ break;
default:
log.warn("Unsupported optical port type {} - can't update", descr.type());
return descr;
}
+ if (port.exactlyEquals(descr.portNumber()) && sa.equals(descr.annotations())) {
+ // result is no-op
+ return descr;
+ }
+ return new DefaultPortDescription(port,
+ descr.isEnabled(),
+ descr.type(),
+ descr.portSpeed(),
+ sa);
}
/**
diff --git a/core/net/src/test/java/org/onosproject/net/device/impl/OpticalPortOperatorTest.java b/core/net/src/test/java/org/onosproject/net/device/impl/OpticalPortOperatorTest.java
index 6f277d6..6d257e4 100644
--- a/core/net/src/test/java/org/onosproject/net/device/impl/OpticalPortOperatorTest.java
+++ b/core/net/src/test/java/org/onosproject/net/device/impl/OpticalPortOperatorTest.java
@@ -28,62 +28,94 @@
import org.onosproject.net.Port;
import org.onosproject.net.PortNumber;
import org.onosproject.net.SparseAnnotations;
-import org.onosproject.net.device.OduCltPortDescription;
-
+import org.onosproject.net.device.PortDescription;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import static org.junit.Assert.assertEquals;
+import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
public class OpticalPortOperatorTest {
private static final DeviceId DID = DeviceId.deviceId("op-test");
- private static final String TPNAME = "test-port-100";
- private static final String SPNAME = "out-port-200";
- private static final String CFGNAME = "cfg-name";
+ private static final long PORT_NUMBER = 100;
+ private static final String CFG_KEY = "optical";
- private static final PortNumber NAMED = PortNumber.portNumber(100, TPNAME);
- private static final PortNumber UNNAMED = PortNumber.portNumber(101);
- private static final ConnectPoint NCP = new ConnectPoint(DID, UNNAMED);
+ private static final String CFG_PORT_NAME = "cfg-name";
+ private static final long CFG_STATIC_LAMBDA = 300L;
+ private static final String DESC_PORT_NAME = "test-port-100";
+ private static final PortNumber NAMED = PortNumber.portNumber(PORT_NUMBER, DESC_PORT_NAME);
+ private static final PortNumber UNNAMED = PortNumber.portNumber(PORT_NUMBER);
+
+ private static final String DESC_STATIC_PORT = "out-port-200";
private static final SparseAnnotations SA = DefaultAnnotations.builder()
- .set(AnnotationKeys.STATIC_PORT, SPNAME)
+ .set(AnnotationKeys.STATIC_PORT, DESC_STATIC_PORT)
.build();
- private static final OduCltPortDescription N_DESC = new OduCltPortDescription(
+ private static final PortDescription N_DESC = oduCltPortDescription(
NAMED, true, CltSignalType.CLT_100GBE, SA);
- private static final OduCltPortDescription FAULTY = new OduCltPortDescription(
- null, true, CltSignalType.CLT_100GBE);
+ private static final PortDescription U_DESC = oduCltPortDescription(
+ UNNAMED, true, CltSignalType.CLT_100GBE, SA);
private final ConfigApplyDelegate delegate = new MockCfgDelegate();
private final ObjectMapper mapper = new ObjectMapper();
- private static final OpticalPortConfig N_OPC = new OpticalPortConfig();
- private static final OpticalPortConfig UNN_OPC = new OpticalPortConfig();
+ private static final ConnectPoint CP = new ConnectPoint(DID, UNNAMED);
+
+ private static final OpticalPortConfig OPC = new OpticalPortConfig();
@Before
public void setUp() {
- N_OPC.init(NCP, TPNAME, JsonNodeFactory.instance.objectNode(), mapper, delegate);
- UNN_OPC.init(NCP, TPNAME, JsonNodeFactory.instance.objectNode(), mapper, delegate);
-
- N_OPC.portName(CFGNAME).portNumberName(101L).portType(Port.Type.ODUCLT).staticLambda(300L);
- UNN_OPC.portType(Port.Type.ODUCLT);
+ OPC.init(CP, CFG_KEY, JsonNodeFactory.instance.objectNode(), mapper, delegate);
}
- @Test(expected = RuntimeException.class)
- public void testDescOps() {
- // port-null desc + opc with port number name
- OduCltPortDescription res = (OduCltPortDescription) OpticalPortOperator.combine(N_OPC, FAULTY);
- assertEquals(CFGNAME, res.portNumber().name());
+ @Test
+ public void testConfigPortName() {
+ OPC.portType(Port.Type.ODUCLT)
+ .portNumberName(PORT_NUMBER)
+ .portName(CFG_PORT_NAME);
+
+ PortDescription res;
// full desc + opc with name
- assertEquals(TPNAME, N_DESC.portNumber().name());
- res = (OduCltPortDescription) OpticalPortOperator.combine(N_OPC, N_DESC);
- long sl = Long.valueOf(res.annotations().value(AnnotationKeys.STATIC_LAMBDA));
- assertEquals(CFGNAME, res.portNumber().name());
- assertEquals(300L, sl);
- // port-null desc + opc without port number name - throws RE
- res = (OduCltPortDescription) OpticalPortOperator.combine(UNN_OPC, FAULTY);
+ res = OpticalPortOperator.combine(OPC, N_DESC);
+ assertEquals("Configured port name expected",
+ CFG_PORT_NAME, res.portNumber().name());
+ assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
+
+ res = OpticalPortOperator.combine(OPC, U_DESC);
+ assertEquals("Configured port name expected",
+ CFG_PORT_NAME, res.portNumber().name());
+ assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
}
+ @Test
+ public void testConfigAddStaticLambda() {
+ OPC.portType(Port.Type.ODUCLT)
+ .portNumberName(PORT_NUMBER)
+ .staticLambda(CFG_STATIC_LAMBDA);
+
+ PortDescription res;
+ res = OpticalPortOperator.combine(OPC, N_DESC);
+ assertEquals("Original port name expected",
+ DESC_PORT_NAME, res.portNumber().name());
+ assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
+ long sl = Long.valueOf(res.annotations().value(AnnotationKeys.STATIC_LAMBDA));
+ assertEquals(CFG_STATIC_LAMBDA, sl);
+ }
+
+ @Test
+ public void testEmptyConfig() {
+ OPC.portType(Port.Type.ODUCLT)
+ .portNumberName(PORT_NUMBER);
+
+ PortDescription res;
+ res = OpticalPortOperator.combine(OPC, N_DESC);
+ assertEquals("Configured port name expected",
+ DESC_PORT_NAME, res.portNumber().name());
+ assertEquals(DESC_STATIC_PORT, res.annotations().value(AnnotationKeys.STATIC_PORT));
+ }
+
+
private class MockCfgDelegate implements ConfigApplyDelegate {
@Override