Ditch static wiring from SoutboundProvider

The inclusion/exclusion lists are a natural part of
ReconciliationManager. Pass them there, so they can be easily picked up.

JIRA: OVSDB-468
Change-Id: Ib12873ca0cc5f8db71e5fa8b6fa58743914000cd
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java
index 6a0a8a54ed128ffe7e7599b13d71d15699452ee9..7cbd5b2e7fa85ce91aa351b2116de62a78a557ee 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java
@@ -64,21 +64,19 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoCloseable {
-
-    private final ConcurrentMap<ConnectionInfo, OvsdbConnectionInstance> clients = new ConcurrentHashMap<>();
     private static final Logger LOG = LoggerFactory.getLogger(OvsdbConnectionManager.class);
     private static final String ENTITY_TYPE = "ovsdb";
     private static final int DB_FETCH_TIMEOUT = 1000;
 
-    private final DataBroker db;
-    private final TransactionInvoker txInvoker;
-    private final Map<OvsdbClient, OvsdbClient> alreadyProcessedClients = new ConcurrentHashMap<>();
-    private final Map<ConnectionInfo,InstanceIdentifier<Node>> instanceIdentifiers =
-            new ConcurrentHashMap<>();
-    private final Map<InstanceIdentifier<Node>, OvsdbConnectionInstance> nodeIdVsConnectionInstance =
+    private final ConcurrentMap<ConnectionInfo, OvsdbConnectionInstance> clients = new ConcurrentHashMap<>();
+    private final ConcurrentMap<OvsdbClient, OvsdbClient> alreadyProcessedClients = new ConcurrentHashMap<>();
+    private final ConcurrentMap<ConnectionInfo,InstanceIdentifier<Node>> instanceIdentifiers =
             new ConcurrentHashMap<>();
-    private final Map<Entity, OvsdbConnectionInstance> entityConnectionMap =
+    private final ConcurrentMap<InstanceIdentifier<Node>, OvsdbConnectionInstance> nodeIdVsConnectionInstance =
             new ConcurrentHashMap<>();
+    private final ConcurrentMap<Entity, OvsdbConnectionInstance> entityConnectionMap = new ConcurrentHashMap<>();
+    private final DataBroker db;
+    private final TransactionInvoker txInvoker;
     private final EntityOwnershipService entityOwnershipService;
     private final OvsdbDeviceEntityOwnershipListener ovsdbDeviceEntityOwnershipListener;
     private final OvsdbConnection ovsdbConnection;
@@ -88,13 +86,16 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
     public OvsdbConnectionManager(final DataBroker db,final TransactionInvoker txInvoker,
                                   final EntityOwnershipService entityOwnershipService,
                                   final OvsdbConnection ovsdbConnection,
-                                  final InstanceIdentifierCodec instanceIdentifierCodec) {
+                                  final InstanceIdentifierCodec instanceIdentifierCodec,
+                                  final List<String> reconcileBridgeInclusionList,
+                                  final List<String> reconcileBridgeExclusionList) {
         this.db = db;
         this.txInvoker = txInvoker;
         this.entityOwnershipService = entityOwnershipService;
         ovsdbDeviceEntityOwnershipListener = new OvsdbDeviceEntityOwnershipListener(this, entityOwnershipService);
         this.ovsdbConnection = ovsdbConnection;
-        reconciliationManager = new ReconciliationManager(db, instanceIdentifierCodec);
+        reconciliationManager = new ReconciliationManager(db, instanceIdentifierCodec,
+            reconcileBridgeInclusionList, reconcileBridgeExclusionList);
         this.instanceIdentifierCodec = instanceIdentifierCodec;
     }
 
@@ -658,11 +659,8 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
     }
 
     private void reconcileBridgeConfigurations(final OvsdbConnectionInstance client) {
-        final InstanceIdentifier<Node> nodeIid = client.getInstanceIdentifier();
-        final ReconciliationTask task = new BridgeConfigReconciliationTask(
-                reconciliationManager, OvsdbConnectionManager.this, nodeIid, client, instanceIdentifierCodec);
-
-        reconciliationManager.enqueue(task);
+        reconciliationManager.enqueue(new BridgeConfigReconciliationTask(reconciliationManager, this,
+            client.getInstanceIdentifier(), client, instanceIdentifierCodec));
     }
 
     private static final class OvsdbDeviceEntityOwnershipListener implements EntityOwnershipListener {

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
index 7eabfa87a0f9dd8e0d049bdac5564e56493d3f85..25b0e5ec28049db2c4276e3bb41c1b96e3f32ca2 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/SouthboundProvider.java
@@ -72,25 +72,6 @@ public class SouthboundProvider implements DataTreeChangeListener<Topology>, Aut
     private static final Logger LOG = LoggerFactory.getLogger(SouthboundProvider.class);
     private static final String ENTITY_TYPE = "ovsdb-southbound-provider";
 
-    // FIXME: get rid of this static
-    private static List<String> reconcileBridgeInclusionList = List.of();
-    // FIXME: get rid of this static
-    private static List<String> reconcileBridgeExclusionList = List.of();
-
-    public static List<String> getBridgesReconciliationInclusionList() {
-        return reconcileBridgeInclusionList;
-    }
-
-    public static List<String> getBridgesReconciliationExclusionList() {
-        return reconcileBridgeExclusionList;
-    }
-
-    @VisibleForTesting
-    @SuppressFBWarnings("EI_EXPOSE_STATIC_REP2")
-    public static void setBridgesReconciliationInclusionList(final List<String> list) {
-        reconcileBridgeInclusionList = list;
-    }
-
     private final OvsdbConnectionManager cm;
     private final TransactionInvoker txInvoker;
     private final OvsdbDataTreeChangeListener ovsdbDataTreeChangeListener;
@@ -135,9 +116,8 @@ public class SouthboundProvider implements DataTreeChangeListener<Topology>, Aut
             List.of(configuration.bridge$_$reconciliation$_$exclusion$_$list()));
     }
 
