From 0a3020b55d61ebc7b9cd4c5cd7231f4bdb6da711 Mon Sep 17 00:00:00 2001
From: Karsten Hopp <karsten@redhat.com>
Date: Sep 23 2018 10:58:40 +0000
Subject: move diff calc to workers


fixes #3599
calculating the differences between branches can take a long time if there are
lots of branches. Move calculation to worker task and return immediately to
unblock server task.

Signed-off-by: Karsten Hopp <karsten@redhat.com>

---

diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py
index c9203c2..48862d1 100644
--- a/pagure/internal/__init__.py
+++ b/pagure/internal/__init__.py
@@ -16,6 +16,7 @@ import collections
 import logging
 import os
 
+import celery
 import flask
 import pygit2
 
@@ -254,11 +255,15 @@ def get_pull_request_ready_branch():
         response.status_code = 400
         return response
 
+    args_reponame = flask.request.form.get("repo", "").strip() or None
+    args_namespace = flask.request.form.get("namespace", "").strip() or None
+    args_user = flask.request.form.get("repouser", "").strip() or None
+
     repo = pagure.lib.get_authorized_project(
         flask.g.session,
-        flask.request.form.get("repo", "").strip() or None,
-        namespace=flask.request.form.get("namespace", "").strip() or None,
-        user=flask.request.form.get("repouser", "").strip() or None,
+        args_reponame,
+        namespace=args_namespace,
+        user=args_user,
     )
 
     if not repo:
@@ -271,8 +276,6 @@ def get_pull_request_ready_branch():
         response.status_code = 404
         return response
 
-    reponame = pagure.utils.get_repo_path(repo)
-    repo_obj = pygit2.Repository(reponame)
     if repo.is_fork and repo.parent:
         if not repo.parent.settings.get("pull_requests", True):
             response = flask.jsonify(
@@ -283,9 +286,6 @@ def get_pull_request_ready_branch():
             )
             response.status_code = 400
             return response
-
-        parentreponame = pagure.utils.get_repo_path(repo.parent)
-        parent_repo_obj = pygit2.Repository(parentreponame)
     else:
         if not repo.settings.get("pull_requests", True):
             response = flask.jsonify(
@@ -296,80 +296,29 @@ def get_pull_request_ready_branch():
             )
             response.status_code = 400
             return response
