OAuth: sync collaborators of repositories with projects only on full syncs

Listing collaborators costs at least one API request per repository,
which is the dominant cost of a full installation sync, and makes large
installations exceed GitHub's per-installation rate limit.

Syncs triggered by organization renames now list collaborators only for
repositories linked to a project, which is what SSO access and
maintainer listings need fresh. Collaborators of other repositories
only feed the list of repositories available to import, and keep being
refreshed when each user signs in or manually re-syncs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjkPjH3EDEQmtywJvDVau

Author: claude

readthedocs/oauth/services/githubapp.py

@@ -174,7 +174,7 @@ def sync_user_access(cls, user):
         if has_error:
             raise SyncServiceError()
 
-    def sync(self):
+    def sync(self, *, sync_all_collaborators=True):
         """
         Sync all repositories and organizations that are accessible to the installation.
 
@@ -183,6 +183,13 @@ def sync(self):
 
         If a remote organization doesn't have any repositories after removing the repositories,
         we remove the organization from the database.
+
+        :param sync_all_collaborators: If ``False``, collaborators are synced only for
+         repositories that are linked to a project. Listing collaborators costs at least
+         one API request per repository, which makes a full sync of a large installation
+         exceed the GitHub API rate limit. Collaborators of repositories without a project
+         are only used to build the list of repositories available to import, which is
+         refreshed when each user signs in or manually re-syncs their repositories.
         """
         try:
             app_installation = self.get_app_installation()
@@ -208,9 +215,22 @@ def sync(self):
             self.installation.delete()
             raise SyncServiceError()
 
+        repos_with_projects = set()
+        if not sync_all_collaborators:
+            repos_with_projects = set(
+                self.installation.repositories.filter(projects__isnull=False).values_list(
+                    "remote_id", flat=True
+                )
+            )
+
         remote_repositories = []
         for gh_repo in app_installation.get_repos():
-            remote_repo = self._create_or_update_repository_from_gh(gh_repo)
+            remote_repo = self._create_or_update_repository_from_gh(
+                gh_repo,
+                resync_collaborators=(
+                    sync_all_collaborators or str(gh_repo.id) in repos_with_projects
+                ),
+            )
             if remote_repo:
                 remote_repositories.append(remote_repo)
 
@@ -266,12 +286,13 @@ def update_or_create_repositories(self, repository_ids: list[int]):
             self.installation.delete_repositories(repositories_to_delete)
 
     def _create_or_update_repository_from_gh(
-        self, gh_repo: GHRepository
+        self, gh_repo: GHRepository, *, resync_collaborators: bool = True
     ) -> RemoteRepository | None:
         """
         Create or update a remote repository from a GitHub repository object.
 
-        We also sync the collaborators of the repository with the database,
+        We also sync the collaborators of the repository with the database
+        (unless ``resync_collaborators`` is ``False``),
         and create or update the organization of the repository.
         """
         target_id = self.installation.target_id
@@ -324,7 +345,8 @@ def _create_or_update_repository_from_gh(
             remote_repo.organization = self.update_or_create_organization(gh_repo.owner.login)
 
         remote_repo.save()
-        self._resync_collaborators(gh_repo, remote_repo)
+        if resync_collaborators:
+            self._resync_collaborators(gh_repo, remote_repo)
         return remote_repo
 
     # NOTE: normally, this should cache only one organization at a time, but just in case...

readthedocs/oauth/tasks.py

@@ -768,8 +768,9 @@ def _handle_organization_event(self):
         # But I wasn't able to trigger neither of those events when renaming an organization.
         # Maybe a bug?
         # If the organization is renamed, we need to sync the repositories, so they use the new name.
+        # Renaming doesn't change permissions, so we don't need to sync all collaborators.
         if action == "renamed":
-            installation.service.sync()
+            installation.service.sync(sync_all_collaborators=False)
             return
 
         if action == "deleted":

readthedocs/oauth/tests/test_githubapp_webhook.py

@@ -1010,7 +1010,7 @@ def test_organization_renamed(self, sync):
         }
         r = self.post_webhook("organization", payload)
         assert r.status_code == 200
-        sync.assert_called_once()
+        sync.assert_called_once_with(sync_all_collaborators=False)
 
     def test_organization_deleted(self):
         organization_id = 1234

readthedocs/rtd_tests/tests/test_oauth.py

@@ -740,6 +740,61 @@ def test_sync(self, request):
         assert relation.account == self.account
         assert relation.admin
 
+    @requests_mock.Mocker(kw="request")
+    def test_sync_collaborators_from_repositories_with_projects_only(self, request):
+        # ``self.remote_repository`` is linked to a project, ``repo2`` is not.
+        repo2 = get(
+            RemoteRepository,
+            remote_id="7777",
+            name="repo2",
+            full_name="user/repo2",
+            vcs_provider=GITHUB_APP,
+            github_app_installation=self.installation,
+        )
+        get(
+            RemoteRepositoryRelation,
+            remote_repository=repo2,
+            user=self.user,
+            account=self.account,
+            admin=True,
+        )
+        request.get(
+            f"{self.api_url}/app/installations/1111",
+            json=self._get_installation_json(id=1111),
+        )
+        request.post(
+            f"{self.api_url}/app/installations/1111/access_tokens",
+            json=self._get_access_token_json(),
+        )
+        request.get(
+            f"{self.api_url}/installation/repositories",
+            json={
+                "repositories": [
+                    self._get_repository_json(
+                        full_name="user/repo", id=int(self.remote_repository.remote_id)
+                    ),
+                    self._get_repository_json(
+                        full_name="user/repo2", id=7777, description="New description"
+                    ),
+                ]
+            },
+        )
+        # Only the repository linked to a project has its collaborators listed,
+        # requesting the collaborators of user/repo2 would fail the test (not mocked).
+        request.get(
+            f"{self.api_url}/repos/user/repo/collaborators",
+            json=[self._get_collaborator_json()],
+        )
+
+        service = self.installation.service
+        service.sync(sync_all_collaborators=False)
+
+        # The metadata of the repository without projects is still updated,
+        # and its existing relations are kept.
+        repo2.refresh_from_db()
+        assert repo2.description == "New description"
+        assert repo2.remote_repository_relations.count() == 1
+
     @requests_mock.Mocker(kw="request")
     def test_sync_delete_remote_repositories(self, request):
         assert self.installation.repositories.count() == 1