feat: scheduled publish support, upload improvements

reeln_cloudflare_plugin/plugin.py

@@ -4,10 +4,12 @@
 
 import logging
 import os
+from pathlib import Path
 from typing import Any
 
 from reeln.models.auth import AuthCheckResult, AuthStatus
 from reeln.models.plugin_schema import ConfigField, PluginConfigSchema
+from reeln.plugins.capabilities import UploaderSkipped
 from reeln.plugins.hooks import Hook, HookContext
 from reeln.plugins.registry import HookRegistry
 
@@ -109,42 +111,56 @@ def register(self, registry: HookRegistry) -> None:
         registry.register(Hook.ON_GAME_FINISH, self.on_game_finish)
         registry.register(Hook.ON_POST_GAME_FINISH, self.on_post_game_finish)
 
-    def on_post_render(self, context: HookContext) -> None:
-        """Handle ``POST_RENDER`` — upload rendered video to R2."""
-        if not self._config.get("upload_video"):
-            return
+    def upload(
+        self, path: Path, *, metadata: dict[str, Any] | None = None
+    ) -> str:
+        """Upload a rendered video to Cloudflare R2 and return its public URL.
+
+        Implements the :class:`reeln.plugins.capabilities.Uploader` protocol
+        so the plugin can be used by ``reeln queue publish`` for truthful
+        per-target status reporting.
+
+        Raises:
+            UploaderSkipped: when ``upload_video`` is disabled in the plugin
+                config. The publish orchestrator maps this to
+                ``PublishStatus.SKIPPED`` (distinct from failure).
+            FileNotFoundError: when ``path`` does not exist on disk.
+            RuntimeError: when R2 credentials cannot be resolved from the
+                configured environment variables.
+            r2.R2Error: when the underlying R2 upload fails.
+        """
+        del metadata  # Not used today; accepted to satisfy Uploader protocol.
 
-        result = context.data.get("result")
-        if result is None:
-            log.warning("Cloudflare plugin: no result in context data, skipping")
-            return
+        if not self._config.get("upload_video"):
+            raise UploaderSkipped(
+                "upload_video disabled in cloudflare plugin config"
+            )
 
