[tor-commits] [stem/master] Replace database function with query

[atagar at torproject.org]

commit f075f94c7a7e7d3efa0104f1b8ba7e03ff4e4dd8
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Aug 27 10:46:57 2017 -0700

    Replace database function with query
    
    On reflection what callers will find useful is an easy function to make sqlite
    queries - not a function to get a database connection. Initially I was wary of
    this due to being unable to know when to close the connection, but from the
    looks of it when accessed in a read-only fashion this doesn't really matter.
---
 stem/manual.py      | 34 +++++++++++++++++++++++-----------
 test/unit/manual.py | 13 ++++++++-----
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/stem/manual.py b/stem/manual.py
index 9b9c0252..7dd2be63 100644
--- a/stem/manual.py
+++ b/stem/manual.py
@@ -34,6 +34,7 @@ us what our torrc options do...
 
 ::
 
+  query - performs a query on our cached sqlite manual information
   is_important - Indicates if a configuration option is of particularly common importance.
   download_man_page - Downloads tor's latest man page.
 
@@ -82,6 +83,7 @@ except ImportError:
 Category = stem.util.enum.Enum('GENERAL', 'CLIENT', 'RELAY', 'DIRECTORY', 'AUTHORITY', 'HIDDEN_SERVICE', 'TESTING', 'UNKNOWN')
 GITWEB_MANUAL_URL = 'https://gitweb.torproject.org/tor.git/plain/doc/tor.1.txt'
 CACHE_PATH = os.path.join(os.path.dirname(__file__), 'cached_tor_manual.sqlite')
+DATABASE = {}  # read-only sqlite database connections
 
 CATEGORY_SECTIONS = OrderedDict((
   ('GENERAL OPTIONS', Category.GENERAL),
@@ -94,33 +96,43 @@ CATEGORY_SECTIONS = OrderedDict((
 ))
 
 
-def database(path = None):
+def query(query, path = None):
   """
-  Provides database connection for our sqlite cache. This database should be
-  treated as being read-only, with this restriction being enforced in the
-  future.
+  Performs the given query on our sqlite manual cache. This database should
+  be treated as being read-only. File permissions generally enforce this, and
+  in the future will be enforced by this function as well.
 
   .. versionadded:: 1.6.0
 
+  :param str query: query to run on the cache
   :param str path: cached manual content to read, if not provided this uses
     the bundled manual information
 
-  :returns: :class:`sqlite3.Connection` for the database cache
+  :returns: :class:`sqlite3.Cursor` with the query results
 
-  :raises: **IOError** if a **path** was provided and we were unable to read it
+  :raises:
+    * **sqlite3.OperationalError** if query fails
+    * **IOError** if a **path** was provided and we were unable to read it
   """
 
+  # The only reason to explicitly close the sqlite connection is to ensure
+  # transactions are committed. Since we're only using read-only access this
+  # doesn't matter, and can allow interpreter shutdown to do the needful.
+  #
+  # TODO: When we only support python 3.4+ we can use sqlite's uri argument
+  # to enforce a read-only connection...
+  #
+  #   https://docs.python.org/3/library/sqlite3.html#sqlite3.connect
+
   if path is None:
     path = CACHE_PATH
   elif not os.path.exists(path):
     raise IOError("%s doesn't exist" % path)
 
-  # TODO: When we only support python 3.4+ we can use sqlite's uri argument to
-  # get a read-only connection...
-  #
-  #   https://docs.python.org/3/library/sqlite3.html#sqlite3.connect
+  if path not in DATABASE:
+    DATABASE[path] = sqlite3.connect(path)
 
-  return sqlite3.connect(path)
+  return DATABASE[path].execute(query)
 
 
 class ConfigOption(object):
diff --git a/test/unit/manual.py b/test/unit/manual.py
index eaca3629..20230e88 100644
--- a/test/unit/manual.py
+++ b/test/unit/manual.py
@@ -5,6 +5,7 @@ Unit testing for the stem.manual module.
 import io
 import os
 import re
+import sqlite3
 import tempfile
 import unittest
 
@@ -102,12 +103,14 @@ def _cached_manual():
 
 
 class TestManual(unittest.TestCase):
-  def test_database(self):
-    with stem.manual.database() as conn:
-      self.assertEqual("If set, this option overrides the default location and file name for Tor's cookie file. (See CookieAuthentication above.)", conn.execute('SELECT description FROM torrc WHERE name="CookieAuthFile"').fetchone()[0])
+  def test_query(self):
+    self.assertEqual("If set, this option overrides the default location and file name for Tor's cookie file. (See CookieAuthentication above.)", stem.manual.query('SELECT description FROM torrc WHERE name="CookieAuthFile"').fetchone()[0])
 
-  def test_missing_database(self):
-    self.assertRaisesRegexp(IOError, "/no/such/path doesn't exist", stem.manual.database, '/no/such/path')
+  def test_query_on_failure(self):
+    self.assertRaisesRegexp(sqlite3.OperationalError, 'near "hello": syntax error', stem.manual.query, 'hello world')
+
+  def test_query_with_missing_database(self):
+    self.assertRaisesRegexp(IOError, "/no/such/path doesn't exist", stem.manual.query, 'SELECT * FROM torrc', '/no/such/path')
 
   def test_has_all_summaries(self):
     """