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`.
This commit is contained in:
Oleh Prypin
2022-11-01 18:53:43 +01:00
committed by GitHub
parent 452c39eae2
commit c99ec45a32
3 changed files with 76 additions and 8 deletions

View File

@@ -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

View File

@@ -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__}]'

View File

@@ -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=[])