BGP Stats collector

simplify synchronization of stats

JIRA: BGPCEP-846
Change-Id: I55d30f9cddb33006fb8d2519ddbe9f8b330de71b
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImpl.java
index c06eb4d80f8496b2a5483c2c5941d6b7a4a5f215..0690720d8ba38fddcff2080b118a5681322017f5 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImpl.java
@@ -8,11 +8,10 @@
 package org.opendaylight.protocol.bgp.rib.impl.state;
 
 import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.stream.Collectors;
-import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 import org.opendaylight.protocol.bgp.rib.spi.state.BGPPeerState;
 import org.opendaylight.protocol.bgp.rib.spi.state.BGPPeerStateConsumer;
@@ -23,70 +22,42 @@ import org.opendaylight.protocol.bgp.rib.spi.state.BGPStateProvider;
 
 @ThreadSafe
 public class BGPStateCollectorImpl implements BGPStateProvider, BGPStateConsumer {
-    @GuardedBy("this")
-    private final List<BGPRibStateConsumer> bgpRibStates = new ArrayList<>();
-    @GuardedBy("this")
-    private final List<BGPPeerStateConsumer> bgpPeerStates = new ArrayList<>();
+    private final List<BGPRibStateConsumer> bgpRibStates = new CopyOnWriteArrayList<>();
+    private final List<BGPPeerStateConsumer> bgpPeerStates = new CopyOnWriteArrayList<>();
 
     @Override
     public List<BGPRibState> getRibStats() {
-        synchronized (this.bgpRibStates) {
-            return ImmutableList.copyOf(this.bgpRibStates
-                    .stream()
-                    .map(BGPRibStateConsumer::getRIBState)
-                    .filter(Objects::nonNull)
-                    .collect(Collectors.toList()));
-        }
+        return ImmutableList.copyOf(this.bgpRibStates.stream()
+            .map(BGPRibStateConsumer::getRIBState)
+            .filter(Objects::nonNull)
+            .collect(Collectors.toList()));
     }
 
     @Override
     public List<BGPPeerState> getPeerStats() {
-        synchronized (this.bgpPeerStates) {
-            return ImmutableList.copyOf(this.bgpPeerStates
-                    .stream()
-                    .map(BGPPeerStateConsumer::getPeerState)
-                    .filter(Objects::nonNull)
-                    .collect(Collectors.toList()));
-        }
+        return ImmutableList.copyOf(this.bgpPeerStates.stream()
+            .map(BGPPeerStateConsumer::getPeerState)
+            .filter(Objects::nonNull)
+            .collect(Collectors.toList()));
     }
 
     @Override
     public void bind(final BGPRibStateConsumer bgpState) {
-        if (bgpState == null) {
-            return;
-        }
-        synchronized (this.bgpRibStates) {
-            this.bgpRibStates.add(bgpState);
-        }
+        this.bgpRibStates.add(bgpState);
     }
 
     @Override
     public void unbind(final BGPRibStateConsumer bgpState) {
-        if (bgpState == null) {
-            return;
-        }
-        synchronized (this.bgpRibStates) {
-            this.bgpRibStates.remove(bgpState);
-        }
+        this.bgpRibStates.remove(bgpState);
     }
 
     @Override
     public void bind(final BGPPeerStateConsumer bgpState) {
-        if (bgpState == null) {
-            return;
-        }
-        synchronized (this.bgpPeerStates) {
-            this.bgpPeerStates.add(bgpState);
-        }
+        this.bgpPeerStates.add(bgpState);
     }
 
     @Override
     public void unbind(final BGPPeerStateConsumer bgpState) {
-        if (bgpState == null) {
-            return;
-        }
-        synchronized (this.bgpPeerStates) {
-            this.bgpPeerStates.remove(bgpState);
-        }
+        this.bgpPeerStates.remove(bgpState);
     }
 }

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImplTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImplTest.java
index 50af403923597de2fc30646cfb14cd876dea08c8..62b1f1d757b775a290ef909372efd0911d5352ea 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImplTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/state/BGPStateCollectorImplTest.java
@@ -29,22 +29,15 @@ public class BGPStateCollectorImplTest {
     private BGPPeerStateConsumer bgpPeerStateConsumer;
 
     @Before
-    public void setUp() throws Exception {
+    public void setUp() {
         MockitoAnnotations.initMocks(this);
     }
 
     @Test
-    public void getRibStatsTest() throws Exception {
+    public void getRibStatsTest() {
         doReturn(mock(BGPPeerState.class)).when(this.bgpPeerStateConsumer).getPeerState();
         doReturn(mock(BGPRibState.class)).when(this.bgpribStateConsumer).getRIBState();
         final BGPStateCollectorImpl collector = new BGPStateCollectorImpl();
-        final BGPRibStateConsumer ribStateConsumerNull = null;
-        collector.bind(ribStateConsumerNull);
-        assertTrue(collector.getRibStats().isEmpty());
-
-        final BGPPeerStateConsumer peerStateConsumerNull = null;
-        collector.bind(peerStateConsumerNull);
-        assertTrue(collector.getPeerStats().isEmpty());
 
         collector.bind(this.bgpribStateConsumer);
         collector.bind(this.bgpPeerStateConsumer);
@@ -58,17 +51,10 @@ public class BGPStateCollectorImplTest {
     }
 
     @Test
-    public void getRibStatsEmptyPeerTest() throws Exception {
+    public void getRibStatsEmptyPeerTest() {
         doReturn(mock(BGPRibState.class)).when(this.bgpribStateConsumer).getRIBState();
         doReturn(null).when(this.bgpPeerStateConsumer).getPeerState();
         final BGPStateCollectorImpl collector = new BGPStateCollectorImpl();
-        final BGPRibStateConsumer ribStateConsumerNull = null;
-        collector.bind(ribStateConsumerNull);
-        assertTrue(collector.getRibStats().isEmpty());
-
-        final BGPPeerStateConsumer peerStateConsumerNull = null;
-        collector.bind(peerStateConsumerNull);
-        assertTrue(collector.getPeerStats().isEmpty());
 
         collector.bind(this.bgpribStateConsumer);
         collector.bind(this.bgpPeerStateConsumer);
@@ -77,16 +63,11 @@ public class BGPStateCollectorImplTest {
     }
 
     @Test
-    public void getRibStatsEmptyRibTest() throws Exception {
+    public void getRibStatsEmptyRibTest() {
         doReturn(null).when(this.bgpribStateConsumer).getRIBState();
         doReturn(null).when(this.bgpPeerStateConsumer).getPeerState();
         final BGPStateCollectorImpl collector = new BGPStateCollectorImpl();
-        final BGPRibStateConsumer ribStateConsumerNull = null;
-        collector.bind(ribStateConsumerNull);
         assertTrue(collector.getRibStats().isEmpty());
-
-        final BGPPeerStateConsumer peerStateConsumerNull = null;
-        collector.bind(peerStateConsumerNull);
         assertTrue(collector.getPeerStats().isEmpty());
 
         collector.bind(this.bgpribStateConsumer);