Merge pull request #150 from gazpachoking/history_optimization

History optimization/improvement fix #149

realms/modules/search/commands.py

@@ -26,10 +26,12 @@ def rebuild_index():
             # Some non-markdown files may have issues
             continue
         # TODO add email?
+        # TODO I have concens about indexing the commit info from latest revision, see #148
+        info = next(page.history)
         body = dict(name=page.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(page.name, body)

realms/modules/wiki/models.py

@@ -109,47 +109,87 @@ class WikiPage(HookMixin):
         return data
 
     @property
-    def info(self):
-        cache_key = self._cache_key('info')
-        cached = cache.get(cache_key)
-        if cached:
-            return cached
-
-        info = self.get_history(limit=1)[0]
-        cache.set(cache_key, info)
-        return info
-
-    def get_history(self, limit=100):
+    def history(self):
         """Get page history.
 
-        :param limit: Limit history size.
-        :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, and caches
+        as it goes.
+
+        :return: iter -- Iterator over dicts
 
         """
+        cache_head = []
+        cache_tail = cache.get(self._cache_key('history')) or [{'_cache_missing': True}]
+        while True:
+            if not cache_tail:
+                return
+            for index, cached_rev in enumerate(cache_tail):
+                if cached_rev.get("_cache_missing"):
+                    break
+                else:
+                    yield cached_rev
+            cache_head.extend(cache_tail[:index])
+            cache_tail = cache_tail[index+1:]
+            start_sha = cached_rev.get('sha')
+            end_sha = cache_tail[0].get('sha') if cache_tail else None
+            for rev in self._iter_revs(start_sha=start_sha, end_sha=end_sha, filename=cached_rev.get('filename')):
+                cache_head.append(rev)
+                placeholder = {
+                    '_cache_missing': True,
+                    'sha': rev['sha'],
+                    'filename': rev['new_filename']
+                }
+                cache.set(self._cache_key('history'), cache_head + [placeholder] + cache_tail)
+                yield rev
+            cache.set(self._cache_key('history'), cache_head + cache_tail)
+
+    def _iter_revs(self, start_sha=None, end_sha=None, filename=None):
+        if end_sha:
+            end_sha = [end_sha]
         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)
+            return
+        filename = filename or self.filename
+        walker = iter(self.wiki.repo.get_walker(paths=[filename],
+                                                include=start_sha,
+                                                exclude=end_sha,
+                                                follow=True))
+        if start_sha:
+            # If we are not starting from HEAD, we already have the start commit
+            next(walker)
+        filename = self.filename
         for entry in walker:
             change_type = None
             for change in entry.changes():
-                if change.old.path == self.filename:
+                if change.new.path == filename:
+                    filename = change.old.path
                     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))
+                    break
 
-        return versions
+            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,
+                     new_filename=change.new.path,
+                     old_filename=change.old.path)
+            yield r
+
+    @property
+    def history_cache(self):
+        """Get info about the history cache.
+
+        :return: tuple -- (cached items, cache complete?)
+        """
+        cached_revs = cache.get(self._cache_key('history'))
+        if not cached_revs:
+            return 0, False
+        elif any(rev.get('_cache_missing') for rev in cached_revs):
+            return len(cached_revs) - 1, False
+        return len(cached_revs), True
 
     @property
     def partials(self):
@@ -196,8 +236,14 @@ class WikiPage(HookMixin):
 
         return username, email
 
