Addressing review comments for 7920

Changes include :
- Changes variable name from tval to untypedValue in Insert and Mutate classes
- Fixed most of the review comments in the IT

Change-Id: I83197b63aa2874f0c7136ec4dc3b7b629b2407a7
Signed-off-by: Madhu Venugopal <mavenugo@gmail.com>

diff --git a/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Insert.java b/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Insert.java
index 83c20b3ad2814c812b2ebe23ccfb6dac1b5c1d71..cad8702888703a53bdd9dbfc78c07e4c07962bf2 100644 (file)
--- a/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Insert.java
+++ b/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Insert.java
@@ -49,14 +49,14 @@ public class Insert<E extends TableSchema<E>> extends Operation<E> {
     }
 
     public <D, C extends TableSchema<C>> Insert<E> value(ColumnSchema<C, D> columnSchema, D value) {
-        Object tval = null;
+        Object untypedValue = null;
         if (columnSchema.getType().isMultiValued()) {
             Preconditions.checkArgument((value instanceof Set),"expected a set for multivalued item") ;
-            tval = OvsDBSet.fromSet((Set) value);
+            untypedValue = OvsDBSet.fromSet((Set) value);
         } else {
-            tval = value;
+            untypedValue = value;
         }
-        row.put(columnSchema.getName(), tval);
+        row.put(columnSchema.getName(), untypedValue);
         return this;
     }
 

diff --git a/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Mutate.java b/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Mutate.java
index e31a7a530d7936bd162a6692d1481251abe0a199..6e9c20789c9520f3684b92f915d1412bdaadf03c 100644 (file)
--- a/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Mutate.java
+++ b/library/src/main/java/org/opendaylight/ovsdb/lib/operations/Mutate.java
@@ -40,14 +40,14 @@ public class Mutate<E extends TableSchema<E>> extends Operation<E> implements Co
 
     public <T extends TableSchema<T>, D> Mutate<E> addMutation(ColumnSchema<T, D> columnSchema, Mutator mutator, D value) {
         columnSchema.validate(value);
-        Object tval = null;
+        Object untypedValue = null;
         if (columnSchema.getType().isMultiValued()) {
             Preconditions.checkArgument((value instanceof Set),"expected a set for multivalued item") ;
-            tval = OvsDBSet.fromSet((Set) value);
+            untypedValue = OvsDBSet.fromSet((Set) value);
         } else {
-            tval = value;
+            untypedValue = value;
         }
-        mutations.add(new Mutation(columnSchema.getName(), mutator, tval));
+        mutations.add(new Mutation(columnSchema.getName(), mutator, untypedValue));
         return this;
     }
 

