Add LIMIT to history query.

For performance, re-introduce web-history-max-items.
As the history query has now become a very specific multi-part query and
history completion was the only consumer of SqlCategory, SqlCategory is
now replaced by a HistoryCategory class.

qutebrowser/completion/models/histcategory.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
 
-"""A completion model backed by SQL tables."""
+"""A completion category that queries the SQL History store."""
 
 import re
 
@@ -26,50 +26,50 @@ from PyQt5.QtSql import QSqlQueryModel
 from qutebrowser.misc import sql
 from qutebrowser.utils import debug
 from qutebrowser.commands import cmdexc
+from qutebrowser.config import config
 
 
-class SqlCategory(QSqlQueryModel):
+class HistoryCategory(QSqlQueryModel):
 
-    """Wraps a SqlQuery for use as a completion category."""
+    """A completion category that queries the SQL History store."""
 
-    def __init__(self, name, *, title=None, filter_fields, sort_by=None,
+    def __init__(self, *, delete_func=None, parent=None):
-                 sort_order=None, select='*',
+        """Create a new History completion category."""
-                 delete_func=None, parent=None):
-        """Create a new completion category backed by a sql table.
-
-        Args:
-            name: Name of the table in the database.
-            title: Title of category, defaults to table name.
-            filter_fields: Names of fields to apply filter pattern to.
-            select: A custom result column expression for the select statement.
-            sort_by: The name of the field to sort by, or None for no sorting.
-            sort_order: Either 'asc' or 'desc', if sort_by is non-None
-            delete_func: Callback to delete a selected item.
-        """
         super().__init__(parent=parent)
-        self.name = title or name
+        self.name = "History"
 
-        querystr = 'select {} from {} where ('.format(select, name)
+        # replace ' to avoid breaking the query
+        timefmt = "strftime('{}', last_atime, 'unixepoch')".format(
+            config.get('completion', 'timestamp-format').replace("'", "`"))
+
+        self._query = sql.Query(' '.join([
+            "SELECT url, title, {}".format(timefmt),
+            "FROM CompletionHistory",
             # the incoming pattern will have literal % and _ escaped with '\'
             # we need to tell sql to treat '\' as an escape character
-        querystr += ' or '.join("{} like :pattern escape '\\'".format(f)
+            "WHERE (url LIKE :pat escape '\\' or title LIKE :pat escape '\\')",
-                                for f in filter_fields)
+            self._atime_expr(),
-        querystr += ')'
+            "ORDER BY last_atime DESC",
+        ]), forward_only=False)
 
-        if sort_by:
+        # advertise that this model filters by URL and title
-            assert sort_order in ['asc', 'desc'], sort_order
+        self.columns_to_filter = [0, 1]
-            querystr += ' order by {} {}'.format(sort_by, sort_order)
-        else:
-            assert sort_order is None, sort_order
-
-        self._query = sql.Query(querystr, forward_only=False)
-
-        # map filter_fields to indices
-        col_query = sql.Query('SELECT * FROM {} LIMIT 1'.format(name))
-        rec = col_query.run().record()
-        self.columns_to_filter = [rec.indexOf(n) for n in filter_fields]
         self.delete_func = delete_func
 
