Merge lp:~midori/midori/makeAstatement into lp:midori

[Cris Dywan]

Introduce high-level prepare/ DatabaseStatement API

[André Stösel (ivaldi)]

"statement = prepare (sqlcmd," is quite confusing if you just look at the diff (I would prefer this.prepare (..) (same for "exec", "exec_script",..)), but I guess it has nothing to do with this patch/branch -> approved

review: Approve

=== modified file 'midori/midori-database.vala'
--- midori/midori-database.vala	2013-10-12 11:13:09 +0000
+++ midori/midori-database.vala	2013-12-10 21:57:30 +0000
@@ -10,13 +10,120 @@
 */
 
 namespace Midori {
+    /*
+     * Since: 0.5.6
+     */
     public errordomain DatabaseError {
         OPEN,
         NAMING,
         FILENAME,
         EXECUTE,
-    }
-
+        COMPILE,
+        TYPE,
+    }
+
+    /*
+     * Since: 0.5.7
+     */
+    public class DatabaseStatement : GLib.Object, GLib.Initable {
+        public Sqlite.Statement? stmt { get { return _stmt; } }
+        protected Sqlite.Statement _stmt = null;
+        public Database? database { get; set construct; }
+        public string? query { get; set construct; }
+
+        public DatabaseStatement (Database database, string query) throws DatabaseError {
+            Object (database: database, query: query);
+            init ();
+        }
+
+        public virtual bool init (GLib.Cancellable? cancellable = null) throws DatabaseError {
+            int result = database.db.prepare_v2 (query, -1, out _stmt, null);
+            if (result != Sqlite.OK)
+                throw new DatabaseError.COMPILE ("Failed to compile statement: %s".printf (query));
+            return true;
+        }
+
+        /*
+         * Bind values to named parameters.
+         * SQL: "SELECT foo FROM bar WHERE id = :session_id"
+         * Vala: statement.bind(":session_id", typeof (int64), 12345);
+         * Supported types: string, int64, double
+         */
+        public void bind (string pname, ...) throws DatabaseError {
+            int pindex = stmt.bind_parameter_index (pname);
+            var args = va_list ();
+            Type ptype = args.arg ();
+            if (ptype == typeof (string))
+                stmt.bind_text (pindex, args.arg ());
+            else if (ptype == typeof (int64))
+                stmt.bind_int64 (pindex, args.arg ());
+            else if (ptype == typeof (double))
+                stmt.bind_double (pindex, args.arg ());
+            else
+                throw new DatabaseError.TYPE ("Invalid type '%s' for '%s' in statement: %s".printf (ptype.name (), pname, query));
+        }
+
+        /*
+         * Execute the statement, it's an error if there are more rows.
+         */
+        public bool exec () throws DatabaseError {
+            if (step ())
+                throw new DatabaseError.EXECUTE ("More rows available - use step instead of exec");
+            return true;
+        }
+
+        /*
+         * Proceed to the next row, returns false when the end is nigh.
+         */
+        public bool step () throws DatabaseError {
+            int result = stmt.step ();
+            if (result != Sqlite.DONE && result != Sqlite.ROW)
+                throw new DatabaseError.EXECUTE (database.db.errmsg ());
+            return result == Sqlite.ROW;
+        }
+
+        private int column_index (string name) throws DatabaseError {
+            for (int i = 0; i < stmt.column_count (); i++) {
+                if (name == stmt.column_name (i))
+                    return i;
+            }
+            throw new DatabaseError.TYPE ("No such column '%s' in row: %s".printf (name, query));
+        }
+
+        /*
+         * Get a string value by its named parameter, for example ":uri".
+         */
+        public string? get_string (string name) throws DatabaseError {
+            int index = column_index (name);
+            if (stmt.column_type (index) != Sqlite.TEXT)
+                throw new DatabaseError.TYPE ("Getting '%s' with wrong type in row: %s".printf (name, query));
+            return stmt.column_text (index);
+        }
+
+        /*
+         * Get an integer value by its named parameter, for example ":day".
+         */
+        public int64 get_int64 (string name) throws DatabaseError {
+            int index = column_index (name);
+            if (stmt.column_type (index) != Sqlite.INTEGER)
+                throw new DatabaseError.TYPE ("Getting '%s' with wrong type in row: %s".printf (name, query));
+            return stmt.column_int64 (index);
+        }
+
+        /*
+         * Get a double value by its named parameter, for example ":session_id".
+         */
+        public double get_double (string name) throws DatabaseError {
+            int index = column_index (name);
+            if (stmt.column_type (index) != Sqlite.FLOAT)
+                throw new DatabaseError.TYPE ("Getting '%s' with wrong type in row: %s".printf (name, query));
+            return stmt.column_double (index);
+        }
+    }
+
+    /*
+     * Since: 0.5.6
+     */
     public class Database : GLib.Object, GLib.Initable {
         public Sqlite.Database? db { get { return _db; } }
         protected Sqlite.Database? _db = null;
@@ -104,5 +211,32 @@
                 throw new DatabaseError.EXECUTE (db.errmsg ());
             return true;
         }
