Merge pull request #343 from ESSS/fb-ASIM-5419-use-plugin-multiple-files

Allow user plugins to import from other modules inside it

CHANGELOG.rst

@@ -5,6 +5,9 @@ CHANGELOG
 0.19.0 (UNRELEASED)
 ===================
 
+* Added support for user plugins have multiple python source files, the "user plugin entry point" logic is unchanged, but extra python code could be placed in `alfasim_sdk_plugins.<plugins_id>` and be imported at runtime;
+
+
 0.18.0 (2023-10-13)
 ===================
 

src/alfasim_sdk/_internal/alfacase/plugin_alfacase_to_case.py

@@ -185,7 +185,7 @@ def import_module(path: Path) -> ModuleType:
     import importlib
 
     assert path.is_file()
-    spec = importlib.util.spec_from_file_location("", path)
+    spec = importlib.util.spec_from_file_location(path.stem, path)
     assert spec is not None
     assert spec.loader is not None
     module = importlib.util.module_from_spec(spec)
@@ -197,14 +197,31 @@ def load_plugin_data_structure(plugin_id: str) -> Optional[List[Type]]:
     """
     Obtain the models for a given plugin.
     """
+    import alfasim_sdk_plugins
+
     for candidate in get_plugin_module_candidates(plugin_id):
         if candidate.is_file():
+            # Update the "alfasim_sdk_plugins" namespace.
+            namespace_package_str = None
+            remove_namespace_package = False
+            namespace_package = candidate.parent / "alfasim_sdk_plugins"
+            if namespace_package.is_dir():
+                namespace_package_str = str(namespace_package.absolute())
+                if namespace_package_str not in alfasim_sdk_plugins.__path__:
+                    alfasim_sdk_plugins.__path__.append(namespace_package_str)
+                    remove_namespace_package = True
+
             module = import_module(candidate)
             if hasattr(module, "alfasim_get_data_model_type"):
                 models = module.alfasim_get_data_model_type()
                 if all(hasattr(m, "_alfasim_metadata") for m in models):
                     return models
 
+            # Cleanup "alfasim_sdk_plugins" namespace.
+            # When a valid plugin is found leave the namespace package unchanged.
+            if remove_namespace_package:
+                alfasim_sdk_plugins.__path__.remove(namespace_package_str)
+
     else:
         return None
 

src/alfasim_sdk/_internal/cli.py

@@ -68,22 +68,28 @@ def new(dst, caption, plugin_id, author_name, author_email):
 
     .. code-block:: bash
 
-        \---myplugin
-        |   CMakeLists.txt
-        |   tasks.py
-        |
-        +---assets
-        |       plugin.yaml
-        |       README.md
-        |
-        \---src
+        <dest>
+        \---<plugin_id>
             |   CMakeLists.txt
-            |   hook_specs.h
-            |   myplugin.cpp
+            |   tasks.py
             |