+    def _atime_expr(self):
+        """If max_items is set, return an expression to limit the query."""
+        max_items = config.get('completion', 'web-history-max-items')
+        if max_items < 0:
+            return ''
+
+        min_atime = sql.Query(' '.join([
+            'SELECT min(last_atime) FROM',
+            '(SELECT last_atime FROM CompletionHistory',
+            'ORDER BY last_atime DESC LIMIT :limit)',
+        ])).run(limit=max_items).value()
+
+        return "AND last_atime >= {}".format(min_atime)
+
     def set_pattern(self, pattern):
         """Set the pattern used to filter results.
 
@@ -83,7 +83,7 @@ class SqlCategory(QSqlQueryModel):
         pattern = re.sub(r' +', '%', pattern)
         pattern = '%{}%'.format(pattern)
         with debug.log_time('sql', 'Running completion query'):
-            self._query.run(pattern=pattern)
+            self._query.run(pat=pattern)
         self.setQuery(self._query)
 
     def delete_cur_item(self, index):

qutebrowser/completion/models/urlmodel.py

@@ -20,8 +20,7 @@
 """Function to return the url completion model for the `open` command."""
 
 from qutebrowser.completion.models import (completionmodel, listcategory,
-                                           sqlcategory)
+                                           histcategory)
-from qutebrowser.config import config
 from qutebrowser.utils import log, objreg
 
 
@@ -66,14 +65,6 @@ def url():
     model.add_category(listcategory.ListCategory(
         'Bookmarks', bookmarks, delete_func=_delete_bookmark))
 
-    # replace 's to avoid breaking the query
+    hist_cat = histcategory.HistoryCategory(delete_func=_delete_history)
-    timefmt = config.get('completion', 'timestamp-format').replace("'", "`")
-    select_time = "strftime('{}', last_atime, 'unixepoch')".format(timefmt)
-    hist_cat = sqlcategory.SqlCategory(
-        'CompletionHistory', title='History',
-        sort_order='desc', sort_by='last_atime',
-        filter_fields=['url', 'title'],
-        select='url, title, {}'.format(select_time),
-        delete_func=_delete_history)
     model.add_category(hist_cat)
     return model

qutebrowser/config/config.py

@@ -382,7 +382,6 @@ class ConfigManager(QObject):
         ('storage', 'offline-storage-default-quota'),
         ('storage', 'offline-web-application-cache-quota'),
         ('content', 'css-regions'),
-        ('completion', 'web-history-max-items'),
     ]
     CHANGED_OPTIONS = {
         ('content', 'cookies-accept'):

qutebrowser/config/configdata.py

@@ -502,6 +502,11 @@ def data(readonly=False):
              "How many commands to save in the command history.\n\n"
              "0: no history / -1: unlimited"),
 
+            ('web-history-max-items',
+             SettingValue(typ.Int(minval=-1), '-1'),
+             "How many URLs to show in the web history.\n\n"
+             "0: no history / -1: unlimited"),
+
             ('quick-complete',
              SettingValue(typ.Bool(), 'true'),
              "Whether to move on to the next part when there's only one "

tests/unit/completion/test_histcategory.py

@@ -0,0 +1,150 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2016-2017 Ryan Roden-Corrent (rcorre) <ryan@rcorre.net>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the web history completion category."""
+
+import unittest.mock
+import time
+
+import pytest
+
+from qutebrowser.misc import sql
+from qutebrowser.completion.models import histcategory
+from qutebrowser.commands import cmdexc
+
+
+@pytest.fixture
+def hist(init_sql, config_stub):
+    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d',
+                                      'web-history-max-items': -1}
+    return sql.SqlTable('CompletionHistory', ['url', 'title', 'last_atime'])
+
+
+@pytest.mark.parametrize('pattern, before, after', [
+    ('foo',
+     [('foo', ''), ('bar', ''), ('aafobbb', '')],
+     [('foo',)]),
+
+    ('FOO',
+     [('foo', ''), ('bar', ''), ('aafobbb', '')],
+     [('foo',)]),
+
+    ('foo',
+     [('FOO', ''), ('BAR', ''), ('AAFOBBB', '')],
+     [('FOO',)]),
+
+    ('foo',
+     [('baz', 'bar'), ('foo', ''), ('bar', 'foo')],
+     [('foo', ''), ('bar', 'foo')]),
+
+    ('foo',
+     [('fooa', ''), ('foob', ''), ('fooc', '')],
+     [('fooa', ''), ('foob', ''), ('fooc', '')]),
+
+    ('foo',
+     [('foo', 'bar'), ('bar', 'foo'), ('biz', 'baz')],
+     [('foo', 'bar'), ('bar', 'foo')]),
+
+    ('foo bar',
+     [('foo', ''), ('bar foo', ''), ('xfooyybarz', '')],
+     [('xfooyybarz', '')]),
+
+    ('foo%bar',
+     [('foo%bar', ''), ('foo bar', ''), ('foobar', '')],
+     [('foo%bar', '')]),
+
+    ('_',
+     [('a_b', ''), ('__a', ''), ('abc', '')],
+     [('a_b', ''), ('__a', '')]),
+
+    ('%',
+     [('\\foo', '\\bar')],
+     []),
+
+    ("can't",
+     [("can't touch this", ''), ('a', '')],
+     [("can't touch this", '')]),
+])
+def test_set_pattern(pattern, before, after, model_validator, hist):
+    """Validate the filtering and sorting results of set_pattern."""
+    for row in before:
+        hist.insert({'url': row[0], 'title': row[1], 'last_atime': 1})
+    cat = histcategory.HistoryCategory()
+    model_validator.set_model(cat)
+    cat.set_pattern(pattern)
+    model_validator.validate(after)
+
+
+@pytest.mark.parametrize('max_items, before, after', [
+    (-1, [
+        ('a', 'a', '2017-04-16'),
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+    ], [
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+        ('a', 'a', '2017-04-16'),
+    ]),
+    (3, [
+        ('a', 'a', '2017-04-16'),
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+    ], [
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+        ('a', 'a', '2017-04-16'),
+    ]),
+    (2, [
+        ('a', 'a', '2017-04-16'),
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+    ], [
+        ('b', 'b', '2017-06-16'),
+        ('c', 'c', '2017-05-16'),
+    ])
+])
+def test_sorting(max_items, before, after, model_validator, hist, config_stub):
+    """Validate the filtering and sorting results of set_pattern."""
+    config_stub.data['completion']['web-history-max-items'] = max_items
+    for url, title, atime in before:
+        timestamp = time.mktime(time.strptime(atime, '%Y-%m-%d'))
+        hist.insert({'url': url, 'title': title, 'last_atime': timestamp})
+    cat = histcategory.HistoryCategory()
+    model_validator.set_model(cat)
+    cat.set_pattern('')
+    model_validator.validate(after)
+
+
+def test_delete_cur_item(hist):
+    hist.insert({'url': 'foo', 'title': 'Foo'})
+    hist.insert({'url': 'bar', 'title': 'Bar'})
+    func = unittest.mock.Mock(spec=[])
+    cat = histcategory.HistoryCategory(delete_func=func)
+    cat.set_pattern('')
+    cat.delete_cur_item(cat.index(0, 0))
+    func.assert_called_with(['foo', 'Foo', ''])
+
+
+def test_delete_cur_item_no_func(hist):
+    hist.insert({'url': 'foo', 'title': 1})
+    hist.insert({'url': 'bar', 'title': 2})
+    cat = histcategory.HistoryCategory()
+    cat.set_pattern('')
+    with pytest.raises(cmdexc.CommandError, match='Cannot delete this item'):
+        cat.delete_cur_item(cat.index(0, 0))

