From d757d43b658bebeea75787d7a3b712e06c9ef69b Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Mon, 27 Jun 2016 11:49:12 +0200 Subject: Fix IPv4 read caching Single cache key was used for each interface during a single read returing same IPs for all the interfaces Change-Id: I8cc05591b257d44a253cc23c9d79d9096459dcdd Signed-off-by: Maros Marsalek --- .../interfacesstate/ip/Ipv4AddressCustomizer.java | 29 ++++---- .../ip/Ipv4AddressCustomizerTest.java | 77 ++++++++++++++++++---- 2 files changed, 82 insertions(+), 24 deletions(-) (limited to 'v3po') diff --git a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java index 641516136..2121337ab 100644 --- a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java +++ b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java @@ -50,7 +50,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Customizer for read operations for {@link Address} of {@link Ipv4} + * Customizer for read operations for {@link Address} of {@link Ipv4}. */ public class Ipv4AddressCustomizer extends FutureJVppCustomizer implements ListReaderCustomizer { @@ -92,8 +92,7 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer builder.setIp(TranslateUtils.arrayToIpv4AddressNoZone(detail.ip)) .setSubnet(new PrefixLengthBuilder() .setPrefixLength(Short.valueOf(detail.prefixLength)).build()); - LOG.info("Address read successfull"); - + LOG.info("Address read successful"); } else { LOG.warn("No address dump present"); } @@ -102,18 +101,22 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer @Override public List getAllIds(@Nonnull InstanceIdentifier
id, @Nonnull ReadContext context) throws ReadFailedException { + // FIXME: this kind of logs provide very little information. At least the ID should be included so we know + // from the logs what exact data is being processed + // + Logs should be consistent in using punctuation LOG.debug("Extracting keys.."); Optional dumpOptional = dumpAddresses(id, context); if (dumpOptional.isPresent() && dumpOptional.get().ipAddressDetails != null) { - List details = dumpOptional.get().ipAddressDetails; - - return details.stream() + return dumpOptional.get().ipAddressDetails.stream() .map(detail -> new AddressKey(TranslateUtils.arrayToIpv4AddressNoZone(detail.ip))) .collect(Collectors.toList()); } else { + // FIXME if this is expected then WARN should not be emitted + // FIXME if this is not expected, throw an exception instead + // Same in readCurrentAttributes() LOG.warn("No dump present"); return Collections.emptyList(); } @@ -124,11 +127,14 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer ((Ipv4Builder) builder).setAddress(readData); } + // TODO refactor after there is a more generic implementation of cache operations + // FIXME update TODO with what exactly should be refactored and how // TODO refactor after there is an more generic implementation of cache // operations private Optional dumpAddresses(InstanceIdentifier
id, ReadContext ctx) - throws ReadFailedException { - Optional dumpFromCache = dumpAddressFromCache(ctx.getModificationCache()); + throws ReadFailedException { + final String cacheKey = CACHE_KEY + id.firstKeyOf(Interface.class).getName(); + Optional dumpFromCache = dumpAddressFromCache(cacheKey, ctx.getModificationCache()); if (dumpFromCache.isPresent()) { return dumpFromCache; @@ -142,15 +148,16 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer } if (dumpFromOperational.isPresent()) { - ctx.getModificationCache().put(CACHE_KEY, dumpFromOperational.get()); + ctx.getModificationCache().put(cacheKey, dumpFromOperational.get()); } return dumpFromOperational; } - private Optional dumpAddressFromCache(ModificationCache cache) { + private Optional dumpAddressFromCache(final String cacheKey, + ModificationCache cache) { LOG.debug("Dumping from cache..."); - return Optional.fromNullable((IpAddressDetailsReplyDump) cache.get(CACHE_KEY)); + return Optional.fromNullable((IpAddressDetailsReplyDump) cache.get(cacheKey)); } private Optional dumpAddressFromOperationalData(final InstanceIdentifier
id, diff --git a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java index 85f528e1e..584464645 100644 --- a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java +++ b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java @@ -19,7 +19,9 @@ package io.fd.honeycomb.v3po.translate.v3po.interfacesstate.ip; import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMapping; import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMappingIid; import static io.fd.honeycomb.v3po.translate.v3po.util.TranslateUtils.reverseBytes; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -38,6 +40,7 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; +import org.hamcrest.CoreMatchers; import org.junit.Test; import org.mockito.Mockito; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.Mappings; @@ -64,7 +67,9 @@ import org.openvpp.jvpp.dto.IpAddressDump; public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest { private static final String IFACE_NAME = "eth0"; + private static final String IFACE_2_NAME = "eth1"; private static final int IFACE_ID = 1; + private static final int IFACE_2_ID = 2; private NamingContext interfacesContext; @@ -80,24 +85,27 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest initCustomizer() { final KeyedInstanceIdentifier eth0Id = getMappingIid(IFACE_NAME, "test-instance"); + final KeyedInstanceIdentifier eth1Id = getMappingIid(IFACE_2_NAME, "test-instance"); final Optional eth0 = getMapping(IFACE_NAME, IFACE_ID); + final Optional eth1 = getMapping(IFACE_2_NAME, IFACE_2_ID); - final List allMappings = Lists.newArrayList(getMapping(IFACE_NAME, IFACE_ID).get()); + final List allMappings = Lists.newArrayList(eth0.get(), eth1.get()); final Mappings allMappingsBaObject = new MappingsBuilder().setMapping(allMappings).build(); doReturn(Optional.of(allMappingsBaObject)).when(mappingContext).read(eth0Id.firstIdentifierOf(Mappings.class)); doReturn(eth0).when(mappingContext).read(eth0Id); + doReturn(eth1).when(mappingContext).read(eth1Id); return new Ipv4AddressCustomizer(api, interfacesContext); } - private static InstanceIdentifier
getId(final String address) { + private static InstanceIdentifier
getId(final String address, final String ifaceName) { return InstanceIdentifier.builder(InterfacesState.class) - .child(Interface.class, new InterfaceKey(IFACE_NAME)) - .augmentation(Interface2.class) - .child(Ipv4.class) - .child(Address.class, new AddressKey(new Ipv4AddressNoZone(new Ipv4Address(address)))) - .build(); + .child(Interface.class, new InterfaceKey(ifaceName)) + .augmentation(Interface2.class) + .child(Ipv4.class) + .child(Address.class, new AddressKey(new Ipv4AddressNoZone(new Ipv4Address(address)))) + .build(); } @Test @@ -118,17 +126,60 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest id = getId("192.168.2.1"); + final InstanceIdentifier
id = getId("192.168.2.1", IFACE_NAME); getCustomizer().readCurrentAttributes(id, builder, ctx); assertEquals("192.168.2.1", builder.getIp().getValue()); } + @Test + public void testReadCurrentAttributesFor2Ifcs() throws ReadFailedException { + ModificationCache cache = new ModificationCache(); + + IpAddressDetails detail1 = new IpAddressDetails(); + IpAddressDetails detail2 = new IpAddressDetails(); + + detail1.ip = reverseBytes( + TranslateUtils.ipv4AddressNoZoneToArray(new Ipv4AddressNoZone(new Ipv4Address("192.168.2.1")))); + detail2.ip = reverseBytes( + TranslateUtils.ipv4AddressNoZoneToArray(new Ipv4AddressNoZone(new Ipv4Address("192.168.2.2")))); + + IpAddressDetailsReplyDump reply = new IpAddressDetailsReplyDump(); + reply.ipAddressDetails = ImmutableList.of(detail1); + IpAddressDetailsReplyDump reply2 = new IpAddressDetailsReplyDump(); + reply2.ipAddressDetails = ImmutableList.of(detail2); + + CompletableFuture future = new CompletableFuture<>(); + future.complete(reply); + CompletableFuture future2 = new CompletableFuture<>(); + future2.complete(reply2); + + when(api.ipAddressDump(Mockito.any(IpAddressDump.class))).thenReturn(future).thenReturn(future2); + when(ctx.getModificationCache()).thenReturn(cache); + + final InstanceIdentifier
id = getId("192.168.2.1", IFACE_NAME); + final InstanceIdentifier
id2 = getId("192.168.2.2", IFACE_2_NAME); + + final List ifc1Ids = getCustomizer().getAllIds(id, ctx); + assertThat(ifc1Ids.size(), is(1)); + assertThat(ifc1Ids, CoreMatchers.hasItem(new AddressKey(new Ipv4AddressNoZone("192.168.2.1")))); + final List ifc2Ids = getCustomizer().getAllIds(id2, ctx); + assertThat(ifc2Ids.size(), is(1)); + assertThat(ifc2Ids, CoreMatchers.hasItem(new AddressKey(new Ipv4AddressNoZone("192.168.2.2")))); + + AddressBuilder builder = new AddressBuilder(); + getCustomizer().readCurrentAttributes(id, builder, ctx); + assertEquals(builder.getIp().getValue(), "192.168.2.1"); + builder = new AddressBuilder(); + getCustomizer().readCurrentAttributes(id2, builder, ctx); + assertEquals(builder.getIp().getValue(), "192.168.2.2"); + } + @Test public void testReadCurrentAttributesFromOperationalData() throws ReadFailedException { ModificationCache cache = new ModificationCache(); @@ -155,7 +206,7 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest id = getId("192.168.2.1"); + final InstanceIdentifier
id = getId("192.168.2.1", IFACE_NAME); getCustomizer().readCurrentAttributes(id, builder, ctx); @@ -180,10 +231,10 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest id = getId("192.168.2.1"); + final InstanceIdentifier
id = getId("192.168.2.1", IFACE_NAME); List ids = getCustomizer().getAllIds(id, ctx).stream() .map(key -> key.getIp()) @@ -220,7 +271,7 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest id = getId("192.168.2.1"); + final InstanceIdentifier
id = getId("192.168.2.1", IFACE_NAME); List ids = getCustomizer().getAllIds(id, ctx).stream() .map(key -> key.getIp()) -- cgit 1.2.3-korg