-            \---python
-                    myplugin.py
-
+            +---assets
+            |       plugin.yaml
+            |       README.md
+            |
+            \---src
+                |   CMakeLists.txt
+                |   hook_specs.h
+                |   <plugin_id>.cpp
+                |
+                \---python
+                    |   <plugin_id>.py
+                    |
+                    \---alfasim_sdk_plugins
+                        \---<plugin_id>
+                                __init__.py
+
+    Any python code placed in ``alfasim_sdk_plugins/<plugin_id>`` is importable by using ``import alfasim_sdk_plugins.<plugin_id>.<my_extra_module>``.
     """
     dst = Path(dst)
     hook_specs_file_path = _get_hook_specs_file_path()
@@ -112,7 +118,11 @@ def new(dst, caption, plugin_id, author_name, author_email):
     source_folder = dst / plugin_id / "src"
     python_folder = source_folder / "python"
     python_folder.mkdir()
-    Path(python_folder / f"{plugin_id}.py").touch()
+    (python_folder / f"{plugin_id}.py").touch()
+
+    importable_module = python_folder / "alfasim_sdk_plugins" / plugin_id
+    importable_module.mkdir(parents=True)
+    (importable_module / "__init__.py").touch()
 
     # remove compile.py created by hookman
     compile_file = dst / plugin_id / "compile.py"

src/alfasim_sdk_plugins/__init__.py

@@ -0,0 +1,13 @@
+"""
+This is not supposed to be a real package.
+It is the namespace of user plugins and is manipulated by the user plugins infrastructure.
+
+The documentation explicitly states that the `namespace packages`_' __path__ is read-only and
+automatically updated if the `sys.path` changes (also `PEP-420`_), initial tests show that it
+is not read-only and was not magically updated, but better to write code to the spec when possible.
+
+Using this to avoid polluting the global `sys.path` with random user plugin stuff.
+
+.. `namespace packages`_: https://docs.python.org/3.10/reference/import.html#namespace-packages
+.. `PEP-420`_: https://peps.python.org/pep-0420/
+"""

tests/alfacase/test_plugin_alfacase_to_case.py

@@ -2,13 +2,16 @@
 import textwrap
 from datetime import datetime
 from pathlib import Path
+from typing import Any
 from typing import Callable
 from typing import List
 from unittest.mock import ANY
 
 import pytest
+from _pytest.monkeypatch import MonkeyPatch
 from barril.units import Array
 from barril.units import Scalar
+from pytest_mock import MockerFixture
 
 from alfasim_sdk import convert_alfacase_to_description
 from alfasim_sdk import PluginDescription
@@ -43,9 +46,80 @@ def test_get_plugin_module_candidates(monkeypatch):
     ]
 
 
-def test_load_plugin_data_structure(abx_plugin):
-    models = load_plugin_data_structure("abx")
-    assert [m.__name__ for m in models] == ["AContainer", "BContainer"]
+class TestLoadPluginDataStructure:
+    def test_without_importable_python(self, abx_plugin: None) -> None:
+        models = load_plugin_data_structure("abx")
+        assert [m.__name__ for m in models] == ["AContainer", "BContainer"]
+
+        with pytest.raises(ModuleNotFoundError):
+            import alfasim_sdk_plugins.abx  # noqa
+
+    def test_with_importable_python(self, importable_plugin: None) -> None:
+        with pytest.raises(ModuleNotFoundError):
+            import alfasim_sdk_plugins.importable  # noqa
+
+        models = load_plugin_data_structure("importable")
+        assert [m.__name__ for m in models] == ["Foo"]
+
+        import alfasim_sdk_plugins.importable  # noqa
+        from alfasim_sdk_plugins.importable import buz  # noqa
+
+        assert buz.BUZ == "fiz buz!"
+
+    def test_keep_namespace_tidy(
+        self,
+        importable_plugin: None,
+        importable_plugin_source: Path,
+        datadir: Path,
+        monkeypatch: MonkeyPatch,
+        mocker: MockerFixture,
+    ) -> None:
+        import alfasim_sdk_plugins
+        from alfasim_sdk._internal.alfacase import plugin_alfacase_to_case
+
+        # Create an invalid "importable" plugin.
+        plugin_root = datadir / "test_invalid_plugins"
+        plugin_file = plugin_root / "importable/artifacts/importable.py"
+        plugin_file.parent.mkdir(parents=True)
+        plugin_file.touch()
+        invalid_namespace = plugin_file.parent / "alfasim_sdk_plugins"
+        (invalid_namespace / "importable").mkdir(parents=True)
+        monkeypatch.setenv(
+            "ALFASIM_PLUGINS_DIR", str(plugin_root), prepend=os.path.pathsep
+        )
+
+        # Prepare to check if the namespace is being updated while trying to load the plugin.
+        valid_namespace = (
+            importable_plugin_source / "importable/artifacts/alfasim_sdk_plugins"
+        )
+        good_namespace = str(valid_namespace.absolute())
+        bad_namespace = str(invalid_namespace.absolute())
+        expected_namespace_state = iter(
+            [
+                {"with": bad_namespace, "without": good_namespace},
+                {"with": good_namespace, "without": bad_namespace},
+            ]
+        )
+        original_import_module = plugin_alfacase_to_case.import_module
+
+        def mock_import_module(*args: Any, **kwargs: Any) -> Any:
+            expected = next(expected_namespace_state)
+            assert expected["with"] in alfasim_sdk_plugins.__path__, "AAA"
+            assert expected["without"] not in alfasim_sdk_plugins.__path__, "BBB"
+            return original_import_module(*args, **kwargs)
+
+        mocker.patch.object(
+            plugin_alfacase_to_case, "import_module", new=mock_import_module
+        )
+
+        load_plugin_data_structure("importable")
+
+        assert bad_namespace not in alfasim_sdk_plugins.__path__
+        assert good_namespace in alfasim_sdk_plugins.__path__
+
+        # Check if `mock import module` has exhausted the expected values.
+        with pytest.raises(StopIteration):
+            next(expected_namespace_state)
 
 
 def test_load_plugin_multiple_containers(datadir, abx_plugin):

tests/conftest.py

@@ -1,3 +1,4 @@
+import os.path
 import shutil
 import textwrap
 from pathlib import Path
@@ -95,5 +96,80 @@ def alfasim_get_data_model_type():
 
 
 @pytest.fixture()
-def abx_plugin(abx_plugin_source: Path, monkeypatch: MonkeyPatch) -> None:
-    monkeypatch.setenv("ALFASIM_PLUGINS_DIR", str(abx_plugin_source))
+def importable_plugin_source(datadir: Path) -> Path:
+    """
+    Create a fake plugin input model configuration.
+
+    The input models consist of two top level containers "AContainer"
+    and "BContainer" and the container children properties are:
+
+    - asd (string);
+    - qwe (string);
+
+    The child model is called "X", that is the reason of the very
+    creative name "abx" plugin.
+    """
+    plugin_root = datadir / "test_plugins"
+    plugin_file = plugin_root / "importable/artifacts/importable.py"
+    plugin_file.parent.mkdir(parents=True)
+    plugin_file.write_text(
+        textwrap.dedent(
+            """\
+            import alfasim_sdk
+            from alfasim_sdk_plugins.importable.models import Foo
+
+            @alfasim_sdk.hookimpl
+            def alfasim_get_data_model_type():
+                return [Foo]
+            """
+        )
+    )
+    namespace = plugin_file.parent / "alfasim_sdk_plugins"
+    models_file = namespace / "importable/models.py"
+    models_file.parent.mkdir(parents=True, exist_ok=True)
+    models_file.write_text(
+        textwrap.dedent(
+            """\
+            import alfasim_sdk
+
+            @alfasim_sdk.data_model(icon="", caption="The Foo")
+            class Foo:
+                bar = alfasim_sdk.String(value="some default bar", caption="Bar")
+            """
+        )
+    )
+    buz_file = namespace / "importable/buz.py"
+    buz_file.parent.mkdir(parents=True, exist_ok=True)
+    buz_file.write_text("BUZ = 'fiz buz!'\n")
+    return plugin_root
+
+
+@pytest.fixture()
+def clear_alfasim_plugins_dir(monkeypatch: MonkeyPatch) -> None:
+    monkeypatch.delenv("ALFASIM_PLUGINS_DIR", raising=False)
+
+
+@pytest.fixture()
+def abx_plugin(
+    clear_alfasim_plugins_dir: None,
+    abx_plugin_source: Path,
+    monkeypatch: MonkeyPatch,
+) -> None:
+    monkeypatch.setenv(
+        "ALFASIM_PLUGINS_DIR",
+        str(abx_plugin_source),
+        prepend=os.path.pathsep,
+    )
+
+
+@pytest.fixture()
+def importable_plugin(
+    clear_alfasim_plugins_dir: None,
+    importable_plugin_source: Path,
+    monkeypatch: MonkeyPatch,
+) -> None:
+    monkeypatch.setenv(
+        "ALFASIM_PLUGINS_DIR",
+        str(importable_plugin_source),
+        prepend=os.path.pathsep,
+    )

tests/test_cli.py

@@ -27,8 +27,8 @@ def test_command_new(tmp_path):
     assert result.exit_code == 0
 
     plugin_dir = tmp_path / "acme"
-    assets_dir = plugin_dir / "assets"
-    compile_file = plugin_dir / "tasks.py"
     assert plugin_dir.is_dir()
-    assert assets_dir.is_dir()
-    assert compile_file.is_file()
+    assert (plugin_dir / "assets").is_dir()
+    assert (plugin_dir / "tasks.py").is_file()
+    importable_package = plugin_dir / "src/python/alfasim_sdk_plugins/acme"
+    assert (importable_package / "__init__.py").is_file()