Committed by
Gerrit Code Review
Change return type of groupId() in FlowRule not to misusing underlying type
Change-Id: Ide90973380f79046650bc372b9ecee00cb290f6a
Showing
4 changed files
with
22 additions
and
10 deletions
... | @@ -21,6 +21,8 @@ import static org.slf4j.LoggerFactory.getLogger; | ... | @@ -21,6 +21,8 @@ import static org.slf4j.LoggerFactory.getLogger; |
21 | import java.util.Objects; | 21 | import java.util.Objects; |
22 | 22 | ||
23 | import org.onlab.onos.core.ApplicationId; | 23 | import org.onlab.onos.core.ApplicationId; |
24 | +import org.onlab.onos.core.DefaultGroupId; | ||
25 | +import org.onlab.onos.core.GroupId; | ||
24 | import org.onlab.onos.net.DeviceId; | 26 | import org.onlab.onos.net.DeviceId; |
25 | import org.slf4j.Logger; | 27 | import org.slf4j.Logger; |
26 | 28 | ||
... | @@ -40,7 +42,7 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -40,7 +42,7 @@ public class DefaultFlowRule implements FlowRule { |
40 | 42 | ||
41 | private final int timeout; | 43 | private final int timeout; |
42 | private final boolean permanent; | 44 | private final boolean permanent; |
43 | - private final short groupId; | 45 | + private final GroupId groupId; |
44 | 46 | ||
45 | 47 | ||
46 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, | 48 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, |
... | @@ -55,19 +57,26 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -55,19 +57,26 @@ public class DefaultFlowRule implements FlowRule { |
55 | this.created = System.currentTimeMillis(); | 57 | this.created = System.currentTimeMillis(); |
56 | 58 | ||
57 | this.appId = (short) (flowId >>> 48); | 59 | this.appId = (short) (flowId >>> 48); |
58 | - this.groupId = (short) ((flowId >>> 32) & 0xFFFF); | 60 | + this.groupId = new DefaultGroupId((short) ((flowId >>> 32) & 0xFFFF)); |
59 | this.id = FlowId.valueOf(flowId); | 61 | this.id = FlowId.valueOf(flowId); |
60 | } | 62 | } |
61 | 63 | ||
62 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, | 64 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, |
63 | TrafficTreatment treatment, int priority, ApplicationId appId, | 65 | TrafficTreatment treatment, int priority, ApplicationId appId, |
64 | int timeout, boolean permanent) { | 66 | int timeout, boolean permanent) { |
65 | - this(deviceId, selector, treatment, priority, appId, (short) 0, timeout, permanent); | 67 | + this(deviceId, selector, treatment, priority, appId, new DefaultGroupId(0), timeout, permanent); |
66 | } | 68 | } |
67 | 69 | ||
70 | + @Deprecated | ||
68 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, | 71 | public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, |
69 | TrafficTreatment treatment, int priority, ApplicationId appId, | 72 | TrafficTreatment treatment, int priority, ApplicationId appId, |
70 | short groupId, int timeout, boolean permanent) { | 73 | short groupId, int timeout, boolean permanent) { |
74 | + this(deviceId, selector, treatment, priority, appId, new DefaultGroupId(groupId), timeout, permanent); | ||
75 | + } | ||
76 | + | ||
77 | + public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector, | ||
78 | + TrafficTreatment treatment, int priority, ApplicationId appId, | ||
79 | + GroupId groupId, int timeout, boolean permanent) { | ||
71 | 80 | ||
72 | if (priority < FlowRule.MIN_PRIORITY) { | 81 | if (priority < FlowRule.MIN_PRIORITY) { |
73 | throw new IllegalArgumentException("Priority cannot be less than " + MIN_PRIORITY); | 82 | throw new IllegalArgumentException("Priority cannot be less than " + MIN_PRIORITY); |
... | @@ -87,8 +96,8 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -87,8 +96,8 @@ public class DefaultFlowRule implements FlowRule { |
87 | * id consists of the following. | 96 | * id consists of the following. |
88 | * | appId (16 bits) | groupId (16 bits) | flowId (32 bits) | | 97 | * | appId (16 bits) | groupId (16 bits) | flowId (32 bits) | |
89 | */ | 98 | */ |
90 | - this.id = FlowId.valueOf((((long) this.appId) << 48) | (((long) this.groupId) << 32) | 99 | + this.id = FlowId.valueOf((((long) this.appId) << 48) | (((long) this.groupId.id()) << 32) |
91 | - | (this.hash() & 0xffffffffL)); | 100 | + | (this.hash() & 0xffffffffL)); |
92 | } | 101 | } |
93 | 102 | ||
94 | public DefaultFlowRule(FlowRule rule) { | 103 | public DefaultFlowRule(FlowRule rule) { |
... | @@ -117,7 +126,7 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -117,7 +126,7 @@ public class DefaultFlowRule implements FlowRule { |
117 | } | 126 | } |
118 | 127 | ||
119 | @Override | 128 | @Override |
120 | - public short groupId() { | 129 | + public GroupId groupId() { |
121 | return groupId; | 130 | return groupId; |
122 | } | 131 | } |
123 | 132 | ... | ... |
... | @@ -15,6 +15,7 @@ | ... | @@ -15,6 +15,7 @@ |
15 | */ | 15 | */ |
16 | package org.onlab.onos.net.flow; | 16 | package org.onlab.onos.net.flow; |
17 | 17 | ||
18 | +import org.onlab.onos.core.GroupId; | ||
18 | import org.onlab.onos.net.DeviceId; | 19 | import org.onlab.onos.net.DeviceId; |
19 | 20 | ||
20 | /** | 21 | /** |
... | @@ -46,7 +47,7 @@ public interface FlowRule extends BatchOperationTarget { | ... | @@ -46,7 +47,7 @@ public interface FlowRule extends BatchOperationTarget { |
46 | * | 47 | * |
47 | * @return an groupId | 48 | * @return an groupId |
48 | */ | 49 | */ |
49 | - short groupId(); | 50 | + GroupId groupId(); |
50 | 51 | ||
51 | /** | 52 | /** |
52 | * Returns the flow rule priority given in natural order; higher numbers | 53 | * Returns the flow rule priority given in natural order; higher numbers | ... | ... |
... | @@ -29,6 +29,8 @@ import java.util.List; | ... | @@ -29,6 +29,8 @@ import java.util.List; |
29 | import java.util.Objects; | 29 | import java.util.Objects; |
30 | import java.util.Set; | 30 | import java.util.Set; |
31 | 31 | ||
32 | +import org.onlab.onos.core.DefaultGroupId; | ||
33 | +import org.onlab.onos.core.GroupId; | ||
32 | import org.onlab.onos.net.DeviceId; | 34 | import org.onlab.onos.net.DeviceId; |
33 | import org.onlab.onos.net.ElementId; | 35 | import org.onlab.onos.net.ElementId; |
34 | import org.onlab.onos.net.Link; | 36 | import org.onlab.onos.net.Link; |
... | @@ -309,8 +311,8 @@ public class IntentTestsMocks { | ... | @@ -309,8 +311,8 @@ public class IntentTestsMocks { |
309 | } | 311 | } |
310 | 312 | ||
311 | @Override | 313 | @Override |
312 | - public short groupId() { | 314 | + public GroupId groupId() { |
313 | - return 0; | 315 | + return new DefaultGroupId(0); |
314 | } | 316 | } |
315 | 317 | ||
316 | @Override | 318 | @Override | ... | ... |
... | @@ -357,7 +357,7 @@ public class StatisticManager implements StatisticService { | ... | @@ -357,7 +357,7 @@ public class StatisticManager implements StatisticService { |
357 | } | 357 | } |
358 | // FIXME: The left hand type and right hand type don't match | 358 | // FIXME: The left hand type and right hand type don't match |
359 | // FlowEntry.groupId() still returns a short value, not int. | 359 | // FlowEntry.groupId() still returns a short value, not int. |
360 | - return flowEntry.groupId() == groupId.get().id(); | 360 | + return flowEntry.groupId().equals(groupId.get()); |
361 | } | 361 | } |
362 | }; | 362 | }; |
363 | } | 363 | } | ... | ... |
-
Please register or login to post a comment