ovsdb-idl: Change interface to conditional monitoring.

Most users of OVSDB react to whatever is currently in their view of the
database, as opposed to keeping track of changes and reacting to those
changes individually.  The interface to conditional monitoring was
different, in that it expected the client to say what to add or remove from
monitoring instead of what to monitor.  This seemed reasonable at the time,
but in practice it turns out that the usual approach actually works better,
because the condition is generally a function of the data visible in the
database.  This commit changes the approach.

This commit also changes the meaning of an empty condition for a table.
Previously, an empty condition meant to replicate every row.  Now, an empty
condition means to replicate no rows.  This is more convenient for code
that gradually constructs conditions, because it does not need special
cases for replicating nothing.

This commit also changes the internal implementation of conditions from
linked lists to arrays.  I just couldn't see an advantage to using linked
lists.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Liran Schour <lirans@il.ibm.com>

3 files changed, 81 insertions, 173 deletions

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 9ed431bf9..d2c1ea6f3 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -393,8 +393,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, false condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition remove simple [false]']],
+  [['condition simple []' \
+    'condition simple [true]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -409,8 +409,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, true condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [true]']],
+  [['condition simple []' \
+    'condition simple [true]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -430,8 +430,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple clauses in condition],
        "row": {"i": 2,
                "r": 3.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1],["i","==",2]]']],
+  [['condition simple []' \
+    'condition simple [["i","==",1],["i","==",2]]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -447,8 +447,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as insert due to condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1]]']],
+  [['condition simple []' \
+    'condition simple [["i","==",1]]']],
   [[000: change conditions
 001: empty
 002: change conditions
@@ -463,9 +463,9 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as delete due to condition],
        "row": {"i": 1,
                "r": 2.0,
                "b": true}}]']],
