Add some updates after review

- Make the version be a object of Version;
- Make the version property be optional in remove_plugin;
- Use the rsplit to obtain the correct name of plugin from its folder;

Author: BeneBr

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

@@ -8,6 +8,7 @@
 from zipfile import ZipFile
 
 from dataclasses import dataclass
+from packaging.version import Version
 
 from hookman import hookman_utils
 from hookman.exceptions import InvalidDestinationPathError
@@ -18,10 +19,10 @@
 @dataclass(frozen=True)
 class InstalledPluginInfo:
     """
-    Responsible to stores the information about the installed plugin.
+    Responsible to store the information about an installed plugin.
     """
     id: str
-    version: str
+    version: Version
 
 class HookSpecs:
     """
@@ -107,7 +108,7 @@ def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> InstalledPl
 
             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.
 
         :param: plugin_file_path:
             The Path for the ``.hmplugin``
@@ -127,8 +128,9 @@ def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> InstalledPl
 
         yaml_content = plugin_file_zip.open("assets/plugin.yaml").read().decode("utf-8")
 
-        plugin_id: str = PluginInfo._load_yaml_file(yaml_content)["id"]
-        plugin_version: str = PluginInfo._load_yaml_file(yaml_content)["version"]
+        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()]
@@ -139,7 +141,7 @@ def install_plugin(self, plugin_file_path: Path, dest_path: Path) -> InstalledPl
         plugin_destination_folder = dest_path / plugin_id_version
         plugin_destination_folder.mkdir(parents=True)
         plugin_file_zip.extractall(plugin_destination_folder)
-        return InstalledPluginInfo(version=plugin_version, id=plugin_id)
+        return InstalledPluginInfo(version=Version(plugin_version), id=plugin_id)
 
     def _move_to_trash(self, root_dir, name):
         """
@@ -172,20 +174,29 @@ def _try_clear_trash(self, root_dir):
                 with suppress(OSError):
                     filename.unlink()
 
-    def remove_plugin(self, caption: str, version: str) -> None:
+    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.
 
         :param version:
-            Version of the plugin to be removed.
+            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 and plugin.version == version:
-                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,11 +2,13 @@
 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 strictyaml import Map
+from packaging.version import Version
+from strictyaml import Map, YAMLValidationError
 from strictyaml import MapPattern
 from strictyaml import Optional
 from strictyaml import Str
@@ -44,7 +46,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,7 +64,7 @@ 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", {})
 
@@ -92,8 +94,8 @@ def _get_plugin_id_from_dll(self, plugin_id_from_plugin_yaml: str) -> str:
             plugin_id_from_shared_lib = plugin_dll.get_plugin_id().decode("utf-8")
             if plugin_id_from_shared_lib != plugin_id_from_plugin_yaml:
                 msg = (
-                    f'Error, the id inside plugin.yaml is "{plugin_id_from_plugin_yaml}" '
-                    f"while the id inside the {self.shared_lib_name} is {plugin_id_from_shared_lib}"
+                    f'Error, the plugin_id inside plugin.yaml is "{plugin_id_from_plugin_yaml}" '
+                    f"while the plugin_id inside the {self.shared_lib_name} is {plugin_id_from_shared_lib}"
                 )
                 raise RuntimeError(msg)
             return plugin_id_from_shared_lib
@@ -133,7 +135,12 @@ 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

@@ -63,7 +63,7 @@ def generate_build_files(ctx):
 
         # Copy all the plugins to the build dir
         for plugin_dir in plugins_dirs:
-            plugin_name, _ = plugin_dir.name.split("-")
+            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(
@@ -169,7 +169,7 @@ def _package_plugins(ctx):
 
         for plugin in plugins_dirs:
             (plugin / "artifacts").mkdir()
-            name, _ = plugin.name.split("-")
+            name, _ = plugin.name.rsplit("-",maxsplit=1)
             if sys.platform == "win32":
                 shutil.copy2(src=artifacts_dir / f"{name}.dll", dst=plugin / "artifacts")
             else:

tests/conftest.py

@@ -52,11 +52,11 @@ def _get_plugin(plugin_name):
         from hookman.plugin_config import PluginInfo
 
         version = PluginInfo(plugin_dir / "assets/plugin.yaml", hooks_available=None).version
-        name, version = plugin_name.split("-")
+        name, version = plugin_name.rsplit("-",maxsplit=1)
         import sys
 
         hm_plugin_name = (
-            f"{name}-{version}-win64.hmplugin" if sys.platform == "win32" else f"{name}-{version}-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
 

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

@@ -3,7 +3,7 @@
 from hookman.hooks import HookMan
 from hookman.hooks import HookSpecs
 from hookman.hooks import PluginInfo
-
+from packaging.version import Version
 
 def _get_plugin_id_set(plugin_info_list):
     """
@@ -233,7 +233,10 @@ def test_remove_plugin(datadir, simple_plugin, simple_plugin_2):
 
     assert _get_plugin_id_set(hm.get_plugins_available()) == {"simple_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","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-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,7 +1,11 @@
 import pytest
+from pytest_mock import MockerFixture
 
 from hookman.plugin_config import PluginInfo
-
+from pathlib import Path
+from textwrap import dedent
+from strictyaml import YAMLValidationError
+import re
 
 def test_load_config_content(datadir, mocker, mock_plugin_id_from_dll):
     mocker.patch.object(PluginInfo, "_get_hooks_implemented", return_value=["a"])
@@ -50,8 +54,38 @@ def test_plugin_id_conflict(simple_plugin, datadir):
     yaml_file.write_text(new_content)
 
     expected_msg = (
-        'Error, the id inside plugin.yaml is "ACME" '
-        f"while the id inside the {acme_lib_name} is simple_plugin"
+        'Error, the plugin_id inside plugin.yaml is "ACME" '
+        f"while the plugin_id inside the {acme_lib_name} is simple_plugin"
     )
     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)
+