Merge pull request #91 from stuartmaxwell:plugin-improvements

Plugin-improvements

src/djpress/exceptions.py

@@ -7,3 +7,7 @@ class PostNotFoundError(Exception):
 
 class PageNotFoundError(Exception):
     """Exception raised when the page is not found in the database."""
+
+
+class PluginLoadError(Exception):
+    """Raised when plugins cannot be loaded."""

src/djpress/models/post.py

@@ -7,6 +7,7 @@
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.db.models import Max
+from django.db.transaction import on_commit
 from django.utils import timezone
 from django.utils.text import slugify
 
@@ -470,9 +471,9 @@ def save(self: "Post", *args, **kwargs) -> None:  # noqa: ANN002, ANN003
         self.full_clean()
         super().save(*args, **kwargs)
 
-        # If the post is a post and it's published, run the post_save_post hook
+        # If the post is a post and it's published, run the post_save_post hook after the transaction is committed
         if self.post_type == "post" and self.is_published:
-            registry.run_hook(Hooks.POST_SAVE_POST, self)
+            on_commit(lambda: registry.run_hook(Hooks.POST_SAVE_POST, self))
 
     def clean(self) -> None:
         """Custom validation for the Post model."""

src/djpress/plugins.py

@@ -7,6 +7,7 @@
 from django.utils.module_loading import import_string
 
 from djpress.conf import settings as djpress_settings
+from djpress.exceptions import PluginLoadError
 
 
 # Hook definitions
@@ -74,7 +75,16 @@ def run_hook(self, hook_name: Hooks, value: Any = None, *args: Any, **kwargs: An
 
         if hook_name in self.hooks:
             for callback in self.hooks[hook_name]:
-                value = callback(value, *args, **kwargs)
+                # Ruff warns us about performance issues running a try/except block in a loop, but I think this is
+                # acceptable since there won't be many hooks, and we don't want one crashing plugin to affect all the
+                # plugins. With this approach, if one fails, the `value` won't be modified and we'll continue to the
+                # next one.
+                try:
+                    callback_value = callback(value, *args, **kwargs)
+                    value = callback_value
+                except Exception:  # noqa: BLE001, PERF203, S112
+                    # Continue with the next callback
+                    continue
 
         return value
 
@@ -93,12 +103,16 @@ def load_plugins(self) -> None:
         plugin_names: list = djpress_settings.PLUGINS
         plugin_settings: dict = djpress_settings.PLUGIN_SETTINGS
 
-        for plugin_path in plugin_names:
-            plugin_class = self._import_plugin_class(plugin_path)
-            plugin = self._instantiate_plugin(plugin_class, plugin_path, plugin_settings)
-            self.plugins.append(plugin)
+        try:
+            for plugin_path in plugin_names:
+                plugin_class = self._import_plugin_class(plugin_path)
+                plugin = self._instantiate_plugin(plugin_class, plugin_path, plugin_settings)
+                self.plugins.append(plugin)
 
-        self._loaded = True
+            self._loaded = True
+        except Exception as exc:
+            msg = f"Failed to load plugins: {exc}"
+            raise PluginLoadError(msg) from exc
 
     def _import_plugin_class(self, plugin_path: str) -> type:
         """Import the plugin class from either custom path or standard location.

tests/test_plugins.py

@@ -1,6 +1,7 @@
 import pytest
 from djpress.plugins import PluginRegistry, DJPressPlugin, Hooks
 from djpress.plugins import registry
+from djpress.exceptions import PluginLoadError
 
 from djpress.conf import settings as djpress_settings
 
@@ -72,8 +73,35 @@ def test_callback(content: str) -> str:
         return content + " modified"
 
     registry.register_hook(Hooks.PRE_RENDER_CONTENT, test_callback)
-    result = registry.run_hook(Hooks.PRE_RENDER_CONTENT, "test")
-    assert result == "test modified"
+    result = registry.run_hook(Hooks.PRE_RENDER_CONTENT, "test value")
+    assert result == "test value modified"
+
+
+# Tests for hook execution (strict)
+def test_run_hook_with_multiple_hooks(clean_registry):
+    """Test running hook with multiple hooks."""
+
+    def test_callback(content: str) -> str:
+        return content + " modified by test_callback"
+
+    def second_test_callback(content: str) -> str:
+        return content + " modified by second test_callback"
+
+    registry.register_hook(Hooks.PRE_RENDER_CONTENT, test_callback)
+    registry.register_hook(Hooks.PRE_RENDER_CONTENT, second_test_callback)
+    result = registry.run_hook(Hooks.PRE_RENDER_CONTENT, "test value")
+    assert result == "test value modified by test_callback modified by second test_callback"
+
+
+def test_run_hook_with_exception(clean_registry):
+    """Test running hook with an exception doesn't modify the value."""
+
+    def test_callback(content: str) -> str:
+        raise ValueError("Test error")
+
+    registry.register_hook(Hooks.PRE_RENDER_CONTENT, test_callback)
+    result = registry.run_hook(Hooks.PRE_RENDER_CONTENT, "test value")
+    assert result == "test value"
 
 
 def test_run_hook_with_string_fails(clean_registry):
@@ -276,6 +304,34 @@ def mock_instantiate(self, plugin_class, path, settings):
     assert isinstance(registry.plugins[0], TestPlugin)
 
 
+def test_load_plugins_exception(clean_registry, monkeypatch):
+    """Test plugin loading with exception."""
+
+    class TestPlugin(DJPressPlugin):
+        name = "test"
+
+        def setup(self, registry):
+            pass
+
+    # Mock the import and instantiate methods to return known values
+    def mock_import(self, path):
+        return TestPlugin
+
+    def mock_instantiate(self, plugin_class, path, settings):
+        raise RuntimeError("Plugin setup failed")
+
+    monkeypatch.setattr(PluginRegistry, "_import_plugin_class", mock_import)
+    monkeypatch.setattr(PluginRegistry, "_instantiate_plugin", mock_instantiate)
+
+    # Set up test settings
+    monkeypatch.setattr(djpress_settings, "PLUGINS", ["test_plugin"])
+    monkeypatch.setattr(djpress_settings, "PLUGIN_SETTINGS", {})
+
+    # Load plugins - will raise exception
+    with pytest.raises(PluginLoadError):
+        registry.load_plugins()
+
+
 def test_plugin_missing_name():
     """Test plugin initialization with missing name."""
 

tests/test_views.py

@@ -213,7 +213,7 @@ def test_date_archives_year_no_posts(client, test_post1):
 def test_date_archives_month(client, settings, test_post1):
     assert settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] == "test-url-archives"
     url = get_archives_url(test_post1.date.year, test_post1.date.month)