-  [['condition add simple [false]' \
-    'condition add simple [["i","==",1],["i","==",2]]' \
-    'condition remove simple [["i","==",1]]' \
+  [['condition simple []' \
+    'condition simple [["i","==",1],["i","==",2]]' \
+    'condition simple [["i","==",2]]' \
     '["idltest",
        {"op": "insert",
        "table": "simple",
@@ -498,10 +498,10 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple tables],
        "table": "link2",
        "row": {"i": 2},
        "uuid-name": "row0"}]']],
-  [['condition add simple [false];condition add link1 [false];condition add link2 [false]' \
-    'condition add simple [["i","==",1]]' \
-    'condition add link1 [["i","==",0]]' \
-    'condition add link2 [["i","==",3]]' \
+  [['condition simple [];link1 [];link2 []' \
+    'condition simple [["i","==",1]]' \
+    'condition link1 [["i","==",0]]' \
+    'condition link2 [["i","==",3]]' \
     '+["idltest",
        {"op": "insert",
        "table": "link2",
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 6a7d46753..968c1de22 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2184,145 +2184,50 @@ find_table_class(const char *name)
 }
 
 static void
-parse_simple_json_clause(struct ovsdb_idl *idl, bool add_cmd,
-                         struct json *json)
-{
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_simple_add_clause_true(idl) :
-            idltest_simple_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_simple_add_clause_false(idl) :
-            idltest_simple_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "b")) {
-        add_cmd ? idltest_simple_add_clause_b(idl, function,
-                                     json_boolean(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_b(idl, function,
-                                     json_boolean(json->u.array.elems[2]));
-    } else if (!strcmp(c, "i")) {
-        add_cmd ? idltest_simple_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
-    } else if (!strcmp(c, "s")) {
-        add_cmd ? idltest_simple_add_clause_s(idl, function,
-                                    json_string(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_s(idl, function,
-                                    json_string(json->u.array.elems[2]));
-    } else if (!strcmp(c, "u")) {
+parse_simple_json_clause(struct ovsdb_idl_condition *cond,
+                         enum ovsdb_function function,
+                         const char *column, const struct json *arg)
+{
+    if (!strcmp(column, "b")) {
+        idltest_simple_add_clause_b(cond, function, json_boolean(arg));
+    } else if (!strcmp(column, "i")) {
+         idltest_simple_add_clause_i(cond, function, json_integer(arg));
+    } else if (!strcmp(column, "s")) {
+        idltest_simple_add_clause_s(cond, function, json_string(arg));
+    } else if (!strcmp(column, "u")) {
         struct uuid uuid;
-        if (!uuid_from_string(&uuid,
-                              json_string(json->u.array.elems[2]))) {
-            ovs_fatal(0, "\"%s\" is not a valid UUID",
-                      json_string(json->u.array.elems[2]));
+        if (!uuid_from_string(&uuid, json_string(arg))) {
+            ovs_fatal(0, "\"%s\" is not a valid UUID", json_string(arg));
         }
-        add_cmd ? idltest_simple_add_clause_u(idl, function, uuid) :
-            idltest_simple_remove_clause_u(idl, function, uuid);
-    } else if (!strcmp(c, "r")) {
-        add_cmd ? idltest_simple_add_clause_r(idl, function,
-                                    json_real(json->u.array.elems[2])) :
-            idltest_simple_remove_clause_r(idl, function,
-                                    json_real(json->u.array.elems[2]));
+        idltest_simple_add_clause_u(cond, function, uuid);
+    } else if (!strcmp(column, "r")) {
+        idltest_simple_add_clause_r(cond, function, json_real(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
 static void
-parse_link1_json_clause(struct ovsdb_idl *idl, bool add_cmd,
-                        struct json *json)
+parse_link1_json_clause(struct ovsdb_idl_condition *cond,
+                        enum ovsdb_function function,
+                        const char *column, const struct json *arg)
 {
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_link1_add_clause_true(idl) :
-            idltest_link1_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_link1_add_clause_false(idl) :
-            idltest_link1_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "i")) {
-        add_cmd ? idltest_link1_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_link1_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
+    if (!strcmp(column, "i")) {
+        idltest_link1_add_clause_i(cond, function, json_integer(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
 static void
-parse_link2_json_clause(struct ovsdb_idl *idl, bool add_cmd, struct json *json)
+parse_link2_json_clause(struct ovsdb_idl_condition *cond,
+                        enum ovsdb_function function,
+                        const char *column, const struct json *arg)
 {
-    const char *c;
-    struct ovsdb_error *error;
-    enum ovsdb_function function;
-
-    if (json->type == JSON_TRUE) {
-        add_cmd ? idltest_link2_add_clause_true(idl) :
-            idltest_link2_remove_clause_true(idl);
-        return;
-    } else if (json->type == JSON_FALSE) {
-        add_cmd ? idltest_link2_add_clause_false(idl) :
-            idltest_link2_remove_clause_false(idl);
-        return;
-    }
-    if (json->type != JSON_ARRAY || json->u.array.n != 3) {
-        ovs_fatal(0, "Error parsing condition");
-    }
-
-    c = json_string(json->u.array.elems[0]);
-    error = ovsdb_function_from_string(json_string(json->u.array.elems[1]),
-                                       &function);
-    if (error) {
-        ovs_fatal(0, "Error parsing clause function %s",
-                  json_string(json->u.array.elems[1]));
-    }
-
-    /* add clause according to column */
-    if (!strcmp(c, "i")) {
-        add_cmd ? idltest_link2_add_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2])) :
-            idltest_link2_remove_clause_i(idl, function,
-                                    json_integer(json->u.array.elems[2]));
+    if (!strcmp(column, "i")) {
+        idltest_link2_add_clause_i(cond, function, json_integer(arg));
     } else {
-        ovs_fatal(0, "Unsupported columns name %s", c);
+        ovs_fatal(0, "Unsupported columns name %s", column);
     }
 }
 
@@ -2331,19 +2236,9 @@ update_conditions(struct ovsdb_idl *idl, char *commands)
 {
     char *cmd, *save_ptr1 = NULL;
     const struct ovsdb_idl_table_class *tc;
-    bool add_cmd = false;
 
     for (cmd = strtok_r(commands, ";", &save_ptr1); cmd;
          cmd = strtok_r(NULL, ";", &save_ptr1)) {
-        if (strstr(cmd, "condition add")) {
-            cmd += strlen("condition add ");
-            add_cmd = true;
-        } else if (strstr(cmd, "condition remove")) {
-            cmd += strlen("condition remove ");
-        } else {
-            ovs_fatal(0, "condition command should be add or remove");
-        }
-
         char *save_ptr2 = NULL;
         char *table_name = strtok_r(cmd, " ", &save_ptr2);
         struct json *json = parse_json(save_ptr2);
@@ -2358,17 +2253,37 @@ update_conditions(struct ovsdb_idl *idl, char *commands)
             ovs_fatal(0, "Table %s does not exist", table_name);
         }
 
-        //ovsdb_idl_condition_reset(idl, tc);
-
+        struct ovsdb_idl_condition cond = OVSDB_IDL_CONDITION_INIT(&cond);
         for (i = 0; i < json->u.array.n; i++) {
-            if (!strcmp(table_name, "simple")) {
-                parse_simple_json_clause(idl, add_cmd, json->u.array.elems[i]);
-            } else if (!strcmp(table_name, "link1")) {
-                parse_link1_json_clause(idl, add_cmd, json->u.array.elems[i]);
-            } else if (!strcmp(table_name, "link2")) {
-                parse_link2_json_clause(idl, add_cmd, json->u.array.elems[i]);
+            const struct json *clause = json->u.array.elems[i];
+            if (clause->type == JSON_TRUE) {
+                ovsdb_idl_condition_add_clause_true(&cond);
+            } else if (clause->type != JSON_ARRAY || clause->u.array.n != 3
+                       || clause->u.array.elems[0]->type != JSON_STRING
+                       || clause->u.array.elems[1]->type != JSON_STRING) {
+                ovs_fatal(0, "Error parsing condition");
+            } else {
+                enum ovsdb_function function;
+                const char *function_s = json_string(clause->u.array.elems[1]);
+                struct ovsdb_error *error = ovsdb_function_from_string(
+                    function_s, &function);
+                if (error) {
+                    ovs_fatal(0, "unknown clause function %s", function_s);
+                }
+
+                const char *column = json_string(clause->u.array.elems[0]);
+                const struct json *arg = clause->u.array.elems[2];
+                if (!strcmp(table_name, "simple")) {
+                    parse_simple_json_clause(&cond, function, column, arg);
+                } else if (!strcmp(table_name, "link1")) {
+                    parse_link1_json_clause(&cond, function, column, arg);
+                } else if (!strcmp(table_name, "link2")) {
+                    parse_link2_json_clause(&cond, function, column, arg);
+                }
             }
         }
+        ovsdb_idl_set_condition(idl, tc, &cond);
+        ovsdb_idl_condition_destroy(&cond);
         json_destroy(json);
     }
 }
@@ -2409,8 +2324,9 @@ do_idl(struct ovs_cmdl_context *ctx)
     setvbuf(stdout, NULL, _IONBF, 0);
 
     symtab = ovsdb_symbol_table_create();
-    if (ctx->argc > 2 && strstr(ctx->argv[2], "condition ")) {
-        update_conditions(idl, ctx->argv[2]);
+    const char cond_s[] = "condition ";
+    if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) {
+        update_conditions(idl, ctx->argv[2] + strlen(cond_s));
         printf("%03d: change conditions\n", step++);
         i = 3;
     } else {
@@ -2451,8 +2367,8 @@ do_idl(struct ovs_cmdl_context *ctx)
         if (!strcmp(arg, "reconnect")) {
             printf("%03d: reconnect\n", step++);
             ovsdb_idl_force_reconnect(idl);
-        }  else if (strstr(arg, "condition ")) {
-            update_conditions(idl, arg);
+        }  else if (!strncmp(arg, cond_s, strlen(cond_s))) {
+            update_conditions(idl, arg + strlen(cond_s));
             printf("%03d: change conditions\n", step++);
         } else if (arg[0] != '[') {
             idl_set(idl, arg, step++);
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 185215ae6..dced56b99 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -531,25 +531,17 @@ def idl_set(idl, commands, step):
 
 
 def update_condition(idl, commands):
-    commands = commands.split(";")
+    commands = commands[len("condition "):].split(";")
     for command in commands:
-        command = command[len("condition "):]
-        if "add" in command:
-            add_cmd = True
-            command = command[len("add "):]
-        else:
-            add_cmd = False
-            command = command[len("remove "):]
-
         command = command.split(" ")
         if(len(command) != 2):
-            sys.stderr.write("Error parsong condition %s\n" % command)
+            sys.stderr.write("Error parsing condition %s\n" % command)
             sys.exit(1)
 
         table = command[0]
         cond = ovs.json.from_string(command[1])
 
-        idl.cond_change(table, add_cmd, cond)
+        idl.cond_change(table, cond)
 
 
 def do_idl(schema_file, remote, *commands):