From 3508bc959e055c76c395e0f878d3c9f138f44114 Mon Sep 17 00:00:00 2001 From: Marek Gradzki Date: Tue, 14 Jun 2016 10:35:53 +0200 Subject: HONEYCOMB-91: fix restoring BD from persisted config. Covers case when bd_id was present in the bdContext Change-Id: I817fc684f175958f772a87ee708fa7f49ceec6f7 Signed-off-by: Marek Gradzki --- .../translate/v3po/vpp/BridgeDomainCustomizer.java | 25 ++-- .../v3po/vpp/BridgeDomainCustomizerTest.java | 25 ++++ .../honeycomb/v3po/translate/v3po/vpp/VppTest.java | 126 ++++++++++++--------- 3 files changed, 110 insertions(+), 66 deletions(-) diff --git a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizer.java b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizer.java index 710a74be5..3bc7ba175 100644 --- a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizer.java +++ b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizer.java @@ -18,6 +18,7 @@ package io.fd.honeycomb.v3po.translate.v3po.vpp; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static io.fd.honeycomb.v3po.translate.v3po.util.TranslateUtils.booleanToByte; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -29,7 +30,6 @@ import io.fd.honeycomb.v3po.translate.write.WriteContext; import io.fd.honeycomb.v3po.translate.write.WriteFailedException; import java.util.List; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.BridgeDomains; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.bridge.domains.BridgeDomain; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.bridge.domains.BridgeDomainKey; @@ -89,12 +89,17 @@ public class BridgeDomainCustomizer final String bdName = dataBefore.getName(); try { - // FIXME we need the bd index to be returned by VPP or we should have a counter field - // (maybe in context similar to artificial name) - // Here we assign the next available ID from bdContext's perspective - int index = 1; - while (bdContext.containsName(index, ctx.getMappingContext())) { - index++; + int index; + if (bdContext.containsIndex(bdName, ctx.getMappingContext())) { + index = bdContext.getIndex(bdName, ctx.getMappingContext()); + } else { + // FIXME we need the bd index to be returned by VPP or we should have a counter field + // (maybe in context similar to artificial name) + // Here we assign the next available ID from bdContext's perspective + index = 1; + while (bdContext.containsName(index, ctx.getMappingContext())) { + index++; + } } addOrUpdateBridgeDomain(index, dataBefore); bdContext.addName(index, bdName, ctx.getMappingContext()); @@ -104,12 +109,6 @@ public class BridgeDomainCustomizer } } - private byte booleanToByte(@Nullable final Boolean aBoolean) { - return aBoolean != null && aBoolean - ? (byte) 1 - : (byte) 0; - } - @Override public void deleteCurrentAttributes(@Nonnull final InstanceIdentifier id, @Nonnull final BridgeDomain dataBefore, diff --git a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizerTest.java b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizerTest.java index ff9c0cf9b..ae9b0c29c 100644 --- a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizerTest.java +++ b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/BridgeDomainCustomizerTest.java @@ -43,6 +43,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.Mappings; +import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.mappings.MappingBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.bridge.domains.BridgeDomain; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.v3po.rev150105.vpp.bridge.domains.BridgeDomainBuilder; import org.openvpp.jvpp.VppInvocationException; @@ -152,8 +153,29 @@ public class BridgeDomainCustomizerTest { final int bdId = 1; final String bdName = "bd1"; final BridgeDomain bd = generateBridgeDomain(bdName); + // Make bdContext.containsName() return false doReturn(Optional.absent()).when(mappingContext) .read(getMappingIid(bdName, "test-instance").firstIdentifierOf(Mappings.class)); + // Make bdContext.containsIndex() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(bdName, "test-instance")); + + whenBridgeDomainAddDelThenSuccess(); + + customizer.writeCurrentAttributes(BridgeDomainTestUtils.bdIdentifierForName(bdName), bd, ctx); + + verifyBridgeDomainAddOrUpdateWasInvoked(bd, bdId); + verify(mappingContext).put(getMappingIid(bdName, "test-instance"), getMapping(bdName, bdId).get()); + } + + @Test + public void testAddBridgeDomainPresentInBdContext() throws Exception { + final int bdId = 1; + final String bdName = "bd1"; + final BridgeDomain bd = generateBridgeDomain(bdName); + // Make bdContext.containsIndex() return true + doReturn(Optional.of(new MappingBuilder().setIndex(bdId).build())).when(mappingContext) + .read(getMappingIid(bdName, "test-instance")); whenBridgeDomainAddDelThenSuccess(); @@ -172,6 +194,9 @@ public class BridgeDomainCustomizerTest { // Returning no Mappings for "test-instance" makes bdContext.containsName() return false doReturn(Optional.absent()).when(mappingContext) .read(getMappingIid(bdName, "test-instance").firstIdentifierOf(Mappings.class)); + // Make bdContext.containsIndex() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(bdName, "test-instance")); whenBridgeDomainAddDelThenFailure(); diff --git a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/VppTest.java b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/VppTest.java index 2bb37d108..e713c623d 100644 --- a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/VppTest.java +++ b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/vpp/VppTest.java @@ -18,6 +18,14 @@ package io.fd.honeycomb.v3po.translate.v3po.vpp; import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMapping; import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMappingIid; +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + import com.google.common.base.Optional; import com.google.common.collect.Iterators; import com.google.common.collect.Lists; @@ -28,6 +36,11 @@ import io.fd.honeycomb.v3po.translate.util.write.DelegatingWriterRegistry; import io.fd.honeycomb.v3po.translate.v3po.util.NamingContext; import io.fd.honeycomb.v3po.translate.write.WriteContext; import io.fd.honeycomb.v3po.translate.write.Writer; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutionException; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -47,20 +60,13 @@ import org.openvpp.jvpp.dto.BridgeDomainAddDel; import org.openvpp.jvpp.dto.BridgeDomainAddDelReply; import org.openvpp.jvpp.future.FutureJVpp; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import java.util.concurrent.ExecutionException; - -import static org.junit.Assert.assertEquals; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.*; - public class VppTest { private static final byte ADD_OR_UPDATE_BD = 1; private static final byte ZERO = 0; + private static final String BD_NAME = "bdn1"; + private static final String BD_CONTEXT_NAME = "test-instance"; + @Mock private FutureJVpp api; @Mock @@ -74,7 +80,7 @@ public class VppTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - NamingContext bdContext = new NamingContext("generatedBdName", "test-instance"); + NamingContext bdContext = new NamingContext("generatedBdName", BD_CONTEXT_NAME); final ModificationCache toBeReturned = new ModificationCache(); doReturn(toBeReturned).when(ctx).getModificationCache(); doReturn(mappingContext).when(ctx).getMappingContext(); @@ -88,25 +94,26 @@ public class VppTest { final List bdmns = Lists.newArrayList(); for (String s : name) { bdmns.add(new BridgeDomainBuilder() - .setName(s) - .setArpTermination(false) - .setFlood(true) - .setForward(false) - .setLearn(true) - .build()); + .setName(s) + .setArpTermination(false) + .setFlood(true) + .setForward(false) + .setLearn(true) + .build()); } return new BridgeDomainsBuilder() - .setBridgeDomain(bdmns) - .build(); + .setBridgeDomain(bdmns) + .build(); } - private void whenBridgeDomainAddDelThen(final int retval) throws ExecutionException, VppInvocationException, InterruptedException { - final CompletionStage replyCS = mock(CompletionStage.class); + private void whenBridgeDomainAddDelThenSuccess() + throws ExecutionException, VppInvocationException, InterruptedException { + final CompletionStage replyCs = mock(CompletionStage.class); final CompletableFuture replyFuture = mock(CompletableFuture.class); - when(replyCS.toCompletableFuture()).thenReturn(replyFuture); + when(replyCs.toCompletableFuture()).thenReturn(replyFuture); final BridgeDomainAddDelReply reply = new BridgeDomainAddDelReply(); when(replyFuture.get()).thenReturn(reply); - when(api.bridgeDomainAddDel(any(BridgeDomainAddDel.class))).thenReturn(replyCS); + when(api.bridgeDomainAddDel(any(BridgeDomainAddDel.class))).thenReturn(replyCs); } private void verifyBridgeDomainAddDel(final BridgeDomain bd, final int bdId) throws VppInvocationException { @@ -145,16 +152,21 @@ public class VppTest { @Test public void writeVppUsingRootRegistry() throws Exception { final int bdId = 1; - final BridgeDomains bdn1 = getBridgeDomains("bdn1"); - whenBridgeDomainAddDelThen(0); - doReturn(Optional - .absent()).when(mappingContext).read(getMappingIid("bdn1", "test-instance").firstIdentifierOf(Mappings.class)); + final BridgeDomains bdn1 = getBridgeDomains(BD_NAME); + whenBridgeDomainAddDelThenSuccess(); + + // Returning no Mappings for "test-instance" makes bdContext.containsName() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME).firstIdentifierOf(Mappings.class)); + // Make bdContext.containsIndex() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME)); rootRegistry.update( - InstanceIdentifier.create(Vpp.class), - null, - new VppBuilder().setBridgeDomains(bdn1).build(), - ctx); + InstanceIdentifier.create(Vpp.class), + null, + new VppBuilder().setBridgeDomains(bdn1).build(), + ctx); verifyBridgeDomainAddDel(Iterators.getOnlyElement(bdn1.getBridgeDomain().iterator()), bdId); } @@ -162,28 +174,38 @@ public class VppTest { @Test public void writeVppUsingVppWriter() throws Exception { final int bdId = 1; - final BridgeDomains bdn1 = getBridgeDomains("bdn1"); - whenBridgeDomainAddDelThen(0); - doReturn(Optional - .absent()).when(mappingContext).read(getMappingIid("bdn1", "test-instance").firstIdentifierOf(Mappings.class)); + final BridgeDomains bdn1 = getBridgeDomains(BD_NAME); + whenBridgeDomainAddDelThenSuccess(); + + // Returning no Mappings for "test-instance" makes bdContext.containsName() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME).firstIdentifierOf(Mappings.class)); + // Make bdContext.containsIndex() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME)); vppWriter.update(InstanceIdentifier.create(Vpp.class), - null, - new VppBuilder().setBridgeDomains(bdn1).build(), - ctx); + null, + new VppBuilder().setBridgeDomains(bdn1).build(), + ctx); verifyBridgeDomainAddDel(Iterators.getOnlyElement(bdn1.getBridgeDomain().iterator()), bdId); - verify(mappingContext).put(getMappingIid("bdn1", "test-instance"), getMapping("bdn1", 1).get()); + verify(mappingContext).put(getMappingIid(BD_NAME, BD_CONTEXT_NAME), getMapping(BD_NAME, 1).get()); } @Test public void writeVppFromRoot() throws Exception { - final BridgeDomains bdn1 = getBridgeDomains("bdn1"); + final BridgeDomains bdn1 = getBridgeDomains(BD_NAME); final int bdId = 1; final Vpp vpp = new VppBuilder().setBridgeDomains(bdn1).build(); - doReturn(Optional - .absent()).when(mappingContext).read(getMappingIid("bdn1", "test-instance").firstIdentifierOf(Mappings.class)); - whenBridgeDomainAddDelThen(0); + whenBridgeDomainAddDelThenSuccess(); + + // Returning no Mappings for "test-instance" makes bdContext.containsName() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME).firstIdentifierOf(Mappings.class)); + // Make bdContext.containsIndex() return false + doReturn(Optional.absent()).when(mappingContext) + .read(getMappingIid(BD_NAME, BD_CONTEXT_NAME)); rootRegistry.update(Collections.emptyMap(), Collections., DataObject>singletonMap(InstanceIdentifier.create(Vpp.class), @@ -194,11 +216,10 @@ public class VppTest { @Test public void deleteVpp() throws Exception { - final String bdName = "bdn1"; - final BridgeDomains bdn1 = getBridgeDomains(bdName); + final BridgeDomains bdn1 = getBridgeDomains(BD_NAME); final int bdId = 1; - whenBridgeDomainAddDelThen(0); - doReturn(getMapping(bdName, bdId)).when(mappingContext).read(getMappingIid(bdName, "test-instance")); + whenBridgeDomainAddDelThenSuccess(); + doReturn(getMapping(BD_NAME, bdId)).when(mappingContext).read(getMappingIid(BD_NAME, BD_CONTEXT_NAME)); rootRegistry.update( InstanceIdentifier.create(Vpp.class), @@ -213,8 +234,8 @@ public class VppTest { public void updateVppNoActualChange() throws Exception { rootRegistry.update( InstanceIdentifier.create(Vpp.class), - new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(), - new VppBuilder().setBridgeDomains(getBridgeDomains("bdn1")).build(), + new VppBuilder().setBridgeDomains(getBridgeDomains(BD_NAME)).build(), + new VppBuilder().setBridgeDomains(getBridgeDomains(BD_NAME)).build(), ctx); verifyZeroInteractions(api); @@ -222,11 +243,10 @@ public class VppTest { @Test public void writeUpdate() throws Exception { - final String bdName = "bdn1"; final int bdn1Id = 1; - doReturn(getMapping(bdName, bdn1Id)).when(mappingContext).read(getMappingIid(bdName, "test-instance")); + doReturn(getMapping(BD_NAME, bdn1Id)).when(mappingContext).read(getMappingIid(BD_NAME, BD_CONTEXT_NAME)); - final BridgeDomains domainsBefore = getBridgeDomains(bdName); + final BridgeDomains domainsBefore = getBridgeDomains(BD_NAME); final BridgeDomain bdn1Before = domainsBefore.getBridgeDomain().get(0); final BridgeDomain bdn1After = new BridgeDomainBuilder(bdn1Before).setFlood(!bdn1Before.isFlood()).build(); @@ -234,7 +254,7 @@ public class VppTest { .setBridgeDomain(Collections.singletonList(bdn1After)) .build(); - whenBridgeDomainAddDelThen(0); + whenBridgeDomainAddDelThenSuccess(); rootRegistry.update( InstanceIdentifier.create(Vpp.class), -- cgit 1.2.3-korg