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/api/src/main/java/org/onosproject/net/PortNumber.java b/core/api/src/main/java/org/onosproject/net/PortNumber.java
index 96c4eb0..e1a1554 100644
--- a/core/api/src/main/java/org/onosproject/net/PortNumber.java
+++ b/core/api/src/main/java/org/onosproject/net/PortNumber.java
@@ -15,6 +15,7 @@
*/
package org.onosproject.net;
+import com.google.common.base.Objects;
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableMap;
@@ -273,4 +274,21 @@
}
return false;
}
+
+ /**
+ * Indicates whether some other PortNumber object is equal to this one
+ * including it's name.
+ *
+ * @param that other {@link PortNumber} instance to compare
+ * @return true if equal, false otherwise
+ */
+ public boolean exactlyEquals(PortNumber that) {
+ if (this == that) {
+ return true;
+ }
+
+ return this.equals(that) &&
+ this.hasName == that.hasName &&
+ Objects.equal(this.name, that.name);
+ }
}
diff --git a/core/api/src/main/java/org/onosproject/net/device/DefaultPortDescription.java b/core/api/src/main/java/org/onosproject/net/device/DefaultPortDescription.java
index a507e49..ca57944 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DefaultPortDescription.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DefaultPortDescription.java
@@ -20,6 +20,7 @@
import org.onosproject.net.PortNumber;
import org.onosproject.net.SparseAnnotations;
+import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.Port.Type;
import com.google.common.base.Objects;
@@ -61,7 +62,7 @@
Type type, long portSpeed,
SparseAnnotations...annotations) {
super(annotations);
- this.number = number;
+ this.number = checkNotNull(number);
this.isEnabled = isEnabled;
this.type = type;
this.portSpeed = portSpeed;
diff --git a/core/api/src/test/java/org/onosproject/net/PortNumberTest.java b/core/api/src/test/java/org/onosproject/net/PortNumberTest.java
index c31e1c8..adba71f 100644
--- a/core/api/src/test/java/org/onosproject/net/PortNumberTest.java
+++ b/core/api/src/test/java/org/onosproject/net/PortNumberTest.java
@@ -21,7 +21,7 @@
import org.onosproject.net.PortNumber.Logical;
import static java.util.stream.Collectors.toList;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
import static org.onosproject.net.PortNumber.portNumber;
import java.util.List;
@@ -77,4 +77,16 @@
ps.forEach(p -> assertEquals(p, PortNumber.fromString(p.toString())));
}
+ @Test
+ public void exactlyEquals() {
+ assertTrue(portNumber(0).exactlyEquals(portNumber(0)));
+ assertTrue(portNumber(0, "foo").exactlyEquals(portNumber(0, "foo")));
+
+ assertFalse(portNumber(0, "foo").exactlyEquals(portNumber(0, "bar")));
+ assertFalse(portNumber(0, "foo").exactlyEquals(portNumber(0)));
+ assertFalse(portNumber(0, "foo").exactlyEquals(portNumber(1, "foo")));
+
+ assertFalse(portNumber(123).exactlyEquals(portNumber(123, "123")));
+ }
+
}
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