diff --git a/odoo_repository/models/odoo_module_branch.py b/odoo_repository/models/odoo_module_branch.py index ba39bff..2229d0f 100644 --- a/odoo_repository/models/odoo_module_branch.py +++ b/odoo_repository/models/odoo_module_branch.py @@ -294,7 +294,7 @@ def _compute_dependency_level(self): (non_std_max_parent_level + 1) if not rec.is_standard else 0 ) - def _get_recursive_dependencies(self, domain=None): + def _get_recursive_dependencies(self, domain=None, _visited=None): """Return all dependencies recursively. A domain can be applied to restrict the modules to return, e.g: @@ -302,13 +302,22 @@ def _get_recursive_dependencies(self, domain=None): >>> mod._get_recursive_dependencies([("org_id", "=", "OCA")]) """ + # NOTE: Circular dependencies are allowed if not domain: domain = [] - dependencies = self.dependency_ids.filtered_domain(domain) + if _visited is None: + _visited = set() + if self.id in _visited: + return self.browse() + _visited.add(self.id) + # Apply domain and exclude self + dependencies = (self.dependency_ids - self).filtered_domain(domain) dep_ids = set(dependencies.ids) for dep in dependencies: dep_ids |= set( - dep._get_recursive_dependencies().filtered_domain(domain).ids + dep._get_recursive_dependencies(domain, _visited) + .filtered_domain(domain) + .ids ) return self.browse(dep_ids) diff --git a/odoo_repository/tests/__init__.py b/odoo_repository/tests/__init__.py index 8f3f0b6..ee060d5 100644 --- a/odoo_repository/tests/__init__.py +++ b/odoo_repository/tests/__init__.py @@ -5,3 +5,4 @@ from . import test_sync_node from . import test_odoo_module_branch from . import test_oca_repository_synchronizer +from . import test_odoo_module_branch_recursive_dependencies diff --git a/odoo_repository/tests/common.py b/odoo_repository/tests/common.py index c0cc1ac..ccadbe4 100644 --- a/odoo_repository/tests/common.py +++ b/odoo_repository/tests/common.py @@ -3,9 +3,9 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) import logging -import os import pathlib import re +import shutil import tempfile from unittest.mock import patch @@ -28,36 +28,36 @@ def setUpClass(cls): cls.env["ir.config_parameter"].set_param( "odoo_repository_storage_path", cls.repositories_path ) - cls._apply_git_config() - cls._handle_cleanup() - - def setUp(self): - super().setUp() - self.repo_name = pathlib.Path(self.repo_upstream_path).parts[-1] - self.org = self.env["odoo.repository.org"].create({"name": self.fork_org}) - self.odoo_repository = self.env["odoo.repository"].create( + # org and repository + cls.repo_name = cls.repo_upstream_path.parts[-1] + cls.org = cls.env["odoo.repository.org"].create({"name": cls.fork_org}) + cls.odoo_repository = cls.env["odoo.repository"].create( { - "org_id": self.org.id, - "name": self.repo_name, - "repo_url": self.repo_upstream_path, - "clone_url": self.repo_upstream_path, + "org_id": cls.org.id, + "name": cls.repo_name, + "repo_url": cls.repo_upstream_path, + "clone_url": cls.repo_upstream_path, "repo_type": "github", } ) # branch1 - self.branch1_name = self.source1.split("/")[1] - self.branch = ( - self.env["odoo.branch"] + cls.branch1_name = cls.source1.split("/")[1] + cls.branch = ( + cls.env["odoo.branch"] .with_context(active_test=False) - .search([("name", "=", self.branch1_name)]) + .search([("name", "=", cls.branch1_name)]) ) - if not self.branch: - self.branch = self.env["odoo.branch"].create( + if not cls.branch: + cls.branch = cls.env["odoo.branch"].create( { - "name": self.branch1_name, + "name": cls.branch1_name, } ) - self.branch.active = True + cls.branch.active = True + cls._handle_cleanup() + + def setUp(self): + super().setUp() # branch2 self.branch2_name = self.source2.split("/")[1] self.branch2 = ( @@ -78,15 +78,6 @@ def setUp(self): self.module_name = self.addon self.module_branch_model = self.env["odoo.module.branch"] - @classmethod - def _apply_git_config(cls): - """Configure git (~/.gitconfig) if no config file exists.""" - git_cfg = pathlib.Path(os.path.expanduser("~/.gitconfig")) - if git_cfg.exists(): - return - os.system("git config --global user.email 'test@example.com'") - os.system("git config --global user.name 'test'") - def _patch_github_class(self): # Patch helper method part of 'odoo_repository' module self.patcher = patch("odoo.addons.odoo_repository.utils.github.request") @@ -145,24 +136,27 @@ def _run_odoo_repository_action_scan(self, branch_id, force=False): branch_ids=[branch_id], force=force ) - def _create_odoo_module(self, name): - return self.env["odoo.module"].create({"name": name}) + @classmethod + def _create_odoo_module(cls, name): + return cls.env["odoo.module"].create({"name": name}) - def _create_odoo_repository_branch(self, repo, branch, **values): + @classmethod + def _create_odoo_repository_branch(cls, repo, branch, **values): vals = { "repository_id": repo.id, "branch_id": branch.id, } vals.update(values) - return self.env["odoo.repository.branch"].create(vals) + return cls.env["odoo.repository.branch"].create(vals) - def _create_odoo_module_branch(self, module, branch, **values): + @classmethod + def _create_odoo_module_branch(cls, module, branch, **values): vals = { "module_id": module.id, "branch_id": branch.id, } vals.update(values) - return self.env["odoo.module.branch"].create(vals) + return cls.env["odoo.module.branch"].create(vals) @classmethod def _handle_cleanup(cls): @@ -187,3 +181,14 @@ def kill_remaining_git_processes(): psutil.wait_procs(children, timeout=10) cls.addClassCleanup(kill_remaining_git_processes) + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + shutil.rmtree(cls.repositories_path) + + def tearDown(self): + super().tearDown() + repositories_path = pathlib.Path(self.repositories_path) + for sub_path in repositories_path.iterdir(): + shutil.rmtree(sub_path) diff --git a/odoo_repository/tests/odoo_repo_mixin.py b/odoo_repository/tests/odoo_repo_mixin.py index b9cd7bc..3fad199 100644 --- a/odoo_repository/tests/odoo_repo_mixin.py +++ b/odoo_repository/tests/odoo_repo_mixin.py @@ -2,6 +2,7 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) import io +import os import shutil import tempfile import threading @@ -29,34 +30,40 @@ def setUpClass(cls): cls.target3 = "origin/18.0" cls.addon = "my_module" cls.target_addon = "my_module_renamed" - - def setUp(self): - super().setUp() # Create a temporary Git repository - self.repo_upstream_path = self._get_upstream_repository_path() - self.addon_path = Path(self.repo_upstream_path) / self.addon - self.manifest_path = self.addon_path / "__manifest__.py" - # By cloning the first repository this will set an 'origin' remote - self.repo_path = self._clone_tmp_git_repository(self.repo_upstream_path) - self._add_fork_remote(self.repo_path) - - def _get_upstream_repository_path(self) -> Path: + cls._apply_git_config() + cls.repo_upstream_path = cls._get_upstream_repository_path() + cls.addon_path = Path(cls.repo_upstream_path) / cls.addon + cls.manifest_path = cls.addon_path / "__manifest__.py" + + @classmethod + def _apply_git_config(cls): + """Configure git (~/.gitconfig) if no config file exists.""" + git_cfg = Path(os.path.expanduser("~/.gitconfig")) + if git_cfg.exists(): + return + os.system("git config --global user.email 'test@example.com'") + os.system("git config --global user.name 'test'") + + @classmethod + def _get_upstream_repository_path(cls) -> Path: """Returns the path of upstream repository. Generate the upstream git repository or re-use the one put in cache if any. """ if hasattr(cache, "archive_data") and cache.archive_data: # Unarchive the repository from memory - repo_path = self._unarchive_upstream_repository(cache.archive_data) + repo_path = cls._unarchive_upstream_repository(cache.archive_data) else: # Prepare and archive the repository in memory - repo_path = self._create_tmp_git_repository() - addon_path = repo_path / self.addon - self._fill_git_repository(repo_path, addon_path) - cache.archive_data = self._archive_upstream_repository(repo_path) + repo_path = cls._create_tmp_git_repository() + addon_path = repo_path / cls.addon + cls._fill_git_repository(repo_path, addon_path) + cache.archive_data = cls._archive_upstream_repository(repo_path) return repo_path - def _archive_upstream_repository(self, repo_path: Path) -> bytes: + @classmethod + def _archive_upstream_repository(cls, repo_path: Path) -> bytes: """Archive the repository located at `repo_path`. Returns binary value of the archive. @@ -70,7 +77,8 @@ def _archive_upstream_repository(self, repo_path: Path) -> bytes: zipf.write(file_path, arcname) return zip_buffer.getvalue() - def _unarchive_upstream_repository(self, archive_data: bytes) -> Path: + @classmethod + def _unarchive_upstream_repository(cls, archive_data: bytes) -> Path: """Unarchive the repository contained in `archive_data`. Returns path of repository. @@ -83,28 +91,25 @@ def _unarchive_upstream_repository(self, archive_data: bytes) -> Path: if path.is_dir() and ".git" in path.name: return path.parent - def _create_tmp_git_repository(self) -> Path: + @classmethod + def _create_tmp_git_repository(cls) -> Path: """Create a temporary Git repository to run tests.""" repo_path = tempfile.mkdtemp() git.Repo.init(repo_path) return Path(repo_path) - def _clone_tmp_git_repository(self, upstream_path: Path) -> Path: - repo_path = tempfile.mkdtemp() - git.Repo.clone_from(upstream_path, repo_path) - return Path(repo_path) - - def _fill_git_repository(self, repo_path: Path, addon_path: Path): + @classmethod + def _fill_git_repository(cls, repo_path: Path, addon_path: Path): """Create branches with some content in the Git repository.""" repo = git.Repo(repo_path) # Commit a file in '15.0' - branch1 = self.source1.split("/")[1] + branch1 = cls.source1.split("/")[1] repo.git.checkout("--orphan", branch1) - self._create_module(addon_path) + cls._create_module(addon_path) repo.index.add(addon_path) - commit = repo.index.commit(f"[ADD] {self.addon}") + commit = repo.index.commit(f"[ADD] {cls.addon}") # Port the commit from 15.0 to 16.0 - branch2 = self.source2.split("/")[1] + branch2 = cls.source2.split("/")[1] repo.git.checkout("--orphan", branch2) repo.git.reset("--hard") # Some git operations do not appear to be atomic, so a delay is added @@ -112,21 +117,22 @@ def _fill_git_repository(self, repo_path: Path, addon_path: Path): time.sleep(1) repo.git.cherry_pick(commit.hexsha) # Create an empty branch 17.0 - branch3 = self.target2.split("/")[1] + branch3 = cls.target2.split("/")[1] repo.git.checkout("--orphan", branch3) repo.git.reset("--hard") repo.git.commit("-m", "Init", "--allow-empty") # Port the commit from 15.0 to 18.0 - branch4 = self.target3.split("/")[1] + branch4 = cls.target3.split("/")[1] repo.git.checkout("--orphan", branch4) repo.git.reset("--hard") time.sleep(1) repo.git.cherry_pick(commit.hexsha) # Rename the module on 18.0 - repo.git.mv(self.addon, self.target_addon) - repo.git.commit("-m", f"Rename {self.addon} to {self.target_addon}") + repo.git.mv(cls.addon, cls.target_addon) + repo.git.commit("-m", f"Rename {cls.addon} to {cls.target_addon}") - def _create_module(self, module_path: Path): + @classmethod + def _create_module(cls, module_path: Path): manifest_lines = [ "# Copyright 2026 Sébastien Alix\n", "# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)\n", @@ -148,13 +154,8 @@ def _create_module(self, module_path: Path): with open(manifest_path, "w") as manifest: manifest.writelines(manifest_lines) - def _add_fork_remote(self, repo_path: Path): - repo = git.Repo(repo_path) - # We do not really care about the remote URL here, re-use origin one - repo.create_remote(self.fork_org, repo.remotes.origin.url) - - def tearDown(self): - super().tearDown() - # Clean up the Git repository - shutil.rmtree(self.repo_upstream_path) - shutil.rmtree(self.repo_path) + @classmethod + def tearDownClass(cls): + super().tearDownClass() + # Clean up upstream Git repository + shutil.rmtree(cls.repo_upstream_path) diff --git a/odoo_repository/tests/test_odoo_module_branch_recursive_dependencies.py b/odoo_repository/tests/test_odoo_module_branch_recursive_dependencies.py new file mode 100644 index 0000000..f6b3094 --- /dev/null +++ b/odoo_repository/tests/test_odoo_module_branch_recursive_dependencies.py @@ -0,0 +1,203 @@ +# Copyright 2026 Sébastien Alix +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +from .common import Common + + +class TestOdooModuleBranchRecursiveDependencies(Common): + @classmethod + def setUpClass(cls): + super().setUpClass() + # Create modules + cls.mod_base = cls._create_odoo_module("base") + cls.mod_a = cls._create_odoo_module("module_a") + cls.mod_b = cls._create_odoo_module("module_b") + cls.mod_c = cls._create_odoo_module("module_c") + cls.mod_d = cls._create_odoo_module("module_d") + cls.mod_e = cls._create_odoo_module("module_e") + # Create module branches + cls.mod_base_branch = cls._create_odoo_module_branch( + cls.mod_base, cls.branch, is_standard=True + ) + cls.mod_a_branch = cls._create_odoo_module_branch(cls.mod_a, cls.branch) + cls.mod_b_branch = cls._create_odoo_module_branch(cls.mod_b, cls.branch) + cls.mod_c_branch = cls._create_odoo_module_branch(cls.mod_c, cls.branch) + cls.mod_d_branch = cls._create_odoo_module_branch(cls.mod_d, cls.branch) + cls.mod_e_branch = cls._create_odoo_module_branch(cls.mod_e, cls.branch) + + def test_get_recursive_dependencies_simple(self): + """Test _get_recursive_dependencies with a simple dependency chain.""" + self.mod_a_branch.dependency_ids = self.mod_base_branch + self.mod_b_branch.dependency_ids = self.mod_a_branch + self.mod_c_branch.dependency_ids = self.mod_b_branch + # Test recursive dependencies for module_c + deps = self.mod_c_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps) + self.assertIn(self.mod_a_branch, deps) + self.assertIn(self.mod_b_branch, deps) + self.assertEqual(len(deps), 3) # base, module_a, module_b + + def test_get_recursive_dependencies_with_domain(self): + """Test _get_recursive_dependencies with domain filtering.""" + # Create organization + org_oca = self.env.ref("odoo_repository.odoo_repository_org_oca") + org_other = self.env["odoo.repository.org"].create({"name": "Other"}) + # Create repositories + repo_oca = self.env["odoo.repository"].create( + { + "org_id": org_oca.id, + "name": "repo_oca", + "repo_url": "https://github.com/OCA/repo_oca", + "clone_url": "https://github.com/OCA/repo_oca", + "repo_type": "github", + } + ) + repo_other = self.env["odoo.repository"].create( + { + "org_id": org_other.id, + "name": "repo_other", + "repo_url": "https://github.com/Other/repo_other", + "clone_url": "https://github.com/Other/repo_other", + "repo_type": "github", + } + ) + # Create modules + mod_oca = self._create_odoo_module("module_oca") + mod_other = self._create_odoo_module("module_other") + mod_mixed = self._create_odoo_module("module_mixed") + # Create repository branches + repo_oca_branch = self._create_odoo_repository_branch(repo_oca, self.branch) + repo_other_branch = self._create_odoo_repository_branch(repo_other, self.branch) + # Create module branches + mod_oca_branch = self._create_odoo_module_branch( + mod_oca, + self.branch, + repository_branch_id=repo_oca_branch.id, + dependency_ids=[(4, self.mod_base_branch.id)], + ) + mod_other_branch = self._create_odoo_module_branch( + mod_other, + self.branch, + repository_branch_id=repo_other_branch.id, + dependency_ids=[(4, self.mod_base_branch.id)], + ) + mod_mixed_branch = self._create_odoo_module_branch( + mod_mixed, + self.branch, + repository_branch_id=repo_oca_branch.id, + dependency_ids=[(4, mod_oca_branch.id), (4, mod_other_branch.id)], + ) + # Test recursive dependencies with OCA filter + deps = mod_mixed_branch._get_recursive_dependencies( + [("org_id", "=", org_oca.id)] + ) + self.assertIn(mod_oca_branch, deps) + self.assertNotIn(mod_other_branch, deps) # filtered out + # base has no org, so it's filtered out by the domain + self.assertNotIn(self.mod_base_branch, deps) + self.assertEqual(len(deps), 1) # only module_oca + + def test_get_recursive_dependencies_circular_1(self): + """Test _get_recursive_dependencies handles circular dependencies (1).""" + # Create module branches with circular dependencies + # base + # ├── a ─┐ + # ├── b │ + # ├── c │ + # └── d <┘ + self.mod_a_branch.dependency_ids = self.mod_base_branch + self.mod_b_branch.dependency_ids = self.mod_a_branch + self.mod_c_branch.dependency_ids = self.mod_b_branch + self.mod_d_branch.dependency_ids = self.mod_c_branch + # Create circular dependency: mod_a -> mod_d + self.mod_a_branch.dependency_ids |= self.mod_d_branch + # Test recursive dependencies - should not infinite loop + deps = self.mod_b_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps) + self.assertIn(self.mod_a_branch, deps) + self.assertIn(self.mod_b_branch, deps) + self.assertIn(self.mod_c_branch, deps) + self.assertIn(self.mod_d_branch, deps) + # Check that we get the expected dependencies + # We expect base, a, b, c, d (mod_b is part from its own dependencies) + self.assertEqual(len(deps), 5) + + def test_get_recursive_dependencies_circular_2(self): + """Test _get_recursive_dependencies handles circular dependencies (2).""" + # Create module branches with circular dependencies + # base + # ├── a ─┐ + # ├── b │ + # ├── c <┘ + # └── d + self.mod_a_branch.dependency_ids = self.mod_base_branch + self.mod_b_branch.dependency_ids = self.mod_a_branch + self.mod_c_branch.dependency_ids = self.mod_b_branch + self.mod_d_branch.dependency_ids = self.mod_c_branch + # Create circular dependency: mod_a -> mod_d + self.mod_a_branch.dependency_ids |= self.mod_c_branch + # Test recursive dependencies - should not infinite loop + deps = self.mod_d_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps) + self.assertIn(self.mod_a_branch, deps) + self.assertIn(self.mod_b_branch, deps) + self.assertIn(self.mod_c_branch, deps) + self.assertNotIn(self.mod_d_branch, deps) + # Check that we get the expected dependencies + # We expect base, a, b, c (mod_d is excluded from its own dependencies) + self.assertEqual(len(deps), 4) + + def test_get_recursive_dependencies_complex_tree(self): + """Test _get_recursive_dependencies with complex dependency tree.""" + # Create module branches with complex dependencies: + # base + # ├── a + # │ ├── c + # │ └── d + # └── b + # └── e + self.mod_a_branch.dependency_ids = self.mod_base_branch + self.mod_b_branch.dependency_ids = self.mod_base_branch + self.mod_c_branch.dependency_ids = self.mod_a_branch + self.mod_d_branch.dependency_ids = self.mod_a_branch + self.mod_e_branch.dependency_ids = self.mod_b_branch + # Test from module_c + deps_c = self.mod_c_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps_c) + self.assertIn(self.mod_a_branch, deps_c) + self.assertNotIn(self.mod_b_branch, deps_c) + self.assertNotIn(self.mod_c_branch, deps_c) + self.assertNotIn(self.mod_d_branch, deps_c) + self.assertNotIn(self.mod_e_branch, deps_c) + self.assertEqual(len(deps_c), 2) # base, module_a + # Test from module_e + deps_e = self.mod_e_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps_e) + self.assertIn(self.mod_b_branch, deps_e) + self.assertNotIn(self.mod_a_branch, deps_e) + self.assertNotIn(self.mod_c_branch, deps_e) + self.assertNotIn(self.mod_d_branch, deps_e) + self.assertEqual(len(deps_e), 2) # base, module_b + + def test_get_recursive_dependencies_empty(self): + """Test _get_recursive_dependencies with no dependencies.""" + # Create module with no dependencies + mod_alone = self._create_odoo_module("module_alone") + mod_alone_branch = self._create_odoo_module_branch(mod_alone, self.branch) + # Test recursive dependencies + deps = mod_alone_branch._get_recursive_dependencies() + self.assertEqual(len(deps), 0) + + def test_get_recursive_dependencies_self_exclusion(self): + """Test that _get_recursive_dependencies excludes self.""" + # Create modules + mod_self = self._create_odoo_module("module_self") + # Create module branches + mod_self_branch = self._create_odoo_module_branch( + mod_self, self.branch, dependency_ids=[(4, self.mod_base_branch.id)] + ) + # Test recursive dependencies - self should not be included + deps = mod_self_branch._get_recursive_dependencies() + self.assertIn(self.mod_base_branch, deps) + self.assertNotIn(mod_self_branch, deps) + self.assertEqual(len(deps), 1)