Assorted small fixes for sql code review.

This commit is contained in:
Ryan Roden-Corrent 2017-06-07 22:45:14 -04:00
parent feed9c8936
commit 6fc61d12fc
16 changed files with 83 additions and 112 deletions

View File

@ -213,7 +213,7 @@ def qute_history(url):
offset = QUrlQuery(url).queryItemValue("offset")
offset = int(offset) if offset else None
except ValueError as e:
raise QuteSchemeError("Query parameter start_time is invalid", e)
raise QuteSchemeError("Query parameter offset is invalid", e)
# Use start_time in query or current time.
try:
start_time = QUrlQuery(url).queryItemValue("start_time")

View File

@ -111,7 +111,6 @@ class CompletionView(QTreeView):
# objreg.get('config').changed.connect(self.init_command_completion)
objreg.get('config').changed.connect(self._on_config_changed)
self._column_widths = (30, 70, 0)
self._active = False
self._delegate = completiondelegate.CompletionItemDelegate(self)
@ -150,7 +149,8 @@ class CompletionView(QTreeView):
def _resize_columns(self):
"""Resize the completion columns based on column_widths."""
width = self.size().width()
pixel_widths = [(width * perc // 100) for perc in self._column_widths]
column_widths = self.model.column_widths
pixel_widths = [(width * perc // 100) for perc in column_widths]
if self.verticalScrollBar().isVisible():
delta = self.style().pixelMetric(QStyle.PM_ScrollBarExtent) + 5
@ -280,14 +280,8 @@ class CompletionView(QTreeView):
model.setParent(self)
self.setModel(model)
self._column_widths = model.column_widths
self._active = True
if (config.get('completion', 'show') == 'always' and
model.count() > 0):
self.show()
else:
self.hide()
self._maybe_show()
for i in range(model.rowCount()):
self.expand(model.index(i, 0))
@ -297,7 +291,14 @@ class CompletionView(QTreeView):
self.model().set_pattern(pattern)
self._resize_columns()
self._maybe_update_geometry()
self._maybe_show()
def _maybe_show(self):
if (config.get('completion', 'show') == 'always' and
model.count() > 0):
self.show()
else:
self.hide()
def _maybe_update_geometry(self):
"""Emit the update_geometry signal if the config says so."""

View File

@ -78,12 +78,14 @@ class CompletionModel(QAbstractItemModel):
if not index.isValid() or role != Qt.DisplayRole:
return None
if not index.parent().isValid():
# category header
if index.column() == 0:
return self._categories[index.row()].name
else:
cat = self._categories[index.parent().row()]
idx = cat.index(index.row(), index.column())
return cat.data(idx)
return None
# item
cat = self._categories[index.parent().row()]
idx = cat.index(index.row(), index.column())
return cat.data(idx)
def flags(self, index):
"""Return the item flags for index.
@ -132,7 +134,7 @@ class CompletionModel(QAbstractItemModel):
# categories have no parent
return QModelIndex()
row = self._categories.index(parent_cat)
return self.createIndex(row, 0, None)
return self.createIndex(row, index.column(), None)
def rowCount(self, parent=QModelIndex()):
"""Override QAbstractItemModel::rowCount."""

View File

@ -71,8 +71,7 @@ class ListCategory(QSortFilterProxyModel):
parent: The parent item QModelIndex.
Return:
True if self.pattern is contained in item, or if it's a root item
(category). False in all other cases
True if self.pattern is contained in item.
"""
if not self.pattern:
return True

View File

@ -28,6 +28,7 @@ from qutebrowser.utils import debug
class SqlCategory(QSqlQueryModel):
"""Wraps a SqlQuery for use as a completion category."""
def __init__(self, name, *, filter_fields, sort_by=None, sort_order=None,
@ -57,12 +58,11 @@ class SqlCategory(QSqlQueryModel):
if group_by:
querystr += ' group by {}'.format(group_by)
if sort_by:
assert sort_order in ['asc', 'desc']
assert sort_order in ['asc', 'desc'], sort_order
querystr += ' order by {} {}'.format(sort_by, sort_order)
self._query = sql.Query(querystr)
self._query = sql.Query(querystr, forward_only=False)
self._param_count = len(filter_fields)
self.columns_to_filter = None
# map filter_fields to indices
col_query = sql.Query('SELECT * FROM {} LIMIT 1'.format(name))

View File

@ -74,7 +74,8 @@ def url():
model.add_category(listcategory.ListCategory('Bookmarks', bookmarks,
columns_to_filter=[0, 1]))
timefmt = config.get('completion', 'timestamp-format')
# replace 's to avoid breaking the query
timefmt = config.get('completion', 'timestamp-format').replace("'", "`")
select_time = "strftime('{}', last_atime, 'unixepoch')".format(timefmt)
hist_cat = sqlcategory.SqlCategory(
'CompletionHistory', sort_order='desc', sort_by='last_atime',

View File

@ -59,13 +59,23 @@ class Query(QSqlQuery):
"""A prepared SQL Query."""
def __init__(self, querystr):
def __init__(self, querystr, forward_only=True):
"""Prepare a new sql query.
Args:
querystr: String to prepare query from.
forward_only: Optimization for queries that will only step forward.
Must be false for completion queries.
"""
super().__init__(QSqlDatabase.database())
log.sql.debug('Preparing SQL query: "{}"'.format(querystr))
self.prepare(querystr)
if not self.prepare(querystr):
raise SqlException('Failed to prepare query "{}"'.format(querystr))
self.setForwardOnly(forward_only)
def __iter__(self):
assert self.isActive(), "Cannot iterate inactive query"
if not self.isActive():
raise SqlException("Cannot iterate inactive query")
rec = self.record()
fields = [rec.fieldName(i) for i in range(rec.count())]
rowtype = collections.namedtuple('ResultRow', fields)
@ -87,8 +97,8 @@ class Query(QSqlQuery):
def value(self):
"""Return the result of a single-value query (e.g. an EXISTS)."""
ok = self.next()
assert ok, "No result for single-result query"
if not self.next():
raise SqlException("No result for single-result query")
return self.record().value(0)
@ -152,7 +162,7 @@ class SqlTable(QObject):
Args:
field: Field to match.
"""
return Query("Select EXISTS(SELECT * FROM {} where {} = ?)"
return Query("SELECT EXISTS(SELECT * FROM {} WHERE {} = ?)"
.format(self._name, field))
def __len__(self):
@ -214,17 +224,19 @@ class SqlTable(QObject):
self.changed.emit()
def delete_all(self):
"""Remove all row from the table."""
"""Remove all rows from the table."""
Query("DELETE FROM {}".format(self._name)).run()
self.changed.emit()
def select(self, sort_by, sort_order, limit=-1):
"""Remove all row from the table.
"""Prepare, run, and return a select statement on this table.
Args:
sort_by: name of column to sort by.
sort_order: 'asc' or 'desc'.
limit: max number of rows in result, defaults to -1 (unlimited).
Return: A prepared and executed select query.
"""
q = Query('SELECT * FROM {} ORDER BY {} {} LIMIT ?'
.format(self._name, sort_by, sort_order))

View File

@ -702,8 +702,6 @@ Feature: Various utility commands.
And I wait for "Renderer process was killed" in the log
And I open data/numbers/3.txt
Then no crash should happen
And the following tabs should be open:
- data/numbers/3.txt (active)
## Other

View File

@ -28,17 +28,16 @@ bdd.scenarios('history.feature')
@bdd.then(bdd.parsers.parse("the history should contain:\n{expected}"))
def check_history(quteproc, expected, httpbin):
with tempfile.TemporaryDirectory() as tmpdir:
path = os.path.join(tmpdir, 'history')
quteproc.send_cmd(':debug-dump-history "{}"'.format(path))
quteproc.wait_for(category='message', loglevel=logging.INFO,
message='Dumped history to {}.'.format(path))
def check_history(quteproc, httpbin, tmpdir, expected):
path = tmpdir / 'history'
quteproc.send_cmd(':debug-dump-history "{}"'.format(path))
quteproc.wait_for(category='message', loglevel=logging.INFO,
message='Dumped history to {}.'.format(path))
with open(path, 'r', encoding='utf-8') as f:
# ignore access times, they will differ in each run
actual = '\n'.join(re.sub('^\\d+-?', '', line).strip()
for line in f.read().splitlines())
with open(path, 'r', encoding='utf-8') as f:
# ignore access times, they will differ in each run
actual = '\n'.join(re.sub('^\\d+-?', '', line).strip()
for line in f.read())
expected = expected.replace('(port)', str(httpbin.port))
assert actual == expected

View File

@ -170,3 +170,16 @@ def abs_datapath():
"""Get the absolute path to the end2end data directory."""
file_abs = os.path.abspath(os.path.dirname(__file__))
return os.path.join(file_abs, '..', 'end2end', 'data')
def validate_model(cat, expected):
"""Check that a category contains the expected items in the given order.
Args:
cat: The category to inspect.
expected: A list of tuples containing the expected items.
"""
assert cat.rowCount() == len(expected)
for row, items in enumerate(expected):
for col, item in enumerate(items):
assert cat.data(cat.index(row, col)) == item

View File

@ -122,7 +122,6 @@ class TestHistoryHandler:
url = QUrl("qute://history/data?start_time=" + str(start_time))
_mimetype, data = qutescheme.qute_history(url)
items = json.loads(data)
items = [item for item in items if 'time' in item] # skip 'next' item
assert len(items) == expected_item_count
@ -132,27 +131,6 @@ class TestHistoryHandler:
assert item['time'] <= start_time * 1000
assert item['time'] > end_time * 1000
@pytest.mark.skip("TODO: do we need next?")
@pytest.mark.parametrize("start_time_offset, next_time", [
(0, 24*60*60),
(24*60*60, 48*60*60),
(48*60*60, -1),
(72*60*60, -1)
])
def test_qutehistory_next(self, start_time_offset, next_time, now):
"""Ensure qute://history/data returns correct items."""
start_time = now - start_time_offset
url = QUrl("qute://history/data?start_time=" + str(start_time))
_mimetype, data = qutescheme.qute_history(url)
items = json.loads(data)
items = [item for item in items if 'next' in item] # 'next' items
assert len(items) == 1
if next_time == -1:
assert items[0]["next"] == -1
else:
assert items[0]["next"] == now - next_time
def test_qute_history_benchmark(self, fake_web_history, benchmark, now):
entries = []
for t in range(100000): # one history per second

View File

@ -107,14 +107,6 @@ def test_entries_before(hist):
assert times == [12348, 12347, 12346]
def test_save(tmpdir, hist):
hist.add_url(QUrl('http://example.com/'), atime=12345)
hist.add_url(QUrl('http://www.qutebrowser.org/'), atime=67890)
hist2 = history.WebHistory()
assert list(hist2) == [('http://example.com/', '', 12345, False),
('http://www.qutebrowser.org/', '', 67890, False)]
def test_clear(qtbot, tmpdir, hist, mocker):
hist.add_url(QUrl('http://example.com/'))
@ -132,12 +124,11 @@ def test_clear_force(qtbot, tmpdir, hist):
assert not len(hist)
@pytest.mark.parametrize('item', [
@pytest.mark.parametrize('url, atime, title, redirect', [
('http://www.example.com', 12346, 'the title', False),
('http://www.example.com', 12346, 'the title', True)
])
def test_add_item(qtbot, hist, item):
(url, atime, title, redirect) = item
def test_add_item(qtbot, hist, url, atime, title, redirect):
hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect)
assert list(hist) == [(url, title, atime, redirect)]
@ -188,13 +179,14 @@ def test_history_interface(qtbot, webview, hist_interface):
def cleanup_init():
# prevent test_init from leaking state
yield
try:
hist = objreg.get('web-history')
hist = objreg.get('web-history', None)
if hist is not None:
hist.setParent(None)
objreg.delete('web-history')
try:
from PyQt5.QtWebKit import QWebHistoryInterface
QWebHistoryInterface.setDefaultInterface(None)
except:
except Exception:
pass

View File

@ -229,7 +229,7 @@ def test_completion_item_del_no_selection(completionview):
model = completionmodel.CompletionModel(delete_cur_item=func)
model.add_category(listcategory.ListCategory('', [('foo',)]))
completionview.set_model(model)
with pytest.raises(cmdexc.CommandError):
with pytest.raises(cmdexc.CommandError, match='No item selected!'):
completionview.completion_item_del()
assert not func.called
@ -240,5 +240,5 @@ def test_completion_item_del_no_func(completionview):
model.add_category(listcategory.ListCategory('', [('foo',)]))
completionview.set_model(model)
completionview.completion_item_focus('next')
with pytest.raises(cmdexc.CommandError):
with pytest.raises(cmdexc.CommandError, match='Cannot delete this item.'):
completionview.completion_item_del()

View File

@ -21,22 +21,10 @@
import pytest
from helpers import utils
from qutebrowser.completion.models import listcategory
def _validate(cat, expected):
"""Check that a category contains the expected items in the given order.
Args:
cat: The category to inspect.
expected: A list of tuples containing the expected items.
"""
assert cat.rowCount() == len(expected)
for row, items in enumerate(expected):
for col, item in enumerate(items):
assert cat.data(cat.index(row, col)) == item
@pytest.mark.parametrize('pattern, filter_cols, before, after', [
('foo', [0],
[('foo', '', ''), ('bar', '', '')],
@ -68,4 +56,4 @@ def test_set_pattern(pattern, filter_cols, before, after):
cat = listcategory.ListCategory('Foo', before,
columns_to_filter=filter_cols)
cat.set_pattern(pattern)
_validate(cat, after)
utils.validate_model(cat, after)

View File

@ -21,6 +21,7 @@
import pytest
from helpers import utils
from qutebrowser.misc import sql
from qutebrowser.completion.models import sqlcategory
@ -28,19 +29,6 @@ from qutebrowser.completion.models import sqlcategory
pytestmark = pytest.mark.usefixtures('init_sql')
def _validate(cat, expected):
"""Check that a category contains the expected items in the given order.
Args:
cat: The category to inspect.
expected: A list of tuples containing the expected items.
"""
assert cat.rowCount() == len(expected)
for row, items in enumerate(expected):
for col, item in enumerate(items):
assert cat.data(cat.index(row, col)) == item
@pytest.mark.parametrize('sort_by, sort_order, data, expected', [
(None, 'asc',
[('B', 'C', 'D'), ('A', 'F', 'C'), ('C', 'A', 'G')],
@ -77,7 +65,7 @@ def test_sorting(sort_by, sort_order, data, expected):
cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], sort_by=sort_by,
sort_order=sort_order)
cat.set_pattern('')
_validate(cat, expected)
utils.validate_model(cat, expected)
@pytest.mark.parametrize('pattern, filter_cols, before, after', [
@ -133,7 +121,7 @@ def test_set_pattern(pattern, filter_cols, before, after):
filter_fields = [['a', 'b', 'c'][i] for i in filter_cols]
cat = sqlcategory.SqlCategory('Foo', filter_fields=filter_fields)
cat.set_pattern(pattern)
_validate(cat, after)
utils.validate_model(cat, after)
def test_select():
@ -141,7 +129,7 @@ def test_select():
table.insert(['foo', 'bar', 'baz'])
cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], select='b, c, a')
cat.set_pattern('')
_validate(cat, [('bar', 'baz', 'foo')])
utils.validate_model(cat, [('bar', 'baz', 'foo')])
def test_where():
@ -150,7 +138,7 @@ def test_where():
table.insert(['baz', 'biz', True])
cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'], where='not c')
cat.set_pattern('')
_validate(cat, [('foo', 'bar', False)])
utils.validate_model(cat, [('foo', 'bar', False)])
def test_group():
@ -162,7 +150,7 @@ def test_group():
cat = sqlcategory.SqlCategory('Foo', filter_fields=['a'],
select='a, max(b)', group_by='a')
cat.set_pattern('')
_validate(cat, [('bar', 3), ('foo', 2)])
utils.validate_model(cat, [('bar', 3), ('foo', 2)])
def test_entry():

View File

@ -798,7 +798,6 @@ def test_chromium_version_unpatched(qapp):
assert version._chromium_version() not in ['', 'unknown', 'unavailable']
# pylint: disable=too-many-locals
@pytest.mark.parametrize(['git_commit', 'frozen', 'style', 'with_webkit',
'known_distribution'], [
(True, False, True, True, True), # normal
@ -812,6 +811,7 @@ def test_chromium_version_unpatched(qapp):
def test_version_output(git_commit, frozen, style, with_webkit,
known_distribution, stubs, monkeypatch, init_sql):
"""Test version.version()."""
# pylint: disable=too-many-locals
class FakeWebEngineProfile:
def httpUserAgent(self):
return 'Toaster/4.0.4 Chrome/CHROMIUMVERSION Teapot/4.1.8'