tests/unit/completion/test_models.py

@@ -137,8 +137,10 @@ def bookmarks(bookmark_manager_stub):
 
 
 @pytest.fixture
-def web_history(init_sql, stubs):
+def web_history(init_sql, stubs, config_stub):
     """Fixture which provides a web-history object."""
+    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d',
+                                      'web-history-max-items': -1}
     stub = history.WebHistory()
     objreg.register('web-history', stub)
     yield stub
@@ -266,7 +268,7 @@ def test_bookmark_completion(qtmodeltester, bookmarks):
     })
 
 
-def test_url_completion(qtmodeltester, config_stub, web_history_populated,
+def test_url_completion(qtmodeltester, web_history_populated,
                         quickmarks, bookmarks):
     """Test the results of url completion.
 
@@ -275,7 +277,6 @@ def test_url_completion(qtmodeltester, config_stub, web_history_populated,
         - entries are sorted by access time
         - only the most recent entry is included for each url
     """
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'}
     model = urlmodel.url()
     model.set_pattern('')
     qtmodeltester.data_display_may_return_none = True
@@ -318,11 +319,10 @@ def test_url_completion(qtmodeltester, config_stub, web_history_populated,
     ('foo%bar', '', '%', 1),
     ('foobar', '', '%', 0),
 ])