-    @SuppressFBWarnings(
-        value = { "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR", "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" },
-        justification = "This is not a final class due to deep mocking. There is a FIXME for the static wiring above")
+    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR",
+        justification = "This is not a final class due to deep mocking")
     public SouthboundProvider(final DataBroker dataBroker,
                               final EntityOwnershipService entityOwnershipServiceDependency,
                               final OvsdbConnection ovsdbConnection,
@@ -146,12 +126,9 @@ public class SouthboundProvider implements DataTreeChangeListener<Topology>, Aut
                               final SystemReadyMonitor systemReadyMonitor,
                               final DiagStatusService diagStatusService,
                               final boolean skipMonitoringManagerStatus,
-                              final List<String> bridgeReconciliationInclusions,
-                              final List<String> bridgeReconciliationExclusions) {
+                              final List<String> bridgeReconciliationInclusionList,
+                              final List<String> bridgeReconciliationExclusionList) {
         this.dataBroker = requireNonNull(dataBroker);
-        // FIXME: get rid of this static wiring
-        reconcileBridgeInclusionList = bridgeReconciliationInclusions;
-        reconcileBridgeExclusionList = bridgeReconciliationExclusions;
         LOG.debug("skipManagerStatus set to {}", skipMonitoringManagerStatus);
         if (skipMonitoringManagerStatus) {
             SouthboundConstants.SKIP_COLUMN_FROM_TABLE.get("Manager").add("status");
@@ -170,7 +147,7 @@ public class SouthboundProvider implements DataTreeChangeListener<Topology>, Aut
         ovsdbStatusProvider.reportStatus(ServiceState.STARTING, "OVSDB initialization in progress");
         txInvoker = new TransactionInvokerImpl(dataBroker);
         cm = new OvsdbConnectionManager(dataBroker, txInvoker, entityOwnershipService, ovsdbConnection,
-                instanceIdentifierCodec);
+                instanceIdentifierCodec, bridgeReconciliationInclusionList, bridgeReconciliationExclusionList);
         ovsdbDataTreeChangeListener = new OvsdbDataTreeChangeListener(dataBroker, cm, instanceIdentifierCodec);
         ovsdbOperGlobalListener = new OvsdbOperGlobalListener(dataBroker, cm, txInvoker);
 

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/ReconciliationManager.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/ReconciliationManager.java
index 14308a2b85c8e38c368d8b572008439b4a0e3cc2..cb898ff074e8fb90ccfea0335f6ee0b0d4887cfd 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/ReconciliationManager.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/ReconciliationManager.java
@@ -88,10 +88,16 @@ public class ReconciliationManager implements AutoCloseable {
     private Registration bridgeCreatedDataTreeChangeRegistration = null;
 
     private final ReconciliationTaskManager reconTaskManager = new ReconciliationTaskManager();
+    private final List<String> reconcileBridgeInclusionList;
+    private final List<String> reconcileBridgeExclusionList;
 
-    public ReconciliationManager(final DataBroker db, final InstanceIdentifierCodec instanceIdentifierCodec) {
+    public ReconciliationManager(final DataBroker db, final InstanceIdentifierCodec instanceIdentifierCodec,
+            final List<String> reconcileBridgeInclusionList, final List<String> reconcileBridgeExclusionList) {
         this.db = db;
         this.instanceIdentifierCodec = instanceIdentifierCodec;
+        this.reconcileBridgeInclusionList = List.copyOf(reconcileBridgeInclusionList);
+        this.reconcileBridgeExclusionList = List.copyOf(reconcileBridgeExclusionList);
+
         reconcilers = SpecialExecutors.newBoundedCachedThreadPool(NO_OF_RECONCILER, RECON_TASK_QUEUE_SIZE,
                 "ovsdb-reconciler", getClass());
 
@@ -102,6 +108,14 @@ public class ReconciliationManager implements AutoCloseable {
         bridgeNodeCache = buildBridgeNodeCache();
     }
 
+    public List<String> getBridgesReconciliationInclusionList() {
+        return reconcileBridgeInclusionList;
+    }
+
+    public List<String> getBridgesReconciliationExclusionList() {
+        return reconcileBridgeExclusionList;
+    }
+
     public boolean isEnqueued(final ReconciliationTask task) {
         return reconTaskManager.isTaskQueued(task);
     }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTask.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTask.java
index d970699c7adbe88d4556e389c605a9ccc539dcf6..1499b281458bf730e093a10222aac0bde9f6528d 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTask.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTask.java
@@ -26,7 +26,6 @@ import org.opendaylight.ovsdb.southbound.OvsdbConnectionInstance;
 import org.opendaylight.ovsdb.southbound.OvsdbConnectionManager;
 import org.opendaylight.ovsdb.southbound.SouthboundConstants;
 import org.opendaylight.ovsdb.southbound.SouthboundMapper;
-import org.opendaylight.ovsdb.southbound.SouthboundProvider;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.BridgeOperationalState;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.DataChangeEvent;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.DataChangesManagedByOvsdbNodeEvent;
@@ -69,12 +68,11 @@ public class BridgeConfigReconciliationTask extends ReconciliationTask {
 
     @Override
     public boolean reconcileConfiguration(final OvsdbConnectionManager connectionManagerOfDevice) {
-
         String nodeIdVal = nodeIid.firstKeyOf(Node.class).getNodeId().getValue();
         List<String> bridgeReconcileIncludeList = getNodeIdForBridges(nodeIdVal,
-            SouthboundProvider.getBridgesReconciliationInclusionList());
+            reconciliationManager.getBridgesReconciliationInclusionList());
         List<String> bridgeReconcileExcludeList = getNodeIdForBridges(nodeIdVal,
-            SouthboundProvider.getBridgesReconciliationExclusionList());
+            reconciliationManager.getBridgesReconciliationExclusionList());
 
         LOG.trace("bridgeReconcileIncludeList : {}", bridgeReconcileIncludeList);
         LOG.trace("bridgeReconcileExcludeList : {}", bridgeReconcileExcludeList);
@@ -118,9 +116,9 @@ public class BridgeConfigReconciliationTask extends ReconciliationTask {
                 // wildcard on node id (ie: ovsdb://uuid/<device uuid>/bridge/*) r attributes
                 readTopologyFuture = tx.read(LogicalDatastoreType.CONFIGURATION, topologyInstanceIdentifier);
             }
-            readTopologyFuture.addCallback(new FutureCallback<Optional<Topology>>() {
+            readTopologyFuture.addCallback(new FutureCallback<>() {
                 @Override
-                public void onSuccess(@Nullable final Optional<Topology> optionalTopology) {
+                public void onSuccess(final Optional<Topology> optionalTopology) {
                     if (optionalTopology != null && optionalTopology.isPresent()) {
                         Map<NodeKey, Node> nodes = optionalTopology.orElseThrow().getNode();
                         if (nodes != null) {

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java
index 02c13e25f8f0c7fd07f4c8cc763267f001af86d8..13b41940192bbddcb46ca2a1a044b320b5c034ec 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManagerTest.java
@@ -21,7 +21,6 @@ import static org.powermock.api.support.membermodification.MemberModifier.suppre
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.Futures;
 import java.net.InetAddress;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -95,7 +94,7 @@ public class OvsdbConnectionManagerTest {
         field(OvsdbConnectionManager.class, "entityOwnershipService").set(ovsdbConnManager, entityOwnershipService);
         field(OvsdbConnectionManager.class, "reconciliationManager").set(ovsdbConnManager, reconciliationManager);
         field(OvsdbConnectionManager.class, "ovsdbConnection").set(ovsdbConnManager, ovsdbConnection);
-        field(OvsdbConnectionManager.class, "alreadyProcessedClients").set(ovsdbConnManager, new HashMap<>());
+        field(OvsdbConnectionManager.class, "alreadyProcessedClients").set(ovsdbConnManager, new ConcurrentHashMap<>());
         entityConnectionMap = new ConcurrentHashMap<>();
 
         OvsdbConnectionInfo info = mock(OvsdbConnectionInfo.class);
@@ -192,7 +191,8 @@ public class OvsdbConnectionManagerTest {
                 OvsdbConnectionInstance.class));
         instanceIdentifiers = new ConcurrentHashMap<>();
         field(OvsdbConnectionManager.class, "instanceIdentifiers").set(ovsdbConnManager, instanceIdentifiers);
-        field(OvsdbConnectionManager.class, "nodeIdVsConnectionInstance").set(ovsdbConnManager, new HashMap<>());
+        field(OvsdbConnectionManager.class, "nodeIdVsConnectionInstance").set(ovsdbConnManager,
+            new ConcurrentHashMap<>());
 
         MemberModifier.suppress(MemberMatcher.method(OvsdbConnectionManager.class, "reconcileConnection",
                 InstanceIdentifier.class, OvsdbNodeAugmentation.class));

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataTreeChangeListenerTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataTreeChangeListenerTest.java
index 1b68abf82be6cfe053f676e3ab7581d2498cdb08..4b9827679337ca309a435bc7effa5ba922346cc7 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataTreeChangeListenerTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbDataTreeChangeListenerTest.java
@@ -11,6 +11,7 @@ import static org.mockito.Mockito.mock;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.List;
 import java.util.concurrent.ExecutionException;
 import org.junit.Before;
 import org.junit.Test;
@@ -51,7 +52,7 @@ public class OvsdbDataTreeChangeListenerTest extends AbstractConcurrentDataBroke
         InstanceIdentifierCodec instanceIdentifierCodec = mock(InstanceIdentifierCodec.class);
         listener = new OvsdbDataTreeChangeListener(dataBroker,
                 new OvsdbConnectionManager(dataBroker, new TransactionInvokerImpl(dataBroker), entityOwnershipService,
-                        ovsdbConnection, instanceIdentifierCodec), instanceIdentifierCodec);
+                        ovsdbConnection, instanceIdentifierCodec, List.of(), List.of()), instanceIdentifierCodec);
     }
 
     @Test

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTaskTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTaskTest.java
index 24bb0b7ebcdd5e67b8364a441e2e2826fee72e4f..baba2084097363a52cb9f2f3893e6e96a7de3a45 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTaskTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/reconciliation/configuration/BridgeConfigReconciliationTaskTest.java
@@ -23,7 +23,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.mdsal.binding.api.DataBroker;
 import org.opendaylight.mdsal.binding.api.ReadTransaction;
@@ -75,19 +74,18 @@ public class BridgeConfigReconciliationTaskTest {
         NodeKey nodeKey = new NodeKey(new NodeId(new Uri(NODE_ID)));
 
         iid = SouthboundMapper.createInstanceIdentifier(nodeKey.getNodeId());
-        SouthboundProvider.setBridgesReconciliationInclusionList(List.of(BR_INT));
         Node brIntNode = createBridgeNode(NODE_ID + "/bridge/" + BR_INT);
         when(reconciliationManager.getDb()).thenReturn(db);
+        when(reconciliationManager.getBridgesReconciliationInclusionList()).thenReturn(List.of(BR_INT));
         ReadTransaction tx = mock(ReadTransaction.class);
-        Mockito.when(db.newReadOnlyTransaction()).thenReturn(tx);
-        Mockito.when(tx.read(any(LogicalDatastoreType.class),any(InstanceIdentifier.class)))
+        when(db.newReadOnlyTransaction()).thenReturn(tx);
+        when(tx.read(any(LogicalDatastoreType.class),any(InstanceIdentifier.class)))
                 .thenReturn(FluentFutures.immediateFluentFuture(Optional.of(brIntNode)));
 
         when(topology.getNode()).thenReturn(Map.of(brIntNode.key(), brIntNode));
 
-        configurationReconciliationTask =
-                new BridgeConfigReconciliationTask(reconciliationManager, ovsdbConnectionManager, iid,
-                        ovsdbConnectionInstance, mock(InstanceIdentifierCodec.class));
+        configurationReconciliationTask = new BridgeConfigReconciliationTask(reconciliationManager,
+            ovsdbConnectionManager, iid, ovsdbConnectionInstance, mock(InstanceIdentifierCodec.class));
     }
 
     @Test