From c99ec45a320194d8f05f6f7124656294e3cb8098 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Tue, 1 Nov 2022 18:53:43 +0100 Subject: [PATCH] Support multiple instances of the same plugin (#3027) Depending on what a plugin does, say if it "adds a thing" into the site, it can be reasonable to allow it to be specified multiple times in the `plugins:` list, so it can "add multiple things". Previously such a use case was completely not predicted in MkDocs, so it silently passes but is bugged - the plugin runs twice but with only one of the configs both times. So, this commit addresses that by registering a plugin `- foo:` as `'foo'`, and then if another plugin `- foo:` appears, it gets registered as `'foo #2'` (the name affects primarily just how it's referred to in warnings and errors). By default, a warning will appear from MkDocs anyway, unless the plugin adds a class variable `supports_multiple_instances = True`. --- mkdocs/config/config_options.py | 28 ++++++++--- mkdocs/plugins.py | 3 ++ mkdocs/tests/config/config_options_tests.py | 53 ++++++++++++++++++++- 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 44cb7492..1a035d19 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -9,7 +9,7 @@ import traceback import types import typing as t import warnings -from collections import UserString +from collections import Counter, UserString from typing import ( Any, Collection, @@ -18,6 +18,7 @@ from typing import ( Iterator, List, Mapping, + MutableMapping, NamedTuple, Tuple, TypeVar, @@ -933,9 +934,9 @@ class Plugins(OptionallyRequired[plugins.PluginCollection]): if not isinstance(value, (list, tuple, dict)): raise ValidationError('Invalid Plugins configuration. Expected a list or dict.') self.plugins = plugins.PluginCollection() + self._instance_counter: MutableMapping[str, int] = Counter() for name, cfg in self._parse_configs(value): - name, plugin = self.load_plugin_with_namespace(name, cfg) - self.plugins[name] = plugin + self.load_plugin_with_namespace(name, cfg) return self.plugins @classmethod @@ -981,7 +982,13 @@ class Plugins(OptionallyRequired[plugins.PluginCollection]): if not isinstance(config, dict): raise ValidationError(f"Invalid config options for the '{name}' plugin.") - plugin = self.plugin_cache.get(name) + self._instance_counter[name] += 1 + inst_number = self._instance_counter[name] + inst_name = name + if inst_number > 1: + inst_name += f' #{inst_number}' + + plugin = self.plugin_cache.get(inst_name) if plugin is None: plugin_cls = self.installed_plugins[name].load() @@ -994,21 +1001,28 @@ class Plugins(OptionallyRequired[plugins.PluginCollection]): plugin = plugin_cls() if hasattr(plugin, 'on_startup') or hasattr(plugin, 'on_shutdown'): - self.plugin_cache[name] = plugin + self.plugin_cache[inst_name] = plugin + + if inst_number > 1 and not getattr(plugin, 'supports_multiple_instances', False): + self.warnings.append( + f"Plugin '{name}' was specified multiple times - this is likely a mistake, " + "because the plugin doesn't declare `supports_multiple_instances`." + ) errors, warns = plugin.load_config( config, self._config.config_file_path if self._config else None ) for warning in warns: if isinstance(warning, str): - self.warnings.append(f"Plugin '{name}': {warning}") + self.warnings.append(f"Plugin '{inst_name}': {warning}") else: key, msg = warning - self.warnings.append(f"Plugin '{name}' option '{key}': {msg}") + self.warnings.append(f"Plugin '{inst_name}' option '{key}': {msg}") errors_message = '\n'.join(f"Plugin '{name}' option '{key}': {msg}" for key, msg in errors) if errors_message: raise ValidationError(errors_message) + self.plugins[inst_name] = plugin return plugin diff --git a/mkdocs/plugins.py b/mkdocs/plugins.py index 40cac267..63fdcb81 100644 --- a/mkdocs/plugins.py +++ b/mkdocs/plugins.py @@ -77,6 +77,9 @@ class BasePlugin(Generic[SomeConfig]): config_scheme: PlainConfigSchema = () config: SomeConfig = {} # type: ignore[assignment] + supports_multiple_instances: bool = False + """Set to true in subclasses to declare support for adding the same plugin multiple times.""" + def __class_getitem__(cls, config_class: Type[Config]): """Eliminates the need to write `config_class = FooConfig` when subclassing BasePlugin[FooConfig]""" name = f'{cls.__name__}[{config_class.__name__}]' diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index a34f5d60..3c72eb16 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1633,7 +1633,7 @@ class _FakePlugin2Config(_FakePluginConfig): class FakePlugin2(BasePlugin[_FakePlugin2Config]): - pass + supports_multiple_instances = True class ThemePlugin(BasePlugin[_FakePluginConfig]): @@ -1804,6 +1804,57 @@ class PluginsTest(TestCase): self.assertEqual(set(conf.plugins), {'overridden'}) self.assertIsInstance(conf.plugins['overridden'], FakePlugin2) + def test_plugin_config_with_multiple_instances(self, mock_class) -> None: + class Schema(Config): + theme = c.Theme(default='mkdocs') + plugins = c.Plugins(theme_key='theme') + + cfg = { + 'plugins': [ + {'sample2': {'foo': 'foo value', 'bar': 42}}, + {'sample2': {'foo': 'foo2 value'}}, + ], + } + conf = self.get_config(Schema, cfg) + + self.assertEqual( + set(conf.plugins), + {'sample2', 'sample2 #2'}, + ) + self.assertEqual(conf.plugins['sample2'].config['bar'], 42) + self.assertEqual(conf.plugins['sample2 #2'].config['bar'], 0) + + def test_plugin_config_with_multiple_instances_and_warning(self, mock_class) -> None: + class Schema(Config): + theme = c.Theme(default='mkdocs') + plugins = c.Plugins(theme_key='theme') + + test_cfgs: List[Dict[str, Any]] = [ + { + 'theme': 'readthedocs', + 'plugins': [{'sub_plugin': {}}, {'sample2': {}}, {'sub_plugin': {}}, 'sample2'], + }, + { + 'theme': 'readthedocs', + 'plugins': ['sub_plugin', 'sample2', 'sample2', 'sub_plugin'], + }, + ] + + for cfg in test_cfgs: + conf = self.get_config( + Schema, + cfg, + warnings=dict( + plugins="Plugin 'readthedocs/sub_plugin' was specified multiple times - " + "this is likely a mistake, because the plugin doesn't declare " + "`supports_multiple_instances`." + ), + ) + self.assertEqual( + set(conf.plugins), + {'readthedocs/sub_plugin', 'readthedocs/sub_plugin #2', 'sample2', 'sample2 #2'}, + ) + def test_plugin_config_empty_list_with_empty_default(self, mock_class) -> None: class Schema(Config): plugins = c.Plugins(default=[])