From e3ae3a1ccb4743d591ca3e10890591672bf5c8cf Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 7 Jul 2016 02:49:52 -0400 Subject: [PATCH 01/24] Lazy load history view with pagination. refs #149 --- realms/modules/wiki/models.py | 103 +++++++++++++++++++++-------- realms/modules/wiki/views.py | 17 ++++- realms/templates/wiki/history.html | 16 +++++ 3 files changed, 107 insertions(+), 29 deletions(-) diff --git a/realms/modules/wiki/models.py b/realms/modules/wiki/models.py index 3c3a160..7531d1d 100644 --- a/realms/modules/wiki/models.py +++ b/realms/modules/wiki/models.py @@ -1,3 +1,5 @@ +import collections +import itertools import os import posixpath import re @@ -98,37 +100,14 @@ class WikiPage(object): cache.set(cache_key, info) return info - def get_history(self, limit=100): + @property + def history(self): """Get page history. - :param limit: Limit history size. :return: list -- List of dicts """ - if not len(self.wiki.repo.open_index()): - # Index is empty, no commits - return [] - - versions = [] - - walker = self.wiki.repo.get_walker(paths=[self.filename], max_entries=limit) - for entry in walker: - change_type = None - for change in entry.changes(): - if change.old.path == self.filename: - change_type = change.type - elif change.new.path == self.filename: - change_type = change.type - author_name, author_email = entry.commit.author.rstrip('>').split('<') - versions.append(dict( - author=author_name.strip(), - author_email=author_email, - time=entry.commit.author_time, - message=entry.commit.message, - sha=entry.commit.id, - type=change_type)) - - return versions + return PageHistory(self, self._cache_key('history')) @property def partials(self): @@ -319,3 +298,75 @@ class WikiPage(object): # We'll get a KeyError if self.sha isn't in the repo, or if self.filename isn't in the tree of our commit return False return True + + +class PageHistory(collections.Sequence): + """Acts like a list, but dynamically loads and caches history revisions as requested.""" + def __init__(self, page, cache_key): + self.page = page + self.cache_key = cache_key + self._store = cache.get(cache_key) or [] + if not self._store: + self._iter_rest = self._get_rest() + elif self._store[-1] == 'TAIL': + self._iter_rest = None + else: + self._iter_rest = self._get_rest(self._store[-1]['sha']) + + def __iter__(self): + # Iterate over the revisions already cached + for r in self._store: + if r == 'TAIL': + return + yield r + # Iterate over the revisions yet to be discovered + if self._iter_rest: + try: + for r in self._iter_rest: + self._store.append(r) + yield r + self._store.append('TAIL') + finally: + # Make sure we cache the results whether or not the iteration was completed + cache.set(self.cache_key, self._store) + + def _get_rest(self, start_sha=None): + if not len(self.page.wiki.repo.open_index()): + # Index is empty, no commits + return + walker = iter(self.page.wiki.repo.get_walker(paths=[self.page.filename], include=start_sha, follow=True)) + if start_sha: + # If we are not starting from HEAD, we already have the start commit + print(next(walker)) + filename = self.page.filename + for entry in walker: + change_type = None + for change in entry.changes(): + if change.new.path == filename: + filename = change.old.path + change_type = change.type + break + + author_name, author_email = entry.commit.author.rstrip('>').split('<') + r = dict(author=author_name.strip(), + author_email=author_email, + time=entry.commit.author_time, + message=entry.commit.message, + sha=entry.commit.id, + type=change_type) + yield r + + def __getitem__(self, index): + if isinstance(index, slice): + return list(itertools.islice(self, index.start, index.stop, index.step)) + else: + try: + return next(itertools.islice(self, index, index+1)) + except StopIteration: + raise IndexError + + def __len__(self): + if not self._store or self._store[-1] != 'TAIL': + # Force generation of all revisions + list(self) + return len(self._store) - 1 # Don't include the TAIL sentinel diff --git a/realms/modules/wiki/views.py b/realms/modules/wiki/views.py index 40571b2..d850ff5 100644 --- a/realms/modules/wiki/views.py +++ b/realms/modules/wiki/views.py @@ -64,11 +64,22 @@ def revert(): def history(name): if current_app.config.get('PRIVATE_WIKI') and current_user.is_anonymous(): return current_app.login_manager.unauthorized() - + ITEMS_PER_PAGE = 15 + page = int(request.args.get('page', 1)) + start_index = (page - 1) * ITEMS_PER_PAGE hist = g.current_wiki.get_page(name).get_history() - for item in hist: + # Grab one extra item to see if there is a next page + items = hist[start_index:start_index+ITEMS_PER_PAGE+1] + more = False + if len(items) > ITEMS_PER_PAGE: + more = True + items.pop() + if page > 1 and not items: + abort(404) + for item in items: item['gravatar'] = gravatar_url(item['author_email']) - return render_template('wiki/history.html', name=name, history=hist) + return render_template('wiki/history.html', name=name, history=items, + pagination={'page': page, 'more': more}) @blueprint.route("/_edit/") diff --git a/realms/templates/wiki/history.html b/realms/templates/wiki/history.html index bdc0cc7..c5c253d 100644 --- a/realms/templates/wiki/history.html +++ b/realms/templates/wiki/history.html @@ -28,6 +28,22 @@ {% endfor %} + + {% if pagination.page > 1 or pagination.more %} +
+ {% if pagination.page > 1 %} + < + {% else %} + < + {% endif %} + {{ pagination.page }} + {% if pagination.more %} + > + {% else %} + > + {% endif %} +
+ {% endif %}

