Git services: improve sync repos (#12428)

Common changes across providers:

- A new method to just update a remote repository from the data returned
by the API was added, this is in order to make it easy to re-use this
method when support for updating a single repo is added (will do this in
another PR on top of this one).
- We were iterating over repositories twice for GitHub and BB, once by
listing all the repositories the user has access to, and then again by
iterating over all the repositories from the organizations the user has
access to, this was also incorrectly adding a relationship between the
user and repositories the use didn't actually have access, as that
endpoint will return all public repositories, even if the user doesn't
belong to it.
- sync_organizations now basically just creates the user-organization
relation only.
- Since several repositories can belong to the same organization, we
don't really need to update the same organization over and over again,
just once, so a cache was introduced for it. This is also a reason why
the creation of the user-organization relationship was moved outside
`create_organization`.

Specific changes:

- For Bitbucket, we weren't updating a repository if the workspace it
belongs was changed, we now update those. This is the same problem we
had with Gitlab
#12233.
- For Bitbucket, we were creating the repo-user relationship and then
updating that relationship for the repositories where the user is an
admin. This wasn't resetting the admin status to false for repos the
user no longer had that permission, we now always default to
admin=False, and then we update the repositories where the user is admin
in the other call.
- In Bitbucket, we don't have organizations, we have projects and
workspaces, we are using workspaces as our organizations. But in BB,
every repository is linked to a workspace (a user can be a workspace),
so we always create an organization for each repository.
- Since all other providers are fetching all repositories in the
`sync_repositories` method, I changed Gitlab do also do that. In order
to do that I reverted to use the previous /projects endpoint
(#12233).
- GitLab sometimes returns avatars with a relative URL `/~/uploads...`
instead of including the domain as well, so we are normalizing all URLs
from Gitlab now.

---------

Co-authored-by: Copilot <[email protected]>

Author: stsewd

readthedocs/oauth/services/base.py

@@ -144,6 +144,10 @@ class UserService(Service):
     def __init__(self, user, account):
         self.user = user
         self.account = account
+        # Cache organizations to avoid multiple DB hits
+        # when syncing repositories that belong to the same organization.
+        # Used by `create_organization` method in subclasses.
+        self._organizations_cache = {}
         structlog.contextvars.bind_contextvars(
             user_username=self.user.username,
             social_provider=self.allauth_provider.id,

readthedocs/oauth/services/bitbucket.py

@@ -63,24 +63,26 @@ def sync_repositories(self):
                 "https://bitbucket.org/api/2.0/repositories/",
                 role="admin",
             )
-            admin_repo_relations = RemoteRepositoryRelation.objects.filter(
+            RemoteRepositoryRelation.objects.filter(
                 user=self.user,
                 account=self.account,
                 remote_repository__vcs_provider=self.vcs_provider_slug,
                 remote_repository__remote_id__in=[r["uuid"] for r in resp],
-            )
-            for remote_repository_relation in admin_repo_relations:
-                remote_repository_relation.admin = True
-                remote_repository_relation.save()
+            ).update(admin=True)
         except (TypeError, ValueError):
-            pass
+            log.warning("Error syncing Bitbucket admin repositories")
 
         return remote_ids
 
     def sync_organizations(self):
-        """Sync Bitbucket workspaces(our RemoteOrganization) and workspace repositories."""
+        """
+        Sync Bitbucket workspaces (organizations).
+
+        This method only creates the relationships between the
+        organizations and the user, as all the repositories
+        are already created in the sync_repositories method.
+        """
         organization_remote_ids = []
-        repository_remote_ids = []
 
         try:
             workspaces = self.paginate(
@@ -89,18 +91,8 @@ def sync_organizations(self):
             )
             for workspace in workspaces:
                 remote_organization = self.create_organization(workspace)
-                repos = self.paginate(workspace["links"]["repositories"]["href"])
-
+                remote_organization.get_remote_organization_relation(self.user, self.account)
                 organization_remote_ids.append(remote_organization.remote_id)
-
-                for repo in repos:
-                    remote_repository = self.create_repository(
-                        repo,
-                        organization=remote_organization,
-                    )
-                    if remote_repository:
-                        repository_remote_ids.append(remote_repository.remote_id)
-
         except ValueError:
             log.warning("Error syncing Bitbucket organizations")
             raise SyncServiceError(
@@ -109,9 +101,9 @@ def sync_organizations(self):
                 )
             )
 
-        return organization_remote_ids, repository_remote_ids
+        return organization_remote_ids, []
 
-    def create_repository(self, fields, privacy=None, organization=None):
+    def create_repository(self, fields, privacy=None):
         """
         Update or create a repository from Bitbucket API response.
 
@@ -136,42 +128,15 @@ def create_repository(self, fields, privacy=None, organization=None):
             repo, _ = RemoteRepository.objects.get_or_create(
                 remote_id=fields["uuid"], vcs_provider=self.vcs_provider_slug
             )
-            repo.get_remote_repository_relation(self.user, self.account)
+            self._update_repository_from_fields(repo, fields)
 
-            if repo.organization and repo.organization != organization:
-                log.debug(
-                    "Not importing repository because mismatched orgs.",
-                    repository=fields["name"],
-                )
-                return None
-
-            repo.organization = organization
-            repo.name = fields["name"]
-            repo.full_name = fields["full_name"]
-            repo.description = fields["description"]
-            repo.private = fields["is_private"]
-
-            # Default to HTTPS, use SSH for private repositories
-            clone_urls = {u["name"]: u["href"] for u in fields["links"]["clone"]}
-            repo.clone_url = self.https_url_pattern.sub(
-                "https://bitbucket.org/",
-                clone_urls.get("https"),
+            # The repositories API doesn't return the admin status of the user,
+            # so we default to False, and then update it later using another API call.
+            remote_repository_relation = repo.get_remote_repository_relation(
+                self.user, self.account
             )
-            repo.ssh_url = clone_urls.get("ssh")
-            if repo.private:
-                repo.clone_url = repo.ssh_url
-
-            repo.html_url = fields["links"]["html"]["href"]
-            repo.vcs = fields["scm"]
-            mainbranch = fields.get("mainbranch") or {}
-            repo.default_branch = mainbranch.get("name")
-
-            avatar_url = fields["links"]["avatar"]["href"] or ""
-            repo.avatar_url = re.sub(r"\/16\/$", r"/32/", avatar_url)
-            if not repo.avatar_url:
-                repo.avatar_url = self.default_user_avatar_url
-
-            repo.save()
+            remote_repository_relation.admin = False
+            remote_repository_relation.save()
 
             return repo
 
@@ -180,17 +145,59 @@ def create_repository(self, fields, privacy=None, organization=None):
             repository=fields["name"],
         )
 
+    def _update_repository_from_fields(self, repo, fields):
+        # All repositories are created under a workspace,
+        # which we consider an organization.
+        organization = self.create_organization(fields["workspace"])
+        repo.organization = organization
+        repo.name = fields["name"]
+        repo.full_name = fields["full_name"]
+        repo.description = fields["description"]
+        repo.private = fields["is_private"]
+
+        # Default to HTTPS, use SSH for private repositories
+        clone_urls = {u["name"]: u["href"] for u in fields["links"]["clone"]}
+        repo.clone_url = self.https_url_pattern.sub(
+            "https://bitbucket.org/",
+            clone_urls.get("https"),
+        )
+        repo.ssh_url = clone_urls.get("ssh")
+        if repo.private:
+            repo.clone_url = repo.ssh_url
+
+        repo.html_url = fields["links"]["html"]["href"]
+        repo.vcs = fields["scm"]
+        mainbranch = fields.get("mainbranch") or {}
+        repo.default_branch = mainbranch.get("name")
+
+        avatar_url = fields["links"]["avatar"]["href"] or ""
+        repo.avatar_url = re.sub(r"\/16\/$", r"/32/", avatar_url)
+        if not repo.avatar_url:
+            repo.avatar_url = self.default_user_avatar_url
+
+        repo.save()
+
     def create_organization(self, fields):
         """
         Update or create remote organization from Bitbucket API response.
 
         :param fields: dictionary response of data from API
         :rtype: RemoteOrganization
+
+        .. note::
+
+           This method caches organizations by their remote ID to avoid
+           unnecessary database queries, specially when creating
+           multiple repositories that belong to the same organization.
         """
+        organization_id = fields["uuid"]
+        if organization_id in self._organizations_cache:
+            return self._organizations_cache[organization_id]
+
         organization, _ = RemoteOrganization.objects.get_or_create(
-            remote_id=fields["uuid"], vcs_provider=self.vcs_provider_slug
+            remote_id=organization_id,
+            vcs_provider=self.vcs_provider_slug,
         )
-        organization.get_remote_organization_relation(self.user, self.account)
 
         organization.slug = fields.get("slug")
         organization.name = fields.get("name")
@@ -201,6 +208,7 @@ def create_organization(self, fields):
 
         organization.save()
 
+        self._organizations_cache[organization_id] = organization
         return organization
 
     def get_next_url_to_paginate(self, response):

readthedocs/oauth/services/github.py

@@ -56,30 +56,22 @@ def sync_repositories(self):
         return remote_ids
 
     def sync_organizations(self):
-        """Sync organizations from GitHub API."""
+        """
+        Sync organizations from GitHub API.
+
+        This method only creates the relationships between the
+        organizations and the user, as all the repositories
+        are already created in the sync_repositories method.
+        """
         organization_remote_ids = []
-        repository_remote_ids = []
 
         try:
             orgs = self.paginate(f"{self.base_api_url}/user/orgs", per_page=100)
             for org in orgs:
                 org_details = self.session.get(org["url"]).json()
-                remote_organization = self.create_organization(
-                    org_details,
-                    create_user_relationship=True,
-                )
+                remote_organization = self.create_organization(org_details)
+                remote_organization.get_remote_organization_relation(self.user, self.account)
                 organization_remote_ids.append(remote_organization.remote_id)
-
-                org_url = org["url"]
-                org_repos = self.paginate(
-                    f"{org_url}/repos",
-                    per_page=100,
-                )
-                for repo in org_repos:
-                    remote_repository = self.create_repository(repo)
-                    if remote_repository:
-                        repository_remote_ids.append(remote_repository.remote_id)
-
         except (TypeError, ValueError):
             log.warning("Error syncing GitHub organizations")
             raise SyncServiceError(
@@ -88,16 +80,14 @@ def sync_organizations(self):
                 )
             )
 
-        return organization_remote_ids, repository_remote_ids
+        return organization_remote_ids, []
 
     def create_repository(self, fields, privacy=None):
         """
         Update or create a repository from GitHub API response.
 
         :param fields: dictionary of response data from API
         :param privacy: privacy level to support
-        :param organization: remote organization to associate with
-        :type organization: RemoteOrganization
         :rtype: RemoteRepository
         """
         privacy = privacy or settings.DEFAULT_PRIVACY_LEVEL
@@ -111,47 +101,7 @@ def create_repository(self, fields, privacy=None):
                 remote_id=str(fields["id"]),
                 vcs_provider=self.vcs_provider_slug,
             )
-
-            owner_type = fields["owner"]["type"]
-            organization = None
-            if owner_type == "Organization":
-                # We aren't creating a remote relationship between the current user
-                # and the organization, since the user can have access to the repository,
-                # but not to the organization.
-                organization = self.create_organization(
-                    fields=fields["owner"],
-                    create_user_relationship=False,
-                )
-
-            # If there is an organization associated with this repository,
-            # attach the organization to the repository.
-            if organization and owner_type == "Organization":
-                repo.organization = organization
-
-            # If the repository belongs to a user,
-            # remove the organization linked to the repository.
-            if owner_type == "User":
-                repo.organization = None
-
-            repo.name = fields["name"]
-            repo.full_name = fields["full_name"]
-            repo.description = fields["description"]
-            repo.ssh_url = fields["ssh_url"]
-            repo.html_url = fields["html_url"]
-            repo.private = fields["private"]
-            repo.vcs = "git"
-            repo.avatar_url = fields.get("owner", {}).get("avatar_url")
-            repo.default_branch = fields.get("default_branch")
-
-            if repo.private:
-                repo.clone_url = fields["ssh_url"]
-            else:
-                repo.clone_url = fields["clone_url"]
-
-            if not repo.avatar_url:
-                repo.avatar_url = self.default_user_avatar_url
-
-            repo.save()
+            self._update_repository_from_fields(repo, fields)
 
             remote_repository_relation = repo.get_remote_repository_relation(
                 self.user, self.account
@@ -166,7 +116,43 @@ def create_repository(self, fields, privacy=None):
             repository=fields["name"],
         )
 
-    def create_organization(self, fields, create_user_relationship=False):
+    def _update_repository_from_fields(self, repo, fields):
+        owner_type = fields["owner"]["type"]
+        organization = None
+        if owner_type == "Organization":
+            organization = self.create_organization(fields=fields["owner"])
+
+        # If there is an organization associated with this repository,
+        # attach the organization to the repository.
+        if organization and owner_type == "Organization":
+            repo.organization = organization
+
+        # If the repository belongs to a user,
+        # remove the organization linked to the repository.
+        if owner_type == "User":
+            repo.organization = None
+
+        repo.name = fields["name"]
+        repo.full_name = fields["full_name"]
+        repo.description = fields["description"]
+        repo.ssh_url = fields["ssh_url"]
+        repo.html_url = fields["html_url"]
+        repo.private = fields["private"]
+        repo.vcs = "git"
+        repo.avatar_url = fields.get("owner", {}).get("avatar_url")
+        repo.default_branch = fields.get("default_branch")
+
+        if repo.private:
+            repo.clone_url = fields["ssh_url"]
+        else:
+            repo.clone_url = fields["clone_url"]
+
+        if not repo.avatar_url:
+            repo.avatar_url = self.default_user_avatar_url
+
+        repo.save()
+
+    def create_organization(self, fields):
         """
         Update or create remote organization from GitHub API response.
 
@@ -175,13 +161,21 @@ def create_organization(self, fields, create_user_relationship=False):
          organization and the current user. If `False`, only the `RemoteOrganization` object
          will be created/updated.
         :rtype: RemoteOrganization
+
+        .. note::
+
+           This method caches organizations by their remote ID to avoid
+           unnecessary database queries, specially when creating
+           multiple repositories that belong to the same organization.
         """
+        organization_id = str(fields["id"])
+        if organization_id in self._organizations_cache:
+            return self._organizations_cache[organization_id]
+
         organization, _ = RemoteOrganization.objects.get_or_create(
-            remote_id=str(fields["id"]),
+            remote_id=organization_id,
             vcs_provider=self.vcs_provider_slug,
         )
-        if create_user_relationship:
-            organization.get_remote_organization_relation(self.user, self.account)
 
         organization.url = fields.get("html_url")
         # fields['login'] contains GitHub Organization slug
@@ -195,6 +189,7 @@ def create_organization(self, fields, create_user_relationship=False):
 
         organization.save()
 
+        self._organizations_cache[organization_id] = organization
         return organization
 
     def get_next_url_to_paginate(self, response):