Merge: Benitlux & sqlite3: fix closing SQLite statements and malformed superstring
authorJean Privat <jean@pryen.org>
Thu, 14 Apr 2016 01:03:03 +0000 (21:03 -0400)
committerJean Privat <jean@pryen.org>
Thu, 14 Apr 2016 01:03:03 +0000 (21:03 -0400)
Fix statements that were left open and did not remove the lock on the database. It now relies on iterators to close the statement after the end of the loop. This requires to actually complete the loop and not return before it is complete. A shortcut is to use `statement.iterator.to_a` when the result is expected to be short, like with a single row.

Note that I did not use `with` because it has the same problem with a `return` skipping the call to `finish`. Neither did I use `Finalizable` because it is invoked by the GC which may be much later.

Also fix one malformed superstring and improve the style of another one.

Pull-Request: #2009
Reviewed-by: Jean Privat <jean@pryen.org>

contrib/benitlux/src/benitlux_db.nit
contrib/benitlux/src/benitlux_social.nit
lib/core/collection/array.nit
lib/sqlite3/sqlite3.nit
tests/sav/test_new_native_alt1.res

index 73739d9..81ec6bf 100644 (file)
@@ -134,8 +134,13 @@ class BenitluxDB
        do
                var stmt = select("ROWID, name, desc FROM beers WHERE ROWID = {id}")
                if stmt == null then return null
-               for row in stmt do return row.to_beer
-               return null
+
+               var res = null
+               for row in stmt do
+                       res = row.to_beer
+                       break
+               end
+               return res
        end
 
        # Days where `beer` was available, all known days if `beer == null`
index ae409a8..bdd83b4 100644 (file)
@@ -110,6 +110,7 @@ GROUP BY beer0, beer1""") else
                        var user_id = row[0].to_i
                        var token = new_token(user_id)
                        var u = new User(user_id, row[1].to_s)
+                       stmt.close
                        return new LoginResult(u, token)
                end
                return null
@@ -153,8 +154,12 @@ GROUP BY beer0, beer1""") else
                # TODO update token timestamp and platform/client hint of last connection.
                # These informations could help detect malicious access to the account.
 
-               for row in stmt do return row[0].to_i
-               return null
+               var res = null
+               for row in stmt do
+                       res = row[0].to_i
+                       break
+               end
+               return res
        end
 
        # Get `User` data from the integer `id`
@@ -163,8 +168,12 @@ GROUP BY beer0, beer1""") else
                var stmt = select("name FROM users WHERE ROWID = {id}")
                assert stmt != null
 
-               for row in stmt do return new User(id, row[0].to_s)
-               return null
+               var res = null
+               for row in stmt do
+                       res = new User(id, row[0].to_s)
+                       break
+               end
+               return res
        end
 
        # Try to sign up a new user, return `true` on success
@@ -211,8 +220,12 @@ GROUP BY beer0, beer1""") else
                var b = beer_from_id(beer)
                if b == null then return null
 
-               for row in stmt do return new BeerStats(b, row[0].to_f, row[1].to_i)
-               return null
+               var res = null
+               for row in stmt do
+                       res = new BeerStats(b, row[0].to_f, row[1].to_i)
+                       break
+               end
+               return res
        end
 
        # Fetch the most recent rating left by `user_id` about `beer`
@@ -220,8 +233,13 @@ GROUP BY beer0, beer1""") else
        do
                var stmt = select("rating FROM reviews WHERE author = {user_id} AND beer = {beer} ORDER BY ROWID DESC LIMIT 1")
                assert stmt != null else print_error "Select 'rating' failed with: {error or else "?"}"
-               for row in stmt do return row[0].to_i
-               return null
+
+               var res = null
+               for row in stmt do
+                       res = row[0].to_i
+                       break
+               end
+               return res
        end
 
        # Register that `user_from` follows `user_to`
@@ -247,7 +265,8 @@ GROUP BY beer0, beer1""") else
                assert stmt != null else
                        print_error "Select 'follows' failed with: {error or else "?"}"
                end
