Skip to content

Commit

Permalink
manager: get rid of _plugin2hookcallers
Browse files Browse the repository at this point in the history
We can do everything necessary with just the `_name2plugin` dict; while
it makes some operations slower (registration and unregistration), they
are not in any hot path (calling) so not a problem.
  • Loading branch information
bluetech committed Jan 15, 2022
1 parent ddb9161 commit 12ea858
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 16 deletions.
35 changes: 23 additions & 12 deletions src/pluggy/_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ class PluginManager:
def __init__(self, project_name: str) -> None:
self.project_name: "Final" = project_name
self._name2plugin: "Final[Dict[str, _Plugin]]" = {}
self._plugin2hookcallers: "Final[Dict[_Plugin, List[_HookCaller]]]" = {}
self._plugin_distinfo: "Final[List[Tuple[_Plugin, DistFacade]]]" = []
self.trace: "Final" = _tracing.TagTracer().get("pluginmanage")
self.hook: "Final" = _HookRelay()
Expand All @@ -138,11 +137,17 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
is already registered."""
plugin_name = name or self.get_canonical_name(plugin)

if plugin_name in self._name2plugin or plugin in self._plugin2hookcallers:
if plugin_name in self._name2plugin:
if self._name2plugin.get(plugin_name, -1) is None:
return None # blocked plugin, return None to indicate no registration
raise ValueError(
"Plugin already registered: %s=%s\n%s"
"Plugin name already registered: %s=%s\n%s"
% (plugin_name, plugin, self._name2plugin)
)

if plugin in self._name2plugin.values():
raise ValueError(
"Plugin already registered under a different name: %s=%s\n%s"
% (plugin_name, plugin, self._name2plugin)
)

Expand All @@ -151,8 +156,6 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
self._name2plugin[plugin_name] = plugin

# register matching hook implementations of the plugin
hookcallers: List[_HookCaller] = []
self._plugin2hookcallers[plugin] = hookcallers
for name in dir(plugin):
hookimpl_opts = self.parse_hookimpl_opts(plugin, name)
if hookimpl_opts is not None:
Expand All @@ -168,7 +171,6 @@ def register(self, plugin: _Plugin, name: Optional[str] = None) -> Optional[str]
self._verify_hook(hook, hookimpl)
hook._maybe_apply_history(hookimpl)
hook._add_hookimpl(hookimpl)
hookcallers.append(hook)
return plugin_name

def parse_hookimpl_opts(
Expand Down Expand Up @@ -201,14 +203,16 @@ def unregister(
if plugin is None:
plugin = self.get_plugin(name)

hookcallers = self.get_hookcallers(plugin)
if hookcallers:
for hookcaller in hookcallers:
hookcaller._remove_plugin(plugin)

# if self._name2plugin[name] == None registration was blocked: ignore
if self._name2plugin.get(name):
assert name is not None
del self._name2plugin[name]

for hookcaller in self._plugin2hookcallers.pop(plugin, []):
hookcaller._remove_plugin(plugin)

return plugin

def set_blocked(self, name: str) -> None:
Expand Down Expand Up @@ -254,11 +258,11 @@ def parse_hookspec_opts(

def get_plugins(self) -> Set[Any]:
"""return the set of registered plugins."""
return set(self._plugin2hookcallers)
return set(self._name2plugin.values())

def is_registered(self, plugin: _Plugin) -> bool:
"""Return ``True`` if the plugin is already registered."""
return plugin in self._plugin2hookcallers
return any(plugin == val for val in self._name2plugin.values())

def get_canonical_name(self, plugin: _Plugin) -> str:
"""Return canonical name for a plugin object. Note that a plugin
Expand Down Expand Up @@ -373,7 +377,14 @@ def list_name_plugin(self) -> List[Tuple[str, _Plugin]]:

def get_hookcallers(self, plugin: _Plugin) -> Optional[List[_HookCaller]]:
"""get all hook callers for the specified plugin."""
return self._plugin2hookcallers.get(plugin)
if self.get_name(plugin) is None:
return None
hookcallers = []
for hookcaller in self.hook.__dict__.values():
for hookimpl in hookcaller.get_hookimpls():
if hookimpl.plugin is plugin:
hookcallers.append(hookcaller)
return hookcallers

def add_hookcall_monitoring(
self, before: _BeforeTrace, after: _AfterTrace
Expand Down
65 changes: 61 additions & 4 deletions testing/test_pluginmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,20 +153,25 @@ def he_method1(self):

def test_register(pm: PluginManager) -> None:
class MyPlugin:
pass
@hookimpl
def he_method1(self):
...

my = MyPlugin()
pm.register(my)
assert my in pm.get_plugins()
assert pm.get_plugins() == {my}
my2 = MyPlugin()
pm.register(my2)
assert {my, my2}.issubset(pm.get_plugins())
assert pm.get_plugins() == {my, my2}

assert pm.is_registered(my)
assert pm.is_registered(my2)
pm.unregister(my)
assert not pm.is_registered(my)
assert my not in pm.get_plugins()
assert pm.get_plugins() == {my2}

with pytest.raises(AssertionError, match=r"not registered"):
pm.unregister(my)


def test_register_unknown_hooks(pm: PluginManager) -> None:
Expand Down Expand Up @@ -468,6 +473,58 @@ class PluginNo:
assert hook_plugins == [plugin1, plugin2]


def test_get_hookcallers(pm: PluginManager) -> None:
class Hooks:
@hookspec
def he_method1(self):
...

@hookspec
def he_method2(self):
...

pm.add_hookspecs(Hooks)

class Plugin1:
@hookimpl
def he_method1(self):
...

@hookimpl
def he_method2(self):
...

class Plugin2:
@hookimpl
def he_method1(self):
...

class Plugin3:
@hookimpl
def he_method2(self):
...

plugin1 = Plugin1()
pm.register(plugin1)
plugin2 = Plugin2()
pm.register(plugin2)
plugin3 = Plugin3()
pm.register(plugin3)

hookcallers1 = pm.get_hookcallers(plugin1)
assert hookcallers1 is not None
assert len(hookcallers1) == 2
hookcallers2 = pm.get_hookcallers(plugin2)
assert hookcallers2 is not None
assert len(hookcallers2) == 1
hookcallers3 = pm.get_hookcallers(plugin3)
assert hookcallers3 is not None
assert len(hookcallers3) == 1
assert hookcallers1 == hookcallers2 + hookcallers3

assert pm.get_hookcallers(object()) is None


def test_add_hookspecs_nohooks(pm: PluginManager) -> None:
class NoHooks:
pass
Expand Down

0 comments on commit 12ea858

Please sign in to comment.