[FIX] databases: neutralize with the custom addons of the database

odev/_version.py

@@ -22,4 +22,4 @@
 # or merged change.
 # ------------------------------------------------------------------------------
 
-__version__ = "4.30.2"
+__version__ = "4.30.3"

odev/commands/database/quickstart.py

@@ -64,16 +64,31 @@ def run(self):
                 raise self.error(f"Database {self._database.name!r} could not be restored")
 
             new_database = LocalDatabase(self.args.name or self._database.name)
+
+            # Link the repository before restoring: `restore` neutralizes the database, and
+            # neutralization looks for `data/neutralize.sql` scripts in the custom modules found
+            # under the addons paths derived from the repository linked to the database.
+            self.link_repository(new_database)
             self.odev.run_command("restore", dump_file.as_posix(), database=new_database)
 
-            if self._database.repository:
-                if isinstance(self._database.repository, Repository):
-                    repo_org = self._database.repository.organization
-                    repo_name = self._database.repository.name
-                else:
-                    repo_org, repo_name = self._database.repository.name.split("/")
+            # `restore` drops and recreates the database, clearing its entry in the data store.
+            self.link_repository(new_database)
+
+    def link_repository(self, database: LocalDatabase) -> None:
+        """Link the repository of the source database to the target local database.
+
+        :param database: The database to link the repository to.
+        """
+        if not self._database.repository:
+            return
+
+        if isinstance(self._database.repository, Repository):
+            repo_org = self._database.repository.organization
+            repo_name = self._database.repository.name
+        else:
+            repo_org, repo_name = self._database.repository.name.split("/")
 
-                new_database.repository = Repository(repo_name, repo_org)
+        database.repository = Repository(repo_name, repo_org)
 
     def get_dump_filename_kwargs(self) -> MutableMapping[str, Any]:
         """Return the keyword arguments to pass to Database.get_dump_filename()."""

odev/common/commands/odoobin.py

@@ -178,14 +178,7 @@ def _guess_addons_paths(self) -> list[Path]:
 
     def _set_addons_paths(self) -> None:
         """Find additional addons paths from the database repository if any."""
-        addons_paths = self._guess_addons_paths()
-
-        globs = [path.glob(f"**/__{manifest}__.py") for path in addons_paths for manifest in ["manifest", "openerp"]]
-        addons_paths = [
-            path.parents[1] for path in (p for g in globs for p in g) if self.odoobin.check_addons_path(path.parents[1])
-        ]
-
-        self.odoobin.additional_addons_paths = sorted(set(addons_paths))
+        self.odoobin.additional_addons_paths = self.odoobin.expand_addons_paths(self._guess_addons_paths())
         self.odoobin.save_database_repository()
 
     def _set_odoobin_process(self, force=False) -> None:

odev/common/databases/local.py

@@ -6,7 +6,7 @@
 import re
 import shutil
 import tempfile
-from collections.abc import Generator, Mapping
+from collections.abc import Generator, Mapping, MutableMapping
 from datetime import datetime
 from functools import cached_property
 from pathlib import Path
@@ -493,12 +493,13 @@ def drop(self) -> bool:
     def neutralize(self):
         """Neutralize the database."""
         max_retries = 5
+        retries = 0
 
         with self.connector.nocache():
             # Artificially wait for SQL transaction to be committed and for the process to be ready
             # before running the neutralize command
             # This is not clean but it works, I guess
-            while (not self.process or not self.version) and (retries := 0) < max_retries:
+            while (not self.process or not self.version) and retries < max_retries:
                 retries += 1
                 sleep(0.2)
 
@@ -525,24 +526,7 @@ def neutralize(self):
             self.process.run(["-d", self.name], subcommand="neutralize")
             self.console.print()
 
