Fixed multiple extension criteria bug
Change-Id: I57157b83b605e7315c3849743a931f270e8f86a8
Showing
3 changed files
with
68 additions
and
4 deletions
... | @@ -28,8 +28,11 @@ import org.onosproject.net.DeviceId; | ... | @@ -28,8 +28,11 @@ import org.onosproject.net.DeviceId; |
28 | import org.onosproject.net.PortNumber; | 28 | import org.onosproject.net.PortNumber; |
29 | import org.onosproject.net.flow.criteria.Criteria; | 29 | import org.onosproject.net.flow.criteria.Criteria; |
30 | import org.onosproject.net.flow.criteria.Criterion; | 30 | import org.onosproject.net.flow.criteria.Criterion; |
31 | +import org.onosproject.net.flow.criteria.ExtensionCriterion; | ||
31 | import org.onosproject.net.flow.criteria.ExtensionSelector; | 32 | import org.onosproject.net.flow.criteria.ExtensionSelector; |
33 | +import org.onosproject.net.flow.criteria.ExtensionSelectorType; | ||
32 | 34 | ||
35 | +import java.util.Collection; | ||
33 | import java.util.Collections; | 36 | import java.util.Collections; |
34 | import java.util.Comparator; | 37 | import java.util.Comparator; |
35 | import java.util.HashMap; | 38 | import java.util.HashMap; |
... | @@ -38,27 +41,38 @@ import java.util.Objects; | ... | @@ -38,27 +41,38 @@ import java.util.Objects; |
38 | import java.util.Set; | 41 | import java.util.Set; |
39 | import java.util.TreeSet; | 42 | import java.util.TreeSet; |
40 | 43 | ||
44 | +import static org.onosproject.net.flow.criteria.Criterion.Type.EXTENSION; | ||
45 | + | ||
41 | /** | 46 | /** |
42 | * Default traffic selector implementation. | 47 | * Default traffic selector implementation. |
43 | */ | 48 | */ |
44 | public final class DefaultTrafficSelector implements TrafficSelector { | 49 | public final class DefaultTrafficSelector implements TrafficSelector { |
45 | 50 | ||
46 | private static final Comparator<? super Criterion> TYPE_COMPARATOR = | 51 | private static final Comparator<? super Criterion> TYPE_COMPARATOR = |
47 | - (c1, c2) -> c1.type().compareTo(c2.type()); | 52 | + (c1, c2) -> { |
53 | + if (c1.type() == EXTENSION && c2.type() == EXTENSION) { | ||
54 | + return ((ExtensionCriterion) c1).extensionSelector().type().toInt() | ||
55 | + - ((ExtensionCriterion) c2).extensionSelector().type().toInt(); | ||
56 | + } else { | ||
57 | + return c1.type().compareTo(c2.type()); | ||
58 | + } | ||
59 | + }; | ||
48 | 60 | ||
49 | private final Set<Criterion> criteria; | 61 | private final Set<Criterion> criteria; |
50 | 62 | ||
51 | private static final TrafficSelector EMPTY | 63 | private static final TrafficSelector EMPTY |
52 | - = new DefaultTrafficSelector(Collections.emptySet()); | 64 | + = new DefaultTrafficSelector(Collections.emptySet(), Collections.emptySet()); |
53 | 65 | ||
54 | /** | 66 | /** |
55 | * Creates a new traffic selector with the specified criteria. | 67 | * Creates a new traffic selector with the specified criteria. |
56 | * | 68 | * |
57 | * @param criteria criteria | 69 | * @param criteria criteria |
70 | + * @param extCriteria extension criteria | ||
58 | */ | 71 | */ |
59 | - private DefaultTrafficSelector(Set<Criterion> criteria) { | 72 | + private DefaultTrafficSelector(Collection<Criterion> criteria, Collection<Criterion> extCriteria) { |
60 | TreeSet<Criterion> elements = new TreeSet<>(TYPE_COMPARATOR); | 73 | TreeSet<Criterion> elements = new TreeSet<>(TYPE_COMPARATOR); |
61 | elements.addAll(criteria); | 74 | elements.addAll(criteria); |
75 | + elements.addAll(extCriteria); | ||
62 | this.criteria = ImmutableSet.copyOf(elements); | 76 | this.criteria = ImmutableSet.copyOf(elements); |
63 | } | 77 | } |
64 | 78 | ||
... | @@ -137,6 +151,7 @@ public final class DefaultTrafficSelector implements TrafficSelector { | ... | @@ -137,6 +151,7 @@ public final class DefaultTrafficSelector implements TrafficSelector { |
137 | public static final class Builder implements TrafficSelector.Builder { | 151 | public static final class Builder implements TrafficSelector.Builder { |
138 | 152 | ||
139 | private final Map<Criterion.Type, Criterion> selector = new HashMap<>(); | 153 | private final Map<Criterion.Type, Criterion> selector = new HashMap<>(); |
154 | + private final Map<ExtensionSelectorType, Criterion> extSelector = new HashMap<>(); | ||
140 | 155 | ||
141 | private Builder() { | 156 | private Builder() { |
142 | } | 157 | } |
... | @@ -149,7 +164,11 @@ public final class DefaultTrafficSelector implements TrafficSelector { | ... | @@ -149,7 +164,11 @@ public final class DefaultTrafficSelector implements TrafficSelector { |
149 | 164 | ||
150 | @Override | 165 | @Override |
151 | public Builder add(Criterion criterion) { | 166 | public Builder add(Criterion criterion) { |
167 | + if (criterion.type() == EXTENSION) { | ||
168 | + extSelector.put(((ExtensionCriterion) criterion).extensionSelector().type(), criterion); | ||
169 | + } else { | ||
152 | selector.put(criterion.type(), criterion); | 170 | selector.put(criterion.type(), criterion); |
171 | + } | ||
153 | return this; | 172 | return this; |
154 | } | 173 | } |
155 | 174 | ||
... | @@ -371,7 +390,7 @@ public final class DefaultTrafficSelector implements TrafficSelector { | ... | @@ -371,7 +390,7 @@ public final class DefaultTrafficSelector implements TrafficSelector { |
371 | 390 | ||
372 | @Override | 391 | @Override |
373 | public TrafficSelector build() { | 392 | public TrafficSelector build() { |
374 | - return new DefaultTrafficSelector(ImmutableSet.copyOf(selector.values())); | 393 | + return new DefaultTrafficSelector(selector.values(), extSelector.values()); |
375 | } | 394 | } |
376 | } | 395 | } |
377 | } | 396 | } | ... | ... |
... | @@ -73,6 +73,15 @@ public class ExtensionSelectorType { | ... | @@ -73,6 +73,15 @@ public class ExtensionSelectorType { |
73 | this.type = type; | 73 | this.type = type; |
74 | } | 74 | } |
75 | 75 | ||
76 | + /** | ||
77 | + * Returns the integer value associated with this type. | ||
78 | + * | ||
79 | + * @return an integer value | ||
80 | + */ | ||
81 | + public int toInt() { | ||
82 | + return this.type; | ||
83 | + } | ||
84 | + | ||
76 | @Override | 85 | @Override |
77 | public int hashCode() { | 86 | public int hashCode() { |
78 | return Objects.hash(type); | 87 | return Objects.hash(type); | ... | ... |
... | @@ -32,6 +32,7 @@ import org.onlab.packet.MplsLabel; | ... | @@ -32,6 +32,7 @@ import org.onlab.packet.MplsLabel; |
32 | import org.onlab.packet.TpPort; | 32 | import org.onlab.packet.TpPort; |
33 | import org.onlab.packet.VlanId; | 33 | import org.onlab.packet.VlanId; |
34 | import org.onosproject.net.ChannelSpacing; | 34 | import org.onosproject.net.ChannelSpacing; |
35 | +import org.onosproject.net.DeviceId; | ||
35 | import org.onosproject.net.GridType; | 36 | import org.onosproject.net.GridType; |
36 | import org.onosproject.net.OchSignal; | 37 | import org.onosproject.net.OchSignal; |
37 | import org.onosproject.net.PortNumber; | 38 | import org.onosproject.net.PortNumber; |
... | @@ -39,10 +40,14 @@ import org.onosproject.net.flow.criteria.Criteria; | ... | @@ -39,10 +40,14 @@ import org.onosproject.net.flow.criteria.Criteria; |
39 | import org.onosproject.net.flow.criteria.Criterion; | 40 | import org.onosproject.net.flow.criteria.Criterion; |
40 | 41 | ||
41 | import com.google.common.testing.EqualsTester; | 42 | import com.google.common.testing.EqualsTester; |
43 | +import org.onosproject.net.flow.criteria.ExtensionSelector; | ||
44 | +import org.onosproject.net.flow.criteria.ExtensionSelectorType; | ||
42 | 45 | ||
43 | import static org.hamcrest.MatcherAssert.assertThat; | 46 | import static org.hamcrest.MatcherAssert.assertThat; |
44 | import static org.hamcrest.Matchers.hasSize; | 47 | import static org.hamcrest.Matchers.hasSize; |
45 | import static org.hamcrest.Matchers.notNullValue; | 48 | import static org.hamcrest.Matchers.notNullValue; |
49 | +import static org.hamcrest.Matchers.is; | ||
50 | +import static org.hamcrest.Matchers.equalTo; | ||
46 | import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable; | 51 | import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable; |
47 | import static org.onosproject.net.flow.criteria.Criterion.Type; | 52 | import static org.onosproject.net.flow.criteria.Criterion.Type; |
48 | 53 | ||
... | @@ -289,5 +294,36 @@ public class DefaultTrafficSelectorTest { | ... | @@ -289,5 +294,36 @@ public class DefaultTrafficSelectorTest { |
289 | selector = DefaultTrafficSelector.builder() | 294 | selector = DefaultTrafficSelector.builder() |
290 | .add(Criteria.matchLambda(new OchSignal(GridType.DWDM, ChannelSpacing.CHL_100GHZ, 1, 1))).build(); | 295 | .add(Criteria.matchLambda(new OchSignal(GridType.DWDM, ChannelSpacing.CHL_100GHZ, 1, 1))).build(); |
291 | assertThat(selector, hasCriterionWithType(Type.OCH_SIGID)); | 296 | assertThat(selector, hasCriterionWithType(Type.OCH_SIGID)); |
297 | + | ||
298 | + selector = DefaultTrafficSelector.builder() | ||
299 | + .matchEthDst(macValue) | ||
300 | + .extension(new MockExtensionSelector(1), DeviceId.NONE) | ||
301 | + .extension(new MockExtensionSelector(2), DeviceId.NONE) | ||
302 | + .build(); | ||
303 | + assertThat(selector.criteria().size(), is(equalTo(3))); | ||
304 | + } | ||
305 | + | ||
306 | + private class MockExtensionSelector extends AbstractExtension implements ExtensionSelector { | ||
307 | + | ||
308 | + ExtensionSelectorType type; | ||
309 | + | ||
310 | + MockExtensionSelector(int typeInt) { | ||
311 | + this.type = new ExtensionSelectorType(typeInt); | ||
312 | + } | ||
313 | + | ||
314 | + @Override | ||
315 | + public ExtensionSelectorType type() { | ||
316 | + return type; | ||
317 | + } | ||
318 | + | ||
319 | + @Override | ||
320 | + public byte[] serialize() { | ||
321 | + return new byte[0]; | ||
322 | + } | ||
323 | + | ||
324 | + @Override | ||
325 | + public void deserialize(byte[] data) { | ||
326 | + | ||
327 | + } | ||
292 | } | 328 | } |
293 | } | 329 | } | ... | ... |
-
Please register or login to post a comment