-
-        parent_repo_obj = repo_obj
-
-    branches = {}
-    if not repo_obj.is_empty and len(repo_obj.listall_branches()) > 0:
-        for branchname in repo_obj.listall_branches():
-            compare_branch = None
-            if (
-                not parent_repo_obj.is_empty
-                and not parent_repo_obj.head_is_unborn
-            ):
-                try:
-                    if pagure.config.config.get(
-                        "PR_TARGET_MATCHING_BRANCH", False
-                    ):
-                        # find parent branch which is the longest substring of
-                        # branch that we're processing
-                        compare_branch = ""
-                        for parent_branch in parent_repo_obj.branches:
-                            if (
-                                not repo.is_fork
-                                and branchname == parent_branch
-                            ):
-                                continue
-                            if branchname.startswith(parent_branch) and len(
-                                parent_branch
-                            ) > len(compare_branch):
-                                compare_branch = parent_branch
-                        compare_branch = (
-                            compare_branch or repo_obj.head.shorthand
-                        )
-                    else:
-                        compare_branch = repo_obj.head.shorthand
-                except pygit2.GitError:
-                    pass  # let compare_branch be None
-
-            # Do not compare a branch to itself
-            if (
-                not repo.is_fork
-                and compare_branch
-                and compare_branch == branchname
-            ):
-                continue
-
-            diff_commits = None
-            try:
-                _, diff_commits, _ = pagure.lib.git.get_diff_info(
-                    repo_obj, parent_repo_obj, branchname, compare_branch
-                )
-            except pagure.exceptions.PagureException:
-                pass
-
-            if diff_commits:
-                branches[branchname] = {
-                    "commits": [c.oid.hex for c in diff_commits],
-                    "target_branch": compare_branch or "master",
-                }
-
-    prs = pagure.lib.search_pull_requests(
-        flask.g.session, project_id_from=repo.id, status="Open"
+    result = pagure.lib.tasks.pull_request_ready_branch.delay(
+        namespace=args_namespace,
+        name=args_reponame,
+        user=args_user,
     )
-    branches_pr = {}
-    for pr in prs:
-        if pr.branch_from in branches:
-            branches_pr[pr.branch_from] = "%s/pull-request/%s" % (
-                pr.project.url_path,
-                pr.id,
-            )
-            del (branches[pr.branch_from])
+
+    try:
+        message = result.get(timeout=100)
+    except celery.exceptions.TimeoutError:
+        result.forget()
+        response = flask.jsonify(
+            {
+                "code": "ERROR",
+                "message": "Timed out while waiting for results",
+            }
+        )
+        response.status_code = 400
+        return response
 
     return flask.jsonify(
         {
             "code": "OK",
-            "message": {"new_branch": branches, "branch_w_pr": branches_pr},
+            "message": message,
         }
     )
 
diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py
index 2249cb1..2ad2193 100644
--- a/pagure/lib/tasks.py
+++ b/pagure/lib/tasks.py
@@ -1051,3 +1051,86 @@ def link_pr_to_ticket(self, session, pr_uid):
     except SQLAlchemyError:
         _log.exception("Could not link ticket to PR :(")
         session.rollback()
+
+
+@conn.task(queue=pagure_config.get("MEDIUM_CELERY_QUEUE", None), bind=True)
+@pagure_task
+def pull_request_ready_branch(self, session, namespace, name, user):
+    repo = pagure.lib._get_project(
+        session, name, user=user, namespace=namespace
+    )
+    repo_obj = pygit2.Repository(pagure.utils.get_repo_path(repo))
+
+    if repo.is_fork and repo.parent:
+        parentreponame = pagure.utils.get_repo_path(repo.parent)
+        parent_repo_obj = pygit2.Repository(parentreponame)
+    else:
+        parent_repo_obj = repo_obj
+
+    branches = {}
+    if not repo_obj.is_empty and len(repo_obj.listall_branches()) > 0:
+        for branchname in repo_obj.listall_branches():
+            compare_branch = None
+            if (
+                not parent_repo_obj.is_empty
+                and not parent_repo_obj.head_is_unborn
+            ):
+                try:
+                    if pagure.config.config.get(
+                        "PR_TARGET_MATCHING_BRANCH", False
+                    ):
+                        # find parent branch which is the longest substring of
+                        # branch that we're processing
+                        compare_branch = ""
+                        for parent_branch in parent_repo_obj.branches:
+                            if (
+                                not repo.is_fork
+                                and branchname == parent_branch
+                            ):
+                                continue
+                            if branchname.startswith(parent_branch) and len(
+                                parent_branch
+                            ) > len(compare_branch):
+                                compare_branch = parent_branch
+                        compare_branch = (
+                            compare_branch or repo_obj.head.shorthand
+                        )
+                    else:
+                        compare_branch = repo_obj.head.shorthand
+                except pygit2.GitError:
+                    pass  # let compare_branch be None
+
+            # Do not compare a branch to itself
+            if (
+                not repo.is_fork
+                and compare_branch
+                and compare_branch == branchname
+            ):
+                continue
+
+            diff_commits = None
+            try:
+                _, diff_commits, _ = pagure.lib.git.get_diff_info(
+                    repo_obj, parent_repo_obj, branchname, compare_branch
+                )
+            except pagure.exceptions.PagureException:
+                pass
+
+            if diff_commits:
+                branches[branchname] = {
+                    "commits": len(list(diff_commits)),
+                    "target_branch": compare_branch or "master",
+                }
+
+    prs = pagure.lib.search_pull_requests(
+        session, project_id_from=repo.id, status="Open"
+    )
+    branches_pr = {}
+    for pr in prs:
+        if pr.branch_from in branches:
+            branches_pr[pr.branch_from] = "%s/pull-request/%s" % (
+                pr.project.url_path,
+                pr.id,
+            )
+            del (branches[pr.branch_from])
+    return {"new_branch": branches, "branch_w_pr": branches_pr}
diff --git a/pagure/templates/repo_branches.html b/pagure/templates/repo_branches.html
index 8d71212..b941fd7 100644
--- a/pagure/templates/repo_branches.html
+++ b/pagure/templates/repo_branches.html
@@ -142,7 +142,7 @@ $(function() {
           html2 = ' \
                 <a class="btn btn-outline-primary border-white" data-toggle="tooltip" \
                 href="' + url + '" title="' + branch +' contains '
-                 + nb_commits.length + ' commit not in the upstream project '
+                 + nb_commits + ' commit not in the upstream project '
                  + nb_target + ' branch. Click to create new PR now.'
                  + '"><i class="fa fa-fw fa-arrow-circle-down"></i> Open Pull Request</a> \
                 </div>';
@@ -150,7 +150,7 @@ $(function() {
           html2 = ' \
                 <a class="btn btn-outline-primary border-white" data-toggle="tooltip" \
                 href="' + url + '" title="' + branch +' contains '
-                 + nb_commits.length + ' commit not in the ' + nb_target
+                 + nb_commits + ' commit not in the ' + nb_target
                  + ' branch. Click to create new PR now.'
                  + '"><i class="fa fa-fw fa-arrow-circle-down"></i> Open Pull Request</a> \
                 </div>';
@@ -158,8 +158,8 @@ $(function() {
           var _b = branch.replace(/\./g, '\\.').replace('/', '__').replace('\+', '\\+');
           $('#branch-' + _b + ' .branch_del').prepend(html2);
           $('[data-toggle="tooltip"]').tooltip({placement : 'bottom'});
-          var commits_string = (nb_commits.length > 1) ? " commits" : " commit"
-          $('#branch-' + _b + ' .commits_ahead_label').append(nb_commits.length + commits_string + ' ahead');
+          var commits_string = (nb_commits > 1) ? " commits" : " commit"
+          $('#branch-' + _b + ' .commits_ahead_label').append(nb_commits + commits_string + ' ahead');
 
 
         }
diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html
index 0a2cad1..0c2b400 100644
--- a/pagure/templates/repo_master.html
+++ b/pagure/templates/repo_master.html
@@ -105,22 +105,7 @@
                     src="{{ url_for('static', filename='images/spinner.gif') }}" />
                   </small>
                 </a>
-                <a class="dropdown-item"
-                   href="{{ url_for('ui_ns.new_request_pull',
-                    repo=repo.name,
-                    username=username,
-                    namespace=repo.namespace,
-                    branch_to=head or 'master',
-                    branch_from=branchname or 'master') }}">
-                    New Pull Request
-                </a>
-                <a class="dropdown-item" href="{{ url_for(
-                    'ui_ns.new_remote_request_pull',
-                    repo=repo.name,
-                    username=username,
-                    namespace=repo.namespace) }}">
-                  New Remote Pull Request
-                </a>
+                <span id="dropdown-item-list" style="display:none;"></span>
               </div>
             </div>
           {%endif%}
