Refactor ExportPolicyTracker and fix AdjRibOutListener 40/38940/7
authorClaudio D. Gasparini <cgaspari@cisco.com>
Wed, 11 May 2016 12:11:59 +0000 (14:11 +0200)
committerClaudio D. Gasparini <cgaspari@cisco.com>
Thu, 19 May 2016 10:28:26 +0000 (10:28 +0000)
-Lamdafication of ExportPolicyTracker
-Fix registration of AdjRiboutlisteners
-Filtering Internal role from PeerExportGroup creation.

Change-Id: If40abc38fe9408971eb92b6351fae364ee0f8601
Signed-off-by: Claudio D. Gasparini <cgaspari@cisco.com>
bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseAbstractRouteEntry.java
bgp/path-selection-mode/src/test/java/org/opendaylight/protocol/bgp/mode/impl/AbstractRouteEntryTest.java
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/AdjRibOutListener.java
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPPeer.java
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/ExportPolicyPeerTrackerImpl.java
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/LocRibWriter.java
bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/PeerExportGroupImpl.java
bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/IdentifierUtils.java
bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/PeerExportGroup.java

index 419e9d6f1978303caf35772deeeed7682a5c2425..fbb7ae5b577f37a490bab9772f3eb287965e7627 100644 (file)
@@ -20,6 +20,7 @@ import org.opendaylight.protocol.bgp.mode.spi.AbstractRouteEntry;
 import org.opendaylight.protocol.bgp.rib.spi.CacheDisconnectedPeers;
 import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker;
 import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup;
+import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup.PeerExporTuple;
 import org.opendaylight.protocol.bgp.rib.spi.RIBSupport;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole;
@@ -161,15 +162,16 @@ public abstract class AddPathAbstractRouteEntry extends AbstractRouteEntry {
             final PeerExportGroup peerGroup = peerPT.getPeerGroup(role);
             if (peerGroup != null) {
                 final ContainerNode effectiveAttributes = peerGroup.effectiveAttributes(routePeerId, attributes);
-                for (final Map.Entry<PeerId, YangInstanceIdentifier> pid : peerGroup.getPeers()) {
+                for (final Map.Entry<PeerId, PeerExporTuple> pid : peerGroup.getPeers()) {
                     final PeerId destPeer = pid.getKey();
                     final boolean destPeerSupAddPath = peerPT.isAddPathSupportedByPeer(destPeer);
                     if (filterRoutes(routePeerId, destPeer, peerPT, localTK, discPeers) && peersSupportsAddPathOrIsFirstBestPath(destPeerSupAddPath, isFirstBestPath)) {
                         if (destPeerSupAddPath) {
-                            update(destPeer, getAdjRibOutYII(ribSup, pid.getValue(), routeIdAddPath, localTK), effectiveAttributes, addPathValue,
+                            update(destPeer, getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeIdAddPath, localTK), effectiveAttributes,
+                                addPathValue,
                                 ribSup, tx);
                         } else {
-                            update(destPeer, getAdjRibOutYII(ribSup, pid.getValue(), routeId, localTK), effectiveAttributes, value, ribSup, tx);
+                            update(destPeer, getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effectiveAttributes, value, ribSup, tx);
                         }
                     }
                 }
index 83588cd79f4a02166ccbf12be31e23a7f9caf967..18ee1ed3c4b898792636adf47b3e0213c92d5276 100644 (file)
@@ -184,7 +184,7 @@ abstract class BaseAbstractRouteEntry extends AbstractRouteEntry {
                 final ContainerNode effAttrib = peerGroup.effectiveAttributes(routePeerId, attributes);
                 peerGroup.getPeers().stream()
                     .filter(pid -> filterRoutes(routePeerId, pid.getKey(), peerPT, localTK, discPeers))
-                    .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue(), routeId, localTK), effAttrib, value, ribSup, tx));
+                    .forEach(pid -> update(pid.getKey(), getAdjRibOutYII(ribSup, pid.getValue().getYii(), routeId, localTK), effAttrib, value, ribSup, tx));
             }
         }
     }
