Bug 6692: clean up MonitorRequestBuilder

MonitorRequest can't be made immutable, so the utility of this builder
is to support fluent-style construction and addColumns() with multiple
columns. This patch removes unneeded methods from MonitorRequest,
drops the static builder() in favour of the constructor, adds support
for addColumns() with multiple strings, and constructs a new instance
for every call to build(). It also performs the skip cleanup in one
step instead of looping over the columns.

Change-Id: I89fd8a554551c3142efc82dbf6bb43417c6148ae
Signed-off-by: Stephen Kitt <skitt@redhat.com>

diff --git a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionInstance.java b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionInstance.java
index 7221a60cdabe6f5565931207fb8f5052ff897f30..c51c0e397a16dc94e2b039e540a61bd685e26eab 100644 (file)
--- a/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionInstance.java
+++ b/hwvtepsouthbound/hwvtepsouthbound-impl/src/main/java/org/opendaylight/ovsdb/hwvtepsouthbound/HwvtepConnectionInstance.java
@@ -126,11 +126,9 @@ public class HwvtepConnectionInstance {
                 LOG.debug("HwvtepSouthbound monitoring table {} in {}", tableName, dbSchema.getName());
                 GenericTableSchema tableSchema = dbSchema.table(tableName, GenericTableSchema.class);
                 Set<String> columns = tableSchema.getColumns();
-                MonitorRequestBuilder<GenericTableSchema> monitorBuilder = MonitorRequestBuilder.builder(tableSchema);
-                for (String column : columns) {
-                    monitorBuilder.addColumn(column);
-                }
-                monitorRequests.add(monitorBuilder.with(new MonitorSelect(true, true, true, true)).build());
+                monitorRequests.add(new MonitorRequestBuilder<>(tableSchema)
+                        .addColumns(columns)
+                        .with(new MonitorSelect(true, true, true, true)).build());
             }
             this.callback.update(monitor(dbSchema, monitorRequests, callback),dbSchema);
         } else {

diff --git a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequest.java b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequest.java
index c29257f5f6c47f185bcb8b208ef8ea79ce71b75c..02a4b43bfcd3601f2a7014e19ed88074e094c62d 100644 (file)
--- a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequest.java
+++ b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequest.java
@@ -9,7 +9,6 @@ package org.opendaylight.ovsdb.lib.message;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
-import com.google.common.collect.Sets;
 import java.util.Set;
 
 /**
@@ -59,11 +58,4 @@ public class MonitorRequest {
     public void setColumns(Set<String> columns) {
         this.columns = columns;
     }
-
-    public void addColumn(String column) {
-        if (columns == null) {
-            columns = Sets.newHashSet();
-        }
-        columns.add(column);
-    }
 }

diff --git a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequestBuilder.java b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequestBuilder.java
index cc5e36809e1c47cced738ae8e5956792ee4cf75b..f04dd19ede9e385aa92672549125e7165bbd8174 100644 (file)
--- a/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequestBuilder.java
+++ b/library/impl/src/main/java/org/opendaylight/ovsdb/lib/message/MonitorRequestBuilder.java
@@ -8,33 +8,24 @@
 
 package org.opendaylight.ovsdb.lib.message;
 
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 import org.opendaylight.ovsdb.lib.schema.ColumnSchema;
 import org.opendaylight.ovsdb.lib.schema.TableSchema;
 
 public class MonitorRequestBuilder<E extends TableSchema<E>> {
 
-    E tableSchema;
-    MonitorRequest monitorRequest;
+    private final E tableSchema;
+    private final Collection<String> columns = new HashSet<>();
+    private MonitorSelect select;
 
-    MonitorRequestBuilder(E tableSchema) {
+    public MonitorRequestBuilder(E tableSchema) {
         this.tableSchema = tableSchema;
     }
 
-    public static <T extends TableSchema<T>> MonitorRequestBuilder<T> builder(T tableSchema) {
-        return new MonitorRequestBuilder<>(tableSchema);
-    }
-
-    MonitorRequest getMonitorRequest() {
-        if (monitorRequest == null) {
-            monitorRequest = new MonitorRequest();
-        }
-        return monitorRequest;
-    }
-
     public MonitorRequestBuilder<E> addColumn(String column) {
-        getMonitorRequest().addColumn(column);
+        this.columns.add(column);
         return this;
     }
 
@@ -43,6 +34,11 @@ public class MonitorRequestBuilder<E extends TableSchema<E>> {
         return this;
     }
 
+    public MonitorRequestBuilder<E> addColumns(Collection<String> columns) {
+        this.columns.addAll(columns);
+        return this;
+    }
+
     public MonitorRequestBuilder<E> addColumns(List<ColumnSchema<E, ?>> columns) {
         for (ColumnSchema<E, ?> schema : columns) {
             this.addColumn(schema);
@@ -50,21 +46,18 @@ public class MonitorRequestBuilder<E extends TableSchema<E>> {
         return this;
     }
 
-    public Set<String> getColumns() {
-        return getMonitorRequest().getColumns();
+    public Collection<String> getColumns() {
+        return this.columns;
     }
 
     public MonitorRequestBuilder<E> with(MonitorSelect select) {
-        getMonitorRequest().setSelect(select);
+        this.select = select;
         return this;
     }
 
     public MonitorRequest build() {
-        MonitorRequest newBuiltMonitorRequest = getMonitorRequest();
-        if (newBuiltMonitorRequest.getSelect() == null) {
-            newBuiltMonitorRequest.setSelect(new MonitorSelect());
-        }
-        newBuiltMonitorRequest.setTableName(tableSchema.getName());
-        return newBuiltMonitorRequest;
+        MonitorRequest request = new MonitorRequest(tableSchema.getName(), new HashSet<>(this.columns));
+        request.setSelect(select == null ? new MonitorSelect() : select);
+        return request;
     }
 }

diff --git a/library/it/src/test/java/org/opendaylight/ovsdb/integrationtest/ovsdbclient/OvsdbClientTestIT.java b/library/it/src/test/java/org/opendaylight/ovsdb/integrationtest/ovsdbclient/OvsdbClientTestIT.java
index 501d1807a1f394ad7a89e137c3ae4d11121c32da..5f56fe20c554ec7cb8902efa964d4cc3778e4481 100644 (file)
--- a/library/it/src/test/java/org/opendaylight/ovsdb/integrationtest/ovsdbclient/OvsdbClientTestIT.java
+++ b/library/it/src/test/java/org/opendaylight/ovsdb/integrationtest/ovsdbclient/OvsdbClientTestIT.java
@@ -100,7 +100,7 @@ public class OvsdbClientTestIT extends LibraryIntegrationTestBase {
         ColumnSchema<GenericTableSchema, Set<Integer>> flood_vlans = bridge.multiValuedColumn("flood_vlans", Integer.class);
         ColumnSchema<GenericTableSchema, Map<String, String>> externalIds = bridge.multiValuedColumn("external_ids", String.class, String.class);
         ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
-        MonitorRequestBuilder<GenericTableSchema> builder = MonitorRequestBuilder.builder(bridge);
+        MonitorRequestBuilder<GenericTableSchema> builder = new MonitorRequestBuilder<>(bridge);
         if (filter) {
             builder.addColumn(bridge.column("name"))
                    .addColumn(bridge.column("fail_mode", String.class))

diff --git a/library/it/src/test/java/org/opendaylight/ovsdb/lib/it/LibraryIntegrationTestBase.java b/library/it/src/test/java/org/opendaylight/ovsdb/lib/it/LibraryIntegrationTestBase.java
index c908a446103728b85f89df5860c7665e52cd9174..4b56e6ffe36ae08ed7193e6ac88842c99e3bc4a9 100644 (file)
--- a/library/it/src/test/java/org/opendaylight/ovsdb/lib/it/LibraryIntegrationTestBase.java
+++ b/library/it/src/test/java/org/opendaylight/ovsdb/lib/it/LibraryIntegrationTestBase.java
@@ -224,7 +224,7 @@ public abstract class LibraryIntegrationTestBase extends AbstractMdsalTestBase {
         TypedBaseTable<GenericTableSchema> table = getClient().createTypedRowWrapper(klazz);
         GenericTableSchema tableSchema = table.getSchema();
         Set<String> columns = tableSchema.getColumns();
-        MonitorRequestBuilder<GenericTableSchema> bridgeBuilder = MonitorRequestBuilder.builder(table.getSchema());
+        MonitorRequestBuilder<GenericTableSchema> bridgeBuilder = new MonitorRequestBuilder<>(table.getSchema());
         for (String column : columns) {
             bridgeBuilder.addColumn(column);
         }
@@ -233,7 +233,7 @@ public abstract class LibraryIntegrationTestBase extends AbstractMdsalTestBase {
 
     public <T extends TableSchema<T>> MonitorRequest getAllColumnsMonitorRequest (T tableSchema) {
         Set<String> columns = tableSchema.getColumns();
-        MonitorRequestBuilder<T> monitorBuilder = MonitorRequestBuilder.builder(tableSchema);
+        MonitorRequestBuilder<T> monitorBuilder = new MonitorRequestBuilder<>(tableSchema);
         for (String column : columns) {
             monitorBuilder.addColumn(column);
         }

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
index ba64232a3bfee03e4a878490122685a15ca364ef..81efd672637009c153921430bfef133444b51963 100644 (file)
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
@@ -15,6 +15,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -166,18 +167,16 @@ public class OvsdbConnectionInstance {
                 if (!SouthboundConstants.SKIP_OVSDB_TABLE.contains(tableName)) {
                     LOG.info("Southbound monitoring OVSDB schema table {}", tableName);
                     GenericTableSchema tableSchema = dbSchema.table(tableName, GenericTableSchema.class);
-                    Set<String> columns = tableSchema.getColumns();
-                    MonitorRequestBuilder<GenericTableSchema> monitorBuilder
-                            = MonitorRequestBuilder.builder(tableSchema);
+                    // We copy the columns so we can clean the set up later
+                    Set<String> columns = new HashSet<>(tableSchema.getColumns());
                     List<String> skipColumns = SouthboundConstants.SKIP_COLUMN_FROM_TABLE.get(tableName);
-                    for (String column : columns) {
-                        if ( skipColumns == null || !skipColumns.contains(column)) {
-                            monitorBuilder.addColumn(column);
-                        } else {
-                            LOG.info("Southbound NOT monitoring column {} in table {}", column, tableName);
-                        }
+                    if (skipColumns != null) {
+                        LOG.info("Southbound NOT monitoring columns {} in table {}", skipColumns, tableName);
+                        columns.removeAll(skipColumns);
                     }
-                    monitorRequests.add(monitorBuilder.with(new MonitorSelect(true, true, true, true)).build());
+                    monitorRequests.add(new MonitorRequestBuilder<>(tableSchema)
+                            .addColumns(columns)
+                            .with(new MonitorSelect(true, true, true, true)).build());
                 }
             }
             this.callback.update(monitor(dbSchema, monitorRequests, callback), dbSchema);

diff --git a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstanceTest.java b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstanceTest.java
index b1e2c2d556fafb48eb0a637854903eb5417a7ab5..aa686787ce750bffb077d0a529dbc32db5fba145 100644 (file)
--- a/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstanceTest.java
+++ b/southbound/southbound-impl/src/test/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstanceTest.java
@@ -41,9 +41,7 @@ import org.opendaylight.ovsdb.lib.MonitorCallBack;
 import org.opendaylight.ovsdb.lib.MonitorHandle;
 import org.opendaylight.ovsdb.lib.OvsdbClient;
 import org.opendaylight.ovsdb.lib.OvsdbConnectionInfo;
-import org.opendaylight.ovsdb.lib.message.MonitorRequest;
 import org.opendaylight.ovsdb.lib.message.MonitorRequestBuilder;
-import org.opendaylight.ovsdb.lib.message.MonitorSelect;
 import org.opendaylight.ovsdb.lib.message.TableUpdates;
 import org.opendaylight.ovsdb.lib.operations.OperationResult;
 import org.opendaylight.ovsdb.lib.operations.TransactionBuilder;
@@ -179,13 +177,6 @@ public class OvsdbConnectionInstanceTest {
         columns.add("_version");
         columns.add("statistics");
         when(tableSchema.getColumns()).thenReturn(columns);
-        MonitorRequestBuilder<GenericTableSchema> monitorBuilder = mock(MonitorRequestBuilder.class);
-        PowerMockito.mockStatic(MonitorRequestBuilder.class);
-        when(MonitorRequestBuilder.builder(any(GenericTableSchema.class))).thenReturn(monitorBuilder);
-        when(monitorBuilder.addColumn(anyString())).thenReturn(monitorBuilder);
-        MonitorRequest monitorReq = mock(MonitorRequest.class);
-        when(monitorBuilder.with(any(MonitorSelect.class))).thenReturn(monitorBuilder);
-        when(monitorBuilder.build()).thenReturn(monitorReq);
 
         suppress(MemberMatcher.method(OvsdbConnectionInstance.class, "monitor", DatabaseSchema.class, List.class,
                 MonitorCallBack.class));
@@ -198,8 +189,6 @@ public class OvsdbConnectionInstanceTest {
         Whitebox.invokeMethod(ovsdbConnectionInstance, "monitorTables", "database", dbSchema);
         PowerMockito.verifyPrivate(ovsdbConnectionInstance, times(1)).invoke("monitorTables", anyString(),
                 any(DatabaseSchema.class));
-
-        verify(monitorBuilder, times(4)).addColumn(anyString());
     }
 
     @SuppressWarnings({ "unchecked" })