Prefer an installed plugin package over a same-named folder in the working directory

documentation/changelog.rst

@@ -147,6 +147,7 @@ Infrastructure / Support
 
 Bugfixes
 -----------
+* An installed plugin listed in ``FLEXMEASURES_PLUGINS`` by name is no longer shadowed by a folder of that name in the working directory (which is easily the case when starting FlexMeasures from the plugin's own repository), where the plugin appeared to load but its routes and CLI commands went missing; spell out a path (e.g. ``./my_plugin``) to load such a folder on purpose [see `PR #2419 <https://www.github.com/FlexMeasures/flexmeasures/pull/2419>`_]
 * Clear cached authentication state between tests, so that one test's login can no longer leak into later tests [see `PR #2424 <https://www.github.com/FlexMeasures/flexmeasures/pull/2424>`_]
 * Include the Excel reader in default installations so XLSX sensor-data uploads work outside test environments [see `PR #2376 <https://www.github.com/FlexMeasures/flexmeasures/pull/2376>`_]
 * In a multi-device flex-model, a device without a stock (e.g. a converter port or curtailable generator) silently disabled constraint validation for all devices after it; validation now covers every device, and also newly checks that each device's power bounds do not contradict each other, so a contradictory hard bound fails with a clear per-time-step message instead of a bare solver infeasibility [see `PR #2252 <https://www.github.com/FlexMeasures/flexmeasures/pull/2252>`_]

documentation/configuration.rst

@@ -102,8 +102,11 @@ This can be a Python list (e.g. ``["plugin1", "plugin2"]``) or a comma-separated
 
 Two types of entries are possible here:
 
-* File paths (absolute or relative) to plugins. Each such path needs to point to a folder, which should contain an ``__init__.py`` file where the Blueprint is defined. 
-* Names of installed Python modules. 
+* File paths (absolute or relative) to plugins. Each such path needs to point to a folder, which should contain an ``__init__.py`` file where the Blueprint is defined.
+* Names of installed Python modules.
+
+An entry that is not spelled out as a path (i.e. a bare name like ``my_plugin``) is loaded as an installed package if one goes by that name, even when a folder of the same name sits in the working directory ― which is easily the case when you start FlexMeasures from your plugin's own repository.
+To load such a folder instead, spell out its path, e.g. ``./my_plugin``.
 
 Added functionality in plugins needs to be based on Flask Blueprints. See :ref:`plugins` for more information and examples.
 

flexmeasures/utils/plugin_utils.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import importlib.machinery
 import importlib.util
 import os
 import sys
@@ -16,6 +17,33 @@
 from flexmeasures.utils.coding_utils import get_classes_module
 
 
+def is_written_as_path(plugin: str) -> bool:
+    """Whether this FLEXMEASURES_PLUGINS entry is spelled out as a file path.
+
+    A bare name like ``my_plugin`` is not: it may well name an installed package.
+    """
+    separators = [sep for sep in (os.sep, os.altsep) if sep is not None]
+    return os.path.isabs(plugin) or any(sep in plugin for sep in separators)
+
+
+def find_importable_package(pkg_name: str) -> importlib.machinery.ModuleSpec | None:
+    """Find the spec of an importable package, if there is one.
+
+    Namespace packages are ignored: a folder without an ``__init__.py`` is importable,
+    but accepting it here would shadow the clearer error that the file path branch of
+    ``register_plugins`` reports for such a folder.
+    """
+    try:
+        spec = importlib.util.find_spec(pkg_name)
+    except (ImportError, ValueError):
+        # ImportError: a dotted name whose parent package is missing.
+        # ValueError: a name that is in sys.modules without a spec.
+        return None
+    if spec is None or spec.origin is None:
+        return None
+    return spec
+
+
 def register_plugins(app: Flask):  # noqa: C901
     """
     Register FlexMeasures plugins as Blueprints.
@@ -29,6 +57,10 @@ def register_plugins(app: Flask):  # noqa: C901
 
     If you load a plugin via a file path, we'll refer to the plugin with the name of your plugin folder
     (last part of the path).
+
+    An entry that is not spelled out as a file path is imported as an installed package
+    if one goes by that name, even when a folder of the same name sits in the working
+    directory. To load such a folder instead, spell out its path (e.g. ``./my_plugin``).
     """
     plugins = app.config.get("FLEXMEASURES_PLUGINS", [])
     if isinstance(plugins, str):
@@ -45,10 +77,24 @@ def register_plugins(app: Flask):  # noqa: C901
         plugin_name = plugin.split("/")[-1]
         app.logger.info(f"Importing plugin {plugin_name} ...")
         module = None
-        if not os.path.exists(plugin):  # assume plugin is a package
-            pkg_name = os.path.split(plugin)[
-                -1
-            ]  # rule out attempts for relative package imports
+        pkg_name = os.path.split(plugin)[
+            -1
+        ]  # rule out attempts for relative package imports
+        # An installed package wins from a folder of the same name in the working directory,
+        # unless the entry is spelled out as a file path. Loading such a folder by path would
+        # execute its __init__.py a second time, under a new module object, while submodules
+        # imported by the first execution keep referring to the old one — so, for instance,
+        # routes end up on a Blueprint that is never registered. See GH issue #2415.
+        prefer_package = not is_written_as_path(plugin) and (
+            find_importable_package(pkg_name) is not None
+        )
+        if not os.path.exists(plugin) or prefer_package:  # assume plugin is a package
+            if prefer_package and os.path.exists(plugin):
+                app.logger.debug(
+                    f"Loading plugin {plugin_name} as an installed package,"
+                    f" ignoring the folder of the same name in the working directory."
+                    f" Spell out its path (e.g. '.{os.sep}{plugin}') to load that folder instead."
+                )
             app.logger.debug(
                 f"Attempting to import {pkg_name} as an installed package ..."
             )
@@ -60,6 +106,12 @@ def register_plugins(app: Flask):  # noqa: C901
                 )
                 continue
         else:  # assume plugin is a file path
