Clean up sql implementation.

Now that sql is only used for history (not quickmarks/bookmarks) a number of
functions are no longer needed. In addition, primary key support was removed as
we actually need to support multiple entries for the same url with different
access times. The completion model will have to handle this by selecting
something like (url, title, max(atime)).

This also fixes up a number of tests that were broken with the last few
sql-related commits.

qutebrowser/browser/history.py

@@ -97,7 +97,7 @@ class WebHistory(sql.SqlTable):
 
     def __init__(self, parent=None):
         super().__init__("History", ['url', 'title', 'atime', 'redirect'],
-                         primary_key='url', parent=parent)
+                         parent=parent)
 
     def __repr__(self):
         return utils.get_repr(self, length=len(self))
@@ -105,7 +105,7 @@ class WebHistory(sql.SqlTable):
     def _add_entry(self, entry):
         """Add an entry to the in-memory database."""
         self.insert([entry.url_str(), entry.title, entry.atime,
-                     entry.redirect], replace=True)
+                     entry.redirect])
 
     def get_recent(self):
         """Get the most recent history entries."""

qutebrowser/misc/sql.py

@@ -101,6 +101,7 @@ def run_batch(querystr, values):
 
     return query
 
+
 class SqlTable(QObject):
 
     """Interface to a sql table.
@@ -108,7 +109,6 @@ class SqlTable(QObject):
     Attributes:
         Entry: The class wrapping row data from this table.
         _name: Name of the SQL table this wraps.
-        _primary_key: The primary key of the table.
 
     Signals:
         changed: Emitted when the table is modified.
@@ -116,7 +116,7 @@ class SqlTable(QObject):
 
     changed = pyqtSignal()
 
-    def __init__(self, name, fields, primary_key, parent=None):
+    def __init__(self, name, fields, parent=None):
         """Create a new table in the sql database.
 
         Raises SqlException if the table already exists.
@@ -124,11 +124,9 @@ class SqlTable(QObject):
         Args:
             name: Name of the table.
             fields: A list of field names.
-            primary_key: Name of the field to serve as the primary key.
         """
         super().__init__(parent)
         self._name = name
-        self._primary_key = primary_key
         run_query("CREATE TABLE IF NOT EXISTS {} ({})"
                   .format(name, ','.join(fields)))
         # pylint: disable=invalid-name
@@ -141,74 +139,47 @@ class SqlTable(QObject):
             rec = result.record()
             yield self.Entry(*[rec.value(i) for i in range(rec.count())])
 
-    def __contains__(self, key):
-        """Return whether the table contains the matching item.
-
-        Args:
-            key: Primary key value to search for.
-        """
-        query = run_query("SELECT * FROM {} where {} = ?".format(
-            self._name, self._primary_key), [key])
-        return query.next()
-
     def __len__(self):
         """Return the count of rows in the table."""
         result = run_query("SELECT count(*) FROM {}".format(self._name))
         result.next()
         return result.value(0)
 
-    def __getitem__(self, key):
+    def delete(self, value, field):
-        """Retrieve the row matching the given key.
-
-        Args:
-            key: Primary key value to fetch.
-        """
-        result = run_query("SELECT * FROM {} where {} = ?".format(
-            self._name, self._primary_key), [key])
-        result.next()
-        rec = result.record()
-        return self.Entry(*[rec.value(i) for i in range(rec.count())])
-
-    def delete(self, value, field=None):
         """Remove all rows for which `field` equals `value`.
 
         Args:
             value: Key value to delete.
-            field: Field to use as the key, defaults to the primary key.
+            field: Field to use as the key.
 
         Return:
             The number of rows deleted.
         """
-        field = field or self._primary_key
         query = run_query("DELETE FROM {} where {} = ?".format(
             self._name, field), [value])
         if not query.numRowsAffected():
             raise KeyError('No row with {} = "{}"'.format(field, value))
         self.changed.emit()
 
-    def insert(self, values, replace=False):
+    def insert(self, values):
         """Append a row to the table.
 
         Args:
             values: A list of values to insert.
-            replace: If true, allow inserting over an existing primary key.
         """
-        cmd = "REPLACE" if replace else "INSERT"
         paramstr = ','.join(['?'] * len(values))
-        run_query("{} INTO {} values({})".format(cmd, self._name, paramstr),
+        run_query("INSERT INTO {} values({})".format(self._name, paramstr),
             values)
         self.changed.emit()
 
-    def insert_batch(self, rows, replace=False):
+    def insert_batch(self, rows):
         """Performantly append multiple rows to the table.
 
         Args:
             rows: A list of lists, where each sub-list is a row.
