Updates related to SDN-IP submitting intent operations:
* Updated IntentSynchronizer.submitPeerIntents() to use intent batch
operation
* Refactored the SDN-IP unit tests:
The verification for intent service operations is moved to a new class
TestIntentServiceHeler
Change-Id: Id6766ebb4d6bf492e8f9787c2c2247ac38d42964
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
index 530e6d3..332ff0b 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
@@ -210,10 +210,11 @@
// Push the intents
if (isElectedLeader && isActivatedLeader) {
log.debug("Submitting all SDN-IP Peer Intents...");
- // TODO: We should use a single Intent batch operation
+ IntentOperations.Builder builder = IntentOperations.builder();
for (Intent intent : intents) {
- intentService.submit(intent);
+ builder.addSubmitOperation(intent);
}
+ intentService.execute(builder.build());
}
}
}
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
index 3d9cd23..0c808ec 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
@@ -5,7 +5,6 @@
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reportMatcher;
import static org.easymock.EasyMock.reset;
import static org.easymock.EasyMock.verify;
import static org.hamcrest.Matchers.is;
@@ -15,13 +14,9 @@
import static org.junit.Assert.assertTrue;
import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import org.apache.commons.collections4.CollectionUtils;
-import org.easymock.IArgumentMatcher;
import org.junit.Before;
import org.junit.Test;
import org.onlab.junit.TestUtils;
@@ -42,14 +37,11 @@
import org.onlab.onos.net.host.HostService;
import org.onlab.onos.net.host.InterfaceIpAddress;
import org.onlab.onos.net.intent.Intent;
-import org.onlab.onos.net.intent.IntentId;
-import org.onlab.onos.net.intent.IntentOperation;
import org.onlab.onos.net.intent.IntentOperations;
import org.onlab.onos.net.intent.IntentService;
import org.onlab.onos.net.intent.IntentState;
import org.onlab.onos.net.intent.MultiPointToSinglePointIntent;
import org.onlab.onos.net.provider.ProviderId;
-import org.onlab.onos.sdnip.IntentSynchronizer.IntentKey;
import org.onlab.onos.sdnip.config.Interface;
import org.onlab.packet.Ethernet;
import org.onlab.packet.IpAddress;
@@ -334,14 +326,16 @@
IntentOperations.Builder builder = IntentOperations.builder();
builder.addWithdrawOperation(intent2.id());
builder.addWithdrawOperation(intent4.id());
- intentService.execute(eqExceptId(builder.build()));
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
builder = IntentOperations.builder();
builder.addSubmitOperation(intent3);
builder.addSubmitOperation(intent4Update);
builder.addSubmitOperation(intent6);
builder.addSubmitOperation(intent7);
- intentService.execute(eqExceptId(builder.build()));
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
// Start the test
@@ -427,129 +421,4 @@
"ingressPoints", intent.ingressPoints());
return intentNew;
}
-
- /*
- * EasyMock matcher that matches {@link IntenOperations} but
- * ignores the {@link IntentId} when matching.
- * <p/>
- * The normal intent equals method tests that the intent IDs are equal,
- * however in these tests we can't know what the intent IDs will be in
- * advance, so we can't set up expected intents with the correct IDs. Thus,
- * the solution is to use an EasyMock matcher that verifies that all the
- * value properties of the provided intent match the expected values, but
- * ignores the intent ID when testing equality.
- */
- private static final class IdAgnosticIntentOperationsMatcher implements
- IArgumentMatcher {
-
- private final IntentOperations intentOperations;
- private String providedString;
-
- /**
- * Constructor taking the expected intent operations to match against.
- *
- * @param intentOperations the expected intent operations
- */
- public IdAgnosticIntentOperationsMatcher(
- IntentOperations intentOperations) {
- this.intentOperations = intentOperations;
- }
-
- @Override
- public void appendTo(StringBuffer strBuffer) {
- strBuffer.append("IntentOperationsMatcher unable to match: "
- + providedString);
- }
-
- @Override
- public boolean matches(Object object) {
- if (!(object instanceof IntentOperations)) {
- return false;
- }
-
- IntentOperations providedIntentOperations =
- (IntentOperations) object;
- providedString = providedIntentOperations.toString();
-
- List<IntentKey> thisSubmitIntents = new LinkedList<>();
- List<IntentId> thisWithdrawIntentIds = new LinkedList<>();
- List<IntentKey> thisReplaceIntents = new LinkedList<>();
- List<IntentKey> thisUpdateIntents = new LinkedList<>();
- List<IntentKey> providedSubmitIntents = new LinkedList<>();
- List<IntentId> providedWithdrawIntentIds = new LinkedList<>();
- List<IntentKey> providedReplaceIntents = new LinkedList<>();
- List<IntentKey> providedUpdateIntents = new LinkedList<>();
-
- extractIntents(intentOperations, thisSubmitIntents,
- thisWithdrawIntentIds, thisReplaceIntents,
- thisUpdateIntents);
- extractIntents(providedIntentOperations, providedSubmitIntents,
- providedWithdrawIntentIds, providedReplaceIntents,
- providedUpdateIntents);
-
- return CollectionUtils.isEqualCollection(thisSubmitIntents,
- providedSubmitIntents) &&
- CollectionUtils.isEqualCollection(thisWithdrawIntentIds,
- providedWithdrawIntentIds) &&
- CollectionUtils.isEqualCollection(thisUpdateIntents,
- providedUpdateIntents) &&
- CollectionUtils.isEqualCollection(thisReplaceIntents,
- providedReplaceIntents);
- }
-
- /**
- * Extracts the intents per operation type. Each intent is encapsulated
- * in IntentKey so it can be compared by excluding the Intent ID.
- *
- * @param intentOperations the container with the intent operations
- * to extract the intents from
- * @param submitIntents the SUBMIT intents
- * @param withdrawIntentIds the WITHDRAW intents IDs
- * @param replaceIntents the REPLACE intents
- * @param updateIntents the UPDATE intens
- */
- private void extractIntents(IntentOperations intentOperations,
- List<IntentKey> submitIntents,
- List<IntentId> withdrawIntentIds,
- List<IntentKey> replaceIntents,
- List<IntentKey> updateIntents) {
- for (IntentOperation oper : intentOperations.operations()) {
- IntentId intentId;
- IntentKey intentKey;
- switch (oper.type()) {
- case SUBMIT:
- intentKey = new IntentKey(oper.intent());
- submitIntents.add(intentKey);
- break;
- case WITHDRAW:
- intentId = oper.intentId();
- withdrawIntentIds.add(intentId);
- break;
- case REPLACE:
- intentKey = new IntentKey(oper.intent());
- replaceIntents.add(intentKey);
- break;
- case UPDATE:
- intentKey = new IntentKey(oper.intent());
- updateIntents.add(intentKey);
- break;
- default:
- break;
- }
- }
- }
- }
-
- /**
- * Matcher method to set an expected intent to match against (ignoring the
- * the intent ID).
- *
- * @param intent the expected intent
- * @return something of type IntentOperations
- */
- private static IntentOperations eqExceptId(
- IntentOperations intentOperations) {
- reportMatcher(new IdAgnosticIntentOperationsMatcher(intentOperations));
- return intentOperations;
- }
}
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
index b7cdbc0..362221c 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
@@ -16,7 +16,6 @@
package org.onlab.onos.sdnip;
import com.google.common.collect.Sets;
-import org.easymock.IArgumentMatcher;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
@@ -31,6 +30,8 @@
import org.onlab.onos.net.flow.TrafficSelector;
import org.onlab.onos.net.flow.TrafficTreatment;
import org.onlab.onos.net.host.InterfaceIpAddress;
+import org.onlab.onos.net.intent.Intent;
+import org.onlab.onos.net.intent.IntentOperations;
import org.onlab.onos.net.intent.IntentService;
import org.onlab.onos.net.intent.PointToPointIntent;
import org.onlab.onos.sdnip.bgp.BgpConstants;
@@ -550,69 +551,6 @@
configInfoService, interfaceService);
}
- /*
- * EasyMock matcher that matches {@link PointToPointIntent}s but
- * ignores the {@link IntentId} when matching.
- * <p/>
- * The normal intent equals method tests that the intent IDs are equal,
- * however in these tests we can't know what the intent IDs will be in
- * advance, so we can't set up expected intents with the correct IDs. Thus,
- * the solution is to use an EasyMock matcher that verifies that all the
- * value properties of the provided intent match the expected values, but
- * ignores the intent ID when testing equality.
- */
- private static final class IdAgnosticPointToPointIntentMatcher implements
- IArgumentMatcher {
-
- private final PointToPointIntent intent;
- private String providedIntentString;
-
- /**
- * Constructor taking the expected intent to match against.
- *
- * @param intent the expected intent
- */
- public IdAgnosticPointToPointIntentMatcher(PointToPointIntent intent) {
- this.intent = intent;
- }
-
- @Override
- public void appendTo(StringBuffer strBuffer) {
- strBuffer.append("PointToPointIntentMatcher unable to match: "
- + providedIntentString);
- }
-
- @Override
- public boolean matches(Object object) {
- if (!(object instanceof PointToPointIntent)) {
- return false;
- }
-
- PointToPointIntent providedIntent = (PointToPointIntent) object;
- providedIntentString = providedIntent.toString();
-
- PointToPointIntent matchIntent =
- new PointToPointIntent(providedIntent.appId(),
- intent.selector(), intent.treatment(),
- intent.ingressPoint(), intent.egressPoint());
-
- return matchIntent.equals(providedIntent);
- }
- }
-
- /**
- * Matcher method to set an expected intent to match against (ignoring the
- * the intent ID).
- *
- * @param intent the expected intent
- * @return something of type PointToPointIntent
- */
- private static PointToPointIntent eqExceptId(
- PointToPointIntent intent) {
- reportMatcher(new IdAgnosticPointToPointIntentMatcher(intent));
- return null;
- }
-
/**
* Tests whether peer connectivity manager can set up correct BGP and
* ICMP intents according to specific configuration.
@@ -625,11 +563,13 @@
reset(intentService);
- // Sets up the expected PointToPoint intents.
- for (int i = 0; i < intentList.size(); i++) {
- intentService.submit(eqExceptId(intentList.get(i)));
+ // Setup the expected intents
+ IntentOperations.Builder builder = IntentOperations.builder();
+ for (Intent intent : intentList) {
+ builder.addSubmitOperation(intent);
}
-
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
// Running the interface to be tested.
@@ -659,6 +599,9 @@
replay(configInfoService);
reset(intentService);
+ IntentOperations.Builder builder = IntentOperations.builder();
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
peerConnectivityManager.start();
verify(intentService);
@@ -682,6 +625,9 @@
replay(configInfoService);
reset(intentService);
+ IntentOperations.Builder builder = IntentOperations.builder();
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
peerConnectivityManager.start();
verify(intentService);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/TestIntentServiceHelper.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/TestIntentServiceHelper.java
new file mode 100644
index 0000000..4cc9053
--- /dev/null
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/TestIntentServiceHelper.java
@@ -0,0 +1,213 @@
+package org.onlab.onos.sdnip;
+
+import java.util.LinkedList;
+import java.util.List;
+
+import org.apache.commons.collections4.CollectionUtils;
+import org.easymock.IArgumentMatcher;
+import org.onlab.onos.net.intent.IntentId;
+import org.onlab.onos.net.intent.IntentOperation;
+import org.onlab.onos.net.intent.IntentOperations;
+import org.onlab.onos.net.intent.PointToPointIntent;
+import org.onlab.onos.sdnip.IntentSynchronizer.IntentKey;
+
+import static org.easymock.EasyMock.reportMatcher;
+
+/**
+ * Helper class for testing operations submitted to the IntentService.
+ */
+public final class TestIntentServiceHelper {
+ /**
+ * Default constructor to prevent instantiation.
+ */
+ private TestIntentServiceHelper() {
+ }
+
+ /**
+ * Matcher method to set the expected intent operations to match against
+ * (ignoring the intent ID for each intent).
+ *
+ * @param intentOperations the expected Intent Operations
+ * @return the submitted Intent Operations
+ */
+ static IntentOperations eqExceptId(IntentOperations intentOperations) {
+ reportMatcher(new IdAgnosticIntentOperationsMatcher(intentOperations));
+ return intentOperations;
+ }
+
+
+ /**
+ * Matcher method to set an expected point-to-point intent to match
+ * against (ignoring the intent ID).
+ *
+ * @param intent the expected point-to-point intent
+ * @return the submitted point-to-point intent
+ */
+ private static PointToPointIntent eqExceptId(
+ PointToPointIntent intent) {
+ reportMatcher(new IdAgnosticPointToPointIntentMatcher(intent));
+ return intent;
+ }
+
+ /*
+ * EasyMock matcher that matches {@link IntenOperations} but
+ * ignores the {@link IntentId} when matching.
+ * <p/>
+ * The normal intent equals method tests that the intent IDs are equal,
+ * however in these tests we can't know what the intent IDs will be in
+ * advance, so we can't set up expected intents with the correct IDs. Thus,
+ * the solution is to use an EasyMock matcher that verifies that all the
+ * value properties of the provided intent match the expected values, but
+ * ignores the intent ID when testing equality.
+ */
+ private static final class IdAgnosticIntentOperationsMatcher implements
+ IArgumentMatcher {
+
+ private final IntentOperations intentOperations;
+ private String providedString;
+
+ /**
+ * Constructor taking the expected intent operations to match against.
+ *
+ * @param intentOperations the expected intent operations
+ */
+ public IdAgnosticIntentOperationsMatcher(
+ IntentOperations intentOperations) {
+ this.intentOperations = intentOperations;
+ }
+
+ @Override
+ public void appendTo(StringBuffer strBuffer) {
+ strBuffer.append("IntentOperationsMatcher unable to match: "
+ + providedString);
+ }
+
+ @Override
+ public boolean matches(Object object) {
+ if (!(object instanceof IntentOperations)) {
+ return false;
+ }
+
+ IntentOperations providedIntentOperations =
+ (IntentOperations) object;
+ providedString = providedIntentOperations.toString();
+
+ List<IntentKey> thisSubmitIntents = new LinkedList<>();
+ List<IntentId> thisWithdrawIntentIds = new LinkedList<>();
+ List<IntentKey> thisReplaceIntents = new LinkedList<>();
+ List<IntentKey> thisUpdateIntents = new LinkedList<>();
+ List<IntentKey> providedSubmitIntents = new LinkedList<>();
+ List<IntentId> providedWithdrawIntentIds = new LinkedList<>();
+ List<IntentKey> providedReplaceIntents = new LinkedList<>();
+ List<IntentKey> providedUpdateIntents = new LinkedList<>();
+
+ extractIntents(intentOperations, thisSubmitIntents,
+ thisWithdrawIntentIds, thisReplaceIntents,
+ thisUpdateIntents);
+ extractIntents(providedIntentOperations, providedSubmitIntents,
+ providedWithdrawIntentIds, providedReplaceIntents,
+ providedUpdateIntents);
+
+ return CollectionUtils.isEqualCollection(thisSubmitIntents,
+ providedSubmitIntents) &&
+ CollectionUtils.isEqualCollection(thisWithdrawIntentIds,
+ providedWithdrawIntentIds) &&
+ CollectionUtils.isEqualCollection(thisUpdateIntents,
+ providedUpdateIntents) &&
+ CollectionUtils.isEqualCollection(thisReplaceIntents,
+ providedReplaceIntents);
+ }
+
+ /**
+ * Extracts the intents per operation type. Each intent is encapsulated
+ * in IntentKey so it can be compared by excluding the Intent ID.
+ *
+ * @param intentOperations the container with the intent operations
+ * to extract the intents from
+ * @param submitIntents the SUBMIT intents
+ * @param withdrawIntentIds the WITHDRAW intents IDs
+ * @param replaceIntents the REPLACE intents
+ * @param updateIntents the UPDATE intens
+ */
+ private void extractIntents(IntentOperations intentOperations,
+ List<IntentKey> submitIntents,
+ List<IntentId> withdrawIntentIds,
+ List<IntentKey> replaceIntents,
+ List<IntentKey> updateIntents) {
+ for (IntentOperation oper : intentOperations.operations()) {
+ IntentId intentId;
+ IntentKey intentKey;
+ switch (oper.type()) {
+ case SUBMIT:
+ intentKey = new IntentKey(oper.intent());
+ submitIntents.add(intentKey);
+ break;
+ case WITHDRAW:
+ intentId = oper.intentId();
+ withdrawIntentIds.add(intentId);
+ break;
+ case REPLACE:
+ intentKey = new IntentKey(oper.intent());
+ replaceIntents.add(intentKey);
+ break;
+ case UPDATE:
+ intentKey = new IntentKey(oper.intent());
+ updateIntents.add(intentKey);
+ break;
+ default:
+ break;
+ }
+ }
+ }
+ }
+
+ /*
+ * EasyMock matcher that matches {@link PointToPointIntent}s but
+ * ignores the {@link IntentId} when matching.
+ * <p/>
+ * The normal intent equals method tests that the intent IDs are equal,
+ * however in these tests we can't know what the intent IDs will be in
+ * advance, so we can't set up expected intents with the correct IDs. Thus,
+ * the solution is to use an EasyMock matcher that verifies that all the
+ * value properties of the provided intent match the expected values, but
+ * ignores the intent ID when testing equality.
+ */
+ private static final class IdAgnosticPointToPointIntentMatcher implements
+ IArgumentMatcher {
+
+ private final PointToPointIntent intent;
+ private String providedIntentString;
+
+ /**
+ * Constructor taking the expected intent to match against.
+ *
+ * @param intent the expected intent
+ */
+ public IdAgnosticPointToPointIntentMatcher(PointToPointIntent intent) {
+ this.intent = intent;
+ }
+
+ @Override
+ public void appendTo(StringBuffer strBuffer) {
+ strBuffer.append("PointToPointIntentMatcher unable to match: "
+ + providedIntentString);
+ }
+
+ @Override
+ public boolean matches(Object object) {
+ if (!(object instanceof PointToPointIntent)) {
+ return false;
+ }
+
+ PointToPointIntent providedIntent = (PointToPointIntent) object;
+ providedIntentString = providedIntent.toString();
+
+ PointToPointIntent matchIntent =
+ new PointToPointIntent(providedIntent.appId(),
+ intent.selector(), intent.treatment(),
+ intent.ingressPoint(), intent.egressPoint());
+
+ return matchIntent.equals(providedIntent);
+ }
+ }
+}