Coverage and SONAR improvements for Objectives classes
- cleaned up constructors to take a builder rather
than a long list of parameters
- improved coverage of unit tests
- added missing APIs to builder interfaces
Change-Id: I4c4eac302d41f785d401f21e9935bc659ca5f892
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultFilteringObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultFilteringObjective.java
index e5589b4..7b5924f 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultFilteringObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultFilteringObjective.java
@@ -47,39 +47,19 @@
private final Operation op;
private final Optional<ObjectiveContext> context;
- private DefaultFilteringObjective(Type type, boolean permanent, int timeout,
- ApplicationId appId, int priority, Criterion key,
- List<Criterion> conditions, Operation op) {
- this.key = key;
- this.type = type;
- this.permanent = permanent;
- this.timeout = timeout;
- this.appId = appId;
- this.priority = priority;
- this.conditions = conditions;
- this.op = op;
- this.context = Optional.empty();
+ private DefaultFilteringObjective(Builder builder) {
+ this.key = builder.key;
+ this.type = builder.type;
+ this.permanent = builder.permanent;
+ this.timeout = builder.timeout;
+ this.appId = builder.appId;
+ this.priority = builder.priority;
+ this.conditions = builder.conditions;
+ this.op = builder.op;
+ this.context = Optional.ofNullable(builder.context);
this.id = Objects.hash(type, key, conditions, permanent,
- timeout, appId, priority);
- }
-
- public DefaultFilteringObjective(Type type, boolean permanent, int timeout,
- ApplicationId appId, int priority, Criterion key,
- List<Criterion> conditions,
- ObjectiveContext context, Operation op) {
- this.key = key;
- this.type = type;
- this.permanent = permanent;
- this.timeout = timeout;
- this.appId = appId;
- this.priority = priority;
- this.conditions = conditions;
- this.op = op;
- this.context = Optional.ofNullable(context);
-
- this.id = Objects.hash(type, key, conditions, permanent,
- timeout, appId, priority);
+ timeout, appId, priority);
}
@Override
@@ -152,6 +132,9 @@
private ApplicationId appId;
private int priority = DEFAULT_PRIORITY;
private Criterion key = Criteria.dummy();
+ private List<Criterion> conditions;
+ private Operation op;
+ private ObjectiveContext context;
@Override
public Builder withKey(Criterion key) {
@@ -204,54 +187,50 @@
@Override
public FilteringObjective add() {
- List<Criterion> conditions = listBuilder.build();
+ conditions = listBuilder.build();
+ op = Operation.ADD;
checkNotNull(type, "Must have a type.");
checkArgument(!conditions.isEmpty(), "Must have at least one condition.");
checkNotNull(appId, "Must supply an application id");
- return new DefaultFilteringObjective(type, permanent, timeout,
- appId, priority, key, conditions,
- Operation.ADD);
+ return new DefaultFilteringObjective(this);
}
@Override
public FilteringObjective remove() {
- List<Criterion> conditions = listBuilder.build();
+ conditions = listBuilder.build();
checkNotNull(type, "Must have a type.");
checkArgument(!conditions.isEmpty(), "Must have at least one condition.");
checkNotNull(appId, "Must supply an application id");
+ op = Operation.REMOVE;
-
- return new DefaultFilteringObjective(type, permanent, timeout,
- appId, priority, key, conditions,
- Operation.REMOVE);
+ return new DefaultFilteringObjective(this);
}
@Override
public FilteringObjective add(ObjectiveContext context) {
- List<Criterion> conditions = listBuilder.build();
+ conditions = listBuilder.build();
checkNotNull(type, "Must have a type.");
checkArgument(!conditions.isEmpty(), "Must have at least one condition.");
checkNotNull(appId, "Must supply an application id");
+ op = Operation.ADD;
+ this.context = context;
- return new DefaultFilteringObjective(type, permanent, timeout,
- appId, priority, key, conditions,
- context, Operation.ADD);
+ return new DefaultFilteringObjective(this);
}
@Override
public FilteringObjective remove(ObjectiveContext context) {
- List<Criterion> conditions = listBuilder.build();
+ conditions = listBuilder.build();
checkNotNull(type, "Must have a type.");
checkArgument(!conditions.isEmpty(), "Must have at least one condition.");
checkNotNull(appId, "Must supply an application id");
+ op = Operation.REMOVE;
+ this.context = context;
-
- return new DefaultFilteringObjective(type, permanent, timeout,
- appId, priority, key, conditions,
- context, Operation.REMOVE);
+ return new DefaultFilteringObjective(this);
}
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultForwardingObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultForwardingObjective.java
index ae47de1..0abf5ab 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultForwardingObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultForwardingObjective.java
@@ -45,47 +45,21 @@
private final int id;
- private DefaultForwardingObjective(TrafficSelector selector,
- Flag flag, boolean permanent,
- int timeout, ApplicationId appId,
- int priority, Integer nextId,
- TrafficTreatment treatment, Operation op) {
- this.selector = selector;
- this.flag = flag;
- this.permanent = permanent;
- this.timeout = timeout;
- this.appId = appId;
- this.priority = priority;
- this.nextId = nextId;
- this.treatment = treatment;
- this.op = op;
- this.context = Optional.empty();
+ private DefaultForwardingObjective(Builder builder) {
+ this.selector = builder.selector;
+ this.flag = builder.flag;
+ this.permanent = builder.permanent;
+ this.timeout = builder.timeout;
+ this.appId = builder.appId;
+ this.priority = builder.priority;
+ this.nextId = builder.nextId;
+ this.treatment = builder.treatment;
+ this.op = builder.op;
+ this.context = Optional.ofNullable(builder.context);
this.id = Objects.hash(selector, flag, permanent,
- timeout, appId, priority, nextId,
- treatment, op);
- }
-
- private DefaultForwardingObjective(TrafficSelector selector,
- Flag flag, boolean permanent,
- int timeout, ApplicationId appId,
- int priority, Integer nextId,
- TrafficTreatment treatment,
- ObjectiveContext context, Operation op) {
- this.selector = selector;
- this.flag = flag;
- this.permanent = permanent;
- this.timeout = timeout;
- this.appId = appId;
- this.priority = priority;
- this.nextId = nextId;
- this.treatment = treatment;
- this.op = op;
- this.context = Optional.ofNullable(context);
-
- this.id = Objects.hash(selector, flag, permanent,
- timeout, appId, priority, nextId,
- treatment, op);
+ timeout, appId, priority, nextId,
+ treatment, op);
}
@@ -164,6 +138,8 @@
private ApplicationId appId;
private Integer nextId;
private TrafficTreatment treatment;
+ private Operation op;
+ private ObjectiveContext context;
@Override
public Builder withSelector(TrafficSelector selector) {
@@ -221,9 +197,8 @@
checkArgument(nextId != null || treatment != null, "Must supply at " +
"least a treatment and/or a nextId");
checkNotNull(appId, "Must supply an application id");
- return new DefaultForwardingObjective(selector, flag, permanent,
- timeout, appId, priority,
- nextId, treatment, Operation.ADD);
+ op = Operation.ADD;
+ return new DefaultForwardingObjective(this);
}
@Override
@@ -233,9 +208,8 @@
checkArgument(nextId != null || treatment != null, "Must supply at " +
"least a treatment and/or a nextId");
checkNotNull(appId, "Must supply an application id");
- return new DefaultForwardingObjective(selector, flag, permanent,
- timeout, appId, priority,
- nextId, treatment, Operation.REMOVE);
+ op = Operation.REMOVE;
+ return new DefaultForwardingObjective(this);
}
@Override
@@ -245,10 +219,10 @@
checkArgument(nextId != null || treatment != null, "Must supply at " +
"least a treatment and/or a nextId");
checkNotNull(appId, "Must supply an application id");
- return new DefaultForwardingObjective(selector, flag, permanent,
- timeout, appId, priority,
- nextId, treatment,
- context, Operation.ADD);
+ op = Operation.ADD;
+ this.context = context;
+
+ return new DefaultForwardingObjective(this);
}
@Override
@@ -258,10 +232,10 @@
checkArgument(nextId != null || treatment != null, "Must supply at " +
"least a treatment and/or a nextId");
checkNotNull(appId, "Must supply an application id");
- return new DefaultForwardingObjective(selector, flag, permanent,
- timeout, appId, priority,
- nextId, treatment,
- context, Operation.REMOVE);
+ op = Operation.REMOVE;
+ this.context = context;
+
+ return new DefaultForwardingObjective(this);
}
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultNextObjective.java b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultNextObjective.java
index 245fd57..20e8929 100644
--- a/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultNextObjective.java
+++ b/core/api/src/main/java/org/onosproject/net/flowobjective/DefaultNextObjective.java
@@ -40,25 +40,13 @@
private final Operation op;
private final Optional<ObjectiveContext> context;
- private DefaultNextObjective(Integer id, List<TrafficTreatment> treatments,
- ApplicationId appId, Type type, Operation op) {
- this.treatments = treatments;
- this.appId = appId;
- this.type = type;
- this.id = id;
- this.op = op;
- this.context = Optional.empty();
- }
-
- private DefaultNextObjective(Integer id, List<TrafficTreatment> treatments,
- ApplicationId appId, ObjectiveContext context,
- Type type, Operation op) {
- this.treatments = treatments;
- this.appId = appId;
- this.type = type;
- this.id = id;
- this.op = op;
- this.context = Optional.ofNullable(context);
+ private DefaultNextObjective(Builder builder) {
+ this.treatments = builder.treatments;
+ this.appId = builder.appId;
+ this.type = builder.type;
+ this.id = builder.id;
+ this.op = builder.op;
+ this.context = Optional.ofNullable(builder.context);
}
@Override
@@ -120,12 +108,15 @@
private ApplicationId appId;
private Type type;
private Integer id;
+ private List<TrafficTreatment> treatments;
+ private Operation op;
+ private ObjectiveContext context;
private final ImmutableList.Builder<TrafficTreatment> listBuilder
= ImmutableList.builder();
@Override
- public NextObjective.Builder withId(int nextId) {
+ public Builder withId(int nextId) {
this.id = nextId;
return this;
}
@@ -164,7 +155,7 @@
}
@Override
- public NextObjective.Builder fromApp(ApplicationId appId) {
+ public Builder fromApp(ApplicationId appId) {
this.appId = appId;
return this;
}
@@ -182,46 +173,50 @@
@Override
public NextObjective add() {
- List<TrafficTreatment> treatments = listBuilder.build();
+ treatments = listBuilder.build();
+ op = Operation.ADD;
checkNotNull(appId, "Must supply an application id");
checkNotNull(id, "id cannot be null");
checkNotNull(type, "The type cannot be null");
checkArgument(!treatments.isEmpty(), "Must have at least one treatment");
- return new DefaultNextObjective(id, treatments, appId, type, Operation.ADD);
+ return new DefaultNextObjective(this);
}
@Override
public NextObjective remove() {
- List<TrafficTreatment> treatments = listBuilder.build();
+ treatments = listBuilder.build();
+ op = Operation.REMOVE;
checkNotNull(appId, "Must supply an application id");
checkNotNull(id, "id cannot be null");
checkNotNull(type, "The type cannot be null");
- return new DefaultNextObjective(id, treatments, appId, type, Operation.REMOVE);
+ return new DefaultNextObjective(this);
}
@Override
public NextObjective add(ObjectiveContext context) {
- List<TrafficTreatment> treatments = listBuilder.build();
+ treatments = listBuilder.build();
+ op = Operation.ADD;
+ this.context = context;
checkNotNull(appId, "Must supply an application id");
checkNotNull(id, "id cannot be null");
checkNotNull(type, "The type cannot be null");
checkArgument(!treatments.isEmpty(), "Must have at least one treatment");
- return new DefaultNextObjective(id, treatments, appId,
- context, type, Operation.ADD);
+ return new DefaultNextObjective(this);
}
@Override
public NextObjective remove(ObjectiveContext context) {
- List<TrafficTreatment> treatments = listBuilder.build();
+ treatments = listBuilder.build();
+ op = Operation.REMOVE;
+ this.context = context;
checkNotNull(appId, "Must supply an application id");
checkNotNull(id, "id cannot be null");
checkNotNull(type, "The type cannot be null");
- return new DefaultNextObjective(id, treatments, appId,
- context, type, Operation.REMOVE);
+ return new DefaultNextObjective(this);
}
}
}
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 9a90cff..1350d7a 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
@@ -112,10 +112,25 @@
*/
Builder addTreatment(TrafficTreatment treatment);
+ /**
+ * Specifies the application which applied the filter.
+ *
+ * @param appId an application id
+ * @return an objective 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);
+
+ /**
* Builds the next objective that will be added.
*
* @return a next objective