From 628306a54c82b14824a397a2fb69bdb6e3729a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Alix?= Date: Tue, 20 Oct 2026 14:59:45 +0200 Subject: [PATCH] [FIX] odoo_repository: handle renamed GitHub repos When syncing OCA repositories. --- .../components/oca_repository_synchronizer.py | 108 ++++++++- .../tests/test_oca_repository_synchronizer.py | 205 +++++++++++++++++- odoo_repository/utils/github.py | 53 ++++- 3 files changed, 341 insertions(+), 25 deletions(-) diff --git a/odoo_repository/components/oca_repository_synchronizer.py b/odoo_repository/components/oca_repository_synchronizer.py index b2d27a41..e0a32e2f 100644 --- a/odoo_repository/components/oca_repository_synchronizer.py +++ b/odoo_repository/components/oca_repository_synchronizer.py @@ -7,6 +7,7 @@ import yaml from odoo import Command +from odoo.exceptions import UserError from odoo.addons.component.core import Component from odoo.addons.queue_job.exception import RetryableJobError @@ -21,8 +22,10 @@ class OCARepositorySynchronizer(Component): This component is syncing the list of OCA repositories in local database from https://github.com/OCA/repo-maintainer-conf repository. - It creates new repositories, updates existing ones, and archive those - no longer declared in the OCA configuration. + It creates new repositories, updates existing ones, archives those + no longer declared in the OCA configuration, and renames those renamed + on GitHub (detected thanks to the permanent redirects GitHub keeps on + renamed repositories). """ _name = "oca.repository.synchronizer" @@ -51,6 +54,7 @@ def run(self): parses the YAML files, and synchronizes the local repository database by: - Creating new repositories that exist in OCA config but not locally - Updating existing repositories with any changes + - Renaming repositories renamed on GitHub - Archiving repositories that no longer exist in OCA config """ try: @@ -70,6 +74,11 @@ def run(self): seconds=e.retry_after, ignore_retry=True, ) from e + except UserError: + # e.g. a repository renamed on GitHub collides with an existing + # record: a manual intervention is required, fail the job + # immediately with the explicit error message, no blind retries + raise except Exception as e: raise RetryableJobError("Failed to fetch OCA repositories") from e @@ -158,9 +167,9 @@ def _sync_oca_repositories(self, current_repos): current_repos: Dict of repository data from OCA configuration Returns: - dict: Statistics about created/updated/archived repositories + dict: Statistics about created/updated/archived/renamed repositories """ - stats = {"created": 0, "updated": 0, "archived": 0} + stats = {"created": 0, "updated": 0, "archived": 0, "renamed": 0} env = self.work.env # Get OCA organization oca_org = env.ref( @@ -174,6 +183,9 @@ def _sync_oca_repositories(self, current_repos): existing_repos = odoo_repository.with_context(active_test=False).search( [("org_id", "=", oca_org.id)] ) + # Rename repositories renamed on GitHub, before any other processing, + # to avoid archiving them and creating a duplicate under their new name + existing_repos = self._rename_repositories(existing_repos, current_repos, stats) # Process current repositories for repo_name, repo_data in current_repos.items(): # Check if repository already exists @@ -199,6 +211,94 @@ def _sync_oca_repositories(self, current_repos): stats["archived"] += 1 return stats + def _rename_repositories(self, existing_repos, current_repos, stats): + """Rename local records of repositories renamed on GitHub. + + GitHub keeps a permanent redirect on renamed repositories. Each + repository missing from the OCA configuration is queried on GitHub: + if the query is redirected, the repository was renamed, and the + local record is renamed accordingly to preserve its collected + data, instead of archiving it and creating a duplicate under the + new name. + + The OCA configuration remains authoritative for the lifecycle of + a repository: a renamed record is archived by the regular sync + process until its new name is declared in the configuration, and + reactivated at that time only (see + '_prepare_oca_repository_update_vals'). + + Args: + existing_repos: Recordset of existing OCA repositories + current_repos: Dict of repository data from OCA configuration + stats: Dict of synchronization statistics to update + + Returns: + recordset: The existing repositories, without any record + deleted to merge duplicates of renamed ones + """ + env = self.work.env + renames = [] + for repo in existing_repos: + if repo.name in current_repos or not repo.active: + continue + # The repository is missing from the OCA configuration: ask + # GitHub about it. A redirected query means the repository + # was renamed (or transferred), and the returned payload + # describes the repository under its current name. + info = github.get_repo_info(env, "OCA", repo.name) + if not info: + # Repository deleted from GitHub: keep the archive process + continue + new_org, _, new_name = info.get("full_name", "").partition("/") + if not new_name or new_name == repo.name or new_org != "OCA": + # Not renamed, or transferred to another organization: + # keep the archive process + continue + renames.append((repo, new_name)) + processed = set() + for repo, new_name in sorted(renames, key=lambda item: item[0].id): + # Chain of renames: several old repositories could redirect + # to the same new name. Only the first one is renamed. + if new_name in processed: + _logger.warning( + "OCA repository %s was renamed to %s on GitHub, but " + "another repository was already renamed to that name: " + "archiving %s instead", + repo.name, + new_name, + repo.name, + ) + continue + target = ( + env["odoo.repository"] + .with_context(active_test=False) + .search([("org_id", "=", repo.org_id.id), ("name", "=", new_name)]) + ) + if target: + # A record already exists under the new name. GitHub + # guarantees both records are the same repository. + # Raise an error, it has to be handled manually to not lose data. + raise UserError( + self.env._( + "OCA repository %(old_name)s was renamed to %(new_name)s " + "on GitHub, but a record already exists under the new name.", + old_name=repo.name, + new_name=new_name, + ) + ) + repo_url = f"https://github.com/OCA/{new_name}" + _logger.info( + "OCA repository %s was renamed to %s on GitHub: renaming " + "local record (%s)", + repo.name, + new_name, + repo.id, + ) + repo.write({"name": new_name, "repo_url": repo_url, "clone_url": repo_url}) + processed.add(new_name) + stats["renamed"] += 1 + return existing_repos.exists() + def _prepare_oca_repository_update_vals(self, repo_data): return { "repo_url": repo_data["repo_url"], diff --git a/odoo_repository/tests/test_oca_repository_synchronizer.py b/odoo_repository/tests/test_oca_repository_synchronizer.py index f668bd65..979b9e72 100644 --- a/odoo_repository/tests/test_oca_repository_synchronizer.py +++ b/odoo_repository/tests/test_oca_repository_synchronizer.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch from odoo import tools +from odoo.exceptions import UserError from odoo.addons.component.tests.common import TransactionComponentCase from odoo.addons.queue_job.exception import RetryableJobError @@ -28,7 +29,16 @@ def setUpClass(cls): } ) - def _mock_requests_get(self, mock_get): + def setUp(self): + super().setUp() + patcher = patch( + "odoo.addons.odoo_repository.utils.github.get_repo_info", + return_value=None, + ) + self.mock_get_repo_info = patcher.start() + self.addCleanup(patcher.stop) + + def _patch_requests_get(self, mock_get): # Mock GitHub API response for directory listing mock_dir_response = MagicMock() mock_dir_response.json.return_value = [ @@ -60,6 +70,30 @@ def get_side_effect(url, **kwargs): mock_get.side_effect = get_side_effect + def _get_current_repos(self, names): + """Return OCA configuration data for the given repository names.""" + return { + name: { + "name": name, + "repo_url": f"https://github.com/OCA/{name}", + "clone_url": f"https://github.com/OCA/{name}", + "repo_type": "github", + } + for name in names + } + + def _mock_github_repo_info(self, info_by_name): + """Map each repository name to the GitHub API payload it should get. + + A ``KeyError`` is raised if the synchronizer queries GitHub + about a repository which is not explicitly mapped. + """ + + def get_repo_info_side_effect(env, org, name): + return info_by_name[name] + + self.mock_get_repo_info.side_effect = get_repo_info_side_effect + def test_parse_oca_repo_configurations(self): """Test OCA repository configurations parsing. @@ -149,14 +183,7 @@ def test_sync_oca_repositories_create(self): mca_backend = self.env.ref("odoo_repository.mca_backend") with mca_backend.work_on("odoo.repository") as work: synchronizer = work.component(usage="oca.repository.synchronizer") - current_repos = { - "new-test-repo": { - "name": "new-test-repo", - "repo_url": "https://github.com/OCA/new-test-repo", - "clone_url": "https://github.com/OCA/new-test-repo", - "repo_type": "github", - } - } + current_repos = self._get_current_repos(["new-test-repo"]) stats = synchronizer._sync_oca_repositories(current_repos) self.assertEqual(stats["created"], 1) # Check that repository was created @@ -188,11 +215,165 @@ def test_sync_oca_repositories_update(self): self.test_repo.repo_url, "https://github.com/OCA/test-archive-repo-updated" ) self.assertTrue(self.test_repo.active) + # The repository is still declared in the OCA configuration: + # GitHub must not have been queried about it + self.mock_get_repo_info.assert_not_called() + + def _get_github_renamed_repo_info(self): + """Return GitHub API payload for a repository renamed on GitHub.""" + return { + "name": "test-renamed-repo", + "full_name": "OCA/test-renamed-repo", + } + + def test_sync_oca_repositories_rename(self): + """Test that a repository renamed on GitHub is renamed locally. + + GitHub redirects the query of a renamed repository to its new + name: the local record is renamed in place instead of being + archived and duplicated under the new name. + """ + self._mock_github_repo_info( + {"test-archive-repo": self._get_github_renamed_repo_info()} + ) + mca_backend = self.env.ref("odoo_repository.mca_backend") + with mca_backend.work_on("odoo.repository") as work: + synchronizer = work.component(usage="oca.repository.synchronizer") + current_repos = self._get_current_repos(["test-renamed-repo"]) + stats = synchronizer._sync_oca_repositories(current_repos) + self.assertEqual(stats["renamed"], 1) + self.assertEqual(stats["created"], 0) + self.assertEqual(stats["updated"], 0) + self.assertEqual(stats["archived"], 0) + self.test_repo.invalidate_model() + # The existing record was renamed in place, not archived + self.assertEqual(self.test_repo.name, "test-renamed-repo") + self.assertEqual( + self.test_repo.repo_url, "https://github.com/OCA/test-renamed-repo" + ) + self.assertEqual( + self.test_repo.clone_url, "https://github.com/OCA/test-renamed-repo" + ) + self.assertTrue(self.test_repo.active) + self.assertTrue(self.test_repo.to_scan) + # No duplicate left under the old name + self.assertFalse( + self.repository_model.with_context(active_test=False).search( + [("name", "=", "test-archive-repo")] + ) + ) + + def test_sync_oca_repositories_rename_not_in_config(self): + """Test rename of a repository not declared (yet) in the OCA config. + + GitHub gives the new identity, but the OCA configuration remains + authoritative for the lifecycle: the record is renamed then + archived, and reactivated as soon as the OCA configuration + declares its new name. + """ + self._mock_github_repo_info( + {"test-archive-repo": self._get_github_renamed_repo_info()} + ) + mca_backend = self.env.ref("odoo_repository.mca_backend") + with mca_backend.work_on("odoo.repository") as work: + synchronizer = work.component(usage="oca.repository.synchronizer") + # First sync: the new name is not (yet) in the OCA configuration + stats = synchronizer._sync_oca_repositories({}) + self.assertEqual(stats["renamed"], 1) + self.assertEqual(stats["created"], 0) + self.assertEqual(stats["archived"], 1) + self.test_repo.invalidate_model() + self.assertEqual(self.test_repo.name, "test-renamed-repo") + self.assertFalse(self.test_repo.active) + self.assertFalse(self.test_repo.to_scan) + # Second sync: the new name is now declared + self.mock_get_repo_info.reset_mock() + current_repos = self._get_current_repos(["test-renamed-repo"]) + stats = synchronizer._sync_oca_repositories(current_repos) + self.assertEqual(stats["renamed"], 0) + self.assertEqual(stats["created"], 0) + self.assertEqual(stats["updated"], 1) + self.test_repo.invalidate_model() + self.assertTrue(self.test_repo.active) + # The reactivation of the record must not query GitHub + self.mock_get_repo_info.assert_not_called() + + def test_sync_oca_repositories_rename_duplicate(self): + """Test rename when a duplicate record exists under the new name. + + GitHub guarantees both records are the same repository: the + sync is then aborted (error raised). + """ + self._mock_github_repo_info( + {"test-archive-repo": self._get_github_renamed_repo_info()} + ) + # Duplicate created by a previous sync under the new name + duplicate = self.repository_model.create( + { + "org_id": self.oca_org.id, + "name": "test-renamed-repo", + "repo_url": "https://github.com/OCA/test-renamed-repo", + "clone_url": "https://github.com/OCA/test-renamed-repo", + "repo_type": "github", + } + ) + mca_backend = self.env.ref("odoo_repository.mca_backend") + with mca_backend.work_on("odoo.repository") as work: + synchronizer = work.component(usage="oca.repository.synchronizer") + current_repos = self._get_current_repos(["test-renamed-repo"]) + with self.assertRaises(UserError): + synchronizer._sync_oca_repositories(current_repos) + # The duplicate is kept + self.assertTrue(duplicate.exists()) + + def test_sync_oca_repositories_transfer(self): + """Test that a repository transferred to another organization is archived.""" + self._mock_github_repo_info( + { + "test-archive-repo": { + "name": "test-renamed-repo", + "full_name": "OtherOrg/test-renamed-repo", + } + } + ) + mca_backend = self.env.ref("odoo_repository.mca_backend") + with mca_backend.work_on("odoo.repository") as work: + synchronizer = work.component(usage="oca.repository.synchronizer") + stats = synchronizer._sync_oca_repositories({}) + self.assertEqual(stats["renamed"], 0) + self.assertEqual(stats["archived"], 1) + self.test_repo.invalidate_model() + self.assertEqual(self.test_repo.name, "test-archive-repo") + self.assertFalse(self.test_repo.active) + + def test_sync_oca_repositories_removed(self): + """Test that a repository still existing on GitHub is archived. + + The repository was removed from the OCA configuration but not + on GitHub: the regular archive process applies. + """ + self._mock_github_repo_info( + { + "test-archive-repo": { + "name": "test-archive-repo", + "full_name": "OCA/test-archive-repo", + } + } + ) + mca_backend = self.env.ref("odoo_repository.mca_backend") + with mca_backend.work_on("odoo.repository") as work: + synchronizer = work.component(usage="oca.repository.synchronizer") + stats = synchronizer._sync_oca_repositories({}) + self.assertEqual(stats["renamed"], 0) + self.assertEqual(stats["archived"], 1) + self.test_repo.invalidate_model() + self.assertEqual(self.test_repo.name, "test-archive-repo") + self.assertFalse(self.test_repo.active) @patch("requests.get") def test_fetch_oca_repo_configurations(self, mock_get): """Test fetching repository configuration from GitHub.""" - self._mock_requests_get(mock_get) + self._patch_requests_get(mock_get) mca_backend = self.env.ref("odoo_repository.mca_backend") with mca_backend.work_on("odoo.repository") as work: synchronizer = work.component(usage="oca.repository.synchronizer") @@ -203,7 +384,7 @@ def test_fetch_oca_repo_configurations(self, mock_get): @patch("requests.get") def test_run_oca_repository_synchronizer(self, mock_get): """Test running the repository synchronizer component (main entrypoint).""" - self._mock_requests_get(mock_get) + self._patch_requests_get(mock_get) mca_backend = self.env.ref("odoo_repository.mca_backend") with mca_backend.work_on("odoo.repository") as work: synchronizer = work.component(usage="oca.repository.synchronizer") @@ -226,7 +407,7 @@ def test_run_oca_repository_synchronizer_error(self): @patch("requests.get") def test_cron_sync_oca_repositories(self, mock_get): """Test cron method syncing OCA repositories from GitHub.""" - self._mock_requests_get(mock_get) + self._patch_requests_get(mock_get) result = self.repository_model.cron_sync_oca_repositories() self.assertTrue(result) test_repo = self.repository_model.search([("name", "=", "test-repo")]) diff --git a/odoo_repository/utils/github.py b/odoo_repository/utils/github.py index e8f3f003..96dd60e6 100644 --- a/odoo_repository/utils/github.py +++ b/odoo_repository/utils/github.py @@ -8,6 +8,9 @@ GITHUB_URL = "https://github.com" GITHUB_API_URL = "https://api.github.com" +# Timeout in seconds applied to every request to the GitHub API, to +# avoid jobs blocked forever on a hanging connection +REQUEST_TIMEOUT = 30 class GitHubRateLimitError(RuntimeError): @@ -36,8 +39,8 @@ def _get_retry_after(response): return None -def request(env, url, method="get", params=None, json=None): - """Request GitHub API.""" +def _get_headers(env): + """Return HTTP headers to request GitHub API.""" headers = {"Accept": "application/vnd.github.groot-preview+json"} key = "odoo_repository_github_token" token = env["ir.config_parameter"].sudo().get_param(key, "") or os.environ.get( @@ -45,16 +48,48 @@ def request(env, url, method="get", params=None, json=None): ) if token: headers.update({"Authorization": f"token {token}"}) - full_url = "/".join([GITHUB_API_URL, url]) - kwargs = {"headers": headers} - if json: - kwargs.update(json=json) - if params: - kwargs.update(params=params) - response = getattr(requests, method)(full_url, **kwargs) + return headers + + +def _check_response(response): + """Return the JSON payload of a GitHub API response. + + Raise on error, taking care of GitHub rate limits. + """ if not response.ok: retry_after = _get_retry_after(response) if retry_after: raise GitHubRateLimitError(response.text, retry_after) raise RuntimeError(response.text) return response.json() + + +def request(env, url, method="get", params=None, json=None, timeout=REQUEST_TIMEOUT): + """Request GitHub API.""" + full_url = f"{GITHUB_API_URL}/{url}" + kwargs = {"headers": _get_headers(env), "timeout": timeout} + if json: + kwargs.update(json=json) + if params: + kwargs.update(params=params) + response = getattr(requests, method)(full_url, **kwargs) + return _check_response(response) + + +def get_repo_info(env, org, name): + """Return information about a GitHub repository, or None if not found. + + If the repository was renamed, GitHub redirects the query to the + current repository: the returned payload describes it under its + current name ('full_name'). + Return ``None`` if the repository does not exist (HTTP 404). + Raise on any other error. + """ + full_url = f"{GITHUB_API_URL}/repos/{org}/{name}" + response = requests.get( + full_url, headers=_get_headers(env), timeout=REQUEST_TIMEOUT + ) + if response.status_code == 404: + # Repository deleted from GitHub + return None + return _check_response(response)