From 2082432c0f3d3ab13d3d693b8e14e5ed96c4f6cd Mon Sep 17 00:00:00 2001 From: Nathan Sprenkle Date: Mon, 19 Aug 2024 11:38:59 -0400 Subject: [PATCH] feat: XBlock overrides (#778) * feat: add ability to override XBlock with xblock.v1.overrides entry_point * test: xblock overrides * docs: update changelog entry and add tutorial for overriding XBlock * chore: bump version to 5.1.0 --------- Co-authored-by: Kyle McCormick --- CHANGELOG.rst | 7 ++ docs/xblock-tutorial/edx_platform/index.rst | 1 + .../edx_platform/overrides.rst | 75 +++++++++++++++++++ xblock/__init__.py | 2 +- xblock/plugin.py | 70 ++++++++++++++--- xblock/test/test_plugin.py | 36 ++++++++- 6 files changed, 180 insertions(+), 11 deletions(-) create mode 100644 docs/xblock-tutorial/edx_platform/overrides.rst diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6626a4786..6a5eff3a9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,13 @@ Change history for XBlock Unreleased ---------- +5.1.0 - 2024-08-07 +------------------ + +* added ability to override an XBlock with the 'xblock.v1.overrides' entry point +* added ability to override an XBlock Aside with the 'xblock_asides.v1.overrides' entry point + + 5.0.0 - 2024-05-30 ------------------ diff --git a/docs/xblock-tutorial/edx_platform/index.rst b/docs/xblock-tutorial/edx_platform/index.rst index c57b5eb59..5b1eba954 100644 --- a/docs/xblock-tutorial/edx_platform/index.rst +++ b/docs/xblock-tutorial/edx_platform/index.rst @@ -11,3 +11,4 @@ XBlocks and the edX Platform edx_lms devstack edx + overrides diff --git a/docs/xblock-tutorial/edx_platform/overrides.rst b/docs/xblock-tutorial/edx_platform/overrides.rst new file mode 100644 index 000000000..aa3eaf2de --- /dev/null +++ b/docs/xblock-tutorial/edx_platform/overrides.rst @@ -0,0 +1,75 @@ +.. _Replace a Preinstalled XBlock With a Custom Implementation: + +########################################################## +Replace a Preinstalled XBlock With a Custom Implementation +########################################################## + +In XBlock ``v5.1.0``, the ability was introduced to override an XBlock with a custom +implementation. + +This can be done by: + +1. Creating an XBlock in a new or existing package installed into ``edx-platform``. + +2. Adding the ``xblock.v1.overrides`` entry point in ``setup.py``, pointing to your + custom XBlock. + +This works with updated logic in ``load_class``'s ``default_select``, which gives +load priority to a class with the ``.overrides`` suffix. + +This can be disabled by providing a different ``select`` kwarg to ``load_class`` which +ignores or otherwise changes override logic. + +******* +Example +******* + +Imagine there is an XBlock installed ``edx-platform``: + +.. code:: python + + # edx-platform/xblocks/video_block.py + class VideoBlock(XBlock): + ... + + # edx-platform/setup.py + setup( + # ... + + entry_points={ + "xblock.v1": [ + "video = xblocks.video_block::VideoBlock" + # ... + ] + } + ) + +If you then create your own Python package with a custom version of that XBlock... + +.. code:: python + + # your_plugin/xblocks/video_block.py + class YourVideoBlock(XBlock): + ... + + # your_plugin/setup.py + setup( + # ... + entry_points={ + "xblock.v1.overrides": [ + "video = your_plugin.xblocks.video_block::YourVideoBlock" + # ... + ], + } + ) + +And install that package into your virtual environment, then your block should be +loaded instead of the existing implementation. + +.. note:: + + The ``load_class`` code will throw an error in the following cases: + + 1. There are multiple classes attempting to override one XBlock implementation. + + 2. There is an override provided where an existing XBlock implementation is not found. diff --git a/xblock/__init__.py b/xblock/__init__.py index 9ba6b90d0..cd74464e7 100644 --- a/xblock/__init__.py +++ b/xblock/__init__.py @@ -2,4 +2,4 @@ XBlock Courseware Components """ -__version__ = '5.0.0' +__version__ = '5.1.0' diff --git a/xblock/plugin.py b/xblock/plugin.py index 3f1574c4b..6b499946c 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -28,9 +28,17 @@ def __init__(self, all_entry_points): super().__init__(msg) -def default_select(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements +class AmbiguousPluginOverrideError(AmbiguousPluginError): + """Raised when a class name produces more than one override for an entry_point.""" + + +def _default_select_no_override(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ - Raise an exception when we have ambiguous entry points. + Selects plugin for the given identifier, raising on error: + + Raises: + - PluginMissingError when we don't have an entry point. + - AmbiguousPluginError when we have ambiguous entry points. """ if len(all_entry_points) == 0: @@ -41,6 +49,37 @@ def default_select(identifier, all_entry_points): # pylint: disable=inconsisten raise AmbiguousPluginError(all_entry_points) +def default_select(identifier, all_entry_points): + """ + Selects plugin for the given identifier with the ability for a Plugin to override + the default entry point. + + Raises: + - PluginMissingError when we don't have an entry point or entry point to override. + - AmbiguousPluginError when we have ambiguous entry points. + """ + + # Split entry points into overrides and non-overrides + overrides = [] + block_entry_points = [] + + for block_entry_point in all_entry_points: + if block_entry_point.group.endswith('.overrides'): + overrides.append(block_entry_point) + else: + block_entry_points.append(block_entry_point) + + # Get the default entry point + default_plugin = _default_select_no_override(identifier, block_entry_points) + + # If we have an unambiguous override, that gets priority. Otherwise, return default. + if len(overrides) == 1: + return overrides[0] + elif len(overrides) > 1: + raise AmbiguousPluginOverrideError(overrides) + return default_plugin + + class Plugin: """Base class for a system that uses entry_points to load plugins. @@ -75,12 +114,20 @@ def _load_class_entry_point(cls, entry_point): def load_class(cls, identifier, default=None, select=None): """Load a single class specified by identifier. - If `identifier` specifies more than a single class, and `select` is not None, - then call `select` on the list of entry_points. Otherwise, choose - the first one and log a warning. + By default, this returns the class mapped to `identifier` from entry_points + matching `{cls.entry_points}.overrides` or `{cls.entry_points}`, in that order. - If `default` is provided, return it if no entry_point matching - `identifier` is found. Otherwise, will raise a PluginMissingError + If multiple classes are found for either `{cls.entry_points}.overrides` or + `{cls.entry_points}`, it will raise an `AmbiguousPluginError`. + + If no classes are found for `{cls.entry_points}`, it will raise a `PluginMissingError`. + + Args: + - identifier: The class to match on. + + Kwargs: + - default: A class to return if no entry_point matching `identifier` is found. + - select: A function to override our default_select functionality. If `select` is provided, it should be a callable of the form:: @@ -100,7 +147,11 @@ def select(identifier, all_entry_points): if select is None: select = default_select - all_entry_points = list(importlib.metadata.entry_points(group=cls.entry_point, name=identifier)) + all_entry_points = [ + *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), + *importlib.metadata.entry_points(group=cls.entry_point, name=identifier) + ] + for extra_identifier, extra_entry_point in iter(cls.extra_entry_points): if identifier == extra_identifier: all_entry_points.append(extra_entry_point) @@ -146,7 +197,7 @@ def load_classes(cls, fail_silently=True): raise @classmethod - def register_temp_plugin(cls, class_, identifier=None, dist='xblock'): + def register_temp_plugin(cls, class_, identifier=None, dist='xblock', group='xblock.v1'): """Decorate a function to run with a temporary plugin available. Use it like this in tests:: @@ -164,6 +215,7 @@ def test_the_thing(): entry_point = Mock( dist=Mock(key=dist), load=Mock(return_value=class_), + group=group ) entry_point.name = identifier diff --git a/xblock/test/test_plugin.py b/xblock/test/test_plugin.py index c2832cdb1..08049fa04 100644 --- a/xblock/test/test_plugin.py +++ b/xblock/test/test_plugin.py @@ -6,7 +6,7 @@ from xblock.core import XBlock from xblock import plugin -from xblock.plugin import AmbiguousPluginError, PluginMissingError +from xblock.plugin import AmbiguousPluginError, AmbiguousPluginOverrideError, PluginMissingError class AmbiguousBlock1(XBlock): @@ -21,6 +21,10 @@ class UnambiguousBlock(XBlock): """A dummy class to find as a plugin.""" +class OverriddenBlock(XBlock): + """A dummy class to find as a plugin.""" + + @XBlock.register_temp_plugin(AmbiguousBlock1, "bad_block") @XBlock.register_temp_plugin(AmbiguousBlock2, "bad_block") @XBlock.register_temp_plugin(UnambiguousBlock, "good_block") @@ -52,6 +56,36 @@ def boom(identifier, entry_points): XBlock.load_class("bad_block", select=boom) +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(UnambiguousBlock, "overridden_block") +def test_plugin_override(): + # Trying to load a block that is overridden returns the correct override + override = XBlock.load_class("overridden_block") + assert override is OverriddenBlock + + +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block", group='xblock.v1.overrides') +def test_plugin_override_missing_original(): + # Trying to override a block that has no original block should raise an error + with pytest.raises(PluginMissingError, match="overridden_block"): + XBlock.load_class("overridden_block") + + +@XBlock.register_temp_plugin(AmbiguousBlock1, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(AmbiguousBlock2, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block") +def test_plugin_override_ambiguous(): + + # Trying to load a block that is overridden, but ambigous, errors. + expected_msg = ( + "Ambiguous entry points for overridden_block: " + "xblock.test.test_plugin.AmbiguousBlock1, " + "xblock.test.test_plugin.AmbiguousBlock2" + ) + with pytest.raises(AmbiguousPluginOverrideError, match=expected_msg): + XBlock.load_class("overridden_block") + + def test_nosuch_plugin(): # We can provide a default class to return for missing plugins. cls = XBlock.load_class("nosuch_block", default=UnambiguousBlock)