From: Stephen Kitt Date: Wed, 19 Sep 2018 14:57:58 +0000 (+0200) Subject: Add more MdsalApiManager test methods, key them X-Git-Tag: release/neon~88 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=fb4fe70dc8580279bc18b318e1eb30ac4f891b9b;p=genius.git Add more MdsalApiManager test methods, key them This adds implementations of some of the new MdsalApiManager methods to TestIMdsalApiManager, allowing them to be used in downstream tests. It also reworks the storage to be keyed, using the same distinguishers as the real datastore, avoiding errors related to duplicate handling. Change-Id: Id87cac5d78d271d6650a513cc8b03c3bd6c88c79 Signed-off-by: Stephen Kitt --- diff --git a/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/TestIMdsalApiManager.java b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/TestIMdsalApiManager.java index 38c8c8a14..9230cc70b 100644 --- a/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/TestIMdsalApiManager.java +++ b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/TestIMdsalApiManager.java @@ -21,13 +21,23 @@ import com.google.common.collect.Lists; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import java.math.BigInteger; -import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; import org.junit.ComparisonFailure; import org.mockito.Mockito; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.genius.infra.Datastore.Configuration; +import org.opendaylight.genius.infra.TypedReadWriteTransaction; +import org.opendaylight.genius.infra.TypedWriteTransaction; import org.opendaylight.genius.mdsalutil.FlowEntity; +import org.opendaylight.genius.mdsalutil.GroupEntity; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; +import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow; +import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.FlowKey; +import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.buckets.Bucket; import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.Group; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,17 +54,26 @@ import org.slf4j.LoggerFactory; * of it using it's static {@link #newInstance()} method. * * @author Michael Vorburger - * @autor Faseela K + * @author Faseela K */ public abstract class TestIMdsalApiManager implements IMdsalApiManager { private static final Logger LOG = LoggerFactory.getLogger(TestIMdsalApiManager.class); - private List flows; - private List groups; + private Map flows; + private Map groups; + private Map buckets; public static TestIMdsalApiManager newInstance() { - return Mockito.mock(TestIMdsalApiManager.class, realOrException()); + TestIMdsalApiManager instance = Mockito.mock(TestIMdsalApiManager.class, realOrException()); + instance.init(); + return instance; + } + + private void init() { + this.flows = new HashMap<>(); + this.groups = new HashMap<>(); + this.buckets = new HashMap<>(); } /** @@ -63,26 +82,12 @@ public abstract class TestIMdsalApiManager implements IMdsalApiManager { * @return immutable copy of list of flows */ public synchronized List getFlows() { - return ImmutableList.copyOf(getOrNewFlows()); - } - - private synchronized List getOrNewFlows() { - if (flows == null) { - flows = new ArrayList<>(); - } - return flows; - } - - private synchronized List getOrNewGroups() { - if (groups == null) { - groups = new ArrayList<>(); - } - return groups; + return ImmutableList.copyOf(retrieveFlows()); } public synchronized void assertFlows(Iterable expectedFlows) { checkNonEmptyFlows(expectedFlows); - List nonNullFlows = getOrNewFlows(); + Collection nonNullFlows = retrieveFlows(); if (!Iterables.isEmpty(expectedFlows)) { assertTrue("No Flows created (bean wiring may be broken?)", !nonNullFlows.isEmpty()); } @@ -94,7 +99,7 @@ public abstract class TestIMdsalApiManager implements IMdsalApiManager { private synchronized void checkNonEmptyFlows(Iterable expectedFlows) { if (!Iterables.isEmpty(expectedFlows)) { - assertTrue("No Flows created (bean wiring may be broken?)", !getOrNewFlows().isEmpty()); + assertTrue("No Flows created (bean wiring may be broken?)", !retrieveFlows().isEmpty()); } } @@ -103,10 +108,15 @@ public abstract class TestIMdsalApiManager implements IMdsalApiManager { public synchronized void assertFlowsInAnyOrder(Iterable expectedFlows) { checkNonEmptyFlows(expectedFlows); // TODO Support Iterable <-> List directly within XtendBeanGenerator - List expectedFlowsAsNewArrayList = Lists.newArrayList(expectedFlows); - List sortedFlows = sortFlows(flows); - List sortedExpectedFlows = sortFlows(expectedFlowsAsNewArrayList); + List sortedFlows = sortFlows(retrieveFlows()); + Map keyedExpectedFlows = new HashMap<>(); + for (FlowEntity expectedFlow : expectedFlows) { + keyedExpectedFlows.put( + new InternalFlowKey(expectedFlow.getDpnId(), expectedFlow.getFlowId(), expectedFlow.getTableId()), + expectedFlow); + } + List sortedExpectedFlows = sortFlows(keyedExpectedFlows.values()); // FYI: This containsExactlyElementsIn() assumes that FlowEntity, and everything in it, // has correctly working equals() implementations. assertEqualBeans() does not assume @@ -135,8 +145,17 @@ public abstract class TestIMdsalApiManager implements IMdsalApiManager { // is, much, more readable), there are cases when looking more closely // at the full toString() output of the flows is still useful, so: // TIP: Use e.g. 'wdiff -n expected.txt actual.txt | colordiff' to compare these! - LOG.warn("assert failed [order ignored!]; expected flows: {}", sortedExpectedFlows); - LOG.warn("assert failed [order ignored!]; actual flows : {}", sortedFlows); + LOG.warn("assert failed [order ignored!]; expected flows ({}): {}", sortedExpectedFlows.size(), + sortedExpectedFlows); + LOG.warn("assert failed [order ignored!]; actual flows ({}): {}", sortedFlows.size(), sortedFlows); + for (int i = 0; i < sortedExpectedFlows.size() && i < sortedFlows.size(); i++) { + if (!sortedExpectedFlows.get(i).equals(sortedFlows.get(i))) { + LOG.warn("First mismatch at index {};", i); + LOG.warn(" expected {}", sortedExpectedFlows.get(i)); + LOG.warn(" got {}", sortedFlows.get(i)); + break; + } + } // The point of now also doing assertEqualBeans() is just that its output, // in case of a comparison failure, is *A LOT* more clearly readable // than what G Truth (or Hamcrest) can do based on toString. @@ -165,49 +184,234 @@ public abstract class TestIMdsalApiManager implements IMdsalApiManager { return sortedFlows; } + private void storeFlow(FlowEntity flowEntity) { + flows.put(new InternalFlowKey(flowEntity.getDpnId(), flowEntity.getFlowId(), flowEntity.getTableId()), + flowEntity); + } + + private Collection retrieveFlows() { + return flows.values(); + } + + private void deleteFlow(BigInteger dpId, String flowId, short tableId) { + flows.remove(new InternalFlowKey(dpId, flowId, tableId)); + } + + private void storeGroup(BigInteger dpnId, Group group) { + groups.put(new InternalGroupKey(dpnId, group.key().getGroupId().getValue()), group); + } + + private Collection retrieveGroups() { + return groups.values(); + } + + private void deleteGroup(BigInteger dpnId, long groupId) { + groups.remove(new InternalGroupKey(dpnId, groupId)); + } + + private void storeBucket(BigInteger dpnId, long groupId, Bucket bucket) { + buckets.put(new InternalBucketKey(dpnId, groupId, bucket.getBucketId().getValue()), bucket); + } + + private Collection retrieveBuckets() { + return buckets.values(); + } + + private void deleteBucket(BigInteger dpnId, long groupId, long bucketId) { + buckets.remove(new InternalBucketKey(dpnId, groupId, bucketId)); + } + @Override - public synchronized CheckedFuture installFlow(FlowEntity flowEntity) { - getOrNewFlows().add(flowEntity); - return Futures.immediateCheckedFuture(null); + public void addFlow(TypedWriteTransaction tx, FlowEntity flowEntity) { + storeFlow(flowEntity); } @Override - public synchronized CheckedFuture installFlow(BigInteger dpId, - FlowEntity flowEntity) { - // TODO should dpId be considered here? how? Copy clone FlowEntity and change its dpId? - return installFlow(flowEntity); + public void addFlow(TypedWriteTransaction tx, BigInteger dpId, Flow flow) { + throw new UnsupportedOperationException("addFlow(..., BigInteger, Flow) isn't supported yet"); + } + + @Override + public void removeFlow(TypedReadWriteTransaction tx, BigInteger dpId, Flow flow) { + removeFlow(tx, dpId, flow.key(), flow.getTableId()); + } + + @Override + public void removeFlow(TypedReadWriteTransaction tx, FlowEntity flowEntity) { + deleteFlow(flowEntity.getDpnId(), flowEntity.getFlowId(), flowEntity.getTableId()); + } + + @Override + public void removeFlow(TypedReadWriteTransaction tx, BigInteger dpId, FlowKey flowKey, + short tableId) { + deleteFlow(dpId, flowKey.getId().getValue(), tableId); + } + + @Override + public void removeFlow(TypedReadWriteTransaction tx, BigInteger dpId, String flowId, + short tableId) { + deleteFlow(dpId, flowId, tableId); } @Override public synchronized CheckedFuture removeFlow(BigInteger dpnId, FlowEntity flowEntity) { - // TODO should dpId be considered here? how? Copy clone FlowEntity and change its dpId? - getOrNewFlows().remove(flowEntity); + deleteFlow(dpnId, flowEntity.getFlowId(), flowEntity.getTableId()); + return Futures.immediateCheckedFuture(null); + } + + @Override + public void addGroup(TypedWriteTransaction tx, GroupEntity groupEntity) { + storeGroup(groupEntity.getDpnId(), groupEntity.getGroupBuilder().build()); + } + + @Override + public void addGroup(TypedWriteTransaction tx, BigInteger dpId, Group group) { + storeGroup(dpId, group); + } + + @Override + public void removeGroup(TypedReadWriteTransaction tx, BigInteger dpId, Group group) { + deleteGroup(dpId, group.getGroupId().getValue()); + } + + @Override + public void removeGroup(TypedReadWriteTransaction tx, BigInteger dpId, long groupId) { + deleteGroup(dpId, groupId); + } + + @Override + public void addBucket(TypedReadWriteTransaction tx, BigInteger dpId, long groupId, + Bucket bucket) { + storeBucket(dpId, groupId, bucket); + } + + @Override + public void removeBucket(TypedReadWriteTransaction tx, BigInteger dpId, long groupId, + long bucketId) { + deleteBucket(dpId, groupId, bucketId); + } + + @Override + public synchronized CheckedFuture installFlow(FlowEntity flowEntity) { + storeFlow(flowEntity); return Futures.immediateCheckedFuture(null); } + @Override + public synchronized CheckedFuture installFlow(BigInteger dpId, + FlowEntity flowEntity) { + // TODO should dpId be considered here? how? Copy clone FlowEntity and change its dpId? + return installFlow(flowEntity); + } + @Override public synchronized void batchedAddFlow(BigInteger dpId, FlowEntity flowEntity) { - getOrNewFlows().add(flowEntity); + storeFlow(flowEntity); } @Override public synchronized void batchedRemoveFlow(BigInteger dpId, FlowEntity flowEntity) { - getOrNewFlows().remove(flowEntity); + deleteFlow(dpId, flowEntity.getFlowId(), flowEntity.getTableId()); } @Override public void syncInstallGroup(BigInteger dpId, Group group, long delayTime) { - getOrNewGroups().add(group); + storeGroup(dpId, group); } @Override public void syncInstallGroup(BigInteger dpId, Group group) { - getOrNewGroups().add(group); + storeGroup(dpId, group); } @Override public void syncRemoveGroup(BigInteger dpId, Group groupEntity) { - getOrNewGroups().remove(groupEntity); + deleteGroup(dpId, groupEntity.getGroupId().getValue()); + } + + private final class InternalFlowKey { + private final BigInteger dpnId; + private final String flowId; + private final short tableId; + + private InternalFlowKey(BigInteger dpnId, String flowId, short tableId) { + this.dpnId = dpnId; + this.flowId = flowId; + this.tableId = tableId; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + InternalFlowKey that = (InternalFlowKey) obj; + return tableId == that.tableId && Objects.equals(dpnId, that.dpnId) && Objects.equals(flowId, that.flowId); + } + + @Override + public int hashCode() { + return Objects.hash(dpnId, flowId, tableId); + } + } + + private final class InternalGroupKey { + private final BigInteger dpnId; + private final long groupId; + + private InternalGroupKey(BigInteger dpnId, long groupId) { + this.dpnId = dpnId; + this.groupId = groupId; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + InternalGroupKey that = (InternalGroupKey) obj; + return groupId == that.groupId && Objects.equals(dpnId, that.dpnId); + } + + @Override + public int hashCode() { + return Objects.hash(dpnId, groupId); + } + } + + private final class InternalBucketKey { + private final BigInteger dpnId; + private final long groupId; + private final long bucketId; + + private InternalBucketKey(BigInteger dpnId, long groupId, long bucketId) { + this.dpnId = dpnId; + this.groupId = groupId; + this.bucketId = bucketId; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + InternalBucketKey that = (InternalBucketKey) obj; + return groupId == that.groupId && bucketId == that.bucketId && Objects.equals(dpnId, that.dpnId); + } + + @Override + public int hashCode() { + return Objects.hash(dpnId, groupId, bucketId); + } } } diff --git a/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/tests/TestIMdsalApiManagerTest.java b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/tests/TestIMdsalApiManagerTest.java index 68a06c719..9d42b2b3b 100644 --- a/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/tests/TestIMdsalApiManagerTest.java +++ b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/interfaces/testutils/tests/TestIMdsalApiManagerTest.java @@ -38,7 +38,7 @@ public class TestIMdsalApiManagerTest { public void testAssertFlowsInAnyOrder() { mdsalApiManager.installFlow(getNewFlow1()); mdsalApiManager.installFlow(getNewFlow2()); - mdsalApiManager.assertFlows(ImmutableList.of(getNewFlow1(), getNewFlow2())); + mdsalApiManager.assertFlows(ImmutableList.of(getNewFlow2(), getNewFlow1())); mdsalApiManager.assertFlowsInAnyOrder(ImmutableList.of(getNewFlow2(), getNewFlow1())); }