-def test_url_completion_pattern(config_stub, web_history,
+def test_url_completion_pattern(web_history, quickmark_manager_stub,
-                                quickmark_manager_stub, bookmark_manager_stub,
+                                bookmark_manager_stub, url, title, pattern,
-                                url, title, pattern, rowcount):
+                                rowcount):
     """Test that url completion filters by url and title."""
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'}
     web_history.add_url(QUrl(url), title)
     model = urlmodel.url()
     model.set_pattern(pattern)
@@ -330,10 +330,9 @@ def test_url_completion_pattern(config_stub, web_history,
     assert model.rowCount(model.index(2, 0)) == rowcount
 
 
-def test_url_completion_delete_bookmark(qtmodeltester, config_stub, bookmarks,
+def test_url_completion_delete_bookmark(qtmodeltester, bookmarks,
                                         web_history, quickmarks):
     """Test deleting a bookmark from the url completion model."""
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'}
     model = urlmodel.url()
     model.set_pattern('')
     qtmodeltester.data_display_may_return_none = True
@@ -353,11 +352,10 @@ def test_url_completion_delete_bookmark(qtmodeltester, config_stub, bookmarks,
     assert len_before == len(bookmarks.marks) + 1
 
 
-def test_url_completion_delete_quickmark(qtmodeltester, config_stub,
+def test_url_completion_delete_quickmark(qtmodeltester,
                                          quickmarks, web_history, bookmarks,
                                          qtbot):
     """Test deleting a bookmark from the url completion model."""
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'}
     model = urlmodel.url()
     model.set_pattern('')
     qtmodeltester.data_display_may_return_none = True
@@ -377,11 +375,10 @@ def test_url_completion_delete_quickmark(qtmodeltester, config_stub,
     assert len_before == len(quickmarks.marks) + 1
 
 
-def test_url_completion_delete_history(qtmodeltester, config_stub,
+def test_url_completion_delete_history(qtmodeltester,
                                        web_history_populated,
                                        quickmarks, bookmarks):
     """Test deleting a history entry."""
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d'}
     model = urlmodel.url()
     model.set_pattern('')
     qtmodeltester.data_display_may_return_none = True
@@ -598,14 +595,11 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     })
 
 
-def test_url_completion_benchmark(benchmark, config_stub,
+def test_url_completion_benchmark(benchmark,
                                   quickmark_manager_stub,
                                   bookmark_manager_stub,
                                   web_history):
     """Benchmark url completion."""
-    config_stub.data['completion'] = {'timestamp-format': '%Y-%m-%d',
-                                      'web-history-max-items': 1000}
-
     r = range(100000)
     entries = {
         'last_atime': list(r),

tests/unit/completion/test_sqlcategory.py

@@ -1,158 +0,0 @@
-# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
-
-# Copyright 2016-2017 Ryan Roden-Corrent (rcorre) <ryan@rcorre.net>
-#
-# This file is part of qutebrowser.
-#
-# qutebrowser is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# qutebrowser is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
-
-"""Test SQL-based completions."""
-
-import unittest.mock
-
-import pytest
-
-from qutebrowser.misc import sql
-from qutebrowser.completion.models import sqlcategory
-from qutebrowser.commands import cmdexc
-
-
-pytestmark = pytest.mark.usefixtures('init_sql')
-
-
-@pytest.mark.parametrize('sort_by, sort_order, data, expected', [
-    (None, None,
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')]),
-
-    ('a', 'asc',
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
-     [('A', 'F', 'C'), ('B', 'C', 'D'), ('C', 'A', 'G')]),
-
-    ('a', 'desc',
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
-     [('C', 'A', 'G'), ('B', 'C', 'D'), ('A', 'F', 'C')]),
-
-    ('b', 'asc',
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
-     [('C', 'A', 'G'), ('B', 'C', 'D'), ('A', 'F', 'C')]),
-
-    ('b', 'desc',
-     [('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
-     [('A', 'F', 'C'), ('B', 'C', 'D'), ('C', 'A', 'G')]),
-
-    ('c', 'asc',
-     [('B', 'C', 2), ('A', 'F', 0), ('C', 'A', 1)],
-     [('A', 'F', 0), ('C', 'A', 1), ('B', 'C', 2)]),
-
-    ('c', 'desc',
-     [('B', 'C', 2), ('A', 'F', 0), ('C', 'A', 1)],
-     [('B', 'C', 2), ('C', 'A', 1), ('A', 'F', 0)]),
-])
-def test_sorting(sort_by, sort_order, data, expected, model_validator):
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
-    for row in data:
-        table.insert({'a': row[0], 'b': row[1], 'c': row[2]})
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], sort_by=sort_by,
-                                  sort_order=sort_order)
-    model_validator.set_model(cat)
-    cat.set_pattern('')
-    model_validator.validate(expected)
-
-
-@pytest.mark.parametrize('pattern, filter_cols, before, after', [
-    ('foo', [0],
-     [('foo', '', ''), ('bar', '', ''), ('aafobbb', '', '')],
-     [('foo',)]),
-
-    ('foo', [0],
-     [('baz', 'bar', 'foo'), ('foo', '', ''), ('bar', 'foo', '')],
-     [('foo', '', '')]),
-
-    ('foo', [0],
-     [('fooa', '', ''), ('foob', '', ''), ('fooc', '', '')],
-     [('fooa', '', ''), ('foob', '', ''), ('fooc', '', '')]),
-
-    ('foo', [1],
-     [('foo', 'bar', ''), ('bar', 'foo', '')],
-     [('bar', 'foo', '')]),
-
-    ('foo', [0, 1],
-     [('foo', 'bar', ''), ('bar', 'foo', ''), ('biz', 'baz', 'foo')],
-     [('foo', 'bar', ''), ('bar', 'foo', '')]),
-
-    ('foo', [0, 1, 2],
-     [('foo', '', ''), ('bar', '', ''), ('baz', 'bar', 'foo')],
-     [('foo', '', ''), ('baz', 'bar', 'foo')]),
-
-    ('foo bar', [0],
-     [('foo', '', ''), ('bar foo', '', ''), ('xfooyybarz', '', '')],
-     [('xfooyybarz', '', '')]),
-
-    ('foo%bar', [0],
-     [('foo%bar', '', ''), ('foo bar', '', ''), ('foobar', '', '')],
-     [('foo%bar', '', '')]),
-
-    ('_', [0],
-     [('a_b', '', ''), ('__a', '', ''), ('abc', '', '')],
-     [('a_b', '', ''), ('__a', '', '')]),
-
-    ('%', [0, 1],
-     [('\\foo', '\\bar', '')],
-     []),
-
-    ("can't", [0],
-     [("can't touch this", '', ''), ('a', '', '')],
-     [("can't touch this", '', '')]),
-])
-def test_set_pattern(pattern, filter_cols, before, after, model_validator):
-    """Validate the filtering and sorting results of set_pattern."""
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
-    for row in before:
-        table.insert({'a': row[0], 'b': row[1], 'c': row[2]})
-    filter_fields = [['a', 'b', 'c'][i] for i in filter_cols]
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=filter_fields)
-    model_validator.set_model(cat)
-    cat.set_pattern(pattern)
-    model_validator.validate(after)
-
-
-def test_select(model_validator):
-    table = sql.SqlTable('Foo', ['a', 'b', 'c'])
-    table.insert({'a': 'foo', 'b': 'bar', 'c': 'baz'})
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], select='b, c, a')
-    model_validator.set_model(cat)
-    cat.set_pattern('')
-    model_validator.validate([('bar', 'baz', 'foo')])
-
-
-def test_delete_cur_item():
-    table = sql.SqlTable('Foo', ['a', 'b'])
-    table.insert({'a': 'foo', 'b': 1})
-    table.insert({'a': 'bar', 'b': 2})
-    func = unittest.mock.Mock(spec=[])
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], delete_func=func)
-    cat.set_pattern('')
-    cat.delete_cur_item(cat.index(0, 0))
-    func.assert_called_with(['foo', 1])
-
-
-def test_delete_cur_item_no_func():
-    table = sql.SqlTable('Foo', ['a', 'b'])
-    table.insert({'a': 'foo', 'b': 1})
-    table.insert({'a': 'bar', 'b': 2})
-    cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'])
-    cat.set_pattern('')
-    with pytest.raises(cmdexc.CommandError, match='Cannot delete this item'):
-        cat.delete_cur_item(cat.index(0, 0))