-    assert url == f"/test-url-archives/{test_post1.date.year}/{test_post1.date.month}/"
+    assert url == f"/test-url-archives/{test_post1.date.year}/{test_post1.date.month:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -223,7 +223,7 @@ def test_date_archives_month(client, settings, test_post1):
 
     settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] = ""
     url = get_archives_url(test_post1.date.year, test_post1.date.month)
-    assert url == f"/{test_post1.date.year}/{test_post1.date.month}/"
+    assert url == f"/{test_post1.date.year}/{test_post1.date.month:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -256,7 +256,7 @@ def test_date_archives_month_no_posts(client, test_post1):
 def test_date_archives_day(client, settings, test_post1):
     assert settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] == "test-url-archives"
     url = get_archives_url(test_post1.date.year, test_post1.date.month, test_post1.date.day)
-    assert url == f"/test-url-archives/{test_post1.date.year}/{test_post1.date.month}/{test_post1.date.day}/"
+    assert url == f"/test-url-archives/{test_post1.date.year}/{test_post1.date.month:02}/{test_post1.date.day:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -266,7 +266,7 @@ def test_date_archives_day(client, settings, test_post1):
 
     settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] = ""
     url = get_archives_url(test_post1.date.year, test_post1.date.month, test_post1.date.day)
-    assert url == f"/{test_post1.date.year}/{test_post1.date.month}/{test_post1.date.day}/"
+    assert url == f"/{test_post1.date.year}/{test_post1.date.month:02}/{test_post1.date.day:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -277,7 +277,7 @@ def test_date_archives_day(client, settings, test_post1):
     assert settings.DJPRESS_SETTINGS["POST_PREFIX"] == "test-posts"
     settings.DJPRESS_SETTINGS["POST_PREFIX"] = "{{ year }}/{{ month }}/{{ day }}"
     url = get_archives_url(test_post1.date.year, test_post1.date.month, test_post1.date.day)
-    assert url == f"/{test_post1.date.year}/{test_post1.date.month}/{test_post1.date.day}/"
+    assert url == f"/{test_post1.date.year}/{test_post1.date.month:02}/{test_post1.date.day:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -301,7 +301,7 @@ def test_conflict_day_archives_and_single_post(client, settings, test_post1):
     settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] = ""
     settings.DJPRESS_SETTINGS["POST_PREFIX"] = "{{ year }}/{{ month }}"
     url = get_archives_url(test_post1.date.year, test_post1.date.month, test_post1.date.day)
-    assert url == f"/{test_post1.date.year}/{test_post1.date.month}/{test_post1.date.day}/"
+    assert url == f"/{test_post1.date.year}/{test_post1.date.month:02}/{test_post1.date.day:02}/"
 
     response = client.get(url)
     assert response.status_code == 200
@@ -322,7 +322,7 @@ def test_conflict_month_archives_and_single_post(client, settings, test_post1):
     settings.DJPRESS_SETTINGS["ARCHIVE_PREFIX"] = ""
     settings.DJPRESS_SETTINGS["POST_PREFIX"] = "{{ year }}"
     url = get_archives_url(test_post1.date.year, test_post1.date.month)
-    assert url == f"/{test_post1.date.year}/{test_post1.date.month}/"
+    assert url == f"/{test_post1.date.year}/{test_post1.date.month:02}/"
 
     response = client.get(url)
     assert response.status_code == 200