-        installed_modules: set[str] = set(self.installed_modules) & {
-            path.name for path in self.process.additional_addons_paths
-        }
-        scripts: list[Path] = [self.odev.static_path / "neutralize-pre.sql"]
-
-        with progress.spinner(f"Looking up neutralization scripts in {len(installed_modules)} installed modules"):
-            for addon in self.process.additional_addons_paths:
-                for module in installed_modules:
-                    neutralize_path: Path = addon / module / "data" / "neutralize.sql"
-
-                    if neutralize_path.is_file():
-                        scripts.append(neutralize_path)
-
-        scripts.append(self.odev.static_path / "neutralize-post.sql")
-
-        if self.version < NEUTRALIZE_BEFORE_ODOO_VERSION:
-            scripts.append(self.odev.static_path / "neutralize-post-before-15.0.sql")
-
+        scripts = self._neutralize_scripts()
         tracker = progress.Progress()
 
         task = tracker.add_task(f"Running {len(scripts)} neutralization scripts", total=len(scripts))
@@ -554,6 +538,38 @@ def neutralize(self):
 
         tracker.stop()
 
+    def _neutralize_scripts(self) -> list[Path]:
+        """Return the neutralization scripts to run, including those shipped by the custom modules
+        installed in the database.
+
+        :return: The paths to the neutralization scripts, in the order they must be run.
+        :rtype: List[Path]
+        """
+        addons_paths = self.process.additional_addons_paths if self.process else []
+        modules: MutableMapping[str, Path] = {
+            module.name: module
+            for path in addons_paths
+            if path.is_dir()
+            for module in path.iterdir()
+            if module.is_dir()
+        }
+        installed_modules: set[str] = set(self.installed_modules) & modules.keys()
+        scripts: list[Path] = [self.odev.static_path / "neutralize-pre.sql"]
+
+        with progress.spinner(f"Looking up neutralization scripts in {len(installed_modules)} installed modules"):
+            scripts.extend(
+                script
+                for module in sorted(installed_modules)
+                if (script := modules[module] / "data" / "neutralize.sql").is_file()
+            )
+
+        scripts.append(self.odev.static_path / "neutralize-post.sql")
+
+        if self.version < NEUTRALIZE_BEFORE_ODOO_VERSION:
+            scripts.append(self.odev.static_path / "neutralize-post-before-15.0.sql")
+
+        return scripts
+
     def dump(self, filestore: bool = False, path: Path | None = None) -> Path:
         if path is None:
             path = self.odev.dumps_path

odev/common/odoobin.py

@@ -167,8 +167,10 @@ def __init__(
         self.repository: GitConnector = GitConnector("odoo/odoo")
         """Github repository of Odoo."""
 
-        self._additional_addons_paths: list[Path] = []
-        """List of additional addons paths to use when starting the Odoo process."""
+        self._additional_addons_paths: list[Path] | None = None
+        """List of additional addons paths to use when starting the Odoo process.
+        None until derived from the repository linked to the database, or assigned explicitly.
+        """
 
         self._venv: PythonEnv | None = None
         """Cached python virtual environment used by the Odoo installation."""
@@ -301,12 +303,14 @@ def odoo_support_repository(self) -> GitConnector:
 
     @property
     def additional_addons_paths(self) -> list[Path]:
-        """Return the list of additional addons paths."""
-        if not self._additional_addons_paths and self.database.repository:
-            repository = GitConnector(self.database.repository.full_name)
-
-            if repository.path not in self._additional_addons_paths:
-                self._additional_addons_paths.append(repository.path)
+        """Return the list of additional addons paths.
+        Derived once from the repository linked to the database, unless assigned explicitly.
+        """
+        if self._additional_addons_paths is None:
+            repository = self.database.repository
+            self._additional_addons_paths = (
+                self.expand_addons_paths([GitConnector(repository.full_name).path]) if repository else []
+            )
 
         return self._additional_addons_paths
 
@@ -925,6 +929,21 @@ def check_addons_path(cls, path: Path) -> bool:
             manifest.is_file() and (manifest.parent / "__init__.py").is_file() for glob in globs for manifest in glob
         )
 
