Finalize Match/Action Objects - ONOS-1728
* Added equals() and hashCode() methods to objects with IDs
* Added/Updates Javadocs
* Implemented rudimentary execute and query methods for
Operations in the MatchActionModule. Operations are preserved
but not actually executed currently.
* Implemented factories for MatchActionOperationsId and
MatchActionOperations
* Added unit tests to check creation and execution of MatchAction
* Added unit tests for new Immutable classes
Change-Id: Id865d04fd1048d00e533736c95c3052148041d95
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java b/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
index 7c3a22d..24790b8 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchAction.java
@@ -8,7 +8,7 @@
import net.onrc.onos.core.util.SwitchPort;
/**
- * A filter and actions for traffic.
+ * A filter and actions for traffic. Objects of this class are immutable.
*/
public final class MatchAction implements BatchOperationTarget {
private final MatchActionId id;
@@ -66,4 +66,18 @@
public List<Action> getActions() {
return actions;
}
+
+ @Override
+ public int hashCode() {
+ return id.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof MatchAction) {
+ MatchAction other = (MatchAction) obj;
+ return (id.equals(other.id));
+ }
+ return false;
+ }
}
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
index 3e485af..b520439 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionId.java
@@ -2,9 +2,17 @@
import net.onrc.onos.api.batchoperation.BatchOperationTarget;
+/**
+ * A unique identifier for a MatchAction. Objects of this class are immutable.
+ */
public final class MatchActionId implements BatchOperationTarget {
private final String value;
+ /**
+ * Creates a new Match Action Identifier based on the given id string.
+ *
+ * @param id unique id string
+ */
public MatchActionId(String id) {
value = id;
}
@@ -23,7 +31,7 @@
public boolean equals(Object obj) {
if (obj instanceof MatchActionId) {
MatchActionId other = (MatchActionId) obj;
- return (this.value.equals(other.value));
+ return (value.equals(other.value));
}
return false;
}
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
index 557932e..9ed031c 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionModule.java
@@ -1,6 +1,9 @@
package net.onrc.onos.core.matchaction;
+import java.util.Collections;
import java.util.EventListener;
+import java.util.HashSet;
+import java.util.List;
import java.util.Set;
import net.onrc.onos.api.flowmanager.ConflictDetectionPolicy;
@@ -11,6 +14,19 @@
*/
public class MatchActionModule implements MatchActionService {
+ private final HashSet<MatchAction> currentOperations = new HashSet<>();
+
+ private boolean processMatchActionEntries(
+ final List<MatchActionOperationEntry> entries) {
+ int successfulOperations = 0;
+ for (final MatchActionOperationEntry entry : entries) {
+ if (currentOperations.add(entry.getTarget())) {
+ successfulOperations++;
+ }
+ }
+ return entries.size() == successfulOperations;
+ }
+
@Override
public boolean addMatchAction(MatchAction matchAction) {
return false;
@@ -18,14 +34,12 @@
@Override
public Set<MatchAction> getMatchActions() {
- // TODO Auto-generated method stub
- return null;
+ return Collections.unmodifiableSet(currentOperations);
}
@Override
public boolean executeOperations(final MatchActionOperations operations) {
- // TODO Auto-generated method stub
- return false;
+ return processMatchActionEntries(operations.getOperations());
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationEntry.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationEntry.java
index b8e150b..2513dd0 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationEntry.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationEntry.java
@@ -2,6 +2,10 @@
import net.onrc.onos.api.batchoperation.BatchOperationEntry;
+/**
+ * This class pairs an Operator and a Match Action to represent an executable
+ * Match Action operation.
+ */
public final class MatchActionOperationEntry
extends BatchOperationEntry<MatchActionOperations.Operator, MatchAction> {
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
index 3aaffad..a17d0f2 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperations.java
@@ -2,8 +2,18 @@
import net.onrc.onos.api.batchoperation.BatchOperation;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * The MatchActionOperations class holds a list of MatchActionOperationEntry
+ * objects to be executed together as one set.
+ * <p/>
+ * Objects of this class are immutable.
+ */
public final class MatchActionOperations
extends BatchOperation<MatchActionOperationEntry> {
+
+ private final MatchActionOperationsId id;
/**
* The MatchAction operators.
*/
@@ -12,5 +22,47 @@
REMOVE,
}
- // TODO waiting on updated BatchOperation as of 8/7
+ /**
+ * Constructs a MatchActionOperations object from an id. Internal
+ * constructor called by a public factory method.
+ *
+ * @param newId match action operations identifier for this instance
+ */
+ private MatchActionOperations(final MatchActionOperationsId newId) {
+ id = checkNotNull(newId);
+ }
+
+ /**
+ * Creates a MatchActionOperations object from an id.
+ *
+ * @param newId match action operations identifier to use for the new object
+ * @return Match Action Operations object
+ */
+ public static MatchActionOperations createMatchActionsOperations(
+ final MatchActionOperationsId newId) {
+ return new MatchActionOperations(newId);
+ }
+
+ /**
+ * Gets the identifier for the Match Action Operations object.
+ *
+ * @return identifier for the Opertions object
+ */
+ public MatchActionOperationsId getOperationsId() {
+ return id;
+ }
+
+ @Override
+ public int hashCode() {
+ return id.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof MatchActionOperations) {
+ final MatchActionOperations other = (MatchActionOperations) obj;
+ return (id.equals(other.id));
+ }
+ return false;
+ }
}
diff --git a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
index 8ea2168..7c9d1db 100644
--- a/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
+++ b/src/main/java/net/onrc/onos/core/matchaction/MatchActionOperationsId.java
@@ -1,5 +1,62 @@
package net.onrc.onos.core.matchaction;
-public interface MatchActionOperationsId {
- // TODO waiting on MatchActionOperations
+import java.util.UUID;
+
+/**
+ * Identifier for a MatchActionOperations object. This is an immutable class
+ * that encapsulates a globally unique identifier for the MatchActionOperations
+ * object.
+ */
+public final class MatchActionOperationsId {
+
+ private static final String OPERATIONS_ID_PREFIX = "MatchActionOperationsId-";
+ private final String id;
+
+ /**
+ * Constructs an Operations identifier and allocates a unique identifier
+ * for it.
+ */
+ private MatchActionOperationsId() {
+ id = OPERATIONS_ID_PREFIX + UUID.randomUUID();
+ }
+
+ /**
+ * Gets the identifier for the Operations object.
+ *
+ * @return Operations object identifier as a string
+ */
+ public String getId() {
+ return id;
+ }
+
+ @Override
+ public boolean equals(final Object other) {
+ if (other == this) {
+ return true;
+ }
+
+ if (!(other instanceof MatchActionOperationsId)) {
+ return false;
+ }
+
+ final MatchActionOperationsId otherMatchActionOperationsId =
+ (MatchActionOperationsId) other;
+
+ return otherMatchActionOperationsId.getId().equals(getId());
+ }
+
+ @Override
+ public int hashCode() {
+ return id.hashCode();
+ }
+
+ /**
+ * Creates a new Id for a MatchActionOperation.
+ *
+ * @return new Id for a MatchActionOperation
+ */
+ public static MatchActionOperationsId createNewOperationsId() {
+ return new MatchActionOperationsId();
+ }
+
}
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java
new file mode 100644
index 0000000..0a4e8d3
--- /dev/null
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionModuleTest.java
@@ -0,0 +1,137 @@
+package net.onrc.onos.core.matchaction;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Set;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+/**
+ * Unit tests for the MatchActionModule.
+ */
+public class MatchActionModuleTest {
+
+ /**
+ * Tests that MatchAction objects added by the executeOperations()
+ * method are properly returned by the getMatchActions() method.
+ */
+ @Test
+ public void testMatchActionModuleGlobalEntriesSet() {
+
+ final int iterations = 5;
+ final MatchActionModule module = new MatchActionModule();
+ final ArrayList<MatchAction> generatedMatchActions = new ArrayList<>();
+
+ // Add some test MatchAction objects. 25 will be added, in 5 blocks
+ // of 5.
+ for (int operationsIteration = 1;
+ operationsIteration <= iterations;
+ operationsIteration++) {
+ final MatchActionOperationsId id =
+ MatchActionOperationsId.createNewOperationsId();
+ assertThat(id, is(notNullValue()));
+ final MatchActionOperations operations =
+ MatchActionOperations.createMatchActionsOperations(id);
+ assertThat(operations, is(notNullValue()));
+
+ for (int entriesIteration = 1;
+ entriesIteration <= iterations;
+ entriesIteration++) {
+
+ final String entryId = "MA" +
+ Integer.toString(operationsIteration) +
+ Integer.toString(entriesIteration);
+ final MatchAction matchAction =
+ new MatchAction(entryId, null, null, null);
+ final MatchActionOperationEntry entry =
+ new MatchActionOperationEntry(MatchActionOperations.Operator.ADD,
+ matchAction);
+ operations.addOperation(entry);
+ generatedMatchActions.add(matchAction);
+ }
+
+ // Add the MatchActions generated by this iteration
+ final boolean result = module.executeOperations(operations);
+ assertThat(result, is(true));
+ }
+
+ // Get the list of generated MatchAction objects and make sure its
+ // length is correct.
+ final int generatedCount = generatedMatchActions.size();
+ final Set<MatchAction> matchActions = module.getMatchActions();
+ assertThat(matchActions, hasSize(generatedCount));
+
+ // Make sure that all the created items are in the list
+ final MatchAction[] generatedArray =
+ generatedMatchActions.toArray(new MatchAction[generatedCount]);
+ assertThat(matchActions, containsInAnyOrder(generatedArray));
+
+ // Make sure that the returned list cannot be modified
+ Throwable errorThrown = null;
+ try {
+ matchActions.add(new MatchAction("", null, null, null));
+ } catch (UnsupportedOperationException e) {
+ errorThrown = e;
+ }
+ assertThat(errorThrown, is(notNullValue()));
+ }
+
+ /**
+ * Tests that adding a duplicate MatchAction via executeOperations()
+ * returns an error.
+ */
+ @Test
+ public void testAddingDuplicateMatchAction() {
+
+ // Create two MatchAction objects using the same ID
+ final MatchAction matchAction =
+ new MatchAction("ID", null, null, null);
+ final MatchAction duplicateMatchAction =
+ new MatchAction("ID", null, null, null);
+
+ // create Operation Entries for the two MatchAction objects
+ final MatchActionOperationEntry entry =
+ new MatchActionOperationEntry(MatchActionOperations.Operator.ADD,
+ matchAction);
+ final MatchActionOperationEntry duplicateEntry =
+ new MatchActionOperationEntry(MatchActionOperations.Operator.ADD,
+ duplicateMatchAction);
+
+ // Create an Operations object to execute the first MatchAction
+ final MatchActionOperationsId id =
+ MatchActionOperationsId.createNewOperationsId();
+ assertThat(id, is(notNullValue()));
+ final MatchActionOperations operations =
+ MatchActionOperations.createMatchActionsOperations(id);
+ operations.addOperation(entry);
+
+ // Create a module to use to execute the Operations.
+ final MatchActionModule module = new MatchActionModule();
+
+ // Execute the first set of Operations. This
+ // should succeed.
+ final boolean result = module.executeOperations(operations);
+ assertThat(result, is(true));
+
+ // Now add the duplicate entry. This should fail.
+ final MatchActionOperationsId idForDuplicate =
+ MatchActionOperationsId.createNewOperationsId();
+ assertThat(idForDuplicate, is(notNullValue()));
+ final MatchActionOperations operationsForDuplicate =
+ MatchActionOperations.createMatchActionsOperations(idForDuplicate);
+ operationsForDuplicate.addOperation(duplicateEntry);
+
+ final boolean resultForDuplicate =
+ module.executeOperations(operationsForDuplicate);
+ assertThat(resultForDuplicate, is(false));
+
+ // Now add the original entry again. This should fail.
+ final boolean resultForAddAgain = module.executeOperations(operations);
+ assertThat(resultForAddAgain, is(false));
+ }
+}
diff --git a/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsTest.java b/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsTest.java
new file mode 100644
index 0000000..fd96de4
--- /dev/null
+++ b/src/test/java/net/onrc/onos/core/matchaction/MatchActionOperationsTest.java
@@ -0,0 +1,79 @@
+package net.onrc.onos.core.matchaction;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+import net.onrc.onos.core.util.TestUtils;
+import org.junit.Test;
+
+/**
+ * Unit tests for Match Action Operations.
+ */
+public class MatchActionOperationsTest {
+
+ /**
+ * Test that creation of MatchActionOperations objects is correct and that
+ * the objects have unique identifiers.
+ */
+ @Test
+ public void testMatchActionoperationsCreate() {
+ final MatchActionOperationsId id1 =
+ MatchActionOperationsId.createNewOperationsId();
+ final MatchActionOperations operations1 =
+ MatchActionOperations.createMatchActionsOperations(id1);
+ assertThat(id1, is(notNullValue()));
+ assertThat(id1, is(equalTo(operations1.getOperationsId())));
+
+ final MatchActionOperationsId id2 =
+ MatchActionOperationsId.createNewOperationsId();
+ final MatchActionOperations operations2 =
+ MatchActionOperations.createMatchActionsOperations(id2);
+ assertThat(id2, is(notNullValue()));
+ assertThat(id2, is(equalTo(operations2.getOperationsId())));
+
+ assertThat(id1.getId(), is(not(equalTo(id2.getId()))));
+ }
+
+ /**
+ * Test the correctness of the equals() operation for
+ * MatchActionOperationsId objects.
+ * objects.
+ */
+ @Test
+ public void testMatchActionOperationsIdEquals() {
+ final MatchActionOperationsId id1 =
+ MatchActionOperationsId.createNewOperationsId();
+ final MatchActionOperationsId id2 =
+ MatchActionOperationsId.createNewOperationsId();
+ final MatchActionOperationsId id1Copy =
+ MatchActionOperationsId.createNewOperationsId();
+
+
+ // Check that null does not match
+ assertThat(id1, is(not(equalTo(null))));
+
+ // Check that different objects do not match
+ assertThat(id1, is(not(equalTo(id2))));
+
+ // Check that copies match
+ TestUtils.setField(id1Copy, "id", id1.getId());
+ assertThat(id1, is(equalTo(id1Copy)));
+
+ // Check that the same object matches
+ assertThat(id1, is(equalTo(id1Copy)));
+ }
+
+ /**
+ * Test the correctness of the hashCode() operation for
+ * MatchActionOperationsId objects.
+ */
+ @Test
+ public void testMatchActionOperationsIdHashCode() {
+ final MatchActionOperationsId id1 =
+ MatchActionOperationsId.createNewOperationsId();
+ assertThat(id1.hashCode(), is(equalTo(id1.getId().hashCode())));
+ }
+}
diff --git a/src/test/java/net/onrc/onos/core/matchaction/TestImmutableClasses.java b/src/test/java/net/onrc/onos/core/matchaction/TestImmutableClasses.java
index f1e0857..bd7364b 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/TestImmutableClasses.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/TestImmutableClasses.java
@@ -8,23 +8,43 @@
*/
public class TestImmutableClasses {
+ /**
+ * MatchAction objects should be immutable.
+ */
@Test
public void checkMatchActionImmutable() {
ImmutableClassChecker.assertThatClassIsImmutable(MatchAction.class);
}
+ /**
+ * MatchActionOperationEntry objects should be immutable.
+ */
@Test
public void checkMatchActionOperationEntryImmutable() {
ImmutableClassChecker.assertThatClassIsImmutable(MatchActionOperationEntry.class);
}
+ /**
+ * MatchActionId objects should be immutable.
+ */
@Test
public void checkMatchActionId() {
ImmutableClassChecker.assertThatClassIsImmutable(MatchActionId.class);
}
+ /**
+ * MatchActionOperations objects should be immutable.
+ */
@Test
public void checkMatchActionOperations() {
ImmutableClassChecker.assertThatClassIsImmutable(MatchActionOperations.class);
}
+
+ /**
+ * MatchActionOperationsId objects should be immutable.
+ */
+ @Test
+ public void checkMatchActionOperationsId() {
+ ImmutableClassChecker.assertThatClassIsImmutable(MatchActionOperationsId.class);
+ }
}
diff --git a/src/test/java/net/onrc/onos/core/matchaction/TestOperationsCreation.java b/src/test/java/net/onrc/onos/core/matchaction/TestOperationsCreation.java
index 9536848..fc33f14 100644
--- a/src/test/java/net/onrc/onos/core/matchaction/TestOperationsCreation.java
+++ b/src/test/java/net/onrc/onos/core/matchaction/TestOperationsCreation.java
@@ -22,11 +22,14 @@
@Test
public void testOperationsCreation() {
// Create the MatchActionOperations
- final MatchActionOperations operations = new MatchActionOperations();
+ final MatchActionOperationsId operationsId =
+ MatchActionOperationsId.createNewOperationsId();
+ final MatchActionOperations operations =
+ MatchActionOperations.createMatchActionsOperations(operationsId);
// Create one MatchActionEntry and add it to the Operations
- final String id1 = "MA1";
- final MatchAction action1 = new MatchAction(id1, null, null, null);
+ final String matchActionId1 = "MA1";
+ final MatchAction action1 = new MatchAction(matchActionId1, null, null, null);
final MatchActionOperationEntry entry1 =
new MatchActionOperationEntry(MatchActionOperations.Operator.ADD, action1);
@@ -44,7 +47,7 @@
assertThat(loadedEntry1, is(notNullValue()));
final MatchAction loadedAction1 = loadedEntry1.getTarget();
- assertThat(loadedAction1.getId().toString(), is(equalTo(id1)));
+ assertThat(loadedAction1.getId().toString(), is(equalTo(matchActionId1)));
final MatchActionOperations.Operator loadedOperator1 = loadedEntry1.getOperator();
assertThat(loadedOperator1, is(equalTo(MatchActionOperations.Operator.ADD)));