diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 86e597bcd..2d5c263bc 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -30,6 +30,10 @@ from qutebrowser.utils import (utils, objreg, log, usertypes, message, from qutebrowser.misc import objects, sql +# increment to indicate that HistoryCompletion must be regenerated +_USER_VERSION = 1 + + class CompletionHistory(sql.SqlTable): """History which only has the newest entry for each URL.""" @@ -48,6 +52,11 @@ class WebHistory(sql.SqlTable): super().__init__("History", ['url', 'title', 'atime', 'redirect'], parent=parent) self.completion = CompletionHistory(parent=self) + if sql.Query('pragma user_version').run().value() < _USER_VERSION: + self.completion.delete_all() + if not self.completion: + # either the table is out-of-date or the user wiped it manually + self._rebuild_completion() self.create_index('HistoryIndex', 'url') self.create_index('HistoryAtimeIndex', 'atime') self._contains_query = self.contains_query('url') @@ -71,6 +80,18 @@ class WebHistory(sql.SqlTable): def __contains__(self, url): return self._contains_query.run(val=url).value() + def _rebuild_completion(self): + data = {'url': [], 'title': [], 'last_atime': []} + # select the latest entry for each url + q = sql.Query('SELECT url, title, max(atime) AS atime FROM History ' + 'WHERE NOT redirect GROUP BY url ORDER BY atime asc') + for entry in q.run(): + data['url'].append(self._format_completion_url(QUrl(entry.url))) + data['title'].append(entry.title) + data['last_atime'].append(entry.atime) + self.completion.insert_batch(data, replace=True) + sql.Query('pragma user_version = {}'.format(_USER_VERSION)).run() + def get_recent(self): """Get the most recent history entries.""" return self.select(sort_by='atime', sort_order='desc', limit=100) @@ -159,13 +180,12 @@ class WebHistory(sql.SqlTable): return atime = int(atime) if (atime is not None) else int(time.time()) - url_str = url.toString(QUrl.FullyEncoded | QUrl.RemovePassword) - self.insert({'url': url_str, + self.insert({'url': self._format_url(url), 'title': title, 'atime': atime, 'redirect': redirect}) if not redirect: - self.completion.insert({'url': url_str, + self.completion.insert({'url': self._format_completion_url(url), 'title': title, 'last_atime': atime}, replace=True) @@ -249,12 +269,13 @@ class WebHistory(sql.SqlTable): if parsed is None: continue url, title, atime, redirect = parsed - data['url'].append(url) + data['url'].append(self._format_url(url)) data['title'].append(title) data['atime'].append(atime) data['redirect'].append(redirect) if not redirect: - completion_data['url'].append(url) + completion_data['url'].append( + self._format_completion_url(url)) completion_data['title'].append(title) completion_data['last_atime'].append(atime) except ValueError as ex: @@ -263,6 +284,12 @@ class WebHistory(sql.SqlTable): self.insert_batch(data) self.completion.insert_batch(completion_data, replace=True) + def _format_url(self, url): + return url.toString(QUrl.RemovePassword | QUrl.FullyEncoded) + + def _format_completion_url(self, url): + return url.toString(QUrl.RemovePassword) + @cmdutils.register(instance='web-history', debug=True) def debug_dump_history(self, dest): """Dump the history to a file in the old pre-SQL format. diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index c109f44eb..2462a1141 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -143,23 +143,27 @@ def test_delete_url(hist): assert completion_diff == {('http://example.com/1', '', 0)} -@pytest.mark.parametrize('url, atime, title, redirect, expected_url', [ - ('http://www.example.com', 12346, 'the title', False, - 'http://www.example.com'), - ('http://www.example.com', 12346, 'the title', True, - 'http://www.example.com'), - ('http://www.example.com/spa ce', 12346, 'the title', False, - 'http://www.example.com/spa%20ce'), - ('https://user:pass@example.com', 12346, 'the title', False, - 'https://user@example.com'), -]) -def test_add_url(qtbot, hist, url, atime, title, redirect, expected_url): +@pytest.mark.parametrize( + 'url, atime, title, redirect, history_url, completion_url', [ + + ('http://www.example.com', 12346, 'the title', False, + 'http://www.example.com', 'http://www.example.com'), + ('http://www.example.com', 12346, 'the title', True, + 'http://www.example.com', None), + ('http://www.example.com/sp ce', 12346, 'the title', False, + 'http://www.example.com/sp%20ce', 'http://www.example.com/sp ce'), + ('https://user:pass@example.com', 12346, 'the title', False, + 'https://user@example.com', 'https://user@example.com'), + ] +) +def test_add_url(qtbot, hist, url, atime, title, redirect, history_url, + completion_url): hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect) - assert list(hist) == [(expected_url, title, atime, redirect)] - if redirect: + assert list(hist) == [(history_url, title, atime, redirect)] + if completion_url is None: assert not len(hist.completion) else: - assert list(hist.completion) == [(expected_url, title, atime)] + assert list(hist.completion) == [(completion_url, title, atime)] def test_add_url_invalid(qtbot, hist, caplog): @@ -349,3 +353,50 @@ def test_debug_dump_history_nonexistent(hist, tmpdir): histfile = tmpdir / 'nonexistent' / 'history' with pytest.raises(cmdexc.CommandError): hist.debug_dump_history(str(histfile)) + + +def test_rebuild_completion(hist): + hist.insert({'url': 'example.com/1', 'title': 'example1', + 'redirect': False, 'atime': 1}) + hist.insert({'url': 'example.com/1', 'title': 'example1', + 'redirect': False, 'atime': 2}) + hist.insert({'url': 'example.com/2%203', 'title': 'example2', + 'redirect': False, 'atime': 3}) + hist.insert({'url': 'example.com/3', 'title': 'example3', + 'redirect': True, 'atime': 4}) + hist.insert({'url': 'example.com/2 3', 'title': 'example2', + 'redirect': False, 'atime': 5}) + hist.completion.delete_all() + + hist2 = history.WebHistory() + assert list(hist2.completion) == [ + ('example.com/1', 'example1', 2), + ('example.com/2 3', 'example2', 5), + ] + + +def test_no_rebuild_completion(hist): + """Ensure that completion is not regenerated unless completely empty.""" + hist.add_url(QUrl('example.com/1'), redirect=False, atime=1) + hist.add_url(QUrl('example.com/2'), redirect=False, atime=2) + hist.completion.delete('url', 'example.com/2') + + hist2 = history.WebHistory() + assert list(hist2.completion) == [('example.com/1', '', 1)] + + +def test_user_version(hist, monkeypatch): + """Ensure that completion is regenerated if user_version is incremented.""" + hist.add_url(QUrl('example.com/1'), redirect=False, atime=1) + hist.add_url(QUrl('example.com/2'), redirect=False, atime=2) + hist.completion.delete('url', 'example.com/2') + + hist2 = history.WebHistory() + assert list(hist2.completion) == [('example.com/1', '', 1)] + + monkeypatch.setattr(history, '_USER_VERSION', history._USER_VERSION + 1) + hist3 = history.WebHistory() + assert list(hist3.completion) == [ + ('example.com/1', '', 1), + ('example.com/2', '', 2), + ]