@@ -409,103 +394,121 @@ window.addEventListener("load", function(event) {
 {% if g.authenticated and not g.repo_obj.is_empty %}
 
 {% if g.repo_committer %}
+function process_async(url, _data, callback) {
+  $.post(url, _data, null, dataType='json')
+    .done(function(data) {
+      callback(data);
+      $("#spinnergif").hide();
+    })
+}
+
+function generate_branch_list(data) {
+  if (data.code == 'OK') {
+     var _b = $("#dropdown-item-list");
+     var first_item = true;
+     for (branch in data.message.new_branch){
+       if (first_item){
+         _b.prepend("<div class='dropdown-divider'></div>")
+         first_item = false;
+       }
+       var url = "{{ url_for(
+         'ui_ns.new_request_pull',
+         repo=repo.name,
+         username=repo.user.user if repo.is_fork else None,
+         namespace=repo.namespace,
+         branch_to='-',
+         branch_from='-') }}";
+       url = url.slice(0, -4) + data.message.new_branch[branch]['target_branch'] + '..' + branch;
+       html = '<a class="dropdown-item " href="' + url + '">'
+         + '<span class="badge badge-light badge-pill border border-secondary font-size-1">'
+           + ' <i class="fa fa-calendar-o fa-rotate-270 fa-fw"></i> '
+           + '{{ repo.name }}'
+         + '</span> '
+         + '<span class="badge badge-secondary border border-secondary badge-pill font-size-1">'
+           + '<span class="fa fa-random fa-fw"> </span> '
+           + branch
+         + '</span></a> ';
+       _b.prepend(html);
+     }
+     _b.show();
+  }
+}
+
+function generate_branch_list_fork(data) {
+  if (data.code == 'OK') {
+     var _b = $("#dropdown-item-list");
+     var first_item = true;
+     for (branch in data.message.new_branch){
+       if (first_item){
+         _b.prepend("<div class='dropdown-divider'></div>")
+         first_item = false;
+       }
+       var url = "{{ url_for(
+         'ui_ns.new_request_pull',
+         repo=repo.name,
+         username=g.fas_user.username,
+         namespace=repo.namespace,
+         branch_to='-',
+         branch_from='-') }}";
+       url = url.slice(0, -4) + data.message.new_branch[branch]['target_branch'] + '..' + branch;
+       html = '<a class="dropdown-item " href="' + url + '">'
+         + '<span class="badge badge-light badge-pill border border-secondary font-size-1">'
+           + ' <i class="fa fa-calendar-o fa-rotate-270 fa-fw"></i> '
+           + '{{ g.fas_user.username }}/{{ repo.name }}</span> '
+         + '</span> '
+         + '<span class="badge badge-secondary border border-secondary badge-pill font-size-1">'
+           + '<span class="fa fa-random fa-fw"> </span> '
+           + branch
+         + '</span></a> ';
+       _b.prepend(html);
+     }
+     _b.show();
+  }
+}
+
 $("#pr-button").one("click",
   function() {
-    $.ajax({
-      url: '{{ url_for("internal_ns.get_pull_request_ready_branch") }}' ,
-      type: 'POST',
-      data: {
+  var _pr_url = "{{ url_for('internal_ns.get_pull_request_ready_branch') }}";
+  var _b = $("#dropdown-item-list");
+  var _s = $("#spinnergif");
+  _s.show();
+  _b.hide();
+  var new_pr_url = "{{ url_for('ui_ns.new_request_pull',
+                repo=repo.name,
+                username=username,
+                namespace=repo.namespace,
+                branch_to=head or 'master',
+                branch_from=branchname or 'master') }}"
+  var new_remote_pr_url = "{{ url_for(
+                'ui_ns.new_remote_request_pull',
+                repo=repo.name,
+                username=username,
+                namespace=repo.namespace) }}"
+  var new_pr = '<a class="dropdown-item" href="' + new_pr_url + '"> \
+                New Pull Request \
+            </a>' +
+            '<a class="dropdown-item" href="' + new_remote_pr_url +'"> \
+              New Remote Pull Request \
+            </a>';
+
+  var data = {
         repo: "{{ repo.name }}",
         namespace: "{{ repo.namespace if repo.namespace }}",
         repouser: "",
         csrf_token: "{{ g.confirmationform.csrf_token.current_token }}",
-      },
-      dataType: 'json',
-      error: function(res) {
-         console.log(res);
-      },
-      success: function(res) {
-        if (res.code == 'OK'){
-          var first_item = true;
-          for (branch in res.message.new_branch){
-            if (first_item){
-              $("#PR-dropdown").prepend("<div class='dropdown-divider'></div>")
-              first_item = false;
-            }
-            var url = "{{ url_for(
-              'ui_ns.new_request_pull',
-              repo=repo.name,
-              username=repo.user.user if repo.is_fork else None,
-              namespace=repo.namespace,
-              branch_to='-',
-              branch_from='-') }}";
-            url = url.slice(0, -4) + res.message.new_branch[branch]['target_branch'] + '..' + branch;
-            html = '<a class="dropdown-item" href="' + url + '">'
-              + '<span class="badge badge-light badge-pill border border-secondary font-size-1">'
-                + ' <i class="fa {{projecticon()}} fa-fw"></i> '
-                + '{{ repo.name }}'
-              + '</span> '
-              + '<span class="badge badge-secondary border border-secondary badge-pill font-size-1">'
-                + '<span class="fa fa-random fa-fw"> </span> '
-                + branch
-              + '</span></a> ';
-            $("#PR-dropdown").prepend(html);
-          }
-        }
-      }
-    });
-  }
-);
-{% endif %}
-
-$("#pr-button").one("click",
-  function() {
-    $.ajax({
-      url: '{{ url_for("internal_ns.get_pull_request_ready_branch") }}' ,
-      type: 'POST',
-      data: {
-        namespace: "{{ repo.namespace if repo.namespace }}",
+      };
+  process_async(_pr_url, data, generate_branch_list);
+  var data = {
         repo: "{{ repo.name }}",
+        namespace: "{{ repo.namespace if repo.namespace }}",
         repouser: "{{ g.fas_user.username }}",
         csrf_token: "{{ g.confirmationform.csrf_token.current_token }}",
-      },
-      dataType: 'json',
-      error: function(res) {
-        $('#spinnergif').hide()
-        console.log(res);
-      },
-      success: function(res) {
-        $('#spinnergif').hide()
-        console.log("done");
-        if (res.code == 'OK'){
-          var first_item = true;
-          for (branch in res.message.new_branch){
-            if (first_item){
-              $("#PR-dropdown").prepend("<div class='dropdown-divider'></div>");
-              first_item = false;
-            }
-            var url = "{{ url_for(
-              'ui_ns.new_request_pull',
-              repo=repo.name,
-              username=g.fas_user.username,
-              namespace=repo.namespace,
-              branch_to='-',
-              branch_from='-') }}";
-            url = url.slice(0, -4) + res.message.new_branch[branch]['target_branch'] + '..' + branch;
-            html = '<a class="dropdown-item" \
-            href="' + url + '"><span class="badge badge-light badge-pill border border-secondary font-size-1">'
-            + '<i class="fa fa-code-fork fa-fw"></i>'
-            +'{{ g.fas_user.username }}/{{ repo.name }}</span> '
-            +'<span class="badge badge-secondary border border-secondary badge-pill font-size-1">'
-            +'<i class="fa fa-random"></i> '
-            + branch + '</span></a> ';
-            $("#PR-dropdown").prepend(html);
-          }
-        }
-      }
-    })
+      };
+  process_async(_pr_url, data, generate_branch_list_fork);
+  _b.prepend(new_pr);
   }
 );
+{% endif %}
 
 {% endif %}
 </script>