Compare Revisions

From 74452fe58f1e79effa47c06dcee49b04397c717f Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 7 Jul 2016 02:50:45 -0400 Subject: [PATCH 02/24] Add ability for 404 page to show error description. --- realms/modules/wiki/views.py | 2 +- realms/templates/errors/404.html | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/realms/modules/wiki/views.py b/realms/modules/wiki/views.py index d850ff5..632b643 100644 --- a/realms/modules/wiki/views.py +++ b/realms/modules/wiki/views.py @@ -75,7 +75,7 @@ def history(name): more = True items.pop() if page > 1 and not items: - abort(404) + abort(404, 'Page is past end of history.') for item in items: item['gravatar'] = gravatar_url(item['author_email']) return render_template('wiki/history.html', name=name, history=items, diff --git a/realms/templates/errors/404.html b/realms/templates/errors/404.html index 66bfd47..11e4044 100644 --- a/realms/templates/errors/404.html +++ b/realms/templates/errors/404.html @@ -2,5 +2,8 @@ {% block body %}

Page Not Found

+ {% if error is defined %} +

{{ error.description }}

+ {% endif %} -{% endblock %} \ No newline at end of file +{% endblock %} From 35471d3c3fe1984c6a470f5e62f690da8fb6c406 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 7 Jul 2016 17:30:49 -0400 Subject: [PATCH 03/24] Don't add a separate class to iterate history --- realms/modules/wiki/models.py | 122 +++++++++++++--------------------- 1 file changed, 47 insertions(+), 75 deletions(-) diff --git a/realms/modules/wiki/models.py b/realms/modules/wiki/models.py index 7531d1d..a73dbaa 100644 --- a/realms/modules/wiki/models.py +++ b/realms/modules/wiki/models.py @@ -96,7 +96,7 @@ class WikiPage(object): if cached: return cached - info = self.get_history(limit=1)[0] + info = next(self.history) cache.set(cache_key, info) return info @@ -104,10 +104,54 @@ class WikiPage(object): def history(self): """Get page history. - :return: list -- List of dicts + History can take a long time to generate for repositories with many commits. + This returns an iterator to avoid having to load them all at once. + + :return: iter -- Iterator over dicts """ - return PageHistory(self, self._cache_key('history')) + cache_complete = False + cached_revs = cache.get(self._cache_key('history')) or [] + start_sha = None + if cached_revs: + if cached_revs[-1] == 'TAIL': + del cached_revs[-1] + cache_complete = True + else: + start_sha = cached_revs[-1]['sha'] + for rev in cached_revs: + yield rev + if cache_complete: + return + if not len(self.wiki.repo.open_index()): + # Index is empty, no commits + return + walker = iter(self.wiki.repo.get_walker(paths=[self.filename], include=start_sha, follow=True)) + if start_sha: + # If we are not starting from HEAD, we already have the start commit + next(walker) + filename = self.filename + try: + for entry in walker: + change_type = None + for change in entry.changes(): + if change.new.path == filename: + filename = change.old.path + change_type = change.type + break + + author_name, author_email = entry.commit.author.rstrip('>').split('<') + r = dict(author=author_name.strip(), + author_email=author_email, + time=entry.commit.author_time, + message=entry.commit.message, + sha=entry.commit.id, + type=change_type) + cached_revs.append(r) + yield r + cached_revs.append('TAIL') + finally: + cache.set(self._cache_key('history'), cached_revs) @property def partials(self): @@ -298,75 +342,3 @@ class WikiPage(object): # We'll get a KeyError if self.sha isn't in the repo, or if self.filename isn't in the tree of our commit return False return True - - -class PageHistory(collections.Sequence): - """Acts like a list, but dynamically loads and caches history revisions as requested.""" - def __init__(self, page, cache_key): - self.page = page - self.cache_key = cache_key - self._store = cache.get(cache_key) or [] - if not self._store: - self._iter_rest = self._get_rest() - elif self._store[-1] == 'TAIL': - self._iter_rest = None - else: - self._iter_rest = self._get_rest(self._store[-1]['sha']) - - def __iter__(self): - # Iterate over the revisions already cached - for r in self._store: - if r == 'TAIL': - return - yield r - # Iterate over the revisions yet to be discovered - if self._iter_rest: - try: - for r in self._iter_rest: - self._store.append(r) - yield r - self._store.append('TAIL') - finally: - # Make sure we cache the results whether or not the iteration was completed - cache.set(self.cache_key, self._store) - - def _get_rest(self, start_sha=None): - if not len(self.page.wiki.repo.open_index()): - # Index is empty, no commits - return - walker = iter(self.page.wiki.repo.get_walker(paths=[self.page.filename], include=start_sha, follow=True)) - if start_sha: - # If we are not starting from HEAD, we already have the start commit - print(next(walker)) - filename = self.page.filename - for entry in walker: - change_type = None - for change in entry.changes(): - if change.new.path == filename: - filename = change.old.path - change_type = change.type - break - - author_name, author_email = entry.commit.author.rstrip('>').split('<') - r = dict(author=author_name.strip(), - author_email=author_email, - time=entry.commit.author_time, - message=entry.commit.message, - sha=entry.commit.id, - type=change_type) - yield r - - def __getitem__(self, index): - if isinstance(index, slice): - return list(itertools.islice(self, index.start, index.stop, index.step)) - else: - try: - return next(itertools.islice(self, index, index+1)) - except StopIteration: - raise IndexError - - def __len__(self): - if not self._store or self._store[-1] != 'TAIL': - # Force generation of all revisions - list(self) - return len(self._store) - 1 # Don't include the TAIL sentinel From 242e317e6564e5eca1de67a995b305704d381ab3 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 7 Jul 2016 17:39:37 -0400 Subject: [PATCH 04/24] Make the history view pagination prettier. --- realms/modules/wiki/views.py | 10 ++++---- realms/templates/wiki/history.html | 38 +++++++++++++++++------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/realms/modules/wiki/views.py b/realms/modules/wiki/views.py index 632b643..a50df83 100644 --- a/realms/modules/wiki/views.py +++ b/realms/modules/wiki/views.py @@ -64,14 +64,14 @@ def revert(): def history(name): if current_app.config.get('PRIVATE_WIKI') and current_user.is_anonymous(): return current_app.login_manager.unauthorized() - ITEMS_PER_PAGE = 15 + items_per_page = 15 page = int(request.args.get('page', 1)) - start_index = (page - 1) * ITEMS_PER_PAGE - hist = g.current_wiki.get_page(name).get_history() + start_index = (page - 1) * items_per_page + hist = g.current_wiki.get_page(name).history # Grab one extra item to see if there is a next page - items = hist[start_index:start_index+ITEMS_PER_PAGE+1] + items = list(itertools.islice(hist, start_index, start_index+items_per_page+1)) more = False - if len(items) > ITEMS_PER_PAGE: + if len(items) > items_per_page: more = True items.pop() if page > 1 and not items: diff --git a/realms/templates/wiki/history.html b/realms/templates/wiki/history.html index c5c253d..c80ccd5 100644 --- a/realms/templates/wiki/history.html +++ b/realms/templates/wiki/history.html @@ -15,6 +15,27 @@ Date + {% if pagination.page > 1 or pagination.more %} + + +
    +
  • + Previous +
    • + {% for p in range(1, pagination.page + 1) %} +
  • + {{ p }} +
    • + {% endfor %} + {% if pagination.more %}
  • …
    • {% endif %} +
  • + Next +
    • +
+ + + {% endif %} + {% for h in history %} @@ -27,23 +48,8 @@ {{ h.time|datetime }} {% endfor %} + - - {% if pagination.page > 1 or pagination.more %} -
- {% if pagination.page > 1 %} - < - {% else %} - < - {% endif %} - {{ pagination.page }} - {% if pagination.more %} - > - {% else %} - > - {% endif %} -
- {% endif %}

Compare Revisions

From 0bcfaba807b7888e335e20b0687ae4ddf298625c Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 7 Jul 2016 17:54:15 -0400 Subject: [PATCH 05/24] Remove WikiPage.info refs #148 --- realms/modules/search/commands.py | 6 ++++-- realms/modules/wiki/models.py | 11 ----------- realms/modules/wiki/tests.py | 2 +- realms/modules/wiki/views.py | 3 ++- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/realms/modules/search/commands.py b/realms/modules/search/commands.py index 43507df..c11edb8 100644 --- a/realms/modules/search/commands.py +++ b/realms/modules/search/commands.py @@ -30,10 +30,12 @@ def rebuild_index(): continue name = filename_to_cname(page['path']) # TODO add email? + # TODO I have concens about indexing the commit info from latest revision, see #148 + info = next(page.history) body = dict(name=name, content=page.data, - message=page.info['message'], - username=page.info['author'], + message=info['message'], + username=info['author'], updated_on=entry['mtime'], created_on=entry['ctime']) search.index_wiki(name, body) diff --git a/realms/modules/wiki/models.py b/realms/modules/wiki/models.py index a73dbaa..aa276a3 100644 --- a/realms/modules/wiki/models.py +++ b/realms/modules/wiki/models.py @@ -89,17 +89,6 @@ class WikiPage(object): cache.set(cache_key, data) return data - @property - def info(self): - cache_key = self._cache_key('info') - cached = cache.get(cache_key) - if cached: - return cached - - info = next(self.history) - cache.set(cache_key, info) - return info - @property def history(self): """Get page history. diff --git a/realms/modules/wiki/tests.py b/realms/modules/wiki/tests.py index 4e9c8eb..565f4f1 100644 --- a/realms/modules/wiki/tests.py +++ b/realms/modules/wiki/tests.py @@ -41,7 +41,7 @@ class WikiTest(WikiBaseTest): self.assert_200(rv) self.assert_context('name', 'test') - eq_(self.get_context_variable('page').info['message'], 'test message') + eq_(next(self.get_context_variable('page').history)['message'], 'test message') eq_(self.get_context_variable('page').data, 'testing') def test_history(self): diff --git a/realms/modules/wiki/views.py b/realms/modules/wiki/views.py index a50df83..f7c474b 100644 --- a/realms/modules/wiki/views.py +++ b/realms/modules/wiki/views.py @@ -96,7 +96,8 @@ def edit(name): return render_template('wiki/edit.html', name=cname, content=page.data, - info=page.info, + # TODO: Remove this? See #148 + info=next(page.history), sha=page.sha, partials=page.partials) From eafff2ae439adb25e368b02c6f5e5bad1ec95a5c Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 8 Jul 2016 21:30:20 -0400 Subject: [PATCH 06/24] Cause an error when trying to hook invalid HookMixin method --- realms/lib/hook.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/realms/lib/hook.py b/realms/lib/hook.py index 61d2885..f4e012a 100644 --- a/realms/lib/hook.py +++ b/realms/lib/hook.py @@ -25,9 +25,12 @@ class HookMixinMeta(type): def __new__(cls, name, bases, attrs): super_new = super(HookMixinMeta, cls).__new__ + hookable = [] for key, value in attrs.items(): if callable(value): attrs[key] = hook_func(key, value) + hookable.append(key) + attrs['_hookable'] = hookable return super_new(cls, name, bases, attrs) @@ -37,9 +40,12 @@ class HookMixin(object): _pre_hooks = {} _post_hooks = {} + _hookable = [] @classmethod def after(cls, method_name): + assert method_name in cls._hookable, "'%s' not a hookable method of '%s'" % (method_name, cls.__name__) + def outer(f, *args, **kwargs): cls._post_hooks.setdefault(method_name, []).append((f, args, kwargs)) return f @@ -47,6 +53,8 @@ class HookMixin(object): @classmethod def before(cls, method_name): + assert method_name in cls._hookable, "'%s' not a hookable method of '%s'" % (method_name, cls.__name__) + def outer(f, *args, **kwargs): cls._pre_hooks.setdefault(method_name, []).append((f, args, kwargs)) return f From b3f6c311b33dfbed658f2fd9271066cc45b69be1 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Sat, 9 Jul 2016 15:50:07 -0400 Subject: [PATCH 07/24] Switch page history view to use jquery datatable --- realms/modules/wiki/models.py | 14 ++++++++ realms/modules/wiki/views.py | 38 +++++++++++++-------- realms/templates/wiki/history.html | 55 +++++++++++------------------- 3 files changed, 57 insertions(+), 50 deletions(-) diff --git a/realms/modules/wiki/models.py b/realms/modules/wiki/models.py index aa276a3..3003978 100644 --- a/realms/modules/wiki/models.py +++ b/realms/modules/wiki/models.py @@ -142,6 +142,20 @@ class WikiPage(object): finally: cache.set(self._cache_key('history'), cached_revs) + @property + def history_cache(self): + """Get info about the history cache. + + :return: tuple -- (cached items, cache complete?) + """ + cache_complete = False + cached_revs = cache.get(self._cache_key('history')) or [] + if cached_revs: + if cached_revs[-1] == 'TAIL': + del cached_revs[-1] + cache_complete = True + return len(cached_revs), cache_complete + @property def partials(self): data = self.data diff --git a/realms/modules/wiki/views.py b/realms/modules/wiki/views.py index f7c474b..0dbc304 100644 --- a/realms/modules/wiki/views.py +++ b/realms/modules/wiki/views.py @@ -64,22 +64,32 @@ def revert(): def history(name): if current_app.config.get('PRIVATE_WIKI') and current_user.is_anonymous(): return current_app.login_manager.unauthorized() - items_per_page = 15 - page = int(request.args.get('page', 1)) - start_index = (page - 1) * items_per_page - hist = g.current_wiki.get_page(name).history - # Grab one extra item to see if there is a next page - items = list(itertools.islice(hist, start_index, start_index+items_per_page+1)) - more = False - if len(items) > items_per_page: - more = True - items.pop() - if page > 1 and not items: - abort(404, 'Page is past end of history.') + return render_template('wiki/history.html', name=name) + + +@blueprint.route("/_history_data/") +def history_data(name): + """Ajax provider for paginated history data.""" + if current_app.config.get('PRIVATE_WIKI') and current_user.is_anonymous(): + return current_app.login_manager.unauthorized() + draw = int(request.args.get('draw', 0)) + start = int(request.args.get('start', 0)) + length = int(request.args.get('length', 10)) + page = g.current_wiki.get_page(name) + items = list(itertools.islice(page.history, start, start + length)) for item in items: item['gravatar'] = gravatar_url(item['author_email']) - return render_template('wiki/history.html', name=name, history=items, - pagination={'page': page, 'more': more}) + total_records, hist_complete = page.history_cache + if not hist_complete: + # Force datatables to fetch more data when it gets to the end + total_records += 1 + return { + 'draw': draw, + 'recordsTotal': total_records, + 'recordsFiltered': total_records, + 'data': items + } + @blueprint.route("/_edit/") diff --git a/realms/templates/wiki/history.html b/realms/templates/wiki/history.html index c80ccd5..8595554 100644 --- a/realms/templates/wiki/history.html +++ b/realms/templates/wiki/history.html @@ -6,49 +6,14 @@ Compare Revisions

- +
- - {% if pagination.page > 1 or pagination.more %} - - - - {% endif %} - - {% for h in history %} - - - - - - - {% endfor %} -
Name Revision Message Date
-
    -
  • - Previous -
    • - {% for p in range(1, pagination.page + 1) %} -
  • - {{ p }} -
    • - {% endfor %} - {% if pagination.more %}
  • …
    • {% endif %} -
  • - Next -
    • -
-
- {% if h.type != 'delete' %} - - {% endif %} - {{ h.author }}View {{ h.message }} {{ h.time|datetime }}

Compare Revisions @@ -58,6 +23,24 @@ {% block js %}