BUG-3300 : Fixed BGP transitive attribute filtering

Namespace in AttributeOperation were hardcode therefore any transitive
attribute with different namespace(inet..) were not valid, reason why
FromExternalImportPolicy did not return any attribute, causing a NPE.
Fixed them by matching extension QName to corresponding Attribute.

Change-Id: Ic3ccf5eb0b1d181baaf8ea5b6dea6f84828b92a4
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AttributeOperations.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AttributeOperations.java
index c997046849585d5afc7fec9acfabd7ae7aaf0ff9..6992b7ffbee9d3e69d385b142ef6f7b5c49fe3e4 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AttributeOperations.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AttributeOperations.java
@@ -12,10 +12,9 @@ import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Sets;
+import com.google.common.collect.ImmutableSet;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.Set;
 import org.opendaylight.protocol.util.Values;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.path.attributes.attributes.AsPath;
@@ -27,10 +26,10 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.mess
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.path.attributes.attributes.UnrecognizedAttributes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.path.attributes.attributes.as.path.Segments;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.ClusterIdentifier;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.NextHop;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.as.path.segment.CSegment;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.as.path.segment.c.segment.a.list._case.AList;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.as.path.segment.c.segment.a.list._case.a.list.AsSequence;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.next.hop.CNextHop;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.QNameModule;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
@@ -69,7 +68,20 @@ final class AttributeOperations {
                 return new AttributeOperations(key);
             }
         });
-
+    private static final LoadingCache<QNameModule, ImmutableSet<QName>> TRANSITIVE_CACHE = CacheBuilder.newBuilder()
+        .weakKeys()
+        .weakValues().build(
+            new CacheLoader<QNameModule, ImmutableSet<QName>>() {
+                @Override
+                public ImmutableSet<QName> load(final QNameModule key) {
+                    return ImmutableSet.of(QName.cachedReference(QName.create(key, Origin.QNAME.getLocalName())),
+                        QName.cachedReference(QName.create(key, AsPath.QNAME.getLocalName())),
+                        QName.cachedReference(QName.create(key, CNextHop.QNAME.getLocalName())),
+                        QName.cachedReference(QName.create(key, Communities.QNAME.getLocalName())),
+                        QName.cachedReference(QName.create(key, ExtendedCommunities.QNAME.getLocalName())));
+                }
+            });
+    private final ImmutableSet<QName> transitiveCollection;
     private final Iterable<PathArgument> originatorIdPath;
     private final Iterable<PathArgument> clusterListPath;
     private final NodeIdentifier originatorIdContainer;
