Merge lp:~rainct/zeitgeist/table-lookup-get-value-query into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

[Michal Hruby (mhr3)]

Looking good.

review: Approve

=== modified file 'src/table-lookup.vala'
--- src/table-lookup.vala	2012-02-10 11:28:05 +0000
+++ src/table-lookup.vala	2012-02-13 20:22:18 +0000
@@ -33,6 +33,7 @@
         private HashTable<int, string> id_to_value;
         private HashTable<string, int> value_to_id;
         private Sqlite.Statement insertion_stmt;
+        private Sqlite.Statement retrieval_stmt;
 
         public TableLookup (Database database, string table_name)
         {
@@ -42,6 +43,7 @@
             value_to_id = new HashTable<string, int>(str_hash, str_equal);
 
             int rc;
+            string sql;
 
             rc = db.exec ("SELECT id, value FROM " + table,
                 (n_columns, values, column_names) =>
@@ -56,10 +58,19 @@
                     rc, db.errmsg ());
             }
 
-            string sql = "INSERT INTO " + table + " (value) VALUES (?)";
+            sql = "INSERT INTO " + table + " (value) VALUES (?)";
             rc = db.prepare_v2 (sql, -1, out insertion_stmt);
             if (rc != Sqlite.OK)
             {
+                // FIXME: throw exception and propagate it up to
+                //        zeitgeist-daemon to abort with DB error?
+                critical ("SQL error: %d, %s\n", rc, db.errmsg ());
+            }
+
+            sql = "SELECT value FROM " + table + " WHERE id=?";
+            rc = db.prepare_v2 (sql, -1, out retrieval_stmt);
+            if (rc != Sqlite.OK)
+            {
                 critical ("SQL error: %d, %s\n", rc, db.errmsg ());
             }
         }
@@ -93,23 +104,27 @@
             unowned string val = id_to_value.lookup (id);
             if (val != null) return val;
 
-            // The above statement isn't exactly true. If this is a standalone
-            // reader in a separate process, the values won't be kept updated
-            // so we need to query the DB if we don't find it.
+            // Unless this is a standalone reader in a separate process, in
+            // which case the values won't be kept updated, so we need to
+            // query the DB if we don't find it.
             int rc;
+            string? text = null;
 
-            rc = db.exec ("SELECT value FROM %s WHERE id=%d".printf (table, id),
-                (n_columns, values, column_names) =>
-                {
-                    id_to_value.insert (id, values[0]);
-                    value_to_id.insert (values[0], id);
-                    return 0;
-                }, null);
-            if (rc != Sqlite.OK)
-            {
-                critical ("Can't get data from table %s: %d, %s\n", table,
+            retrieval_stmt.reset ();
+            retrieval_stmt.bind_int64 (1, id);
+            if ((rc = retrieval_stmt.step()) == Sqlite.ROW)
+            {
+                text = retrieval_stmt.column_text (0);
+                id_to_value.insert (id, text);
+                value_to_id.insert (text, id);
+                rc = retrieval_stmt.step ();
+            }
+            if (rc != Sqlite.DONE || text == null)
+            {
+                critical ("Error getting data from table: %d, %s\n",
                     rc, db.errmsg ());
             }
+
             return id_to_value.lookup (id);
         }
 

=== modified file 'test/direct/Makefile.am'
--- test/direct/Makefile.am	2012-02-05 14:52:13 +0000
+++ test/direct/Makefile.am	2012-02-13 20:22:18 +0000
@@ -10,10 +10,10 @@
 
 TESTS = \
 	marshalling-test \
+	mimetype-test \
 	query-operators-test \
+	table-lookup-test \
 	where-clause-test \
-	table-lookup-test \
-	mimetype-test \
 	$(NULL)
 
 SRC_FILES = \
@@ -56,19 +56,19 @@
 
 DISTCLEANFILES = \
 	marshalling-test \
+	mimetype-test \
 	query-operators-test \
+	table-lookup-test \
 	where-clause-test \
-	table-lookup-test \
-	mimetype-test \
 	$(NULL)
 
 EXTRA_DIST = \
+	assertions.vapi \
 	marshalling-test.vala \
+	mimetype-test.vala \
 	query-operators-test.vala \
+	table-lookup-test.vala \
 	where-clause-test.vala \
-	table-lookup-test.vala \
-	mimetype-test.vala \
-	assertions.vapi \
 	$(NULL)
 
 VALA_V = $(VALA_V_$(V))

=== modified file 'test/direct/table-lookup-test.vala'
--- test/direct/table-lookup-test.vala	2012-02-05 14:52:13 +0000
+++ test/direct/table-lookup-test.vala	2012-02-13 20:22:18 +0000
@@ -38,6 +38,7 @@
 
     Test.add_func ("/WhereClause/basic", basic_test);
     Test.add_func ("/WhereClause/delete_hook", engine_test);
+    Test.add_func ("/WhereClause/get_value_query", get_value_with_query_test);
 
     return Test.run ();
 }
@@ -56,16 +57,28 @@
     unowned Sqlite.Database db = database.database;
     TableLookup table_lookup = new TableLookup (database, "actor");
 
-    assert_cmpint (table_lookup.get_id ("1st-actor"), OperatorType.EQUAL, 1);
-    assert_cmpint (table_lookup.get_id ("2nd-actor"), OperatorType.EQUAL, 2);
-    assert_cmpint (table_lookup.get_id ("1st-actor"), OperatorType.EQUAL, 1);
+    int id = table_lookup.get_id ("1st-actor");
+    assert_cmpint (table_lookup.get_id ("2nd-actor"), OperatorType.EQUAL, id+1);
+    assert_cmpint (table_lookup.get_id ("1st-actor"), OperatorType.EQUAL, id);
 
     int rc = db.exec ("DELETE FROM actor WHERE value='1st-actor'");
     assert (rc == Sqlite.OK);
 
     table_lookup.remove (1);
-    assert_cmpint (table_lookup.get_id ("2nd-actor"), OperatorType.EQUAL, 2);
-    assert_cmpint (table_lookup.get_id ("1st-actor"), OperatorType.EQUAL, 3);
+    assert_cmpint (table_lookup.get_id ("2nd-actor"), OperatorType.EQUAL, id+1);
+    assert_cmpint (table_lookup.get_id ("1st-actor"), OperatorType.EQUAL, id+2);
+}
+
+public void get_value_with_query_test ()
+{
+    Database database = new Zeitgeist.SQLite.Database ();
+    unowned Sqlite.Database db = database.database;
+    TableLookup table_lookup = new TableLookup (database, "actor");
+
+    int rc = db.exec ("INSERT INTO actor (id, value) VALUES (100, 'new-actor')");
+    assert (rc == Sqlite.OK);
+
+    assert_cmpstr (table_lookup.get_value (100), OperatorType.EQUAL, "new-actor");
 }
 
 public void engine_test ()
@@ -82,8 +95,6 @@
     int rc = db.exec ("DELETE FROM actor WHERE value='something'");
     assert (rc == Sqlite.OK);
 
-    assert_cmpint (
-        table_lookup.get_id ("sqlite-reuses-the-id"), OperatorType.EQUAL, 1);
     assert_cmpint (table_lookup.get_id ("something"), OperatorType.EQUAL, 2);
 }