fix(bitbucket_server): Considers pagination for (#19542)

bitbucket-server-integration

Implements support for pagination in the bitucket-server-api when the
commits between two commit ids or filechanges are requested

Fixes GH-19201

src/sentry/integrations/bitbucket_server/client.py

@@ -1,11 +1,15 @@
 from __future__ import absolute_import
 
+import logging
+
 from oauthlib.oauth1 import SIGNATURE_RSA
 from requests_oauthlib import OAuth1
 from six.moves.urllib.parse import parse_qsl
 from sentry.integrations.client import ApiClient
 from sentry.shared_integrations.exceptions import ApiError
 
+logger = logging.getLogger("sentry.integrations.bitbucket_server")
+
 
 class BitbucketServerAPIPath(object):
     """
@@ -139,11 +143,15 @@ class BitbucketServer(ApiClient):
             auth=self.get_auth(),
         )
 
-    def get_commits(self, project, repo, fromHash, toHash):
-        return self.get(
+    def get_commits(self, project, repo, from_hash, to_hash, limit=1000):
+        logger.info(
+            "Loading commits",
+            extra={"repo": repo, "project": project, "from_hash": from_hash, "to_hash": to_hash},
+        )
+
+        return self._get_values(
             BitbucketServerAPIPath.repository_commits.format(project=project, repo=repo),
-            auth=self.get_auth(),
-            params={"since": fromHash, "until": toHash, "merges": "exclude"},
+            {"limit": limit, "since": from_hash, "until": to_hash, "merges": "exclude"},
         )
 
     def get_last_commits(self, project, repo, limit=10):
@@ -151,12 +159,16 @@ class BitbucketServer(ApiClient):
             BitbucketServerAPIPath.repository_commits.format(project=project, repo=repo),
             auth=self.get_auth(),
             params={"merges": "exclude", "limit": limit},
+        )["values"]
+
+    def get_commit_filechanges(self, project, repo, commit, limit=1000):
+        logger.info(
+            "Loading filechanges", extra={"repo": repo, "project": project, "commit": commit}
         )
 
-    def get_commit_filechanges(self, project, repo, commit):
-        return self.get(
+        return self._get_values(
             BitbucketServerAPIPath.commit_changes.format(project=project, repo=repo, commit=commit),
-            auth=self.get_auth(),
+            {"limit": limit},
         )
 
     def get_auth(self):
@@ -168,3 +180,40 @@ class BitbucketServer(ApiClient):
             signature_method=SIGNATURE_RSA,
             signature_type="auth_header",
         )
+
+    def _get_values(self, uri, params, max_pages=1000000):
+        values = []
+        start = 0
+
+        logger.info(
+            "Loading values for paginated uri",
+            extra={"uri": uri, "max_pages": max_pages, "params": params},
+        )
+
+        for i in range(max_pages):
+            new_params = dict.copy(params)
+            new_params["start"] = start
+            logger.debug(
+                "Loading values for paginated uri starting from %s",
+                start,
+                extra={"uri": uri, "params": new_params},
+            )
+            data = self.get(uri, auth=self.get_auth(), params=new_params)
+            logger.debug(
+                "%s values loaded", len(data["values"]), extra={"uri": uri, "params": new_params}
+            )
+
+            values += data["values"]
+
+            if "isLastPage" not in data or data["isLastPage"]:
+                logger.debug("Reached last page for paginated uri", extra={"uri": uri})
+                return values
+            else:
+                start = data["nextPageStart"]
+
+        logger.warn(
+            "Reached max number of pages (%s) for paginated uri. Values will be missing.",
+            max_pages,
+            extra={"uri": uri, "params": params},
+        )
+        return values

src/sentry/integrations/bitbucket_server/repository.py

@@ -108,7 +108,7 @@ class BitbucketServerRepositoryProvider(IntegrationRepositoryProvider):
                     client, repo.config["project"], repo.config["repo"], c["id"]
                 ),
             }
-            for c in commit_list["values"]
+            for c in commit_list
         ]
 
     def compare_commits(self, repo, start_sha, end_sha):
@@ -117,12 +117,12 @@ class BitbucketServerRepositoryProvider(IntegrationRepositoryProvider):
 
         try:
             if "0" * 40 == start_sha or start_sha is None:
-                res = client.get_last_commits(repo.config["project"], repo.config["repo"])
+                commit_list = client.get_last_commits(repo.config["project"], repo.config["repo"])
             else:
-                res = client.get_commits(
+                commit_list = client.get_commits(
                     repo.config["project"], repo.config["repo"], start_sha, end_sha
                 )
-            return self._format_commits(client, repo, res)
+            return self._format_commits(client, repo, commit_list)
         except Exception as e:
             installation.raise_error(e)
 
@@ -142,13 +142,13 @@ class BitbucketServerRepositoryProvider(IntegrationRepositoryProvider):
 
         return self._transform_patchset(commit_files)
 
-    def _transform_patchset(self, diff):
+    def _transform_patchset(self, values):
         """Convert the patch data from Bitbucket into our internal format
 
         See sentry.models.Release.set_commits
         """
         changes = []
-        for change in diff["values"]:
+        for change in values:
             if change["type"] == "MODIFY":
                 changes.append({"path": change["path"]["toString"], "type": "M"})
             if change["type"] == "ADD":

tests/sentry/integrations/bitbucket_server/test_repository.py

@@ -11,7 +11,17 @@ from sentry.models import Integration, Repository, IdentityProvider, Identity, I
 from sentry.testutils import APITestCase
 from sentry.integrations.bitbucket_server.repository import BitbucketServerRepositoryProvider
 from sentry.shared_integrations.exceptions import IntegrationError
-from .testutils import EXAMPLE_PRIVATE_KEY, COMPARE_COMMITS_EXAMPLE, REPO, COMMIT_CHANGELIST_EXAMPLE
+from .testutils import (
+    EXAMPLE_PRIVATE_KEY,
+    COMPARE_COMMITS_EXAMPLE,
+    REPO,
+    COMMIT_CHANGELIST_EXAMPLE,
+    COMPARE_COMMITS_WITH_PAGES_1_2_EXAMPLE,
+    COMPARE_COMMITS_WITH_PAGES_2_2_EXAMPLE,
+    COMMIT_CHANGELIST_WITH_PAGES_FIRST_COMMIT_EXAMPLE,
+    COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_1_2,
+    COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_2_2,
+)
 
 
 class BitbucketServerRepositoryProviderTest(APITestCase):
@@ -92,6 +102,77 @@ class BitbucketServerRepositoryProviderTest(APITestCase):
             }
         ]
 
+    @responses.activate
+    def test_compare_commits_with_two_pages(self):
+        repo = Repository.objects.create(
+            provider="bitbucket_server",
+            name="sentryuser/newsdiffs",
+            organization_id=self.organization.id,
+            config={"name": "sentryuser/newsdiffs", "project": "sentryuser", "repo": "newsdiffs"},
+            integration_id=self.integration.id,
+        )
+
+        responses.add(
+            responses.GET,
+            "https://bitbucket.example.com/rest/api/1.0/projects/sentryuser/repos/newsdiffs/commits?merges=exclude&limit=1000&since=d0352305beb41afb3a4ea79e3a97bf6a97520339&start=0&until=042bc8434e0c178d8745c7d9f90bddab9c927887",
+            json=COMPARE_COMMITS_WITH_PAGES_1_2_EXAMPLE,
+        )
+
+        responses.add(
+            responses.GET,
+            "https://bitbucket.example.com/rest/api/1.0/projects/sentryuser/repos/newsdiffs/commits?merges=exclude&limit=1000&since=d0352305beb41afb3a4ea79e3a97bf6a97520339&start=1&until=042bc8434e0c178d8745c7d9f90bddab9c927887",
+            json=COMPARE_COMMITS_WITH_PAGES_2_2_EXAMPLE,
+        )
+
+        responses.add(
+            responses.GET,
+            "https://bitbucket.example.com/rest/api/1.0/projects/sentryuser/repos/newsdiffs/commits/d0352305beb41afb3a4ea79e3a97bf6a97520339/changes",
+            json=COMMIT_CHANGELIST_WITH_PAGES_FIRST_COMMIT_EXAMPLE,
+        )
+
+        responses.add(
+            responses.GET,
+            "https://bitbucket.example.com/rest/api/1.0/projects/sentryuser/repos/newsdiffs/commits/042bc8434e0c178d8745c7d9f90bddab9c927887/changes?merges=exclude&limit=100&start=0",
+            json=COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_1_2,
+        )
+
+        responses.add(
+            responses.GET,
+            "https://bitbucket.example.com/rest/api/1.0/projects/sentryuser/repos/newsdiffs/commits/042bc8434e0c178d8745c7d9f90bddab9c927887/changes?merges=exclude&limit=100&start=1",
+            json=COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_2_2,
+        )
+
+        res = self.provider.compare_commits(
+            repo,
+            "d0352305beb41afb3a4ea79e3a97bf6a97520339",
+            "042bc8434e0c178d8745c7d9f90bddab9c927887",
+        )
+
+        assert res == [
+            {
+                "author_email": "sentryuser@getsentry.com",
+                "author_name": "Sentry User",
+                "message": "Fist commit",
+                "id": "d0352305beb41afb3a4ea79e3a97bf6a97520339",
+                "repository": "sentryuser/newsdiffs",
+                "patch_set": [{"path": "a.txt", "type": "M"}, {"path": "b.txt", "type": "A"}],
+                "timestamp": datetime.datetime(2019, 12, 19, 13, 56, 56, tzinfo=timezone.utc),
+            },
+            {
+                "author_email": "sentryuser@getsentry.com",
+                "author_name": "Sentry User",
+                "message": "Second commit",
+                "id": "042bc8434e0c178d8745c7d9f90bddab9c927887",
+                "repository": "sentryuser/newsdiffs",
+                "patch_set": [
+                    {"path": "c.txt", "type": "D"},
+                    {"path": "e.txt", "type": "D"},
+                    {"path": "d.txt", "type": "A"},
+                ],
+                "timestamp": datetime.datetime(2019, 12, 19, 13, 56, 56, tzinfo=timezone.utc),
+            },
+        ]
+
     @responses.activate
     def test_build_repository_config(self):
         project = u"laurynsentry"

tests/sentry/integrations/bitbucket_server/testutils.py

@@ -179,3 +179,140 @@ REPO = {
         ]
     ),
 }
+
+
+COMPARE_COMMITS_WITH_PAGES_1_2_EXAMPLE = {
+    "values": [
+        {
+            "id": "d0352305beb41afb3a4ea79e3a97bf6a97520339",
+            "displayId": "d0352305beb",
+            "author": {
+                "name": "SentryU",
+                "displayName": "Sentry User",
+                "emailAddress": "sentryuser@getsentry.com",
+                "type": "NORMAL",
+            },
+            "message": "Fist commit",
+            "authorTimestamp": 1576763816000,
+        }
+    ],
+    "size": 1,
+    "isLastPage": False,
+    "start": 0,
+    "limit": 1,
+    "nextPageStart": 1,
+}
+
+COMPARE_COMMITS_WITH_PAGES_2_2_EXAMPLE = {
+    "values": [
+        {
+            "id": "042bc8434e0c178d8745c7d9f90bddab9c927887",
+            "displayId": "042bc8434e0",
+            "author": {
+                "name": "SentryU",
+                "displayName": "Sentry User",
+                "emailAddress": "sentryuser@getsentry.com",
+                "type": "NORMAL",
+            },
+            "message": "Second commit",
+            "authorTimestamp": 1576763816000,
+        }
+    ],
+    "size": 1,
+    "isLastPage": True,
+    "start": 1,
+    "limit": 1,
+    "nextPageStart": None,
+}
+
+COMMIT_CHANGELIST_WITH_PAGES_FIRST_COMMIT_EXAMPLE = {
+    "values": [
+        {
+            "path": {
+                "components": ["a.txt"],
+                "parent": "",
+                "name": "a.txt",
+                "extension": "txt",
+                "toString": "a.txt",
+            },
+            "executable": False,
+            "percentUnchanged": -1,
+            "type": "MODIFY",
+            "nodeType": "FILE",
+            "srcExecutable": False,
+            "properties": {"gitChangeType": "MODIFY"},
+        },
+        {
+            "path": {
+                "components": ["b.txt"],
+                "parent": "",
+                "name": "b.txt",
+                "extension": "txt",
+                "toString": "b.txt",
+            },
+            "executable": False,
+            "percentUnchanged": -1,
+            "type": "ADD",
+            "nodeType": "FILE",
+            "srcExecutable": False,
+            "properties": {"gitChangeType": "ADD"},
+        },
+    ]
+}
+
+COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_1_2 = {
+    "values": [
+        {
+            "path": {
+                "components": ["c.txt"],
+                "parent": "",
+                "name": "c.txt",
+                "extension": "txt",
+                "toString": "c.txt",
+            },
+            "executable": False,
+            "percentUnchanged": -1,
+            "type": "DELETE",
+            "nodeType": "FILE",
+            "srcExecutable": False,
+            "properties": {"gitChangeType": "DELETE"},
+        }
+    ],
+    "size": 1,
+    "isLastPage": False,
+    "start": 0,
+    "limit": 1,
+    "nextPageStart": 1,
+}
+
+COMMIT_CHANGELIST_WITH_PAGES_SECOND_COMMIT_EXAMPLE_2_2 = {
+    "values": [
+        {
+            "path": {
+                "components": ["e.txt"],
+                "parent": "",
+                "name": "d.txt",
+                "extension": "txt",
+                "toString": "d.txt",
+            },
+            "srcPath": {
+                "components": ["d.txt"],
+                "parent": "",
+                "name": "e.txt",
+                "extension": "txt",
+                "toString": "e.txt",
+            },
+            "executable": False,
+            "percentUnchanged": -1,
+            "type": "MOVE",
+            "nodeType": "FILE",
+            "srcExecutable": False,
+            "properties": {"gitChangeType": "MOVE"},
+        },
+    ],
+    "size": 1,
+    "isLastPage": True,
+    "start": 1,
+    "limit": 1,
+    "nextPageStart": None,
+}