-            replace: If true, allow inserting over an existing primary key.
         """
-        cmd = "REPLACE" if replace else "INSERT"
         paramstr = ','.join(['?'] * len(rows[0]))
-        run_batch("{} INTO {} values({})".format(cmd, self._name, paramstr),
+        run_batch("INSERT INTO {} values({})".format(self._name, paramstr),
             rows)
         self.changed.emit()
 

qutebrowser/utils/version.py

@@ -327,7 +327,8 @@ def version():
 
     lines += ['pdf.js: {}'.format(_pdfjs_version())]
 
-    sql.init()
+    # we can use an in-memory database as we just want to query the version
+    sql.init('')
     lines += ['sqlite: {}'.format(sql.version())]
     sql.close()
 

tests/end2end/features/test_history_bdd.py

@@ -20,31 +20,29 @@
 import os.path
 
 import pytest_bdd as bdd
+
+from PyQt5.QtSql import QSqlDatabase, QSqlQuery
+
 bdd.scenarios('history.feature')
 
 
 @bdd.then(bdd.parsers.parse("the history file should contain:\n{expected}"))
 def check_history(quteproc, httpbin, expected):
-    history_file = os.path.join(quteproc.basedir, 'data', 'history')
+    path = os.path.join(quteproc.basedir, 'data', 'history.sqlite')
-    quteproc.send_cmd(':save history')
+    db = QSqlDatabase.addDatabase('QSQLITE')
-    quteproc.wait_for(message=':save saved history')
+    db.setDatabaseName(path)
-
+    assert db.open(), 'Failed to open history database'
-    expected = expected.replace('(port)', str(httpbin.port)).splitlines()
+    query = db.exec_('select * from History')
-
+    actual = []
-    with open(history_file, 'r', encoding='utf-8') as f:
+    while query.next():
-        lines = []
+        rec = query.record()
-        for line in f:
+        url = rec.value(0)
-            if not line.strip():
+        title = rec.value(1)
-                continue
+        redirect = rec.value(3)
-            print('history line: ' + line)
+        actual.append('{} {} {}'.format('r' * redirect, url, title).strip())
-            atime, line = line.split(' ', maxsplit=1)
+    db = None
-            line = line.rstrip()
+    QSqlDatabase.removeDatabase(QSqlDatabase.database().connectionName())
-            if '-' in atime:
+    assert actual == expected.replace('(port)', str(httpbin.port)).splitlines()
-                flags = atime.split('-')[1]
-                line = '{} {}'.format(flags, line)
-            lines.append(line)
-
-    assert lines == expected
 
 
 @bdd.then("the history file should be empty")

tests/unit/browser/test_qutescheme.py

@@ -98,7 +98,7 @@ class TestHistoryHandler:
     @pytest.fixture
     def fake_web_history(self, fake_save_manager, tmpdir, init_sql):
         """Create a fake web-history and register it into objreg."""
-        web_history = history.WebHistory(tmpdir.dirname, 'fake-history')
+        web_history = history.WebHistory()
         objreg.register('web-history', web_history)
         yield web_history
         objreg.delete('web-history')
@@ -108,7 +108,6 @@ class TestHistoryHandler:
         """Create fake history."""
         for item in entries:
             fake_web_history._add_entry(item)
-        fake_web_history.save()
 
     @pytest.mark.parametrize("start_time_offset, expected_item_count", [
         (0, 4),

tests/unit/browser/webkit/test_history.py

@@ -65,14 +65,6 @@ def test_len(hist):
     assert len(hist) == 1
 
 
-def test_updated_entries(tmpdir, hist):
-    hist.add_url(QUrl('http://example.com/'), atime=67890)
-    assert list(hist) == [('http://example.com/', '', 67890, False)]
-
-    hist.add_url(QUrl('http://example.com/'), atime=99999)
-    assert list(hist) == [('http://example.com/', '', 99999, False)]
-
-
 def test_get_recent(hist):
     hist.add_url(QUrl('http://www.qutebrowser.org/'), atime=67890)
     hist.add_url(QUrl('http://example.com/'), atime=12345)
@@ -117,7 +109,7 @@ def test_clear_force(qtbot, tmpdir, hist):
 def test_add_item(qtbot, hist, item):
     (url, atime, title, redirect) = item
     hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect)
-    assert hist[url] == (url, title, atime, redirect)
+    assert list(hist) == [(url, title, atime, redirect)]
 
 
 def test_add_item_invalid(qtbot, hist, caplog):
@@ -139,15 +131,6 @@ def test_add_from_tab(hist, level, url, req_url, expected, mock_time, caplog):
     assert set(list(hist)) == set(expected)
 
 
-def test_add_item_redirect_update(qtbot, tmpdir, hist):
-    """A redirect update added should override a non-redirect one."""
-    url = 'http://www.example.com/'
-    hist.add_url(QUrl(url), atime=5555)
-    hist.add_url(QUrl(url), redirect=True, atime=67890)
-
-    assert hist[url] == (url, '', 67890, True)
-
-
 @pytest.fixture
 def hist_interface():
     # pylint: disable=invalid-name

tests/unit/completion/test_models.py

@@ -157,8 +157,7 @@ def bookmarks(bookmark_manager_stub):
 @pytest.fixture
 def web_history(stubs, init_sql):
     """Pre-populate the web-history database."""
-    table = sql.SqlTable("History", ['url', 'title', 'atime', 'redirect'],
+    table = sql.SqlTable("History", ['url', 'title', 'atime', 'redirect'])
-                         primary_key='url')
     table.insert(['http://some-redirect.example.com', 'redirect',
                   datetime(2016, 9, 5).timestamp(), True])
     table.insert(['http://qutebrowser.org', 'qutebrowser',

tests/unit/completion/test_sqlcategory.py

@@ -71,7 +71,7 @@ def _validate(cat, expected):
      [('B', 'C', 2), ('C', 'A', 1), ('A', 'F', 0)]),
 ])
 def test_sorting(sort_by, sort_order, data, expected):
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
     for row in data:
         table.insert(row)
     cat = sqlcategory.SqlCategory('Foo', sort_by=sort_by,
@@ -126,7 +126,7 @@ def test_sorting(sort_by, sort_order, data, expected):
 ])
 def test_set_pattern(pattern, filter_cols, before, after):
     """Validate the filtering and sorting results of set_pattern."""
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
     for row in before:
         table.insert(row)
     cat = sqlcategory.SqlCategory('Foo')
@@ -135,14 +135,14 @@ def test_set_pattern(pattern, filter_cols, before, after):
 
 
 def test_select():
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
     table.insert(['foo', 'bar', 'baz'])
     cat = sqlcategory.SqlCategory('Foo', select='b, c, a')
     _validate(cat, [('bar', 'baz', 'foo')])
 
 
 def test_where():
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
     table.insert(['foo', 'bar', False])
     table.insert(['baz', 'biz', True])
     cat = sqlcategory.SqlCategory('Foo', where='not c')
@@ -150,7 +150,7 @@ def test_where():
 
 
 def test_entry():
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
     assert hasattr(table.Entry, 'a')
     assert hasattr(table.Entry, 'b')
     assert hasattr(table.Entry, 'c')

tests/unit/misc/test_sql.py

@@ -27,24 +27,21 @@ pytestmark = pytest.mark.usefixtures('init_sql')
 
 
 def test_init():
-    sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     # should not error if table already exists
-    sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    sql.SqlTable('Foo', ['name', 'val', 'lucky'])
 
 
 def test_insert(qtbot):
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     with qtbot.waitSignal(table.changed):
         table.insert(['one', 1, False])
     with qtbot.waitSignal(table.changed):
         table.insert(['wan', 1, False])
-    with pytest.raises(sql.SqlException):
-        # duplicate primary key
-        table.insert(['one', 1, False])
 
 
 def test_iter():
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     table.insert(['one', 1, False])
     table.insert(['nine', 9, False])
     table.insert(['thirteen', 13, True])
@@ -59,29 +56,21 @@ def test_iter():
     ([[2, 5], [1, 6], [3, 4]], 'b', 'desc', 2, [(1, 6), (2, 5)])
 ])
 def test_select(rows, sort_by, sort_order, limit, result):
-    table = sql.SqlTable('Foo', ['a', 'b'], primary_key='a')
+    table = sql.SqlTable('Foo', ['a', 'b'])
     for row in rows:
         table.insert(row)
     assert list(table.select(sort_by, sort_order, limit)) == result
 
 
-def test_replace(qtbot):
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
-    table.insert(['one', 1, False])
-    with qtbot.waitSignal(table.changed):
-        table.insert(['one', 1, True], replace=True)
-    assert list(table) == [('one', 1, True)]
-
-
 def test_delete(qtbot):
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     table.insert(['one', 1, False])
     table.insert(['nine', 9, False])
     table.insert(['thirteen', 13, True])
     with pytest.raises(KeyError):
-        table.delete('nope')
+        table.delete('nope', 'name')
     with qtbot.waitSignal(table.changed):
-        table.delete('thirteen')
+        table.delete('thirteen', 'name')
     assert list(table) == [('one', 1, False), ('nine', 9, False)]
     with qtbot.waitSignal(table.changed):
         table.delete(False, field='lucky')
@@ -89,7 +78,7 @@ def test_delete(qtbot):
 
 
 def test_len():
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     assert len(table) == 0
     table.insert(['one', 1, False])
     assert len(table) == 1
@@ -99,32 +88,8 @@ def test_len():
     assert len(table) == 3
 
 
-def test_contains():
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
-    table.insert(['one', 1, False])
-    table.insert(['nine', 9, False])
-    table.insert(['thirteen', 13, True])
-    assert 'oone' not in table
-    assert 'ninee' not in table
-    assert 1 not in table
-    assert '*' not in table
-    assert 'one' in table
-    assert 'nine' in table
-    assert 'thirteen' in table
-
-
-def test_getitem():
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
-    table.insert(['one', 1, False])
-    table.insert(['nine', 9, False])
-    table.insert(['thirteen', 13, True])
-    assert table['one'] == ('one', 1, False)
-    assert table['nine'] == ('nine', 9, False)
-    assert table['thirteen'] == ('thirteen', 13, True)
-
-
 def test_delete_all(qtbot):
-    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'], primary_key='name')
+    table = sql.SqlTable('Foo', ['name', 'val', 'lucky'])
     table.insert(['one', 1, False])
     table.insert(['nine', 9, False])
     table.insert(['thirteen', 13, True])