-    def _clear_cache(self):
-        cache.delete_many(*(self._cache_key(p) for p in ['data', 'info']))
+    def _invalidate_cache(self, save_history=None):
+        cache.delete(self._cache_key('data'))
+        if save_history:
+            if not save_history[0].get('_cache_missing'):
+                save_history = [{'_cache_missing': True}] + save_history
+            cache.set(self._cache_key('history'), save_history)
+        else:
+            cache.delete(self._cache_key('history'))
 
     def delete(self, username=None, email=None, message=None):
         """Delete page.
@@ -218,7 +264,7 @@ class WikiPage(HookMixin):
                                   email=email,
                                   message=message,
                                   files=[self.filename])
-        self._clear_cache()
+        self._invalidate_cache()
         return commit
 
     def rename(self, new_name, username=None, email=None, message=None):
@@ -256,11 +302,12 @@ class WikiPage(HookMixin):
                                   message=message,
                                   files=[old_filename, new_filename])
 
-        self._clear_cache()
+        old_history = cache.get(self._cache_key('history'))
+        self._invalidate_cache()
         self.name = new_name
         self.filename = new_filename
         # We need to clear the cache for the new name as well as the old
-        self._clear_cache()
+        self._invalidate_cache(save_history=old_history)
 
         return commit
 
@@ -296,7 +343,8 @@ class WikiPage(HookMixin):
                                message=message,
                                files=[self.filename])
 
-        self._clear_cache()
+        old_history = cache.get(self._cache_key('history'))
+        self._invalidate_cache(save_history=old_history)
         return ret
 
     def revert(self, commit_sha, message, username, email):

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):

realms/modules/wiki/views.py

@@ -1,5 +1,6 @@
 import itertools
 import sys
+from datetime import datetime
 from flask import abort, g, render_template, request, redirect, Blueprint, flash, url_for, current_app
 from flask.ext.login import login_required, current_user
 from realms.lib.util import to_canonical, remove_ext, gravatar_url
@@ -64,11 +65,37 @@ def revert():
 def history(name):
     if current_app.config.get('PRIVATE_WIKI') and current_user.is_anonymous():
         return current_app.login_manager.unauthorized()
+    return render_template('wiki/history.html', name=name)
 
-    hist = g.current_wiki.get_page(name).get_history()
-    for item in hist:
+
+@blueprint.route("/_history_data/<path:name>")
+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=hist)
+        item['DT_RowId'] = item['sha']
+        date = datetime.fromtimestamp(item['time'])
+        item['date'] = date.strftime(current_app.config.get('DATETIME_FORMAT', '%b %d, %Y %I:%M %p'))
+        item['link'] = url_for('.commit', name=name, sha=item['sha'])
+    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,
+        'fully_loaded': hist_complete
+    }
+
 
 
 @blueprint.route("/_edit/<path:name>")
@@ -85,7 +112,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)
 

realms/templates/errors/404.html

@@ -2,5 +2,8 @@
 {% block body %}
 
   <h1>Page Not Found</h1>
+  {% if error is defined %}
+  <h4>{{ error.description }}</h4>
+  {% endif %}
 
-{% endblock %}
+{% endblock %}

realms/templates/wiki/history.html

@@ -6,27 +6,17 @@
     <a class="btn btn-default btn-sm compare-revisions">Compare Revisions</a>
   </p>
 
-  <table class="table table-bordered revision-tbl">
+  <table class="table table-bordered revision-tbl dataTable DTTT_selectable">
     <thead>
     <tr>
-      <th></th>
       <th>Name</th>
       <th>Revision Message</th>
       <th>Date</th>
     </tr>
     </thead>
-    {% for h in history %}
-      <tr>
-        <td class="checkbox-cell text-center">
-          {% if h.type != 'delete' %}
-          <input type="checkbox" name="versions[]" value="{{ h.sha }}" />
-          {% endif %}
-        </td>
-        <td><img src="{{ h.gravatar }}?s=20" class="avatar"/> {{ h.author }}</td>
-        <td><a href="{{ url_for('wiki.commit', name=name, sha=h.sha) }}" class='label label-primary'>View</a> {{ h.message }} </td>
-        <td>{{ h.time|datetime }}</td>
-      </tr>
-    {% endfor %}
+    <tbody>
+    <tr><td colspan="3" style="text-align: center">Loading file history...</td></tr>
+    </tbody>
   </table>
   <p>
     <a class="btn btn-default btn-sm compare-revisions">Compare Revisions</a>
@@ -34,25 +24,95 @@
 
 {% endblock %}
 
+{% block css %}
+  <style type="text/css">
+    table.dataTable td {
+      transition: background-color 0.5s linear, color 0.5s linear;
+      transition-delay: 0.1s;
+    }
+    table.dataTable tr.active td {
+      transition: background-color 0.1s linear, color 0.1s linear;
+      transition-delay: 0s
+    }
+    table.dataTable tbody tr:hover {
+      background-color: #d8d8d8 !important;
+    }
+  </style>
+{% endblock %}
+
 {% block js %}
   <script>
-    $(function(){
-      $('.revision-tbl :checkbox').change(function () {
-        var $cs=$(this).closest('.revision-tbl').find(':checkbox:checked');
-        if ($cs.length > 2) {
-          this.checked=false;
+    $(document).ready(function() {
+      var selected = [];
+      var selected_pos = [];
+
+      $('.dataTable').dataTable({
+        serverSide: true,
+        ajax: {
+          url: '{{ url_for('.history_data', name=name) }}',
+          dataSrc: function (data) {
+            $('.dataTable').data('fully_loaded', data.fully_loaded);
+            return data.data
+          }
+        },
+        ordering: false,
+        bFilter: false,
+        columns: [
+          {
+            "data": null,
+            "render": function (data) {
+              return '<img src="' + data.gravatar + '?s=20" class="avatar" />&nbsp;&nbsp;' + data.author
+            }
+          },
+          {
+            "data": null,
+            "render": function (data) {
+              return '<a href="' + data.link + '" class="label label-primary">View</a>&nbsp;&nbsp;' + data.message
+            }
+          },
+          { "data": "date" }
+        ],
+        rowCallback: function( row, data, index ) {
+            index += $('.dataTable').DataTable().page.info().start;
+            $(row).data('index', index);
+            if ( $.inArray(data.DT_RowId, selected) !== -1 ) {
+                $(row).addClass('active');
+            }
+        },
+        infoCallback: function( settings, start, end, max, total, pre ) {
+          if (!$('.dataTable').data('fully_loaded')) {
+            total += "+"
+          }
+          return "Showing " + start +" to "+ end + " of " + total + " revisions.";
         }
       });
 
+      $('.dataTable tbody').on('click', 'tr', function () {
+        var id = this.id;
+        var selected_index = $.inArray(id, selected);
+
+        if ( selected_index === -1 ) {
+            selected.push( id );
+            selected_pos.push( $(this).data('index') );
+            if ( selected.length > 2) {
+                // Only 2 selected at once
+                var shifted = selected.shift();
+                selected_pos.shift();
+                $('#' + shifted).removeClass('active');
+            }
+        } else {
+            selected.splice( selected_index, 1 );
+            selected_pos.splice( selected_index, 1);
+        }
+
+        $(this).toggleClass('active');
+      });
       $(".compare-revisions").click(function(){
-        var $cs = $('.revision-tbl').find(':checkbox:checked');
-        if ($cs.length != 2) return;
-        var revs = [];
-        $.each($cs, function(i, v){
-          revs.push(v.value);
-        });
-        revs.reverse();
-        revs = revs.join("..");
+        if (selected.length != 2) return;
+        if (selected_pos[1] > selected_pos[0]) {
+          selected.reverse()
+        }
+        revs = selected.join("..");
         location.href = "{{ config.RELATIVE_PATH }}/_compare/{{ name }}/" + revs;
       });
     });