Instruction related fixes
- Removed redundant equality check. (ONOS-975)
- Enforced using Instruction Factory methods.
- cosmetic fixes.
Change-Id: I178b55f8568c1a9132f0aa88465b8b34dc2b2df2
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
index 246d21a..926d267 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
@@ -260,11 +260,13 @@
return new TableTypeTransition(type);
}
- /*
- * Drop instructions
+ /**
+ * Drop instruction.
*/
-
public static final class DropInstruction implements Instruction {
+
+ private DropInstruction() {}
+
@Override
public Type type() {
return Type.DROP;
@@ -287,18 +289,15 @@
return true;
}
if (obj instanceof DropInstruction) {
- DropInstruction that = (DropInstruction) obj;
- return Objects.equals(type(), that.type());
-
+ return true;
}
return false;
}
}
- /*
- * Output Instruction
+ /**
+ * Output Instruction.
*/
-
public static final class OutputInstruction implements Instruction {
private final PortNumber port;
@@ -339,10 +338,9 @@
}
}
- /*
- * Group Instruction
+ /**
+ * Group Instruction.
*/
-
public static final class GroupInstruction implements Instruction {
private final GroupId groupId;
@@ -389,7 +387,7 @@
public static class TableTypeTransition implements Instruction {
private final FlowRule.Type tableType;
- public TableTypeTransition(FlowRule.Type type) {
+ TableTypeTransition(FlowRule.Type type) {
this.tableType = type;
}
@@ -405,7 +403,7 @@
@Override
public String toString() {
return toStringHelper(type().toString())
- .add("group ID", this.tableType).toString();
+ .add("tableType", this.tableType).toString();
}
@Override
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
index 69d6938..ba5b7d7 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
@@ -36,7 +36,7 @@
public abstract L0SubType subtype();
@Override
- public Type type() {
+ public final Type type() {
return Type.L0MODIFICATION;
}
@@ -48,7 +48,7 @@
private final L0SubType subtype;
private final short lambda;
- public ModLambdaInstruction(L0SubType subType, short lambda) {
+ ModLambdaInstruction(L0SubType subType, short lambda) {
this.subtype = subType;
this.lambda = lambda;
}
@@ -81,7 +81,6 @@
if (obj instanceof ModLambdaInstruction) {
ModLambdaInstruction that = (ModLambdaInstruction) obj;
return Objects.equals(lambda, that.lambda) &&
- Objects.equals(this.type(), that.type()) &&
Objects.equals(subtype, that.subtype);
}
return false;
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
index 5135557..2288898 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
@@ -80,7 +80,7 @@
public abstract L2SubType subtype();
@Override
- public Type type() {
+ public final Type type() {
return Type.L2MODIFICATION;
}
@@ -92,7 +92,7 @@
private final L2SubType subtype;
private final MacAddress mac;
- public ModEtherInstruction(L2SubType subType, MacAddress addr) {
+ ModEtherInstruction(L2SubType subType, MacAddress addr) {
this.subtype = subType;
this.mac = addr;
@@ -126,16 +126,13 @@
if (obj instanceof ModEtherInstruction) {
ModEtherInstruction that = (ModEtherInstruction) obj;
return Objects.equals(mac, that.mac) &&
- Objects.equals(this.type(), that.type()) &&
Objects.equals(subtype, that.subtype);
-
}
return false;
}
-
-
}
+ // TODO This instruction is reused for Pop-Mpls. Consider renaming.
public static final class PushHeaderInstructions extends
L2ModificationInstruction {
@@ -191,7 +188,7 @@
private final VlanId vlanId;
- public ModVlanIdInstruction(VlanId vlanId) {
+ ModVlanIdInstruction(VlanId vlanId) {
this.vlanId = vlanId;
}
@@ -222,15 +219,10 @@
}
if (obj instanceof ModVlanIdInstruction) {
ModVlanIdInstruction that = (ModVlanIdInstruction) obj;
- return Objects.equals(vlanId, that.vlanId) &&
- Objects.equals(this.type(), that.type()) &&
- Objects.equals(this.subtype(), that.subtype());
-
+ return Objects.equals(vlanId, that.vlanId);
}
return false;
}
-
-
}
/**
@@ -240,7 +232,7 @@
private final Byte vlanPcp;
- public ModVlanPcpInstruction(Byte vlanPcp) {
+ ModVlanPcpInstruction(Byte vlanPcp) {
this.vlanPcp = vlanPcp;
}
@@ -271,26 +263,22 @@
}
if (obj instanceof ModVlanPcpInstruction) {
ModVlanPcpInstruction that = (ModVlanPcpInstruction) obj;
- return Objects.equals(vlanPcp, that.vlanPcp) &&
- Objects.equals(this.type(), that.type()) &&
- Objects.equals(this.subtype(), that.subtype());
-
+ return Objects.equals(vlanPcp, that.vlanPcp);
}
return false;
}
-
}
/**
* Represents a MPLS label modification.
*/
- public static final class ModMplsLabelInstruction extends
- L2ModificationInstruction {
+ public static final class ModMplsLabelInstruction
+ extends L2ModificationInstruction {
private final MplsLabel mplsLabel;
- public ModMplsLabelInstruction(MplsLabel mplsLabel) {
+ ModMplsLabelInstruction(MplsLabel mplsLabel) {
this.mplsLabel = mplsLabel;
}
@@ -321,10 +309,7 @@
}
if (obj instanceof ModMplsLabelInstruction) {
ModMplsLabelInstruction that = (ModMplsLabelInstruction) obj;
- return Objects.equals(mplsLabel, that.mplsLabel) &&
- Objects.equals(this.type(), that.type());
-
-
+ return Objects.equals(mplsLabel, that.mplsLabel);
}
return false;
}
@@ -333,10 +318,10 @@
/**
* Represents a MPLS TTL modification.
*/
- public static final class ModMplsTtlInstruction extends
- L2ModificationInstruction {
+ public static final class ModMplsTtlInstruction
+ extends L2ModificationInstruction {
- public ModMplsTtlInstruction() {
+ ModMplsTtlInstruction() {
}
@Override
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
index 88a26b9..3d0944e 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
@@ -80,7 +80,7 @@
public abstract L3SubType subtype();
@Override
- public Type type() {
+ public final Type type() {
return Type.L3MODIFICATION;
}
@@ -92,7 +92,7 @@
private final L3SubType subtype;
private final IpAddress ip;
- public ModIPInstruction(L3SubType subType, IpAddress addr) {
+ ModIPInstruction(L3SubType subType, IpAddress addr) {
this.subtype = subType;
this.ip = addr;
@@ -126,9 +126,7 @@
if (obj instanceof ModIPInstruction) {
ModIPInstruction that = (ModIPInstruction) obj;
return Objects.equals(ip, that.ip) &&
- Objects.equals(this.type(), that.type()) &&
Objects.equals(this.subtype(), that.subtype());
-
}
return false;
}
@@ -148,7 +146,7 @@
*
* @param flowLabel the IPv6 flow label to set in the treatment (20 bits)
*/
- public ModIPv6FlowLabelInstruction(int flowLabel) {
+ ModIPv6FlowLabelInstruction(int flowLabel) {
this.flowLabel = flowLabel & MASK;
}
@@ -185,9 +183,7 @@
if (obj instanceof ModIPv6FlowLabelInstruction) {
ModIPv6FlowLabelInstruction that =
(ModIPv6FlowLabelInstruction) obj;
- return Objects.equals(flowLabel, that.flowLabel) &&
- Objects.equals(this.type(), that.type()) &&
- Objects.equals(this.subtype(), that.subtype());
+ return Objects.equals(flowLabel, that.flowLabel);
}
return false;
}
@@ -200,7 +196,7 @@
private final L3SubType subtype;
- public ModTtlInstruction(L3SubType subtype) {
+ ModTtlInstruction(L3SubType subtype) {
this.subtype = subtype;
}
@@ -227,9 +223,7 @@
}
if (obj instanceof ModTtlInstruction) {
ModTtlInstruction that = (ModTtlInstruction) obj;
- return Objects.equals(this.subtype(), that.subtype()) &&
- Objects.equals(this.type(), that.type());
-
+ return Objects.equals(this.subtype(), that.subtype());
}
return false;
}
diff --git a/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java b/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
index 78685c1..949707b 100644
--- a/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
+++ b/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
@@ -73,7 +73,7 @@
@Test
public void dropInstructionTest() {
final Instructions.DropInstruction instruction =
- new Instructions.DropInstruction();
+ Instructions.createDrop();
final ObjectNode instructionJson =
instructionCodec.encode(instruction, context);
assertThat(instructionJson, matchesInstruction(instruction));