[ASIM - 6224] - Update hookman to deal with multiple versions of a plugin

hookman/hookman_generator.py

@@ -330,7 +330,7 @@ def generate_plugin_package(
             contents_dict["requirements"] = dict(sorted(requirements.items()))
             contents = contents_dict.as_yaml()
 
-        hmplugin_base_name_components = [package_name, plugin_info.version]
+        hmplugin_base_name_components = [package_name, plugin_info.version.base_version]
         if package_name_suffix is not None:
             hmplugin_base_name_components.append(package_name_suffix)
 
@@ -404,12 +404,13 @@ def _validate_plugin_config_file(self, plugin_config_file: Path) -> None:
         All checks are made in the __init__
         """
         plugin_file_content = PluginInfo(plugin_config_file, hooks_available=None)
+        content_version = plugin_file_content.version.base_version
         semantic_version_re = re.compile(r"^(\d+)\.(\d+)\.(\d+)")  # Ex.: 1.0.0 or 2025.1.0
-        version = semantic_version_re.match(plugin_file_content.version)
+        version = semantic_version_re.match(content_version)
 
         if not version:
             raise ValueError(
-                f"Version attribute does not follow the calendar or semantic versioning, got {plugin_file_content.version!r}"
+                f"Version attribute does not follow the calendar or semantic versioning, got {content_version!r}"
             )
 
     def _hook_specs_header_content(self, plugin_id: str) -> str:

hookman/hooks.py

@@ -2,18 +2,31 @@
 import shutil
 from collections.abc import Callable
 from collections.abc import Sequence
+from dataclasses import dataclass
 from pathlib import Path
 from typing import List
 from typing import Optional
 from zipfile import ZipFile
 
+from packaging.version import Version
+
 from hookman import hookman_utils
 from hookman.exceptions import InvalidDestinationPathError
 from hookman.exceptions import PluginAlreadyInstalledError
 from hookman.hookman_utils import change_path_env
 from hookman.plugin_config import PluginInfo
 
 
+@dataclass(frozen=True)
+class InstalledPluginInfo:
+    """
+    Responsible to store the information about an installed plugin.
+    """
+
+    id: str
+    version: Version
+
+
 class HookSpecs:
     """
     A class that holds the specification of the hooks, currently the following specification are available:
@@ -89,19 +102,22 @@ def __init__(self, *, specs: HookSpecs, plugin_dirs: List[Path]):
             for hook in specs.hooks
         }
 
-    def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> str:
+    def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> InstalledPluginInfo:
         """
         Extract the content of the zip file into dest_path.
-        If the installation occurs successfully the name of the installed plugin will be returned.
+        If the installation occurs successfully a InstalledPluginInfo will be returned.
 
         The following checks will be executed to validate the consistency of the inputs:
 
             1. The destination Path should be one of the paths informed during the initialization of HookMan (plugins_dirs field).
 
-            2. The plugins_dirs cannot have two plugins with the same name.
+            2. The plugins_dirs cannot have two plugins with the same name and version.
 
-        :plugin_file_path: The Path for the ``.hmplugin``
-        :dest_path: The destination to where the plugin should be placed.
+        :param: plugin_file_path:
+            The Path for the ``.hmplugin``
+
+        :param dest_path:
+            The destination to where the plugin should be placed.
         """
         plugin_file_zip = ZipFile(plugin_file_path)
         PluginInfo.validate_plugin_file(plugin_file_zip=plugin_file_zip)
@@ -114,17 +130,21 @@ def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> str:
             )
 
         yaml_content = plugin_file_zip.open("assets/plugin.yaml").read().decode("utf-8")
-        plugin_id = PluginInfo._load_yaml_file(yaml_content)["id"]
+
+        yaml_data = PluginInfo._load_yaml_file(yaml_content)
+        plugin_id: str = yaml_data["id"]
+        plugin_version: str = yaml_data["version"]
+        plugin_id_version = f"{plugin_id}-{plugin_version}"
 
         plugins_dirs = [x for x in dest_path.iterdir() if x.is_dir()]
 
-        if plugin_id in [x.name for x in plugins_dirs]:
+        if plugin_id_version in [x.name for x in plugins_dirs]:
             raise PluginAlreadyInstalledError("Plugin already installed")
 
-        plugin_destination_folder = dest_path / plugin_id
+        plugin_destination_folder = dest_path / plugin_id_version
         plugin_destination_folder.mkdir(parents=True)
         plugin_file_zip.extractall(plugin_destination_folder)
