From f100b1a7d1689194f08bb22e1b849c4d840dadd0 Mon Sep 17 00:00:00 2001 From: Marek Gradzki Date: Thu, 24 Aug 2017 13:08:49 +0200 Subject: NamingContext.getNameIfPresent should not fail if name is missing Also makes InterfaceChangeNotificationProducer notification translation code more defensive. The issue was revealed by HC2VPP-216 and HC2VPP-220. Change-Id: I20792a51743ae621d86c1b9066d680bc2303ed82 Signed-off-by: Marek Gradzki --- .../InterfaceChangeNotificationProducer.java | 8 ++++- .../common/translate/util/NamingContext.java | 20 +++++++++--- .../common/translate/util/NamingContextTest.java | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/notification/InterfaceChangeNotificationProducer.java b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/notification/InterfaceChangeNotificationProducer.java index ffab221ee..b4ea48cf7 100644 --- a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/notification/InterfaceChangeNotificationProducer.java +++ b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/notification/InterfaceChangeNotificationProducer.java @@ -80,7 +80,13 @@ public final class InterfaceChangeNotificationProducer implements ManagedNotific swInterfaceSetFlagsNotification -> { LOG.trace("Interface notification received: {}", swInterfaceSetFlagsNotification); // TODO HONEYCOMB-166 this should be lazy - collector.onNotification(transformNotification(swInterfaceSetFlagsNotification)); + try { + collector.onNotification(transformNotification(swInterfaceSetFlagsNotification)); + } catch (Exception e) { + // There is no need to propagate exception to jvpp rx thread in case of unexpected failures. + // We can't do much about it, so lets log the exception. + LOG.warn("Failed to process interface notification {}", swInterfaceSetFlagsNotification, e); + } } ); } diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/hc2vpp/common/translate/util/NamingContext.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/hc2vpp/common/translate/util/NamingContext.java index 5e0eea71b..8965de616 100644 --- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/hc2vpp/common/translate/util/NamingContext.java +++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/hc2vpp/common/translate/util/NamingContext.java @@ -22,7 +22,9 @@ import static com.google.common.base.Preconditions.checkState; import com.google.common.base.Optional; import io.fd.honeycomb.translate.MappingContext; import io.fd.honeycomb.translate.util.RWUtils; +import java.util.List; import java.util.stream.Collector; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.Contexts; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.NamingContextKey; @@ -92,13 +94,21 @@ public final class NamingContext implements AutoCloseable { public synchronized Optional getNameIfPresent(final int index, @Nonnull final MappingContext mappingContext) { final Optional read = mappingContext.read(namingContextIid.child(Mappings.class)); + if (!read.isPresent()) { + return Optional.absent(); + } - return read.isPresent() - ? Optional.of(read.get().getMapping().stream() + final List mappings = read.get().getMapping().stream() .filter(mapping -> mapping.getIndex().equals(index)) - .collect(SINGLE_ITEM_COLLECTOR) - .getName()) - : Optional.absent(); + .collect(Collectors.toList()); + + if (mappings.size() > 1) { + throw new IllegalStateException("Multiple mappings defined with index=" + index + ": " + mappings); + } else if (mappings.size() == 1) { + return Optional.of(mappings.get(0).getName()); + } else { + return Optional.absent(); + } } /** diff --git a/vpp-common/vpp-translate-utils/src/test/java/io/fd/hc2vpp/common/translate/util/NamingContextTest.java b/vpp-common/vpp-translate-utils/src/test/java/io/fd/hc2vpp/common/translate/util/NamingContextTest.java index 4e66315e1..b9c0f2c0f 100644 --- a/vpp-common/vpp-translate-utils/src/test/java/io/fd/hc2vpp/common/translate/util/NamingContextTest.java +++ b/vpp-common/vpp-translate-utils/src/test/java/io/fd/hc2vpp/common/translate/util/NamingContextTest.java @@ -17,11 +17,14 @@ package io.fd.hc2vpp.common.translate.util; import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.base.Optional; +import com.google.common.collect.Lists; import io.fd.honeycomb.test.tools.HoneycombTestRunner; import io.fd.honeycomb.test.tools.annotations.InjectTestData; import io.fd.honeycomb.test.tools.annotations.InjectablesProcessor; @@ -40,6 +43,7 @@ import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.cont import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.Contexts; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.NamingContextKey; 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.MappingsBuilder; import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.mappings.Mapping; 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.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.mappings.MappingKey; @@ -104,6 +108,40 @@ public class NamingContextTest implements InjectablesProcessor { .build()); } + @Test(expected = IllegalArgumentException.class) + public void getAndThrow() { + when(mappingContext.read(any())).thenReturn(Optional.absent()); + namingContext.getIndex("non-existing", mappingContext); + } + + @Test(expected = IllegalStateException.class) + public void getNameIfPresentFails() { + final Mapping mapping1 = mock(Mapping.class); + final Mapping mapping2 = mock(Mapping.class); + final Mappings mappings = new MappingsBuilder().setMapping(Lists.newArrayList(mapping1, mapping2)).build(); + when(mappingContext.read(namingContextIid.child(Mappings.class))).thenReturn(Optional.of(mappings)); + + namingContext.getNameIfPresent(0, mappingContext); + } + + @Test + public void getNameIfPresentReturnsAbsent() { + final Mapping mapping1 = new MappingBuilder().setIndex(1).setName(NAME_1).build(); + final Mappings mappings = new MappingsBuilder().setMapping(Lists.newArrayList(mapping1)).build(); + when(mappingContext.read(namingContextIid.child(Mappings.class))).thenReturn(Optional.of(mappings)); + + assertEquals(Optional.absent(), namingContext.getNameIfPresent(0, mappingContext)); + } + + @Test + public void getNameIfPresent() { + final Mapping mapping1 = new MappingBuilder().setIndex(1).setName(NAME_1).build(); + final Mappings mappings = new MappingsBuilder().setMapping(Lists.newArrayList(mapping1)).build(); + when(mappingContext.read(namingContextIid.child(Mappings.class))).thenReturn(Optional.of(mappings)); + + assertEquals(Optional.of(NAME_1), namingContext.getNameIfPresent(1, mappingContext)); + } + private Mapping filterForParent(final String parent) { return mappings.getMapping().stream() .filter(mapping -> mapping.getName().equals(parent)) -- cgit 1.2.3-korg