index 8f156e59ea46d63bf285513bbe463c0f1ad25b9b..bad58b64398ed0b8bf63a7fb28cebe8917eacb87 100644 (file)
@@ -11,16 +11,11 @@ package org.opendaylight.protocol.bgp.mode.impl;
 import static org.opendaylight.protocol.bgp.mode.impl.base.BasePathSelectorTest.ATTRS_EXTENSION_Q;
 import static org.opendaylight.protocol.bgp.mode.impl.base.BasePathSelectorTest.SEGMENTS_NID;
 
-import com.google.common.base.Function;
-import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.primitives.UnsignedInteger;
-import java.util.AbstractMap;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.mockito.Mock;
@@ -82,8 +77,6 @@ public class AbstractRouteEntryTest {
     private static final NodeIdentifier ORIGIN_VALUE_NID = new NodeIdentifier(QName.create(ATTRS_EXTENSION_Q, "value").intern());
     private static final NodeIdentifier AS_PATH_NID = new NodeIdentifier(QName.create(ATTRS_EXTENSION_Q, AsPath.QNAME.getLocalName()).intern());
     private static final NodeIdentifier ATOMIC_NID = new NodeIdentifier(QName.create(ATTRS_EXTENSION_Q, AtomicAggregate.QNAME.getLocalName()));
-    private static final Function<YangInstanceIdentifier, Map.Entry<PeerId, YangInstanceIdentifier>> GENERATE_PEER_ID =
-        input -> PEER_YII.equals(input) ? new AbstractMap.SimpleImmutableEntry<>(PEER_ID, input) : new AbstractMap.SimpleImmutableEntry<>(PEER_ID2, input);
     private static final QName Q_NAME = BindingReflections.findQName(Ipv4Routes.class).intern();
     private static final NodeIdentifier ROUTE_ATTRIBUTES_IDENTIFIER = new NodeIdentifier(QName.create(Q_NAME, Attributes.QNAME.getLocalName().intern()));
     private static final QName PREFIX_QNAME = QName.create(Ipv4Route.QNAME, "prefix").intern();
@@ -179,11 +172,14 @@ public class AbstractRouteEntryTest {
     private void mockExportGroup() {
         Mockito.doReturn(this.attributes).when(this.peg).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class));
         Mockito.doReturn(null).when(this.pegNot).effectiveAttributes(Mockito.any(PeerId.class), Mockito.any(ContainerNode.class));
-        Collection<YangInstanceIdentifier> value = Arrays.asList(PEER_YII, PEER_YII2);
-        final Collection<Map.Entry<PeerId, YangInstanceIdentifier>> peers = ImmutableList.copyOf(Collections2.transform(value, GENERATE_PEER_ID));
-        final Collection<Map.Entry<PeerId, YangInstanceIdentifier>> peersEmpty = ImmutableList.copyOf(Collections2.transform(Collections.<YangInstanceIdentifier>emptyList(), GENERATE_PEER_ID));
-        Mockito.doReturn(peers).when(this.peg).getPeers();
-        Mockito.doReturn(peersEmpty).when(this.pegNot).getPeers();
+
+        Map<PeerId, PeerExportGroup.PeerExporTuple> peers = new HashMap<>();
+        Mockito.doReturn(ImmutableList.copyOf(peers.entrySet())).when(this.pegNot).getPeers();
+
+        peers.put(PEER_ID, new PeerExportGroup.PeerExporTuple(PEER_YII, PeerRole.Ibgp));
+        peers.put(PEER_ID2, new PeerExportGroup.PeerExporTuple(PEER_YII2, PeerRole.Ibgp));
+
+        Mockito.doReturn(ImmutableList.copyOf(peers.entrySet())).when(this.peg).getPeers();
     }
 
     private void mockExportPolicies() {
index 193c6287238cb7b27b668eccad2d93be2513568d..ab41b7a2566eaacbd6000b4553f33b55b2851b67 100644 (file)
@@ -164,4 +164,8 @@ final class AdjRibOutListener implements DOMDataTreeChangeListener {
     public void close() {
         this.registerDataTreeChangeListener.close();
     }
+
+    boolean isMpSupported() {
+        return this.mpSupport;
+    }
 }
index 9e332413d2ac451c2ca93ff894f211f6af2586cb..60ffb5a89a8baa96b67dcf470a13b1703d293efd 100644 (file)
@@ -155,7 +155,7 @@ public class BGPPeer implements BGPSessionListener, Peer, AutoCloseable, BGPPeer
         if (listener != null) {
             listener.close();
             this.adjRibOutListenerSet.remove(listener);
-            createAdjRibOutListener(RouterIds.createPeerId(session.getBgpId()), key, true);
+            createAdjRibOutListener(RouterIds.createPeerId(session.getBgpId()), key, listener.isMpSupported());
         } else {
             LOG.info("Ignoring RouteRefresh message. Afi/Safi is not supported: {}, {}.", rrAfi, rrSafi);
         }
@@ -284,8 +284,7 @@ public class BGPPeer implements BGPSessionListener, Peer, AutoCloseable, BGPPeer
     //try to add a support for old-school BGP-4, if peer did not advertise IPv4-Unicast MP capability
     private void addBgp4Support(final PeerId peerId, final boolean announceNone) {
         final TablesKey key = new TablesKey(Ipv4AddressFamily.class, UnicastSubsequentAddressFamily.class);
-        this.tables.add(key);
-        if (!announceNone) {
+        if (this.tables.add(key) && !announceNone) {
             createAdjRibOutListener(peerId, key, false);
         }
     }
index 35b1d78c5e61b87a1bb4198e4017f708e459848c..197c9c793fcff3f0ad48f962b4096f2945ef4fc2 100644 (file)
@@ -7,28 +7,25 @@
  */
 package org.opendaylight.protocol.bgp.rib.impl;
 
-import com.google.common.base.Function;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Collections2;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
-import java.util.AbstractMap;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker;
 import org.opendaylight.protocol.bgp.rib.spi.IdentifierUtils;
 import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup;
+import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup.PeerExporTuple;
 import org.opendaylight.protocol.bgp.rib.spi.RibSupportUtils;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev130919.SendReceive;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
@@ -48,13 +45,6 @@ import org.slf4j.LoggerFactory;
 
 final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker {
     private static final Logger LOG = LoggerFactory.getLogger(ExportPolicyPeerTrackerImpl.class);
-    private static final Function<YangInstanceIdentifier, Entry<PeerId, YangInstanceIdentifier>> GENERATE_PEER_ID = new Function<YangInstanceIdentifier, Entry<PeerId, YangInstanceIdentifier>>() {
-        @Override
-        public Entry<PeerId, YangInstanceIdentifier> apply(final YangInstanceIdentifier input) {
-            final PeerId peerId = IdentifierUtils.peerId((NodeIdentifierWithPredicates) input.getLastPathArgument());
-            return new AbstractMap.SimpleImmutableEntry<>(peerId, input);
-        }
-    };
     private static final QName SEND_RECEIVE = QName.create(SupportedTables.QNAME, "send-receive").intern();
     private static final NodeIdentifier SEND_RECEIVE_NID = new NodeIdentifier(SEND_RECEIVE);
     private final Map<YangInstanceIdentifier, PeerRole> peerRoles = new HashMap<>();
@@ -74,24 +64,12 @@ final class ExportPolicyPeerTrackerImpl implements ExportPolicyPeerTracker {
             return Collections.emptyMap();
         }
 
-        // Index things nicely for easy access
-        final Multimap<PeerRole, YangInstanceIdentifier> roleToIds = ArrayListMultimap.create(PeerRole.values().length, 2);
-        final Map<PeerId, PeerRole> idToRole = new HashMap<>();
-        for (final Entry<YangInstanceIdentifier, PeerRole> e : peerPathRoles.entrySet()) {
-            roleToIds.put(e.getValue(), e.getKey());
-            idToRole.put(IdentifierUtils.peerId((NodeIdentifierWithPredicates) e.getKey().getLastPathArgument()), e.getValue());
-        }
-
-        // Optimized immutable copy, reused for all PeerGroups
-        final Map<PeerId, PeerRole> allPeerRoles = ImmutableMap.copyOf(idToRole);
-
-        final Map<PeerRole, PeerExportGroup> ret = new EnumMap<>(PeerRole.class);
-        for (final Entry<PeerRole, Collection<YangInstanceIdentifier>> e : roleToIds.asMap().entrySet()) {
-            final AbstractExportPolicy policy = this.policyDatabase.exportPolicyForRole(e.getKey());
-            final Collection<Entry<PeerId, YangInstanceIdentifier>> peers = ImmutableList.copyOf(Collections2.transform(e.getValue(), GENERATE_PEER_ID));
+        final Map<PeerId, PeerExporTuple> immutablePeers = ImmutableMap.copyOf(peerPathRoles.entrySet().stream()
+            .collect(toMap(peer -> IdentifierUtils.peerKeyToPeerId(peer.getKey()), peer -> new PeerExporTuple(peer.getKey(), peer.getValue()))));
 
-            ret.put(e.getKey(), new PeerExportGroupImpl(peers, allPeerRoles, policy));
-        }
+        final Map<PeerRole, PeerExportGroup> ret = peerPathRoles.values().stream().collect(Collectors.toSet()).stream().filter(role -> role != PeerRole.Internal)
+            .collect(toMap(identity(), role -> new PeerExportGroupImpl(immutablePeers, this.policyDatabase.exportPolicyForRole(role)),
+                (oldKey, newKey) -> oldKey, () -> new EnumMap<>(PeerRole.class)));
 
         return ret;
     }
index 0519a9a930d436bbc46a8ecfd1ae33765c744658..1f6668e8ac77d01cb998e8c085b32c718ebdcc52 100644 (file)
@@ -158,8 +158,8 @@ final class LocRibWriter implements AutoCloseable, DOMDataTreeChangeListener {
         for (final DataTreeCandidate tc : changes) {
             final YangInstanceIdentifier rootPath = tc.getRootPath();
             final DataTreeCandidateNode rootNode = tc.getRootNode();
-            final NodeIdentifierWithPredicates peerKey = IdentifierUtils.peerKey(rootPath);
-            final PeerId peerId = IdentifierUtils.peerId(peerKey);
+            final PeerId peerId = IdentifierUtils.peerKeyToPeerId(rootPath);
+
             filterOutPeerRole(peerId, rootNode, rootPath);
             filterOutChangesToSupportedTables(peerId, rootNode);
             filterOutAnyChangeOutsideEffRibsIn(peerId, rootNode, ret, rootPath, tx);
@@ -215,8 +215,8 @@ final class LocRibWriter implements AutoCloseable, DOMDataTreeChangeListener {
             LOG.debug("Data Changed for Peer role {} path {}, dataBefore {}, dataAfter {}", roleChange.getIdentifier(),
                 peerPath , roleChange.getDataBefore(), maybePeerRole);
             final PeerRole role = PeerRoleUtil.roleForChange(maybePeerRole);
-            SimpleRoutingPolicy srp = getSimpleRoutingPolicy(rootNode);
-            if(PeerRole.Internal == role || SimpleRoutingPolicy.AnnounceNone == srp) {
+            final SimpleRoutingPolicy srp = getSimpleRoutingPolicy(rootNode);
+            if(SimpleRoutingPolicy.AnnounceNone == srp) {
                 return;
             }
             this.peerPolicyTracker.peerRoleChanged(peerPath, role);
index 300fc4553dc09b359d9e2982ee5ae601c7ad8771..53bdbdc32203ff0e5150ae4d76434cd190935fe8 100644 (file)
@@ -10,32 +10,27 @@ package org.opendaylight.protocol.bgp.rib.impl;
 import com.google.common.base.Preconditions;
 import java.util.Collection;
 import java.util.Map;
-import java.util.Map.Entry;
 import org.opendaylight.protocol.bgp.rib.spi.PeerExportGroup;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 
 final class PeerExportGroupImpl implements PeerExportGroup {
-    private final Collection<Entry<PeerId, YangInstanceIdentifier>> peers;
-    private final Map<PeerId, PeerRole> peerRoles;
+    private final Map<PeerId, PeerExporTuple> peers;
     private final AbstractExportPolicy policy;
 
-    PeerExportGroupImpl(final Collection<Entry<PeerId, YangInstanceIdentifier>> peers, final Map<PeerId, PeerRole> peerRoles, final AbstractExportPolicy policy) {
+    public PeerExportGroupImpl(final Map<PeerId, PeerExporTuple> peers, final AbstractExportPolicy policy) {
         this.peers = Preconditions.checkNotNull(peers);
-        this.peerRoles = Preconditions.checkNotNull(peerRoles);
         this.policy = Preconditions.checkNotNull(policy);
     }
 
     @Override
     public ContainerNode effectiveAttributes(final PeerId sourcePeerId, final ContainerNode attributes) {
-        final PeerRole peerRole = peerRoles.get(sourcePeerId);
-        return attributes == null || peerRole == null ? null :  policy.effectiveAttributes(peerRole, attributes);
+        final PeerExporTuple peer = this.peers.get(sourcePeerId);
+        return attributes == null || peer == null ? null : policy.effectiveAttributes(peer.getRole(), attributes);
     }
 
     @Override
-    public Collection<Entry<PeerId, YangInstanceIdentifier>> getPeers() {
-        return peers;
+    public Collection<Map.Entry<PeerId, PeerExporTuple>> getPeers() {
+        return peers.entrySet();
     }
 }
\ No newline at end of file
index 65f3e3c644e5173c90b008b5b3f3b341bfdb8a7e..af9195bd671d06d0b2af2293979116387c363553 100644 (file)
@@ -19,18 +19,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdent
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 
 public final class IdentifierUtils {
-    private static final Predicate<PathArgument> IS_PEER = new Predicate<PathArgument>() {
-        @Override
-        public boolean apply(final PathArgument input) {
-            return input instanceof NodeIdentifierWithPredicates && Peer.QNAME.equals(input.getNodeType());
-        }
-    };
-    private static final Predicate<PathArgument> IS_TABLES = new Predicate<PathArgument>() {
-        @Override
-        public boolean apply(final PathArgument input) {
-            return Tables.QNAME.equals(input.getNodeType());
-        }
-    };
+    private static final Predicate<PathArgument> IS_PEER = input -> input instanceof NodeIdentifierWithPredicates && Peer.QNAME.equals(input.getNodeType());
+    private static final Predicate<PathArgument> IS_TABLES = input -> Tables.QNAME.equals(input.getNodeType());
     private static final QName PEER_ID = QName.create(Peer.QNAME, "peer-id").intern();
 
     private IdentifierUtils() {
@@ -64,6 +54,11 @@ public final class IdentifierUtils {
         return new PeerId((String) peerKey.getKeyValues().get(PEER_ID));
     }
 
+    public static PeerId peerKeyToPeerId(final YangInstanceIdentifier id) {
+        return peerId(peerKey(id));
+    }
+
+
     static NodeIdentifierWithPredicates tableKey(final YangInstanceIdentifier id) {
         return firstKeyOf(id, IS_TABLES);
     }
index 0b38810fde2a285a67280283ffc6bc3fbd7fa94b..b9c82878987bce113287e8fc4f4128d2d43924f1 100644 (file)
@@ -11,6 +11,7 @@ package org.opendaylight.protocol.bgp.rib.spi;
 import java.util.Collection;
 import java.util.Map;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.PeerRole;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 
@@ -18,8 +19,27 @@ import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
  * A collection of peers sharing the same export policy.
  */
 public interface PeerExportGroup {
+    final class PeerExporTuple {
+        private final YangInstanceIdentifier yii;
+        private final PeerRole role;
+
+        public PeerExporTuple(final YangInstanceIdentifier yii, final PeerRole role) {
+            this.yii = yii;
+            this.role = role;
+        }
+
+        public YangInstanceIdentifier getYii() {
+            return yii;
+        }
+
+        public PeerRole getRole() {
+            return role;
+        }
+    }
+
     /**
      * Transform outgoing attributes according to policy per Peer
+     *
      * @param sourcePeerId root Peer
      * @param attributes attributes container
      * @return return attributes container after apply policy
@@ -27,8 +47,7 @@ public interface PeerExportGroup {
     ContainerNode effectiveAttributes(PeerId sourcePeerId, ContainerNode attributes);
 
     /**
-     *
      * @return map of peer
      */
-    Collection<Map.Entry<PeerId, YangInstanceIdentifier>> getPeers();
+    Collection<Map.Entry<PeerId, PeerExporTuple>> getPeers();
 }