+
+        /*
+         * Prepare a statement with optionally binding parameters by name.
+         * See also DatabaseStatement.bind().
+         * Since: 0.5.7
+         */
+        public DatabaseStatement prepare (string query, ...) throws DatabaseError {
+            var statement = new DatabaseStatement (this, query);
+            var args = va_list ();
+            unowned string? pname = args.arg ();
+            while (pname != null) {
+                Type ptype = args.arg ();
+                if (ptype == typeof (string)) {
+                    string pvalue = args.arg ();
+                    statement.bind (pname, ptype, pvalue);
+                } else if (ptype == typeof (int64)) {
+                    int64 pvalue = args.arg ();
+                    statement.bind (pname, ptype, pvalue);
+                } else if (ptype == typeof (double)) {
+                    double pvalue = args.arg ();
+                    statement.bind (pname, ptype, pvalue);
+                } else
+                    throw new DatabaseError.TYPE ("Invalid type '%s' in statement: %s".printf (ptype.name (), query));
+                pname = args.arg ();
+            }
+            return statement;
+        }
     }
 }

=== modified file 'midori/midori-historydatabase.vala'
--- midori/midori-historydatabase.vala	2013-10-11 22:43:02 +0000
+++ midori/midori-historydatabase.vala	2013-12-10 21:57:30 +0000
@@ -52,58 +52,49 @@
         public async List<HistoryItem>? query (string sqlcmd, string? filter, int day, int max_items, Cancellable cancellable) {
             return_val_if_fail (db != null, null);
 
-            Sqlite.Statement stmt;
-            int result;
-
-            result = db.prepare_v2 (sqlcmd, -1, out stmt, null);
-            if (result != Sqlite.OK) {
-                critical (_("Failed to select from history: %s"), db.errmsg ());
-                return null;
-            }
-
-            if (":filter" in sqlcmd) {
+            Midori.DatabaseStatement statement;
+
+            try {
                 string real_filter = "%" + filter.replace (" ", "%") + "%";
-                stmt.bind_text (stmt.bind_parameter_index (":filter"), real_filter);
-            }
-            if (":day" in sqlcmd)
-                stmt.bind_int64 (stmt.bind_parameter_index (":day"), day);
-            if (":limit" in sqlcmd)
-                stmt.bind_int64 (stmt.bind_parameter_index (":limit"), max_items);
-
-            result = stmt.step ();
-            if (!(result == Sqlite.DONE || result == Sqlite.ROW)) {
-                critical (_("Failed to select from history: %s"), db.errmsg ());
+                statement = prepare (sqlcmd,
+                    ":filter", typeof (string), real_filter,
+                    ":day", typeof (int64), day,
+                    ":limit", typeof (int64), max_items);
+            } catch (Error error) {
+                critical (_("Failed to select from history: %s"), error.message);
                 return null;
             }
 
             var items = new List<HistoryItem> ();
-            while (result == Sqlite.ROW) {
-                int64 type = stmt.column_int64 (0);
-                int64 date = stmt.column_int64 (1);
-                switch (type) {
-                    case 1:
-                        string uri = stmt.column_text (2);
-                        string title = stmt.column_text (3);
-                        items.append (new HistoryWebsite (uri, title, date));
-                        break;
-                    case 2:
-                        string uri = stmt.column_text (2);
-                        string title = stmt.column_text (3);
-                        items.append (new HistorySearch (uri, title, date));
-                        break;
-                    default:
-                        warn_if_reached ();
-                        break;
+            try {
+                while (statement.step ()) {
+                    int64 type = statement.get_int64 ("type");
+                    int64 date = statement.get_int64 ("date");
+                    switch (type) {
+                        case 1:
+                            string uri = statement.get_string ("uri");
+                            string title = statement.get_string ("title");
+                            items.append (new HistoryWebsite (uri, title, date));
+                            break;
+                        case 2:
+                            string uri = statement.get_string ("uri");
+                            string title = statement.get_string ("title");
+                            items.append (new HistorySearch (uri, title, date));
+                            break;
+                        default:
+                            warn_if_reached ();
+                            break;
+                    }
+
+                    uint src = Idle.add (query.callback);
+                    yield;
+                    Source.remove (src);
+
+                    if (cancellable.is_cancelled ())
+                        return null;
                 }
-
-                uint src = Idle.add (query.callback);
-                yield;
-                Source.remove (src);
-
-                if (cancellable.is_cancelled ())
-                    return null;
-
-                result = stmt.step ();
+            } catch (Error error) {
+                critical (_("Failed to select from history: %s"), error.message);
             }
 
             if (cancellable.is_cancelled ())