Improve flow objectives data objects code coverage
- enhance some APIs to return the right type of builder to enhance fluent interface
- override the copy() methods to return the proper types so it can be used in a fluent manner
- enhance unit tests to improve coverage on objective types
- add unit tests for objective context
Change-Id: I6beb7027fe5eb9929a8a4fea147444d9930c10ac
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/FilteringObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/FilteringObjective.java
index 898504f..623de3b 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/FilteringObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/FilteringObjective.java
@@ -93,6 +93,14 @@
TrafficTreatment meta();
/**
+ * Returns a new builder set to create a copy of this objective.
+ *
+ * @return new builder
+ */
+ @Override
+ Builder copy();
+
+ /**
* Builder of Filtering objective entities.
*/
interface Builder extends Objective.Builder {
@@ -145,6 +153,23 @@
Builder fromApp(ApplicationId appId);
/**
+ * Sets the priority for this objective.
+ *
+ * @param priority an integer
+ * @return an objective builder
+ */
+ @Override
+ Builder withPriority(int priority);
+
+ /**
+ * Makes the filtering objective permanent.
+ *
+ * @return an objective builder
+ */
+ @Override
+ public Builder makePermanent();
+
+ /**
* Builds the filtering objective that will be added.
*
* @return a filtering objective
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/ForwardingObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/ForwardingObjective.java
index 7c19171..1d62a23 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/ForwardingObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/ForwardingObjective.java
@@ -16,6 +16,7 @@
package org.onosproject.net.flowobjective;
import com.google.common.annotations.Beta;
+import org.onosproject.core.ApplicationId;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
@@ -97,6 +98,14 @@
TrafficSelector meta();
/**
+ * Returns a new builder set to create a copy of this objective.
+ *
+ * @return new builder
+ */
+ @Override
+ Builder copy();
+
+ /**
* A forwarding objective builder.
*/
interface Builder extends Objective.Builder {
@@ -142,6 +151,32 @@
Builder withMeta(TrafficSelector selector);
/**
+ * Assigns an application id.
+ *
+ * @param appId an application id
+ * @return a filtering builder
+ */
+ @Override
+ Builder fromApp(ApplicationId appId);
+
+ /**
+ * Sets the priority for this objective.
+ *
+ * @param priority an integer
+ * @return an objective builder
+ */
+ @Override
+ Builder withPriority(int priority);
+
+ /**
+ * Makes the filtering objective permanent.
+ *
+ * @return an objective builder
+ */
+ @Override
+ Builder makePermanent();
+
+ /**
* Builds the forwarding objective that will be added.
*
* @return a forwarding objective
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/NextObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/NextObjective.java
index c514baf..725ebf5 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/NextObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/NextObjective.java
@@ -95,6 +95,14 @@
TrafficSelector meta();
/**
+ * Returns a new builder set to create a copy of this objective.
+ *
+ * @return new builder
+ */
+ @Override
+ Builder copy();
+
+ /**
* A next step builder.
*/
interface Builder extends Objective.Builder {
diff --git a/core/api/src/test/java/org/onosproject/net/flowobjective/ObjectiveTest.java b/core/api/src/test/java/org/onosproject/net/flowobjective/ObjectiveTest.java
index 3c152af..34acd42 100644
--- a/core/api/src/test/java/org/onosproject/net/flowobjective/ObjectiveTest.java
+++ b/core/api/src/test/java/org/onosproject/net/flowobjective/ObjectiveTest.java
@@ -15,7 +15,9 @@
*/
package org.onosproject.net.flowobjective;
+import com.google.common.testing.EqualsTester;
import org.junit.Test;
+import org.onosproject.TestApplicationId;
import org.onosproject.net.flow.DefaultTrafficSelector;
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
@@ -23,6 +25,7 @@
import org.onosproject.net.flow.criteria.Criteria;
import org.onosproject.net.flow.criteria.Criterion;
+import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
@@ -32,9 +35,12 @@
import static org.onosproject.net.NetTestTools.APP_ID;
import static org.onosproject.net.flowobjective.FilteringObjective.Type.DENY;
import static org.onosproject.net.flowobjective.ForwardingObjective.Flag.SPECIFIC;
+import static org.onosproject.net.flowobjective.ForwardingObjective.Flag.VERSATILE;
import static org.onosproject.net.flowobjective.NextObjective.Type.HASHED;
import static org.onosproject.net.flowobjective.Objective.Operation.ADD;
+import static org.onosproject.net.flowobjective.Objective.Operation.ADD_TO_EXISTING;
import static org.onosproject.net.flowobjective.Objective.Operation.REMOVE;
+import static org.onosproject.net.flowobjective.Objective.Operation.REMOVE_FROM_EXISTING;
/**
* Unit tests for forwarding objective class.
@@ -49,21 +55,6 @@
private final Criterion key = Criteria.dummy();
/**
- * Mock objective context.
- */
- private static class MockObjectiveContext implements ObjectiveContext {
- @Override
- public void onSuccess(Objective objective) {
- // stub
- }
-
- @Override
- public void onError(Objective objective, ObjectiveError error) {
- // stub
- }
- }
-
- /**
* Checks immutability of objective classes.
*/
@Test
@@ -85,6 +76,7 @@
.withSelector(selector)
.withTreatment(treatment)
.withFlag(SPECIFIC)
+ .withMeta(selector)
.fromApp(APP_ID)
.withPriority(22)
.makeTemporary(5)
@@ -102,6 +94,7 @@
assertThat(objective.permanent(), is(false));
assertThat(objective.timeout(), is(5));
assertThat(objective.selector(), is(selector));
+ assertThat(objective.meta(), is(selector));
assertThat(objective.treatment(), is(treatment));
assertThat(objective.flag(), is(SPECIFIC));
assertThat(objective.appId(), is(APP_ID));
@@ -131,7 +124,7 @@
*/
@Test
public void testForwardingAddWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkForwardingBase(baseForwardingBuilder().add(context), ADD, context);
}
@@ -150,7 +143,7 @@
*/
@Test
public void testForwardingRemoveWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkForwardingBase(baseForwardingBuilder().remove(context), REMOVE, context);
}
@@ -165,6 +158,7 @@
return DefaultFilteringObjective.builder()
.withKey(key)
.withPriority(5)
+ .withMeta(treatment)
.addCondition(criterion)
.fromApp(APP_ID)
.makeTemporary(2)
@@ -181,6 +175,7 @@
ObjectiveContext expectedContext) {
assertThat(objective.key(), is(key));
assertThat(objective.conditions(), hasItem(criterion));
+ assertThat(objective.meta(), is(treatment));
assertThat(objective.permanent(), is(false));
assertThat(objective.timeout(), is(2));
assertThat(objective.priority(), is(5));
@@ -210,7 +205,7 @@
*/
@Test
public void testFilteringAddWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkFilteringBase(baseFilteringBuilder().add(context), ADD, context);
}
@@ -229,7 +224,7 @@
*/
@Test
public void testFilteringRemoveWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkFilteringBase(baseFilteringBuilder().remove(context), REMOVE, context);
}
@@ -245,6 +240,7 @@
.addTreatment(treatment)
.withId(12)
.withType(HASHED)
+ .withMeta(selector)
.makeTemporary(777)
.withPriority(33)
.fromApp(APP_ID);
@@ -262,6 +258,7 @@
assertThat(objective.appId(), is(APP_ID));
assertThat(objective.type(), is(HASHED));
assertThat(objective.next(), hasItem(treatment));
+ assertThat(objective.meta(), is(selector));
assertThat(objective.permanent(), is(false));
assertThat(objective.timeout(), is(0));
assertThat(objective.priority(), is(0));
@@ -288,7 +285,7 @@
*/
@Test
public void testNextAddWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkNextBase(baseNextBuilder().add(context), ADD, context);
}
@@ -307,7 +304,240 @@
*/
@Test
public void testNextRemoveWithContext() {
- ObjectiveContext context = new MockObjectiveContext();
+ ObjectiveContext context = new DefaultObjectiveContext(null, null);
checkNextBase(baseNextBuilder().remove(context), REMOVE, context);
}
+
+ /**
+ * Tests equality, hash, and toString operations on next objectives.
+ */
+ @Test
+ public void testEqualsNextObjective() {
+ NextObjective next1 = baseNextBuilder().add();
+ NextObjective sameAsNext1 = next1.copy().add();
+ NextObjective next2 = baseNextBuilder().verify();
+ NextObjective next3 = baseNextBuilder().fromApp(new TestApplicationId("foo2")).verify();
+ NextObjective next4 = baseNextBuilder().withType(NextObjective.Type.FAILOVER).verify();
+ NextObjective next5 = baseNextBuilder().addTreatment(DefaultTrafficTreatment.emptyTreatment()).verify();
+ new EqualsTester()
+ .addEqualityGroup(next1, sameAsNext1)
+ .addEqualityGroup(next2)
+ .addEqualityGroup(next3)
+ .addEqualityGroup(next4)
+ .addEqualityGroup(next5)
+ .testEquals();
+ }
+
+ /**
+ * Tests add to an existing next objective.
+ */
+ @Test
+ public void testToFromExistingNextObjective() {
+ NextObjective next1 = baseNextBuilder().addToExisting();
+
+ checkNextBase(next1, ADD_TO_EXISTING, null);
+ }
+
+ /**
+ * Tests remove from an existing next objective.
+ */
+ @Test
+ public void testRemoveFromExistingNextObjective() {
+ NextObjective next1 = baseNextBuilder().removeFromExisting();
+
+ checkNextBase(next1, REMOVE_FROM_EXISTING, null);
+ }
+
+ /**
+ * Tests equality, hash, and toString operations on filtering objectives.
+ */
+ @Test
+ public void testEqualsFilteringObjective() {
+ FilteringObjective filter1 = baseFilteringBuilder().add();
+ FilteringObjective sameAsFilter1 = filter1.copy().add();
+ FilteringObjective filter2 = baseFilteringBuilder().permit().add();
+ FilteringObjective filter3 = baseFilteringBuilder().fromApp(new TestApplicationId("foo2")).add();
+ FilteringObjective filter4 = baseFilteringBuilder().permit().remove();
+ FilteringObjective filter5 = baseFilteringBuilder().withPriority(55).add();
+ FilteringObjective filter6 = baseFilteringBuilder().makePermanent().add();
+ new EqualsTester()
+ .addEqualityGroup(filter1, sameAsFilter1)
+ .addEqualityGroup(filter2)
+ .addEqualityGroup(filter3)
+ .addEqualityGroup(filter4)
+ .addEqualityGroup(filter5)
+ .addEqualityGroup(filter6)
+ .testEquals();
+ }
+
+ /**
+ * Tests add to an existing filtering objective.
+ */
+ @Test
+ public void testToFromExistingFilteringObjective() {
+ FilteringObjective filter1 = baseFilteringBuilder().add(null);
+
+ checkFilteringBase(filter1, ADD, null);
+ }
+
+ /**
+ * Tests remove from an existing filtering objective.
+ */
+ @Test
+ public void testRemoveFromExistingFilteringObjective() {
+ FilteringObjective filter1 = baseFilteringBuilder().remove(null);
+
+ checkFilteringBase(filter1, REMOVE, null);
+ }
+
+ /**
+ * Tests equality, hash, and toString operations on filtering objectives.
+ */
+ @Test
+ public void testEqualsForwardingObjective() {
+ ForwardingObjective forward1 = baseForwardingBuilder().add();
+ ForwardingObjective sameAsForward1 = forward1.copy().add();
+ ForwardingObjective forward2 = baseForwardingBuilder().withFlag(VERSATILE).add();
+ ForwardingObjective forward3 = baseForwardingBuilder().fromApp(new TestApplicationId("foo2")).add();
+ ForwardingObjective forward4 = baseForwardingBuilder().remove();
+ ForwardingObjective forward5 = baseForwardingBuilder().withPriority(55).add();
+ ForwardingObjective forward6 = baseForwardingBuilder().makePermanent().add();
+ new EqualsTester()
+ .addEqualityGroup(forward1, sameAsForward1)
+ .addEqualityGroup(forward2)
+ .addEqualityGroup(forward3)
+ .addEqualityGroup(forward4)
+ .addEqualityGroup(forward5)
+ .addEqualityGroup(forward6)
+ .testEquals();
+ }
+
+ /**
+ * Tests add to an existing forwarding objective.
+ */
+ @Test
+ public void testToFromExistingForwardingObjective() {
+ ForwardingObjective forward1 = baseForwardingBuilder().add(null);
+
+ checkForwardingBase(forward1, ADD, null);
+ }
+
+ /**
+ * Tests remove from an existing forwarding objective.
+ */
+ @Test
+ public void testRemoveFromExistingForwardingObjective() {
+ ForwardingObjective forward1 = baseForwardingBuilder().remove(null);
+
+ checkForwardingBase(forward1, REMOVE, null);
+ }
+
+
+ enum ContextType {
+ BOTH,
+ ERROR_ONLY,
+ SUCCESS_ONLY
+ }
+
+ class ObjectiveContextAdapter implements ObjectiveContext {
+ DefaultObjectiveContext context;
+ int successCount = 0;
+ int errorCount = 0;
+ ObjectiveError objectiveError = ObjectiveError.UNKNOWN;
+ Objective objectiveInError = null;
+
+ ObjectiveContextAdapter(ContextType type) {
+ switch (type) {
+
+ case ERROR_ONLY:
+ context = new DefaultObjectiveContext(
+ (failedObjective, error) -> {
+ errorCount++;
+ objectiveInError = failedObjective;
+ objectiveError = error;
+ });
+ break;
+
+ case SUCCESS_ONLY:
+ context = new DefaultObjectiveContext(
+ (successfulObjective) -> successCount++);
+ break;
+
+ case BOTH:
+ default:
+ context = new DefaultObjectiveContext(
+ (successfulObjective) -> successCount++,
+ (failedObjective, error) -> {
+ errorCount++;
+ objectiveInError = failedObjective;
+ objectiveError = error;
+ });
+ break;
+ }
+ }
+
+ @Override
+ public void onSuccess(Objective objective) {
+ context.onSuccess(objective);
+ }
+
+ @Override
+ public void onError(Objective objective, ObjectiveError error) {
+ context.onError(objective, error);
+ }
+
+ int successCount() {
+ return successCount;
+ }
+
+ int errorCount() {
+ return errorCount;
+ }
+
+ ObjectiveError objectiveError() {
+ return objectiveError;
+ }
+
+ Objective objectiveInError() {
+ return objectiveInError;
+ }
+ }
+
+ /**
+ * Tests default objective context.
+ */
+ @Test
+ public void testDefaultContext() {
+ Objective objective = baseFilteringBuilder().add();
+ ObjectiveContextAdapter context;
+
+ context = new ObjectiveContextAdapter(ContextType.BOTH);
+ context.onSuccess(objective);
+ assertThat(context.successCount(), is(1));
+ assertThat(context.errorCount(), is(0));
+ assertThat(context.objectiveError(), is(ObjectiveError.UNKNOWN));
+ assertThat(context.objectiveInError(), nullValue());
+
+ context = new ObjectiveContextAdapter(ContextType.BOTH);
+ context.onError(objective, ObjectiveError.UNSUPPORTED);
+ assertThat(context.successCount(), is(0));
+ assertThat(context.errorCount(), is(1));
+ assertThat(context.objectiveError(), is(ObjectiveError.UNSUPPORTED));
+ assertThat(context.objectiveInError(), equalTo(objective));
+
+ context = new ObjectiveContextAdapter(ContextType.SUCCESS_ONLY);
+ context.onSuccess(objective);
+ assertThat(context.successCount(), is(1));
+ assertThat(context.errorCount(), is(0));
+ assertThat(context.objectiveError(), is(ObjectiveError.UNKNOWN));
+ assertThat(context.objectiveInError(), nullValue());
+
+ context = new ObjectiveContextAdapter(ContextType.ERROR_ONLY);
+ context.onError(objective, ObjectiveError.UNSUPPORTED);
+ assertThat(context.successCount(), is(0));
+ assertThat(context.errorCount(), is(1));
+ assertThat(context.objectiveError(), is(ObjectiveError.UNSUPPORTED));
+ assertThat(context.objectiveInError(), equalTo(objective));
+ }
+
}