-        output = getattr(result, "output", None)
-        if output is None or not output.exists():
-            log.warning(
-                "Cloudflare plugin: output file not found: %s, skipping", output
+        if not path.exists():
+            raise FileNotFoundError(
+                f"Cloudflare upload source not found: {path}"
             )
-            return
 
         credentials = self._resolve_credentials()
         if credentials is None:
-            return
-
+            raise RuntimeError(
+                "Cloudflare R2 credentials not configured "
+                "(environment variables missing)"
+            )
         access_key_id, secret_access_key = credentials
 
         prefix = self._config.get("upload_prefix", "")
-        filename = output.name
-        key = f"{prefix}/{filename}" if prefix else filename
+        key = f"{prefix}/{path.name}" if prefix else path.name
 
         if self._config.get("dry_run"):
             base = self._config.get("public_url_base", "").rstrip("/")
-            url = f"{base}/{key}"
+            synthetic_url = f"{base}/{key}"
             log.info(
                 "Cloudflare plugin: [DRY RUN] would upload %s → %s",
-                output,
-                url,
+                path,
+                synthetic_url,
             )
-            return
+            return synthetic_url
 
         config = r2.R2Config(
             endpoint=self._config.get("r2_endpoint", ""),
@@ -156,15 +172,52 @@ def on_post_render(self, context: HookContext) -> None:
             upload_max_kbps=int(self._config.get("upload_max_kbps", 0)),
         )
 
+        public_url = r2.upload_file(config, path, key)
+        self._uploaded_keys.append(key)
+        log.info("Cloudflare plugin: uploaded %s → %s", path, public_url)
+        return public_url
+
+    def on_post_render(self, context: HookContext) -> None:
+        """Handle ``POST_RENDER`` — delegate to :meth:`upload`.
+
+        Preserves the auto-publish-during-render contract: exceptions are
+        swallowed (an upload failure must never break the render pipeline)
+        and ``context.shared["video_url"]`` is populated on real success
+        so downstream plugins (e.g. meta reels) can consume it.
+        Dry-run behavior is preserved: the synthetic URL is logged but
+        NOT written to ``context.shared`` — downstream plugins should not
+        believe a real object exists when one does not.
+        """
+        result = context.data.get("result")
+        if result is None:
+            log.warning(
+                "Cloudflare plugin: no result in context data, skipping"
+            )
+            return
+
+        output = getattr(result, "output", None)
+        if output is None or not output.exists():
+            log.warning(
+                "Cloudflare plugin: output file not found: %s, skipping",
+                output,
+            )
+            return
+
         try:
-            public_url = r2.upload_file(config, output, key)
-        except r2.R2Error as exc:
-            log.warning("Cloudflare plugin: upload failed (non-fatal): %s", exc)
+            public_url = self.upload(
+                output, metadata=context.data.get("publish_metadata")
+            )
+        except UploaderSkipped as exc:
+            log.info("Cloudflare plugin: %s", exc)
+            return
+        except Exception as exc:
+            log.warning(
+                "Cloudflare plugin: upload failed (non-fatal): %s", exc
+            )
             return
 
-        self._uploaded_keys.append(key)
-        context.shared["video_url"] = public_url
-        log.info("Cloudflare plugin: uploaded %s → %s", output, public_url)
+        if not self._config.get("dry_run"):
+            context.shared["video_url"] = public_url
 
     def _resolve_credentials(self) -> tuple[str, str] | None:
         """Read R2 credentials from environment variables named in config.

tests/unit/test_plugin.py

@@ -137,6 +137,40 @@ def test_feature_flag_missing(self) -> None:
         plugin.on_post_render(context)
         assert "video_url" not in context.shared
 
+    def test_feature_flag_disabled_with_valid_result_swallows_skipped(
+        self,
+        video_file: Path,
+        r2_env: None,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        """on_post_render catches UploaderSkipped from upload() and returns.
+
+        This covers the auto-publish-during-render path: when upload_video
+        is disabled but the render pipeline still reaches on_post_render
+        with a valid result, the wrapper must swallow UploaderSkipped
+        (a render failure would break the pipeline) and not populate
+        video_url.
+        """
+        plugin = CloudflarePlugin(
+            {
+                "upload_video": False,
+                "r2_endpoint": "https://x.r2.cloudflarestorage.com",
+                "r2_bucket": "b",
+                "r2_access_key_env": "R2_ACCESS_KEY_ID",
+                "r2_secret_key_env": "R2_SECRET_ACCESS_KEY",
+                "public_url_base": "https://cdn.example.com",
+            }
+        )
+        result = FakeRenderResult(output=video_file)
+        context = _make_context(data={"result": result})
+
+        with caplog.at_level(logging.INFO):
+            plugin.on_post_render(context)
+
+        assert "video_url" not in context.shared
+        # The UploaderSkipped message should be logged at INFO.
+        assert "upload_video" in caplog.text
+
 
 class TestOnPostRenderMissingData:
     def test_missing_result(
@@ -777,3 +811,195 @@ def test_hint_mentions_env_vars(self) -> None:
         assert "environment variables" in results[0].hint
         assert "r2_access_key_env" in results[0].hint
         assert "r2_secret_key_env" in results[0].hint
+
+
+# ------------------------------------------------------------------
+# upload() — Uploader protocol implementation (manual publish path)
+# ------------------------------------------------------------------
+
+
+class TestUpload:
+    """Tests for the ``upload()`` method used by ``reeln queue publish``.
+
+    These cover the new path where the publish orchestrator calls the
+    plugin directly (instead of emitting POST_RENDER). They must raise on
+    failure and return the public URL on success — see
+    ``reeln.core.queue.publish_queue_item``.
+    """
+
+    def test_upload_video_disabled_raises_skipped(
+        self,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        from reeln.plugins.capabilities import UploaderSkipped
+
+        plugin_config["upload_video"] = False
+        plugin = CloudflarePlugin(plugin_config)
+
+        with pytest.raises(UploaderSkipped, match="upload_video"):
+            plugin.upload(video_file)
+
+    def test_upload_video_missing_raises_skipped(
+        self,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        from reeln.plugins.capabilities import UploaderSkipped
+
+        del plugin_config["upload_video"]
+        plugin = CloudflarePlugin(plugin_config)
+
+        with pytest.raises(UploaderSkipped, match="upload_video"):
+            plugin.upload(video_file)
+
+    def test_upload_missing_source_raises_file_not_found(
+        self,
+        plugin_config: dict[str, Any],
+        tmp_path: Path,
+        r2_env: None,
+    ) -> None:
+        missing = tmp_path / "nonexistent.mp4"
+        plugin = CloudflarePlugin(plugin_config)
+
+        with pytest.raises(FileNotFoundError, match=r"nonexistent\.mp4"):
+            plugin.upload(missing)
+
+    def test_upload_missing_credentials_raises_runtime_error(
+        self,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        monkeypatch.delenv("R2_ACCESS_KEY_ID", raising=False)
+        monkeypatch.delenv("R2_SECRET_ACCESS_KEY", raising=False)
+        plugin = CloudflarePlugin(plugin_config)
+
+        with pytest.raises(RuntimeError, match="credentials"):
+            plugin.upload(video_file)
+
+    def test_upload_dry_run_returns_synthetic_url(
+        self,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        plugin_config["dry_run"] = True
+        plugin = CloudflarePlugin(plugin_config)
+
+        with patch("reeln_cloudflare_plugin.r2.upload_file") as mock_upload:
+            url = plugin.upload(video_file)
+
+        assert url == "https://cdn.example.com/highlight.mp4"
+        mock_upload.assert_not_called()
+        assert plugin._uploaded_keys == []
+
+    def test_upload_dry_run_strips_trailing_slash_on_base(
+        self,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        plugin_config["dry_run"] = True
+        plugin_config["public_url_base"] = "https://cdn.example.com/"
+        plugin = CloudflarePlugin(plugin_config)
+
+        url = plugin.upload(video_file)
+
+        assert url == "https://cdn.example.com/highlight.mp4"
+
+    @patch("reeln_cloudflare_plugin.r2.upload_file")
+    def test_upload_success_returns_real_url(
+        self,
+        mock_upload: Any,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        mock_upload.return_value = "https://cdn.example.com/highlight.mp4"
+        plugin = CloudflarePlugin(plugin_config)
+
+        url = plugin.upload(video_file)
+
+        assert url == "https://cdn.example.com/highlight.mp4"
+        mock_upload.assert_called_once()
+        assert plugin._uploaded_keys == ["highlight.mp4"]
+
+    @patch("reeln_cloudflare_plugin.r2.upload_file")
+    def test_upload_with_prefix_uses_prefixed_key(
+        self,
+        mock_upload: Any,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        mock_upload.return_value = "https://cdn.example.com/reels/highlight.mp4"
+        plugin_config["upload_prefix"] = "reels"
+        plugin = CloudflarePlugin(plugin_config)
+
+        plugin.upload(video_file)
+
+        call_args = mock_upload.call_args
+        assert call_args[0][2] == "reels/highlight.mp4"
+        assert plugin._uploaded_keys == ["reels/highlight.mp4"]
+
+    @patch("reeln_cloudflare_plugin.r2.upload_file")
+    def test_upload_accepts_metadata_kwarg(
+        self,
+        mock_upload: Any,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        """The Uploader protocol requires a metadata kwarg; we accept and ignore it."""
+        mock_upload.return_value = "https://cdn.example.com/highlight.mp4"
+        plugin = CloudflarePlugin(plugin_config)
+
+        url = plugin.upload(
+            video_file,
+            metadata={"title": "Goal!", "description": "What a shot"},
+        )
+
+        assert url == "https://cdn.example.com/highlight.mp4"
+
+    @patch("reeln_cloudflare_plugin.r2.upload_file")
+    def test_upload_r2_error_propagates(
+        self,
+        mock_upload: Any,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        mock_upload.side_effect = R2Error("bucket not found")
+        plugin = CloudflarePlugin(plugin_config)
+
+        with pytest.raises(R2Error, match="bucket not found"):
+            plugin.upload(video_file)
+
+        assert plugin._uploaded_keys == []
+
+    @patch("reeln_cloudflare_plugin.r2.upload_file")
+    def test_upload_passes_config_to_r2(
+        self,
+        mock_upload: Any,
+        plugin_config: dict[str, Any],
+        video_file: Path,
+        r2_env: None,
+    ) -> None:
+        mock_upload.return_value = "https://cdn.example.com/highlight.mp4"
+        plugin_config["r2_region"] = "us-east-1"
+        plugin_config["upload_max_kbps"] = 1000
+        plugin = CloudflarePlugin(plugin_config)
+
+        plugin.upload(video_file)
+
+        r2_config = mock_upload.call_args[0][0]
+        assert r2_config.region == "us-east-1"
+        assert r2_config.upload_max_kbps == 1000
+        assert r2_config.endpoint == "https://account-id.r2.cloudflarestorage.com"
+        assert r2_config.bucket == "test-bucket"
+        assert r2_config.public_url_base == "https://cdn.example.com"
+        assert r2_config.access_key_id == "test-access-key"
+        assert r2_config.secret_access_key == "test-secret-key"