@@ -84,10 +96,6 @@ final class AttributeOperations {
     private final NodeIdentifier asPathId;
     private final NodeIdentifier transitiveLeaf;
 
-    // FIXME: get this list instead of hardcoding
-    private static final Set<QName> TRANSITIVES = Sets.newHashSet(Origin.QNAME, AsPath.QNAME, NextHop.QNAME, Communities.QNAME,
-        ExtendedCommunities.QNAME);
-
     private AttributeOperations(final QNameModule namespace) {
         this.asPathContainer = new NodeIdentifier(QName.cachedReference(QName.create(namespace, AsPath.QNAME.getLocalName())));
         this.asPathSegments = new NodeIdentifier(QName.cachedReference(QName.create(namespace, Segments.QNAME.getLocalName())));
@@ -104,10 +112,11 @@ final class AttributeOperations {
         this.originatorIdPath = ImmutableList.<PathArgument>of(this.originatorIdContainer, this.originatorIdLeaf);
 
         this.transitiveLeaf = new NodeIdentifier(QName.cachedReference(QName.create(UnrecognizedAttributes.QNAME, "transitive")));
+        this.transitiveCollection = TRANSITIVE_CACHE.getUnchecked(namespace);
     }
 
     static AttributeOperations getInstance(final ContainerNode attributes) {
-        return ATTRIBUTES_CACHE.getUnchecked(QNameModule.cachedReference(attributes.getNodeType().getModule()));
+        return ATTRIBUTES_CACHE.getUnchecked(attributes.getNodeType().getModule());
     }
 
     private Collection<UnkeyedListEntryNode> reusableSequence(final UnkeyedListEntryNode segment) {
@@ -197,7 +206,6 @@ final class AttributeOperations {
         return b.build();
     }
 
-
     // Attributes when reflecting a route
     ContainerNode reflectedAttributes(final ContainerNode attributes, final Ipv4Address originatorId, final ClusterIdentifier clusterId) {
         final DataContainerNodeAttrBuilder<NodeIdentifier, ContainerNode> b = Builders.containerBuilder(attributes);
@@ -250,13 +258,13 @@ final class AttributeOperations {
             final AugmentationIdentifier ai = (AugmentationIdentifier) child.getIdentifier();
             for (final QName name : ai.getPossibleChildNames()) {
                 LOG.trace("Augmented QNAME {}", name);
-                if (TRANSITIVES.contains(name)) {
+                if (transitiveCollection.contains(name)) {
                     return true;
                 }
             }
             return false;
         }
-        if (TRANSITIVES.contains(child.getNodeType())) {
+        if (transitiveCollection.contains(child.getNodeType())) {
             return true;
         }
         if (UnrecognizedAttributes.QNAME.equals(child.getNodeType())) {
@@ -321,11 +329,10 @@ final class AttributeOperations {
 
         final NormalizedNode<?, ?> originatorId = maybeOriginatorId.get();
         if (originatorId instanceof LeafNode) {
-            return ((LeafNode<?>)originatorId).getValue();
+            return ((LeafNode<?>) originatorId).getValue();
         }
 
         LOG.warn("Unexpected ORIGINATOR_ID node {}, ignoring it", originatorId);
         return null;
     }
-
 }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/FromExternalImportPolicyTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/FromExternalImportPolicyTest.java
new file mode 100644 (file)
index 0000000..0c57c33
--- /dev/null
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/FromExternalImportPolicyTest.java
@@ -0,0 +1,100 @@
+package org.opendaylight.protocol.bgp.rib.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
+import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode;
+import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeAttrBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeSchemaAwareBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableLeafNodeBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableLeafSetEntryNodeBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableLeafSetNodeBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableUnkeyedListNodeBuilder;
+
+/**
+ * Created by cgasparini on 10.6.2015.
+ */
+public class FromExternalImportPolicyTest {
+
+    private static final QName DATA_QNAME = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "attributes"));
+    private static final QName LOCALPREF = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "local-pref"));
+    private static final QName CLUSTERID = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "cluster-id"));
+    private static final QName CLUSTER = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "cluster"));
+    private static final QName ASPATH = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "as-path"));
+    private static final QName SEGMENT = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "segments"));
+    private static final QName UNRECOGNIZED = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "unrecognized-attributes"));
+    private static final QName NEXTHOP = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "c-next-hop"));
+    private static final QName IPV4NH = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "ipv4-next-hop"));
+    private static final QName ORIGINATOR = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "originator-id"));
+    private static final QName MED = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "multi-exit-disc"));
+    private static final QName ORIGIN = QName.cachedReference(QName.create("urn:opendaylight:params:xml:ns:yang:bgp-inet", "2013-09-19", "origin"));
+
+    @Test
+    public void testEffectiveAttributes() {
+        DataContainerNodeAttrBuilder<NodeIdentifier, ContainerNode> dataContBuilder = createContBuilder(this.DATA_QNAME);
+        // local pref
+        dataContBuilder.addChild(createContBuilder(LOCALPREF).addChild(createValueBuilder(100L, LOCALPREF, "pref").build()).build());
+
+        // cluster pref
+        String s = "404.40.40.40";
+        LeafSetEntryNode<Object> entry1 = ImmutableLeafSetEntryNodeBuilder.create().withNodeIdentifier(
+            new NodeWithValue(CLUSTER, s)).withValue(s).build();
+
+        dataContBuilder.addChild(createContBuilder(CLUSTERID).addChild(ImmutableLeafSetNodeBuilder.create().withNodeIdentifier(
+            new NodeIdentifier(QName.create(CLUSTER, "cluster"))).withChild(entry1).build()).build());
+
+        // as-path pref
+        final ContainerNode asPath = createContBuilder(ASPATH).addChild(ImmutableUnkeyedListNodeBuilder.create()
+            .withNodeIdentifier(new NodeIdentifier(SEGMENT)).build()).build();
+        dataContBuilder.addChild(asPath);
+
+        // unrecognized
+        dataContBuilder.addChild(ImmutableNodes.mapNodeBuilder(UNRECOGNIZED).build());
+
+        // c-next-hop pref
+        final DataContainerNodeBuilder<NodeIdentifier, ChoiceNode> nextHop = Builders.choiceBuilder();
+        nextHop.withNodeIdentifier(new NodeIdentifier(NEXTHOP));
+        final ContainerNode cNextHop = createContBuilder(IPV4NH).addChild(createValueBuilder("199.20.160.41", IPV4NH, "global").build()).build();
+        final ChoiceNode resultNextHop = nextHop.addChild(cNextHop).build();
+        dataContBuilder.addChild(resultNextHop);
+
+        // originator pref
+        dataContBuilder.addChild(createContBuilder(ORIGINATOR).addChild(createValueBuilder("41.41.41.41", ORIGINATOR, "originator").build()).build());
+
+        // origin pref
+        final ContainerNode origin = createContBuilder(ORIGIN).addChild(createValueBuilder("igp", ORIGIN, "value").build()).build();
+        dataContBuilder.addChild(origin);
+
+        // multi-exit-disc pref
+        dataContBuilder.addChild(createContBuilder(MED).addChild(createValueBuilder("0", MED, "med").build()).build());
+        FromExternalImportPolicy importPol = new FromExternalImportPolicy();
+        final ContainerNode result = importPol.effectiveAttributes(dataContBuilder.build());
+
+        DataContainerNodeAttrBuilder<NodeIdentifier, ContainerNode> dataContExpected = createContBuilder(this.DATA_QNAME);
+
+        dataContExpected.addChild(asPath);
+        dataContExpected.addChild(resultNextHop);
+        dataContExpected.addChild(origin);
+
+        assertEquals(dataContExpected.build(), result);
+    }
+
+    private DataContainerNodeAttrBuilder<NodeIdentifier, ContainerNode> createContBuilder(final QName qname) {
+        return ImmutableContainerNodeSchemaAwareBuilder.create().withNodeIdentifier(new NodeIdentifier(qname));
+    }
+
+    private <T> ImmutableLeafNodeBuilder<T> createValueBuilder(final T value, final QName qname, final String localName) {
+        final ImmutableLeafNodeBuilder<T> valueBuilder = new ImmutableLeafNodeBuilder<>();
+        valueBuilder.withNodeIdentifier(new NodeIdentifier(QName.create(qname, localName))).withValue(value);
+        return valueBuilder;
+    }
+
+}
\ No newline at end of file