+            if not is_written_as_path(plugin):
+                app.logger.warning(
+                    f"Loading plugin {plugin_name} from the folder of that name in the working directory,"
+                    f" as no installed package goes by that name."
+                    f" Spell out its path (e.g. '.{os.sep}{plugin}') to make this explicit."
+                )
             if not os.path.exists(os.path.join(plugin, "__init__.py")):
                 app.logger.error(
                     f"Plugin {plugin_name} is a valid file path, but does not contain an '__init__.py' file. Cannot load plugin {plugin_name}."

flexmeasures/utils/tests/test_plugin_utils.py

@@ -0,0 +1,170 @@
+"""Tests for loading plugins, i.e. for the FLEXMEASURES_PLUGINS setting."""
+
+from __future__ import annotations
+
+import os
+import sys
+
+import pytest
+from flask import Flask
+
+from flexmeasures.utils.plugin_utils import register_plugins
+
+
+def write_plugin(root, pkg_name: str, marker: str, with_init: bool = True):
+    """Write a minimal plugin package, whose Blueprint gets its route from a submodule.
+
+    This mirrors the common plugin layout: the Blueprint is created in ``__init__.py``,
+    and ``views.py`` imports it to attach routes. The marker distinguishes two copies
+    of the same plugin, and rides along on the module and its route.
+    """
+    pkg = root / pkg_name
+    pkg.mkdir(parents=True)
+    if not with_init:
+        return pkg
+    (pkg / "__init__.py").write_text(
+        "from flask import Blueprint\n"
+        f"MARKER = '{marker}'\n"
+        f"__version__ = '{marker}'\n"
+        f"bp = Blueprint('{pkg_name}_{marker}', __name__)\n"
+        f"import {pkg_name}.views  # noqa: E402,F401 (attaches the routes to bp)\n"
+    )
+    (pkg / "views.py").write_text(
+        f"from {pkg_name} import bp\n"
+        "\n"
+        f"@bp.route('/{marker}')\n"
+        "def a_route():\n"
+        f"    return '{marker}'\n"
+    )
+    return pkg
+
+
+@pytest.fixture
+def clean_import_state():
+    """Undo the sys.path and sys.modules changes that loading a plugin makes."""
+    original_path = list(sys.path)
+    original_modules = dict(sys.modules)
+    yield
+    sys.path[:] = original_path
+    for name in set(sys.modules) - set(original_modules):
+        del sys.modules[name]
+    sys.modules.update(original_modules)
+
+
+def make_app(plugins: list[str]) -> Flask:
+    app = Flask(__name__)
+    app.config["FLEXMEASURES_PLUGINS"] = plugins
+    return app
+
+
+def test_installed_package_wins_from_folder_in_working_directory(
+    tmp_path, monkeypatch, clean_import_state
+):
+    """A bare plugin name resolves to the installed package, not to a folder in the cwd.
+
+    Regression test for GH issue #2415: loading the folder executed ``__init__.py`` a second
+    time, so the routes that ``views.py`` had attached to the first Blueprint were lost, and
+    the Blueprint that got registered was empty. Both copies here define a route, so the
+    routing table tells us which module was loaded, and whether its routes survived.
+    """
+    installed = tmp_path / "site-packages"
+    write_plugin(installed, "my_plugin", marker="installed")
+    monkeypatch.syspath_prepend(str(installed))
+
+    working_directory = tmp_path / "plugin-repo"
+    write_plugin(working_directory, "my_plugin", marker="shadow")
+    monkeypatch.chdir(working_directory)
+    assert os.path.exists("my_plugin"), "the shadowing folder must be in the cwd"
+
+    app = make_app(["my_plugin"])
+    register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {"my_plugin": "installed"}
+    assert sys.modules["my_plugin"].MARKER == "installed"
+    routes = [str(rule) for rule in app.url_map.iter_rules()]
+    assert "/installed" in routes, "the installed plugin's route must be registered"
+    assert "/shadow" not in routes
+
+
+def test_folder_in_working_directory_is_loaded_when_nothing_is_installed(
+    tmp_path, monkeypatch, clean_import_state, caplog
+):
+    """Without an installed package of that name, a bare name still loads the cwd folder."""
+    working_directory = tmp_path / "plugin-repo"
+    write_plugin(working_directory, "lonely_plugin", marker="from-cwd")
+    monkeypatch.chdir(working_directory)
+    monkeypatch.syspath_prepend(str(working_directory))
+
+    app = make_app(["lonely_plugin"])
+    register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {"lonely_plugin": "from-cwd"}
+    assert "/from-cwd" in [str(rule) for rule in app.url_map.iter_rules()]
+
+
+def test_path_entry_still_loads_the_folder_it_points_to(
+    tmp_path, monkeypatch, clean_import_state
+):
+    """Spelling out a path loads that folder, even when a package of that name is installed."""
+    installed = tmp_path / "site-packages"
+    write_plugin(installed, "my_plugin", marker="installed")
+    monkeypatch.syspath_prepend(str(installed))
+
+    working_directory = tmp_path / "plugin-repo"
+    write_plugin(working_directory, "my_plugin", marker="by-path")
+    monkeypatch.chdir(working_directory)
+
+    app = make_app([f".{os.sep}my_plugin"])
+    register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {"my_plugin": "by-path"}
+    assert "/by-path" in [str(rule) for rule in app.url_map.iter_rules()]
+
+
+def test_absolute_path_entry_loads_the_folder_it_points_to(
+    tmp_path, monkeypatch, clean_import_state
+):
+    """An absolute path is a path, too, wherever the process happens to run from."""
+    plugin = write_plugin(tmp_path / "elsewhere", "my_plugin", marker="absolute")
+    monkeypatch.syspath_prepend(str(tmp_path / "elsewhere"))
+    monkeypatch.chdir(tmp_path)
+
+    app = make_app([str(plugin)])
+    register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {"my_plugin": "absolute"}
+
+
+def test_folder_without_init_file_reports_a_clear_error(
+    tmp_path, monkeypatch, clean_import_state, caplog
+):
+    """A folder without __init__.py is importable as a namespace package, but we don't.
+
+    Reporting the missing ``__init__.py`` is more useful than loading an empty namespace
+    package and then complaining that it defines no Blueprints.
+    """
+    working_directory = tmp_path / "plugin-repo"
+    write_plugin(working_directory, "no_init_plugin", marker="", with_init=False)
+    monkeypatch.chdir(working_directory)
+    monkeypatch.syspath_prepend(str(working_directory))
+
+    app = make_app(["no_init_plugin"])
+    with caplog.at_level("ERROR"):
+        register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {}
+    assert "does not contain an '__init__.py' file" in caplog.text
+
+
+def test_missing_plugin_reports_that_it_is_not_installed(
+    tmp_path, monkeypatch, clean_import_state, caplog
+):
+    """A name that is neither installed nor a folder is reported as not installed."""
+    monkeypatch.chdir(tmp_path)
+
+    app = make_app(["there_is_no_such_plugin"])
+    with caplog.at_level("ERROR"):
+        register_plugins(app)
+
+    assert app.config["LOADED_PLUGINS"] == {}
+    assert "it is not installed" in caplog.text