Allow dynamic plugin data classes & convert feature plugins

Plugins' `_data_class` is no longer the source of plugins' data classes.
Instead, new `get_data_class()` method is added (see [1]). `_data_class`
is not gone, it serves as the default for `get_data_class()`, but
plugins now can provide their own implementation.

Which is exactly what `prepare/feature` and feature plugins do now, the
plugin builds its data class from collected smaller data classes, one
for each feature plugin.

There are some loose ends, namely type annorations, but the code works,
both in runtime and when rendering docs.

[1] a `data_class` property would be nice, but the attribute must be a
class-level attribute and those cannot be properties.

docs/scripts/generate-plugins.py

@@ -120,7 +120,7 @@ def plugin_iterator():
         for plugin_id in registry.iter_plugin_ids():
             plugin = registry.get_plugin(plugin_id).class_
 
-            yield plugin_id, plugin, plugin._data_class
+            yield plugin_id, plugin, plugin.get_data_class()
 
     return plugin_iterator
 

tmt/steps/__init__.py

@@ -1027,7 +1027,8 @@ def _patch_raw_datum(
                 else:
                     debug3('incompatible step data')
 
-                    data_base = self._plugin_base_class._data_class
+                    data_base = cast(type[BasePlugin[StepData, Any]],
+                                     self._plugin_base_class).get_data_class()
 
                     debug3('compatible base', f'{data_base.__module__}.{data_base.__name__}')
                     debug3('compatible keys', ', '.join(k for k in data_base.keys()))  # noqa: SIM118
@@ -1272,14 +1273,26 @@ class BasePlugin(Phase, Generic[StepDataT, PluginReturnValueT]):
     _supported_methods: 'tmt.plugins.PluginRegistry[Method]'
 
     _data_class: type[StepDataT]
+
+    @classmethod
+    def get_data_class(cls) -> type[StepDataT]:
+        """
+        Return step data class for this plugin.
+
+        By default, :py:attr:`_data_class` is returned, but plugin may
+        override this method to provide different class.
+        """
+
+        return cls._data_class
+
     data: StepDataT
 
     # TODO: do we need this list? Can whatever code is using it use _data_class directly?
     # List of supported keys
     # (used for import/export to/from attributes during load and save)
     @property
     def _keys(self) -> list[str]:
-        return list(self._data_class.keys())
+        return list(self.get_data_class().keys())
 
     def __init__(
             self,
@@ -1330,8 +1343,8 @@ def options(cls, how: Optional[str] = None) -> list[tmt.options.ClickOptionDecor
         return [
             metadata.option
             for _, _, _, _, metadata in (
-                container_field(cls._data_class, key)
-                for key in container_keys(cls._data_class)
+                container_field(cls.get_data_class(), key)
+                for key in container_keys(cls.get_data_class())
                 )
             if metadata.option is not None
             ] + (
@@ -1461,7 +1474,8 @@ def delegate(
                     f"for the '{how}' method.", level=2)
 
                 plugin_class = method.class_
-                plugin_data_class = plugin_class._data_class
+                plugin_data_class = cast(
+                    type[BasePlugin[StepDataT, PluginReturnValueT]], plugin_class).get_data_class()
 
                 # If we're given raw data, construct a step data instance, applying
                 # normalization in the process.
@@ -1498,7 +1512,7 @@ def delegate(
     def default(self, option: str, default: Optional[Any] = None) -> Any:
         """ Return default data for given option """
 
-        value = self._data_class.default(option_to_key(option), default=default)
+        value = self.get_data_class().default(option_to_key(option), default=default)
 
         if value is None:
             return default
@@ -1651,9 +1665,9 @@ def wake(self) -> None:
         selected ones.
         """
 
-        assert self.data.__class__ is self._data_class, \
+        assert self.data.__class__ is self.get_data_class(), \
             (f'Plugin {self.__class__.__name__} woken with incompatible '
-             f'data {self.data}, expects {self._data_class.__name__}')
+             f'data {self.data}, expects {self.get_data_class().__name__}')
 
         if self.step.status() == 'done':
             self.debug('step is done, not overwriting plugin data')

tmt/steps/prepare/feature/__init__.py

@@ -12,7 +12,7 @@
 from tmt.plugins import PluginRegistry
 from tmt.result import PhaseResult
 from tmt.steps.provision import Guest
-from tmt.utils import Path, field
+from tmt.utils import Path
 
 FEATURE_PLAYEBOOK_DIRECTORY = tmt.utils.resource_files('steps/prepare/feature')
 
@@ -54,11 +54,32 @@ def find_plugin(name: str) -> 'FeatureClass':
     return plugin
 
 
+@dataclasses.dataclass
+class PrepareFeatureData(tmt.steps.prepare.PrepareStepData):
+    pass
+
+
 class Feature(tmt.utils.Common):
     """ Base class for ``feature`` prepare plugin implementations """
 
     NAME: str
 
+    #: Plugin's data class listing keys this feature plugin accepts.
+    #: It is eventually composed together with other feature plugins
+    #: into a single class, :py:class:`PrepareFeatureData`.
+    _data_class: type[PrepareFeatureData] = PrepareFeatureData
+
+    @classmethod
+    def get_data_class(cls) -> type[PrepareFeatureData]:
+        """
+        Return step data class for this plugin.
+
+        By default, :py:attr:`_data_class` is returned, but plugin may
+        override this method to provide different class.
+        """
+
+        return cls._data_class
+
     def __init__(
             self,
             *,
@@ -103,17 +124,6 @@ def _run_playbook(
         guest.ansible(playbook_path)
 
 
-@dataclasses.dataclass
-class PrepareFeatureData(tmt.steps.prepare.PrepareStepData):
-    # TODO: Change it to be able to create and discover custom fields to feature step data
-    epel: Optional[str] = field(
-        default=None,
-        option='--epel',
-        metavar='enabled|disabled',
-        help='Whether EPEL repository should be installed & enabled or disabled.'
-        )
-
-
 @tmt.steps.provides_method('feature')
 class PrepareFeature(tmt.steps.prepare.PreparePlugin[PrepareFeatureData]):
     """
@@ -151,6 +161,47 @@ class PrepareFeature(tmt.steps.prepare.PreparePlugin[PrepareFeatureData]):
 
     _data_class = PrepareFeatureData
 
+    @classmethod
+    def get_data_class(cls) -> type[PrepareFeatureData]:
+        """
+        Return step data class for this plugin.
+
+        ``prepare/feature`` builds the class in a dynamic way: class'
+        fields are defined by discovered feature plugins. Plugins define
+        their own data classes, these are collected, their fields
+        extracted and merged together with the base data class fields
+        (``name``, ``order``, ...) into the final data class of
+        ``prepare/feature`` plugin.
+        """
+
+        # If this class' data class is not `PrepareFeatureData` anymore,
+        # it means this method already constructed the dynamic class.
+        if cls._data_class == PrepareFeatureData:
+            # Collect fields in the base class, we must filter them out
+            # from classes returned by plugins. These fields will be
+            # provided by the base class, and repeating them would raise
+            # an exception.
+            baseclass_fields = list(tmt.utils.container_fields(PrepareFeatureData))
+            baseclass_field_names = [field.name for field in baseclass_fields]
+
+            component_fields = [
+                field
+                for plugin in _FEATURE_PLUGIN_REGISTRY.iter_plugins()
+                for field in tmt.utils.container_fields(plugin.get_data_class())
+                if field.name not in baseclass_field_names
+                ]
+
+            cls._data_class = cast(
+                type[PrepareFeatureData],
+                dataclasses.make_dataclass(
+                    'PrepareFeatureData',
+                    [
+                        (field.name, field.type, field)
+                        for field in component_fields],
+                    bases=(PrepareFeatureData,)))
+
+        return cls._data_class
+
     def go(
             self,
             *,

tmt/steps/prepare/feature/epel.py

@@ -1,14 +1,29 @@
-from typing import Any
+import dataclasses
+from typing import Any, Optional
 
 import tmt.log
-from tmt.steps.prepare.feature import Feature, provides_feature
+import tmt.steps.prepare
+import tmt.utils
+from tmt.steps.prepare.feature import Feature, PrepareFeatureData, provides_feature
 from tmt.steps.provision import Guest
+from tmt.utils import field
+
+
+@dataclasses.dataclass
+class EpelStepData(PrepareFeatureData):
+    epel: Optional[str] = field(
+        default=None,
+        option='--epel',
+        metavar='enabled|disabled',
+        help='Whether EPEL repository should be installed & enabled or disabled.')
 
 
 @provides_feature('epel')
 class Epel(Feature):
     NAME = "epel"
 
+    _data_class = EpelStepData
+
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
 

tmt/steps/provision/__init__.py

@@ -899,6 +899,17 @@ class Guest(tmt.utils.Common):
     # Used by save() to construct the correct container for keys.
     _data_class: type[GuestData] = GuestData
 
+    @classmethod
+    def get_data_class(cls) -> type[GuestData]:
+        """
+        Return step data class for this plugin.
+
+        By default, :py:attr:`_data_class` is returned, but plugin may
+        override this method to provide different class.
+        """
+
+        return cls._data_class
+
     role: Optional[str]
 
     #: Primary hostname or IP address for tmt/guest communication.
@@ -919,7 +930,7 @@ class Guest(tmt.utils.Common):
     # (used for import/export to/from attributes during load and save)
     @property
     def _keys(self) -> list[str]:
-        return list(self._data_class.keys())
+        return list(self.get_data_class().keys())
 
     def __init__(self,
                  *,
@@ -1011,7 +1022,7 @@ def save(self) -> GuestData:
         the guest. Everything needed to attach to a running instance
         should be added into the data dictionary by child classes.
         """
-        return self._data_class.extract_from(self)
+        return self.get_data_class().extract_from(self)
 
     def wake(self) -> None:
         """

tmt/trying.py

@@ -16,13 +16,15 @@
 import tmt.log
 import tmt.steps
 import tmt.steps.execute
+import tmt.steps.prepare
+import tmt.steps.prepare.feature
 import tmt.steps.provision
 import tmt.templates
 import tmt.utils
 from tmt import Plan
 from tmt.base import RunData
 from tmt.steps.prepare import PreparePlugin
-from tmt.utils import MetadataError, Path
+from tmt.utils import GeneralError, MetadataError, Path
 
 USER_PLAN_NAME = "/user/plan"
 
@@ -416,12 +418,23 @@ def handle_epel(self, plan: Plan) -> None:
         """ Enable EPEL repository """
 
         # tmt run prepare --how feature --epel enabled
-        from tmt.steps.prepare.feature import PrepareFeatureData
-
-        data = PrepareFeatureData(
+        # cast: linters do not detect the class `get_class_data()`
+        # returns, it's reported as `type[Unknown]`. mypy does not care,
+        # pyright does.
+        prepare_data_class = cast(  # type: ignore[redundant-cast]
+            type[tmt.steps.prepare.feature.PrepareFeatureData],
+            tmt.steps.prepare.feature.PrepareFeature.get_data_class())
+
+        if not tmt.utils.container_has_field(prepare_data_class, 'epel'):
+            raise GeneralError("Feature 'epel' is not available.")
+
+        # ignore[reportCallIssue,call-arg,unused-ignore]: thanks to
+        # dynamic nature of the data class, the field is indeed unknown
+        # to type checkers.
+        data = prepare_data_class(
             name="tmt-try-epel",
             how='feature',
-            epel="enabled")
+            epel="enabled")  # type: ignore[reportCallIssue,call-arg,unused-ignore]
 
         phase: PreparePlugin[Any] = cast(
             PreparePlugin[Any],

tmt/utils/__init__.py

@@ -3038,6 +3038,10 @@ def container_fields(container: Container) -> Iterator[dataclasses.Field[Any]]:
     yield from dataclasses.fields(container)
 
 
+def container_has_field(container: Container, key: str) -> bool:
+    return key in list(container_keys(container))
+
+
 def container_keys(container: Container) -> Iterator[str]:
     """ Iterate over key names in a container """