diff --git a/library/src/test/java/org/opendaylight/ovsdb/lib/OvsDBClientTestIT.java b/library/src/test/java/org/opendaylight/ovsdb/lib/OvsDBClientTestIT.java
index d3e5f0395f85c36198f86f58891fb5372ebc132d..e26977928011b32a05c8d86e252a5a92bd918fd9 100644 (file)
--- a/library/src/test/java/org/opendaylight/ovsdb/lib/OvsDBClientTestIT.java
+++ b/library/src/test/java/org/opendaylight/ovsdb/lib/OvsDBClientTestIT.java
@@ -13,7 +13,6 @@ import static org.opendaylight.ovsdb.lib.operations.Operations.op;
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -58,21 +57,85 @@ public class OvsDBClientTestIT extends OvsdbTestBase {
     public void testTransact() throws IOException, InterruptedException, ExecutionException {
         Assert.assertNotNull(dbSchema);
         TableSchema<GenericTableSchema> bridge = dbSchema.table("Bridge", GenericTableSchema.class);
-        GenericTableSchema ovsTable = dbSchema.table("Open_vSwitch", GenericTableSchema.class);
-        ColumnSchema<GenericTableSchema, Set<UUID>> bridges = ovsTable.multiValuedColumn("bridges", UUID.class);
+        ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
+
+        createBridgeTransaction();
+        abortTransaction();
+        assertTransaction();
+    }
+
+    @Test
+    public void testMonitorRequest() throws ExecutionException, InterruptedException, IOException {
+        Assert.assertNotNull(dbSchema);
+        // Create Test Bridge before testing the Monitor operation
+        createBridgeTransaction();
+
+        GenericTableSchema bridge = dbSchema.table("Bridge", GenericTableSchema.class);
+
+        List<MonitorRequest<GenericTableSchema>> monitorRequests = Lists.newArrayList();
+        ColumnSchema<GenericTableSchema, Set<Integer>> flood_vlans = bridge.multiValuedColumn("flood_vlans", Integer.class);
+
+        monitorRequests.add(
+                MonitorRequestBuilder.builder(bridge)
+                        .addColumn(bridge.column("name"))
+                        .addColumn(bridge.column("fail_mode", String.class))
+                        .addColumn(flood_vlans)
+                        .with(new MonitorSelect(true, true, true, true))
+                        .build());
+
+        final List<Object> results = Lists.newArrayList();
+
+        MonitorHandle monitor = ovs.monitor(dbSchema, monitorRequests, new MonitorCallBack() {
+            @Override
+            public void update(TableUpdates result) {
+                results.add(result);
+                System.out.println("result = " + result);
+            }
+
+            @Override
+            public void exception(Throwable t) {
+                results.add(t);
+                System.out.println("t = " + t);
+            }
+        });
 
-        for (Map.Entry<String, ColumnSchema> names : bridge.getColumnSchemas().entrySet()) {
-            System.out.println("names = " + names.getKey());
-            System.out.println("names.getValue().getType() = " + names.getValue().getType().getBaseType());
+        for (int i = 0; i < 5 ; i++) { //wait 5 seconds to get a result
+            System.out.println("waiting");
+            Thread.sleep(1000);
+        }
+
+        Assert.assertTrue(!results.isEmpty());
+        Object result = results.get(0);
+        Assert.assertTrue(result instanceof TableUpdates);
+        TableUpdates updates = (TableUpdates) result;
+        org.opendaylight.ovsdb.lib.message.TableUpdate<GenericTableSchema> update = updates.getUpdate(bridge);
+        Row<GenericTableSchema> aNew = update.getNew();
+        for (Column<GenericTableSchema, ?> column: aNew.getColumns()) {
+            if (column.getSchema().equals(flood_vlans)) {
+                Set<Integer> data = column.getData(flood_vlans);
+                Assert.assertTrue(!data.isEmpty());
+            }
         }
+    }
+
+    private void createBridgeTransaction() throws IOException, InterruptedException, ExecutionException {
+        Assert.assertNotNull(dbSchema);
+        TableSchema<GenericTableSchema> bridge = dbSchema.table("Bridge", GenericTableSchema.class);
+        GenericTableSchema ovsTable = dbSchema.table("Open_vSwitch", GenericTableSchema.class);
 
         ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
         ColumnSchema<GenericTableSchema, String> fail_mode = bridge.column("fail_mode", String.class);
         ColumnSchema<GenericTableSchema, Set<Integer>> flood_vlans = bridge.multiValuedColumn("flood_vlans", Integer.class);
+        ColumnSchema<GenericTableSchema, Set<UUID>> bridges = ovsTable.multiValuedColumn("bridges", UUID.class);
 
         String namedUuid = "br_test";
         int nOperations = 7;
+        int insertOperationIndex = 0;
         ListenableFuture<List<OperationResult>> results = ovs.transactBuilder()
+                 /*
+                  * Make sure that the position of insert operation matches the insertOperationIndex.
+                  * This will be used later when the Results are processed.
+                  */
                 .add(op.insert(bridge)
                         .withId(namedUuid)
                         .value(name, testBridgeName)
@@ -100,93 +163,53 @@ public class OvsDBClientTestIT extends OvsdbTestBase {
         // Check if Results matches the number of operations in transaction
         Assert.assertEquals(nOperations, operationResults.size());
         System.out.println("Insert & Update operation results = " + operationResults);
-        testBridgeUuid = operationResults.get(0).getUuid();
+        testBridgeUuid = operationResults.get(insertOperationIndex).getUuid();
+    }
+
+    private void assertTransaction() throws InterruptedException, ExecutionException {
+        Assert.assertNotNull(dbSchema);
+        TableSchema<GenericTableSchema> bridge = dbSchema.table("Bridge", GenericTableSchema.class);
+        ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
 
         /*
-         * Adding a separate Abort operation in a transaction. Lets not mix this with other
+         * Adding a separate Assert operation in a transaction. Lets not mix this with other
          * valid transactions as above.
          */
-        results = ovs.transactBuilder()
+        ListenableFuture<List<OperationResult>> results = ovs.transactBuilder()
                 .add(op.delete(bridge)
                         .where(name.opEqual(testBridgeName))
                         .build())
-                .add(op.abort())
+                .add(op.assertion("Assert12345")) // Failing intentionally
                 .execute();
 
-        operationResults = results.get();
+        List<OperationResult> operationResults = results.get();
         Assert.assertFalse(operationResults.isEmpty());
-        /* Testing for Abort Error */
+        /* Testing for an Assertion Error */
         Assert.assertFalse(operationResults.get(1).getError() == null);
-        System.out.println("Abort operation results = " + operationResults);
+        System.out.println("Assert operation results = " + operationResults);
+    }
+
+    private void abortTransaction() throws InterruptedException, ExecutionException {
+        Assert.assertNotNull(dbSchema);
+        TableSchema<GenericTableSchema> bridge = dbSchema.table("Bridge", GenericTableSchema.class);
+        ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
 
         /*
-         * Adding a separate Assert operation in a transaction. Lets not mix this with other
+         * Adding a separate Abort operation in a transaction. Lets not mix this with other
          * valid transactions as above.
          */
-        results = ovs.transactBuilder()
+        ListenableFuture<List<OperationResult>> results = ovs.transactBuilder()
                 .add(op.delete(bridge)
                         .where(name.opEqual(testBridgeName))
                         .build())
-                .add(op.assertion("Assert12345")) // Failing intentionally
+                .add(op.abort())
                 .execute();
 
-        operationResults = results.get();
+        List<OperationResult> operationResults = results.get();
         Assert.assertFalse(operationResults.isEmpty());
-        /* Testing for an Assertion Error */
+        /* Testing for Abort Error */
         Assert.assertFalse(operationResults.get(1).getError() == null);
-        System.out.println("Assert operation results = " + operationResults);
-    }
-
-    @Test
-    public void testMonitorRequest() throws ExecutionException, InterruptedException, IOException {
-        Assert.assertNotNull(dbSchema);
-        testTransact(); // Reuse the br-test create transactions
-        GenericTableSchema bridge = dbSchema.table("Bridge", GenericTableSchema.class);
-
-        List<MonitorRequest<GenericTableSchema>> monitorRequests = Lists.newArrayList();
-        ColumnSchema<GenericTableSchema, Set<Integer>> flood_vlans = bridge.multiValuedColumn("flood_vlans", Integer.class);
-
-        monitorRequests.add(
-                MonitorRequestBuilder.builder(bridge)
-                        .addColumn(bridge.column("name"))
-                        .addColumn(bridge.column("fail_mode", String.class))
-                        .addColumn(flood_vlans)
-                        .with(new MonitorSelect(true, true, true, true))
-                        .build());
-
-        final List<Object> results = Lists.newArrayList();
-
-        MonitorHandle monitor = ovs.monitor(dbSchema, monitorRequests, new MonitorCallBack() {
-            @Override
-            public void update(TableUpdates result) {
-                results.add(result);
-                System.out.println("result = " + result);
-            }
-
-            @Override
-            public void exception(Throwable t) {
-                results.add(t);
-                System.out.println("t = " + t);
-            }
-        });
-
-        for (int i = 0; i < 5 ; i++) { //wait 5 seconds to get a result
-            System.out.println("waiting");
-            Thread.sleep(1000);
-        }
-
-        Assert.assertTrue(!results.isEmpty());
-        Object result = results.get(0);
-        Assert.assertTrue(result instanceof TableUpdates);
-        TableUpdates updates = (TableUpdates) result;
-        org.opendaylight.ovsdb.lib.message.TableUpdate<GenericTableSchema> update = updates.getUpdate(bridge);
-        Row<GenericTableSchema> aNew = update.getNew();
-        for (Column<GenericTableSchema, ?> column: aNew.getColumns()) {
-            if (column.getSchema().equals(flood_vlans)) {
-                Set<Integer> data = column.getData(flood_vlans);
-                Assert.assertTrue(!data.isEmpty());
-            }
-        }
+        System.out.println("Abort operation results = " + operationResults);
     }
 
     public void testGetDBs() throws ExecutionException, InterruptedException {
@@ -204,7 +227,7 @@ public class OvsDBClientTestIT extends OvsdbTestBase {
     }
 
     @Before
-    public  void initalize() throws IOException, ExecutionException, InterruptedException {
+    public  void setUp() throws IOException, ExecutionException, InterruptedException {
         if (ovs != null) {
             return;
         }
@@ -216,13 +239,10 @@ public class OvsDBClientTestIT extends OvsdbTestBase {
         ovs = new OvsDBClientImpl(rpc, executorService);
         testGetDBs();
         dbSchema = ovs.getSchema(OPEN_VSWITCH_SCHEMA, true).get();
-        Assert.assertNotNull(dbSchema);
     }
 
     @After
-    public void cleanup() throws InterruptedException, ExecutionException {
-        Assert.assertNotNull(dbSchema);
-        Assert.assertNotNull(testBridgeUuid);
+    public void tearDown() throws InterruptedException, ExecutionException {
         TableSchema<GenericTableSchema> bridge = dbSchema.table("Bridge", GenericTableSchema.class);
         ColumnSchema<GenericTableSchema, String> name = bridge.column("name", String.class);
         GenericTableSchema ovsTable = dbSchema.table("Open_vSwitch", GenericTableSchema.class);
@@ -238,7 +258,6 @@ public class OvsDBClientTestIT extends OvsdbTestBase {
                 .execute();
 
         List<OperationResult> operationResults = results.get();
-        Assert.assertFalse(operationResults.isEmpty());
         System.out.println("Delete operation results = " + operationResults);
     }