+    @classmethod
+    def expand_addons_paths(cls, paths: Sequence[Path]) -> list[Path]:
+        """Expand paths to the actual Odoo addons directories they contain.
+
+        Modules are looked up recursively so that repositories keeping their modules in
+        subdirectories are handled the same way as those keeping them at their root.
+
+        :param paths: Paths to expand.
+        :return: Sorted list of unique valid addons paths.
+        :rtype: List[Path]
+        """
+        globs = (path.glob(f"**/__{manifest}__.py") for path in paths for manifest in ["manifest", "openerp"])
+        candidates = {manifest.parents[1] for glob in globs for manifest in glob}
+        return sorted(path for path in candidates if cls.check_addons_path(path))
+
     @classmethod
     def check_addon_path(cls, path: Path) -> bool:
         """Return whether the given path is a valid Odoo addon.

tests/resources/repositories/test/test-addons/addon_01/data/neutralize.sql

@@ -0,0 +1,2 @@
+-- Neutralization script shipped by a custom module, used by tests.
+SELECT 1;

tests/tests/commands/test_quickstart.py

@@ -0,0 +1,96 @@
+from argparse import Namespace
+from pathlib import Path
+from unittest.mock import MagicMock
+
+from odev.commands.database.quickstart import QuickStartCommand
+from odev.common.databases import LocalDatabase, Repository
+
+from tests.fixtures import OdevTestCase
+
+
+class TestQuickStartLinksRepository(OdevTestCase):
+    """The repository must be linked before the dump is restored, and still be linked afterwards."""
+
+    def make_command(self, repository: Repository | None) -> QuickStartCommand:
+        command = QuickStartCommand.__new__(QuickStartCommand)
+        command._framework = self.odev
+        command.args = Namespace(branch=None, version=None, name="quickstart-target", filestore=False)
+
+        source = MagicMock()
+        source.name = "quickstart-source"
+        source.repository = repository
+        source.platform.name = "remote"
+        source._get_dump_filename.return_value = "dump.zip"
+        command._database = source
+        return command
+
+    def make_dump_file(self) -> Path:
+        dump_file = self.run_path / "dump.zip"
+        dump_file.parent.mkdir(parents=True, exist_ok=True)
+        dump_file.touch()
+        return dump_file
+
+    def test_repository_is_linked_before_restore(self):
+        """Regression for #98: neutralization runs from within `restore`, so a repository linked
+        only after the restore call is invisible to it.
+        """
+        repository = Repository("psbe-project", "odoo-ps")
+        command = self.make_command(repository)
+        target = MagicMock(spec=LocalDatabase)
+        target.repository = None
+        seen: list[Repository | None] = []
+
+        def fake_run_command(name, *_args, database=None, **_kwargs):
+            if name == "restore":
+                seen.append(database.repository)
+            return True
+
+        dump_file = self.make_dump_file()
+
+        with (
+            self.patch(self.odev, "run_command", side_effect=fake_run_command),
+            self.patch_property(type(self.odev), "dumps_path", dump_file.parent),
+            self.patch("odev.commands.database.quickstart", "LocalDatabase", return_value=target),
+        ):
+            command.run()
+
+        self.assertEqual(seen, [Repository("psbe-project", "odoo-ps")])
+
+    def test_repository_is_still_linked_after_restore(self):
+        """`restore` drops and recreates the database, which clears its entry in the data store."""
+        repository = Repository("psbe-project", "odoo-ps")
+        command = self.make_command(repository)
+        target = MagicMock(spec=LocalDatabase)
+        target.repository = None
+
+        def fake_run_command(name, *_args, database=None, **_kwargs):
+            if name == "restore":
+                database.repository = None  # `restore` wipes the store entry
+            return True
+
+        dump_file = self.make_dump_file()
+
+        with (
+            self.patch(self.odev, "run_command", side_effect=fake_run_command),
+            self.patch_property(type(self.odev), "dumps_path", dump_file.parent),
+            self.patch("odev.commands.database.quickstart", "LocalDatabase", return_value=target),
+        ):
+            command.run()
+
+        self.assertEqual(target.repository, Repository("psbe-project", "odoo-ps"))
+
+    def test_no_repository_on_source_links_nothing(self):
+        command = self.make_command(None)
+        target = MagicMock(spec=LocalDatabase)
+        target.repository = None
+
+        dump_file = self.make_dump_file()
+
+        with (
+            self.patch(self.odev, "run_command", return_value=True),
+            self.patch_property(type(self.odev), "dumps_path", dump_file.parent),
+            self.patch("odev.commands.database.quickstart", "LocalDatabase", return_value=target),
+        ):
+            command.run()
+
+        self.assertIsNone(target.repository)

tests/tests/common/test_local_database.py

@@ -0,0 +1,79 @@
+from pathlib import Path
+from unittest.mock import MagicMock
+
+from odev.common.databases import LocalDatabase
+from odev.common.version import OdooVersion
+
+from tests.fixtures import OdevTestCase
+
+
+class TestNeutralizeScripts(OdevTestCase):
+    """Neutralization must pick up the `data/neutralize.sql` scripts shipped by custom modules."""
+
+    def setUp(self):
+        super().setUp()
+        self.addons_path = self.res_path / "repositories" / "test" / "test-addons"
+
+    def make_database(self, addons_paths: list[Path] | None = None) -> LocalDatabase:
+        database = LocalDatabase.__new__(LocalDatabase)
+        database._framework = self.odev
+        database._process = MagicMock()
+        database._process.additional_addons_paths = [self.addons_path] if addons_paths is None else addons_paths
+        return database
+
+    def neutralize_scripts(
+        self,
+        installed_modules: list[str],
+        addons_paths: list[Path] | None = None,
+        version: str = "18.0",
+    ) -> list[Path]:
+        database = self.make_database(addons_paths)
+
+        with (
+            self.patch_property(LocalDatabase, "installed_modules", installed_modules),
+            self.patch_property(LocalDatabase, "process", database._process),
+            self.patch_property(LocalDatabase, "version", OdooVersion(version)),
+        ):
+            return database._neutralize_scripts()
+
+    def test_custom_module_script_is_collected(self):
+        """Regression for #98: the module scripts were filtered against the *addons directory*
+        names rather than the module names, so no custom script was ever collected.
+        """
+        scripts = self.neutralize_scripts(["base", "addon_01"])
+        self.assertEqual(
+            scripts,
+            [
+                self.odev.static_path / "neutralize-pre.sql",
+                self.addons_path / "addon_01" / "data" / "neutralize.sql",
+                self.odev.static_path / "neutralize-post.sql",
+            ],
+        )
+
+    def test_module_without_script_is_skipped(self):
+        scripts = self.neutralize_scripts(["addon_02"], addons_paths=[self.addons_path / "submodule"])
+        self.assertEqual(
+            scripts,
+            [
+                self.odev.static_path / "neutralize-pre.sql",
+                self.odev.static_path / "neutralize-post.sql",
+            ],
+        )
+
+    def test_module_not_installed_is_skipped(self):
+        scripts = self.neutralize_scripts(["base"])
+        self.assertNotIn(self.addons_path / "addon_01" / "data" / "neutralize.sql", scripts)
+
+    def test_no_addons_paths_yields_static_scripts_only(self):
+        scripts = self.neutralize_scripts(["addon_01"], addons_paths=[])
+        self.assertEqual(
+            scripts,
+            [
+                self.odev.static_path / "neutralize-pre.sql",
+                self.odev.static_path / "neutralize-post.sql",
+            ],
+        )
+
+    def test_older_versions_get_the_legacy_script(self):
+        scripts = self.neutralize_scripts(["base"], version="14.0")
+        self.assertIn(self.odev.static_path / "neutralize-post-before-15.0.sql", scripts)