-        return plugin_id
+        return InstalledPluginInfo(version=Version(plugin_version), id=plugin_id)
 
     def _move_to_trash(self, root_dir, name):
         """
@@ -157,16 +177,29 @@ def _try_clear_trash(self, root_dir):
                 with suppress(OSError):
                     filename.unlink()
 
-    def remove_plugin(self, caption: str):
+    def remove_plugin(self, caption: str, version: Version | None = None) -> None:
         """
-        This method receives the name of the plugin as input, and will remove completely the plugin from ``plugin_dirs``.
+        This method receives the name and version of plugin as input, and will remove completely the
+        plugin from ``plugin_dirs``.
+
+        :param caption:
+            Name of the plugin to be removed.
 
-        :caption: Name of the plugin to be removed
+        :param version:
+            Optional parameter used to remove a specific version of plugin. Case it is not specified,
+            all versions of a given plugin will be removed.
         """
         for plugin in self.get_plugins_available():
-            if plugin.id == caption:
-                plugin_dir = plugin.yaml_location.parents[1]
-                root_dir = plugin_dir.parent
+            plugin_dir = plugin.yaml_location.parents[1]
+            root_dir = plugin_dir.parent
+            remove_plugin = False
+
+            if version is None and caption in plugin_dir.name:
+                remove_plugin = True
+            elif plugin.id == caption and version == plugin.version:
+                remove_plugin = True
+
+            if remove_plugin:
                 self._move_to_trash(root_dir, plugin_dir.name)
                 self._try_clear_trash(root_dir)
                 break

hookman/plugin_config.py

@@ -2,15 +2,18 @@
 import sys
 from collections.abc import Sequence
 from pathlib import Path
+from textwrap import dedent
 from zipfile import ZipFile
 
 from attr import define
 from attr import field
+from packaging.version import Version
 from strictyaml import Map
 from strictyaml import MapPattern
 from strictyaml import Optional
 from strictyaml import Str
 from strictyaml import YAML
+from strictyaml import YAMLValidationError
 
 from hookman.exceptions import SharedLibraryNotFoundError
 from hookman.hookman_utils import load_shared_lib
@@ -44,7 +47,7 @@ class PluginInfo:
     caption: str = field(init=False)
     shared_lib_name: str = field(init=False)
     shared_lib_path: Path = field(init=False)
-    version: str = field(init=False)
+    version: Version = field(init=False)
     requirements: dict[str, str] = field(init=False)
     extras: dict = field(init=False)
     id: str = field(init=False)
@@ -62,11 +65,11 @@ def __attrs_post_init__(self) -> None:
         self.author = plugin_config_file_content["author"]
         self.caption = plugin_config_file_content["caption"]
         self.email = plugin_config_file_content["email"]
-        self.version = plugin_config_file_content["version"]
+        self.version = Version(plugin_config_file_content["version"])
         self.requirements = plugin_config_file_content.get("requirements", {})
         self.extras = plugin_config_file_content.get("extras", {})
 
-        # The id bellow guarantee to me that the plugin_id to be used in the application was not changed by a config file.
+        # The id bellow guarantee to me that the id to be used in the application was not changed by a config file.
         self.id = self._get_plugin_id_from_dll(plugin_config_file_content["id"])
 
         readme_file = self.yaml_location.parent / "README.md"
@@ -133,7 +136,16 @@ def is_implemented_on_plugin(cls, plugin_dll: ctypes.CDLL, hook_name: str) -> bo
     def _load_yaml_file(cls, yaml_content: str) -> YAML:
         import strictyaml
 
-        plugin_config_file_content = strictyaml.load(yaml_content, PLUGIN_CONFIG_SCHEMA).data
+        try:
+            plugin_config_file_content = strictyaml.load(yaml_content, PLUGIN_CONFIG_SCHEMA).data
+        except YAMLValidationError:
+            current_plugin_schema = "\n".join(
+                f"{key} : {value}" for key, value in PLUGIN_CONFIG_SCHEMA._validator.items()
+            )
+            raise ValueError(
+                f"The plugin.yaml does not follow the PLUGIN_CONFIG_SCHEMA: {current_plugin_schema}"
+            )
+
         if sys.platform == "win32":
             plugin_config_file_content["shared_lib_name"] = (
                 f"{plugin_config_file_content['id']}.dll"

tasks.py

@@ -62,11 +62,12 @@ def generate_build_files(ctx):
         ]
 
         # Copy all the plugins to the build dir
-        for plugin in plugins_dirs:
-            plugin_dir_build = project_dir_for_build / f"plugin/{plugin.name}"
-            shutil.copytree(src=plugin, dst=plugin_dir_build)
+        for plugin_dir in plugins_dirs:
+            plugin_name, _ = plugin_dir.name.rsplit("-", maxsplit=1)
+            plugin_dir_build = project_dir_for_build / f"plugin/{plugin_dir.name}"
+            shutil.copytree(src=plugin_dir, dst=plugin_dir_build)
             (plugin_dir_build / "src/hook_specs.h").write_text(
-                hm_generator._hook_specs_header_content(plugin.stem)
+                hm_generator._hook_specs_header_content(plugin_name)
             )
 
         # Create the CMakeFile on root of the project to include others CMake files.
@@ -168,11 +169,12 @@ def _package_plugins(ctx):
 
         for plugin in plugins_dirs:
             (plugin / "artifacts").mkdir()
+            name, _ = plugin.name.rsplit("-", maxsplit=1)
             if sys.platform == "win32":
-                shutil.copy2(src=artifacts_dir / f"{plugin.name}.dll", dst=plugin / "artifacts")
+                shutil.copy2(src=artifacts_dir / f"{name}.dll", dst=plugin / "artifacts")
             else:
-                shutil.copy2(src=artifacts_dir / f"lib{plugin.name}.so", dst=plugin / "artifacts")
+                shutil.copy2(src=artifacts_dir / f"lib{name}.so", dst=plugin / "artifacts")
 
             hm_generator.generate_plugin_package(
-                package_name=plugin.name, plugin_dir=plugin, dst_path=plugins_zip
+                package_name=name, plugin_dir=plugin, dst_path=plugins_zip
             )

tests/conftest.py

@@ -52,11 +52,13 @@ def _get_plugin(plugin_name):
         from hookman.plugin_config import PluginInfo
 
         version = PluginInfo(plugin_dir / "assets/plugin.yaml", hooks_available=None).version
-        name = f"{plugin_name}-{version}"
+        name, version = plugin_name.rsplit("-", maxsplit=1)
         import sys
 
         hm_plugin_name = (
-            f"{name}-win64.hmplugin" if sys.platform == "win32" else f"{name}-linux64.hmplugin"
+            f"{plugin_name}-win64.hmplugin"
+            if sys.platform == "win32"
+            else f"{plugin_name}-linux64.hmplugin"
         )
         plugin_zip_path = plugins_zip_folder / hm_plugin_name
 
@@ -67,12 +69,12 @@ def _get_plugin(plugin_name):
 
 @pytest.fixture
 def simple_plugin(get_plugin):
-    return get_plugin("simple_plugin")
+    return get_plugin("simple_plugin-1.0.0")
 
 
 @pytest.fixture
 def simple_plugin_2(get_plugin):
-    return get_plugin("simple_plugin_2")
+    return get_plugin("simple_plugin_2-1.0.0")
 
 
 @pytest.fixture

tests/test_hookman_generator.py

@@ -242,7 +242,7 @@ def test_generate_plugin_package(
 
     from hookman.plugin_config import PluginInfo
 
-    version = PluginInfo(Path(tmpdir / "acme/assets/plugin.yaml"), None).version
+    version = PluginInfo(Path(tmpdir / "acme/assets/plugin.yaml"), None).version.base_version
 
     base_plugin_name_components = [plugin_id, version]
     if package_name_extra is not None:

tests/test_hooks.py

@@ -1,4 +1,5 @@
 import pytest
+from packaging.version import Version
 
 from hookman.hooks import HookMan
 from hookman.hooks import HookSpecs
@@ -136,11 +137,11 @@ def test_plugins_available_ignore_trash(datadir, simple_plugin, simple_plugin_2)
     plugins = hm.get_plugins_available()
     assert {p.id for p in plugins} == {"simple_plugin", "simple_plugin_2"}
 
-    hm._move_to_trash(datadir / "plugins", "simple_plugin")
+    hm._move_to_trash(datadir / "plugins", "simple_plugin-1.0.0")
     plugins = hm.get_plugins_available()
     assert {p.id for p in plugins} == {"simple_plugin_2"}
 
-    hm._move_to_trash(datadir / "plugins", "simple_plugin_2")
+    hm._move_to_trash(datadir / "plugins", "simple_plugin_2-1.0.0")
     plugins = hm.get_plugins_available()
     assert {p.id for p in plugins} == set()
 
@@ -154,7 +155,7 @@ def test_try_clean_cache_ignore_os_errors(datadir, simple_plugin, monkeypatch):
     plugin_dir = datadir / "plugins"
     trash_folder = plugin_dir / ".trash"
     hm = HookMan(specs=simple_plugin["specs"], plugin_dirs=plugin_dir)
-    hm._move_to_trash(plugin_dir, "simple_plugin")
+    hm._move_to_trash(plugin_dir, "simple_plugin-1.0.0")
     (trash_item_dir,) = trash_folder.glob("*")
     # Change cwd to inside the trash item folder, windows will not be able to delete a directory
     # in use.
@@ -209,9 +210,9 @@ def test_install_plugin_duplicate(simple_plugin):
     hm = HookMan(specs=simple_plugin["specs"], plugin_dirs=[simple_plugin["path"].parent])
     import os
 
-    os.makedirs(simple_plugin["path"] / "simple_plugin")
+    os.makedirs(simple_plugin["path"] / "simple_plugin-1.0.0")
 
-    # Trying to install the plugin in a folder that already has a folder with the same name as the plugin
+    # Trying to install the plugin in a folder that already has a folder with the same name and version of plugin.
     from hookman.exceptions import PluginAlreadyInstalledError
 
     with pytest.raises(PluginAlreadyInstalledError, match=f"Plugin already installed"):
@@ -222,18 +223,24 @@ def test_install_plugin_duplicate(simple_plugin):
 
 def test_install_plugin(datadir, simple_plugin):
     hm = HookMan(specs=simple_plugin["specs"], plugin_dirs=[simple_plugin["path"]])
-    assert (simple_plugin["path"] / "simple_plugin").exists() == False
+    assert (simple_plugin["path"] / "simple_plugin-1.0.0").exists() == False
     hm.install_plugin(plugin_file_path=simple_plugin["zip"], dest_path=simple_plugin["path"])
-    assert (simple_plugin["path"] / "simple_plugin").exists() == True
+    assert (simple_plugin["path"] / "simple_plugin-1.0.0").exists() == True
 
 
 def test_remove_plugin(datadir, simple_plugin, simple_plugin_2):
     plugins_dirs = [simple_plugin["path"], simple_plugin_2["path"]]
     hm = HookMan(specs=simple_plugin["specs"], plugin_dirs=plugins_dirs)
 
     assert _get_plugin_id_set(hm.get_plugins_available()) == {"simple_plugin", "simple_plugin_2"}
-    assert _get_names_inside_folder(datadir / "plugins") == {"simple_plugin", "simple_plugin_2"}
-    hm.remove_plugin("simple_plugin_2")
+    assert _get_names_inside_folder(datadir / "plugins") == {
+        "simple_plugin-1.0.0",
+        "simple_plugin_2-1.0.0",
+    }
+    hm.remove_plugin("simple_plugin_2", Version("1.0.0"))
     assert _get_plugin_id_set(hm.get_plugins_available()) == {"simple_plugin"}
-    assert _get_names_inside_folder(datadir / "plugins") == {"simple_plugin", ".trash"}
+    assert _get_names_inside_folder(datadir / "plugins") == {"simple_plugin-1.0.0", ".trash"}
     assert _get_names_inside_folder(datadir / "plugins" / ".trash") == set()
+
+    hm.remove_plugin("simple_plugin")
+    assert _get_plugin_id_set(hm.get_plugins_available()) == set()

tests/test_plugin_config.py

@@ -1,4 +1,10 @@
+import re
+from pathlib import Path
+from textwrap import dedent
+
 import pytest
+from pytest_mock import MockerFixture
+from strictyaml import YAMLValidationError
 
 from hookman.plugin_config import PluginInfo
 
@@ -55,3 +61,35 @@ def test_plugin_id_conflict(simple_plugin, datadir):
     )
     with pytest.raises(RuntimeError, match=expected_msg):
         PluginInfo(yaml_file, None)
+
+
+def testPluginInfoInvalidSchema(tmp_path: Path, mocker: MockerFixture) -> None:
+    invalid_yaml_content = f"""\
+            caption: 'Plugin'
+            version: '1.0.0'
+            author: 'ESSS'
+            developer: 'Developer 1'
+            email: 'alfasim-dev@esss.co'
+            id: 'plugin'
+            """
+
+    invalid_yaml_file = tmp_path / "config.yaml"
+    invalid_yaml_file.write_text(invalid_yaml_content)
+
+    mocker.patch.object(PluginInfo, "_check_if_shared_lib_exists", autospec=True)
+    mocker.patch.object(PluginInfo, "_get_plugin_id_from_dll", autospc=True, return_value="plugin")
+
+    current_schema = dedent(
+        """\
+    caption : Str()
+    version : Str()
+    author : Str()
+    email : Str()
+    id : Str()
+    Optional("requirements") : MapPattern(Str(), Str())
+    Optional("extras") : MapPattern(Str(), Str())
+    """
+    ).strip()
+    expected_message = f"The plugin.yaml does not follow the PLUGIN_CONFIG_SCHEMA: {current_schema}"
+    with pytest.raises(ValueError, match=re.escape(expected_message)):
+        info = PluginInfo(invalid_yaml_file)