-               for row in stmt do return true
+
+               for row in stmt.iterator.to_a do return true
                return false
        end
 
@@ -293,9 +312,10 @@ GROUP BY beer0, beer1""") else
        # List reciprocal friends of `user_id`
        fun followed_followers(user_id: Int): nullable Array[User]
        do
-               var stmt = select("ROWID, name FROM users WHERE " +
-                       "ROWID in (SELECT user_from FROM follows WHERE user_to = {user_id}) AND " +
-                       "ROWID in (SELECT user_to FROM follows WHERE user_from = {user_id})")
+               var stmt = select("""
+ROWID, name FROM users WHERE
+       users.ROWID in (SELECT user_from FROM follows WHERE user_to = {{{user_id}}}) AND
+       users.ROWID in (SELECT user_to FROM follows WHERE user_from = {{{user_id}}})""")
                assert stmt != null else print_error "Select 'followed_followers' failed with: {error or else "?"}"
 
                var users = new Array[User]
@@ -495,8 +515,8 @@ ORDER BY average LIMIT 1""")
                var sql = """
 ROWID, name FROM users
 WHERE 1 in (SELECT is_in FROM checkins WHERE user = users.ROWID ORDER BY ROWID DESC LIMIT 1)
-       AND ROWID in (SELECT user_from FROM follows WHERE user_to = {user_id})
-       AND ROWID in (SELECT user_to FROM follows WHERE user_from = {user_id})"""
+       AND ROWID in (SELECT user_from FROM follows WHERE user_to = {{{user_id}}})
+       AND ROWID in (SELECT user_to FROM follows WHERE user_from = {{{user_id}}})"""
 
                var stmt = select(sql)
                if stmt == null then
index e28e735..79e7a70 100644 (file)
@@ -955,6 +955,7 @@ redef class Iterator[E]
                        res.add(item)
                        next
                end
+               finish
                return res
        end
 end
index 1d6287a..62fe8e5 100644 (file)
@@ -42,6 +42,8 @@ class Sqlite3DB
        # Close this connection to the DB and all open statements
        fun close
        do
+               if not is_open then return
+
                is_open = false
 
                # close open statements
@@ -110,16 +112,32 @@ class Sqlite3DB
        fun last_insert_rowid: Int do return native_connection.last_insert_rowid
 end
 
-# A prepared Sqlite3 statement, created from `Sqlite3DB::prepare` or `Sqlite3DB::select`
+# Prepared Sqlite3 statement
+#
+# Instances of this class are created from `Sqlite3DB::prepare` and
+# its shortcuts: `create_table`, `insert`, `replace` and `select`.
+# The results should be explored with an `iterator`,
+# and each call to `iterator` resets the request.
+# If `close_with_iterator` the iterator calls `close`
+# on this request upon finishing.
 class Statement
        private var native_statement: NativeStatement
 
        # Is this statement usable?
        var is_open = true
 
+       # Should any `iterator` close this statement on `Iterator::finish`?
+       #
+       # If `true`, the default, any `StatementIterator` created by calls to
+       # `iterator` invokes `close` on this request when finished iterating.
+       # Otherwise, `close` must be called manually.
+       var close_with_iterator = true is writable
+
        # Close and finalize this statement
        fun close
        do
+               if not is_open then return
+
                is_open = false
                native_statement.finalize
        end
@@ -276,6 +294,8 @@ class StatementIterator
                        is_ok = false
                end
        end
+
+       redef fun finish do if statement.close_with_iterator then statement.close
 end
 
 # A data type supported by Sqlite3
index 897691a..9e5ceab 100644 (file)
@@ -1,4 +1,4 @@
-Runtime error: Cast failed. Expected `E`, got `Bool` (../lib/core/collection/array.nit:988)
+Runtime error: Cast failed. Expected `E`, got `Bool` (../lib/core/collection/array.nit:989)
 NativeString
 0x4e
 Nit