Move more of the logic to split commit to diff/patch to the backend

This lowers the amount of code in the front end.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

pagure/lib/git.py

@@ -43,8 +43,9 @@ from pagure.lib import tasks
 _log = logging.getLogger(__name__)
 
 
-def commit_to_patch(repo_obj, commits,
-                    diff_view=False, find_similar=False):
+def commit_to_patch(
+        repo_obj, commits, diff_view=False, find_similar=False,
+        separated=False):
     ''' For a given commit (PyGit2 commit object) of a specified git repo,
     returns a string representation of the changes the commit did in a
     format that allows it to be used as patch.
@@ -60,6 +61,11 @@ def commit_to_patch(repo_obj, commits,
     :kwarg find_similar: a boolean specifying if what we run find_similar
         on the diff to group renamed files
     :type find_similar: boolean
+    :kwarg separated: a boolean specifying if the data returned should be
+        returned as one text blob or not. If diff_view is True, then the diff
+        are also split by file, otherwise, the different patches are returned
+        as different text blob.
+    :type separated: boolean
     :return: the patch or diff representation of the provided commits
     :rtype: str
 
@@ -79,7 +85,12 @@ def commit_to_patch(repo_obj, commits,
             diff.find_similar()
 
         if diff_view:
-            patch.append(diff.patch)
+            if separated:
+                for el in diff.patch.split('diff --git a/'):
+                    if el:
+                        patch.append('diff --git a/' + el)
+            else:
+                patch.append(diff.patch)
         else:
 
             subject = message = ''
@@ -110,7 +121,11 @@ Subject: {subject}
                 subject=subject,
                 msg=message,
                 patch=diff.patch))
-    return ''.join(patch)
+
+    if separated:
+        return patch
+    else:
+        return ''.join(patch)
 
 
 def generate_gitolite_acls(project=None, group=None):

pagure/ui/repo.py

@@ -891,35 +891,13 @@ def view_commit_patch_or_diff(
             flask.abort(404, 'Commit not found')
 
     if is_js:
-        if commit.parents:
-            diff_obj = repo_obj.diff(commit.parents[0], commit)
-        else:
-            diff_obj = commit.tree.diff_to_tree(swap=True)
-
-        # Patch.patch was introduced in pygit 0.26.2 so
-        # check that the Patch Object has this
-        if hasattr(diff_obj[0], "patch"):
-            if diff_obj:
-                diff_obj.find_similar()
-            diffs = {}
-            count = 0
-            for patch in diff_obj:
-                count = count + 1
-                diffs[str(count)] = patch.patch
-        else:
-            # since we can't get a individual patch item for each
-            # file, we get the whole diff, and manually split the
-            # string.
-            patch = pagure.lib.git.commit_to_patch(
-                repo_obj, commit, diff_view=diff, find_similar=True)
-
-            patches = filter(None, patch.split("diff --git a/"))
-
-            diffs = {}
-            count = 0
-            for p in patches:
-                count = count + 1
-                diffs[str(count)] = "diff --git a/" + p
+        patches = pagure.lib.git.commit_to_patch(
+            repo_obj, commit, diff_view=True, find_similar=True,
+            separated=True)
+
+        diffs = {}
+        for idx, patch in enumerate(patches):
+            diffs[idx + 1] = patch
 
         return flask.jsonify(diffs)
     else:

tests/test_pagure_lib_git.py

@@ -1387,152 +1387,6 @@ repo requests/forks/pingou/test3
         os.unlink(outputconf)
         self.assertFalse(os.path.exists(outputconf))
 
-    def test_commit_to_patch(self):
-        """ Test the commit_to_patch function of pagure.lib.git. """
-        # Create a git repo to play with
-        self.gitrepo = os.path.join(self.path, 'repos', 'test_repo.git')
-        os.makedirs(self.gitrepo)
-        repo = pygit2.init_repository(self.gitrepo)
-
-        # Create a file in that git repo
-        with open(os.path.join(self.gitrepo, 'sources'), 'w') as stream:
-            stream.write('foo\n bar')
-        repo.index.add('sources')
-        repo.index.write()
-
-        # Commits the files added
-        tree = repo.index.write_tree()
-        author = pygit2.Signature(
-            'Alice Author', 'alice@authors.tld')
-        committer = pygit2.Signature(
-            'Cecil Committer', 'cecil@committers.tld')
-        repo.create_commit(
-            'refs/heads/master',  # the name of the reference to update
-            author,
-            committer,
-            'Add sources file for testing',
-            # binary string representing the tree object ID
-            tree,
-            # list of binary strings representing parents of the new commit
-            []
-        )
-
-        first_commit = repo.revparse_single('HEAD')
-
-        # Edit the sources file again
-        with open(os.path.join(self.gitrepo, 'sources'), 'w') as stream:
-            stream.write('foo\n bar\nbaz\n boose')
-        repo.index.add('sources')
-        repo.index.write()
-
-        # Commits the files added
-        tree = repo.index.write_tree()
-        author = pygit2.Signature(
-            'Alice Author', 'alice@authors.tld')
-        committer = pygit2.Signature(
-            'Cecil Committer', 'cecil@committers.tld')
-        repo.create_commit(
-            'refs/heads/master',  # the name of the reference to update
-            author,
-            committer,
-            'Add baz and boose to the sources\n\n There are more objects to '
-            'consider',
-            # binary string representing the tree object ID
-            tree,
-            # list of binary strings representing parents of the new commit
-            [first_commit.oid.hex]
-        )
-
-        second_commit = repo.revparse_single('HEAD')
-
-        # Generate a patch for 2 commits
-        patch = pagure.lib.git.commit_to_patch(
-            repo, [first_commit, second_commit])
-        exp = """Mon Sep 17 00:00:00 2001
-From: Alice Author <alice@authors.tld>
-Subject: [PATCH 1/2] Add sources file for testing
-
-
----
-
-diff --git a/sources b/sources
-new file mode 100644
-index 0000000..9f44358
---- /dev/null
-+++ b/sources
-@@ -0,0 +1,2 @@
-+foo
-+ bar
-\ No newline at end of file
-
-Mon Sep 17 00:00:00 2001
-From: Alice Author <alice@authors.tld>
-Subject: [PATCH 2/2] Add baz and boose to the sources
-
-
- There are more objects to consider
----
-
-diff --git a/sources b/sources
-index 9f44358..2a552bb 100644
---- a/sources
-+++ b/sources
-@@ -1,2 +1,4 @@
- foo
-- bar
-\ No newline at end of file
-+ bar
-+baz
-+ boose
-\ No newline at end of file
-
-"""
-        npatch = []
-        for row in patch.split('\n'):
-            if row.startswith('Date:'):
-                continue
-            if row.startswith('From '):
-                row = row.split(' ', 2)[2]
-            npatch.append(row)
-
-        patch = '\n'.join(npatch)
-        self.assertEqual(patch, exp)
-
-        # Generate a patch for a single commit
-        patch = pagure.lib.git.commit_to_patch(repo, second_commit)
-        exp = """Mon Sep 17 00:00:00 2001
-From: Alice Author <alice@authors.tld>
-Subject: Add baz and boose to the sources
-
-
- There are more objects to consider
----
-
-diff --git a/sources b/sources
-index 9f44358..2a552bb 100644
---- a/sources
-+++ b/sources
-@@ -1,2 +1,4 @@
- foo
-- bar
-\ No newline at end of file
-+ bar
-+baz
-+ boose
-\ No newline at end of file
-
-"""
-        npatch = []
-        for row in patch.split('\n'):
-            if row.startswith('Date:'):
-                continue
-            if row.startswith('From '):
-                row = row.split(' ', 2)[2]
-            npatch.append(row)
-
-        patch = '\n'.join(npatch)
-        self.assertEqual(patch, exp)
-
     @patch('pagure.lib.notify.send_email')
     def test_update_git(self, email_f):
         """ Test the update_git of pagure.lib.git. """
@@ -3211,5 +3065,405 @@ index 0000000..60f7480
         )
 
 
+class PagureLibGitCommitToPatchtests(tests.Modeltests):
+    """ Tests for pagure.lib.git """
+
+    maxDiff = None
+
+    def setUp(self):
+        """ Set up the environment for the tests. """
+        super(PagureLibGitCommitToPatchtests, self).setUp()
+
+        # Create a git repo to play with
+        self.gitrepo = os.path.join(self.path, 'repos', 'test_repo.git')
+        os.makedirs(self.gitrepo)
+        repo = pygit2.init_repository(self.gitrepo)
+
+        # Create a file in that git repo
+        with open(os.path.join(self.gitrepo, 'sources'), 'w') as stream:
+            stream.write('foo\n bar')
+        repo.index.add('sources')
+        repo.index.write()
+
+        # Commits the files added
+        tree = repo.index.write_tree()
+        author = pygit2.Signature(
+            'Alice Author', 'alice@authors.tld')
+        committer = pygit2.Signature(
+            'Cecil Committer', 'cecil@committers.tld')
+        repo.create_commit(
+            'refs/heads/master',  # the name of the reference to update
+            author,
+            committer,
+            'Add sources file for testing',
+            # binary string representing the tree object ID
+            tree,
+            # list of binary strings representing parents of the new commit
+            []
+        )
+
+        self.first_commit = repo.revparse_single('HEAD')
+
+        # Edit the sources file again
+        with open(os.path.join(self.gitrepo, 'sources'), 'w') as stream:
+            stream.write('foo\n bar\nbaz\n boose')
+        repo.index.add('sources')
+        repo.index.write()
+
+        # Commits the files added
+        tree = repo.index.write_tree()
+        author = pygit2.Signature(
+            'Alice Author', 'alice@authors.tld')
+        committer = pygit2.Signature(
+            'Cecil Committer', 'cecil@committers.tld')
+        repo.create_commit(
+            'refs/heads/master',  # the name of the reference to update
+            author,
+            committer,
+            'Add baz and boose to the sources\n\n There are more objects to '
+            'consider',
+            # binary string representing the tree object ID
+            tree,
+            # list of binary strings representing parents of the new commit
+            [self.first_commit.oid.hex]
+        )
+
+        self.second_commit = repo.revparse_single('HEAD')
+
+    def test_commit_to_patch_first_commit(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(repo, self.first_commit)
+        exp = """Mon Sep 17 00:00:00 2001
+From: Alice Author <alice@authors.tld>
+Subject: Add sources file for testing
+
+
+---
+
+diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_single_commit(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(repo, self.second_commit)
+        exp = """Mon Sep 17 00:00:00 2001
+From: Alice Author <alice@authors.tld>
+Subject: Add baz and boose to the sources
+
+
+ There are more objects to consider
+---
+
+diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_2_commits(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(
+            repo, [self.first_commit, self.second_commit])
+        exp = """Mon Sep 17 00:00:00 2001
+From: Alice Author <alice@authors.tld>
+Subject: [PATCH 1/2] Add sources file for testing
+
+
+---
+
+diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+
+Mon Sep 17 00:00:00 2001
+From: Alice Author <alice@authors.tld>
+Subject: [PATCH 2/2] Add baz and boose to the sources
+
+
+ There are more objects to consider
+---
+
+diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_first_commit_diff(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(
+            repo, self.first_commit, diff_view=True)
+        exp = """diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_single_commit_diff(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(
+            repo, self.second_commit, diff_view=True)
+        exp = """diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_two_commits_diff(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patch = pagure.lib.git.commit_to_patch(
+            repo, [self.first_commit, self.second_commit], diff_view=True)
+        exp = """diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+"""
+        npatch = []
+        for row in patch.split('\n'):
+            if row.startswith('Date:'):
+                continue
+            if row.startswith('From '):
+                row = row.split(' ', 2)[2]
+            npatch.append(row)
+
+        patch = '\n'.join(npatch)
+        self.assertEqual(patch, exp)
+
+    def test_commit_to_patch_first_commit_diff_separated(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patches = pagure.lib.git.commit_to_patch(
+            repo, self.first_commit, diff_view=True, separated=True)
+        exp = """diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+"""
+        output = []
+        for patch in patches:
+            npatch = []
+            for row in patch.split('\n'):
+                if row.startswith('Date:'):
+                    continue
+                if row.startswith('From '):
+                    row = row.split(' ', 2)[2]
+                npatch.append(row)
+
+            patch = '\n'.join(npatch)
+            output.append(patch)
+
+        self.assertEqual(output, [exp])
+
+    def test_commit_to_patch_single_commit_diff_separated(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patches = pagure.lib.git.commit_to_patch(
+            repo, self.second_commit, diff_view=True, separated=True)
+        exp = """diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+"""
+        output = []
+        for patch in patches:
+            npatch = []
+            for row in patch.split('\n'):
+                if row.startswith('Date:'):
+                    continue
+                if row.startswith('From '):
+                    row = row.split(' ', 2)[2]
+                npatch.append(row)
+
+            patch = '\n'.join(npatch)
+            output.append(patch)
+
+        self.assertEqual(output, [exp])
+
+    def test_commit_to_patch_two_commits_diff_separated(self):
+        """ Test the commit_to_patch function of pagure.lib.git. """
+        repo = pygit2.init_repository(self.gitrepo)
+
+        patches = pagure.lib.git.commit_to_patch(
+            repo, [self.first_commit, self.second_commit], diff_view=True,
+            separated=True)
+        exp = ["""diff --git a/sources b/sources
+new file mode 100644
+index 0000000..9f44358
+--- /dev/null
++++ b/sources
+@@ -0,0 +1,2 @@
++foo
++ bar
+\ No newline at end of file
+""",
+"""diff --git a/sources b/sources
+index 9f44358..2a552bb 100644
+--- a/sources
++++ b/sources
+@@ -1,2 +1,4 @@
+ foo
+- bar
+\ No newline at end of file
++ bar
++baz
++ boose
+\ No newline at end of file
+"""]
+        output = []
+        for patch in patches:
+            npatch = []
+            for row in patch.split('\n'):
+                if row.startswith('Date:'):
+                    continue
+                if row.startswith('From '):
+                    row = row.split(' ', 2)[2]
+                npatch.append(row)
+
+            patch = '\n'.join(npatch)
+            output.append(patch)
+
+        self.assertEqual(output, exp)
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)