From 9183d7ddec039fcebf0e35fc8433fe273ddcdcf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Hells=C3=A9n?= Date: Sun, 31 May 2026 04:15:04 +0200 Subject: [PATCH] Recommend feeds when creating new feed if broken --- discord_rss_bot/feeds.py | 7 +- discord_rss_bot/main.py | 97 +++++++++++++++++++-- discord_rss_bot/settings.py | 28 +++++- discord_rss_bot/templates/add.html | 39 +++++++-- tests/test_main.py | 41 ++++++++- tests/test_settings.py | 135 +++++++++++++++++++++++++++++ 6 files changed, 328 insertions(+), 19 deletions(-) diff --git a/discord_rss_bot/feeds.py b/discord_rss_bot/feeds.py index 833bf33..9a54535 100644 --- a/discord_rss_bot/feeds.py +++ b/discord_rss_bot/feeds.py @@ -79,6 +79,10 @@ type SentWebhookRecord = dict[str, JsonValue] type UpdateCallback = Callable[[], UpdatedFeed | None] +class FeedUpdateError(HTTPException): + """Raised when the initial update for a newly added feed fails.""" + + class JsonResponseLike(Protocol): """Response interface needed for Discord webhook JSON parsing.""" @@ -1899,6 +1903,7 @@ def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None: webhook_dropdown: The webhook we should send entries to. Raises: + FeedUpdateError: If the initial feed update fails. HTTPException: If webhook_dropdown does not equal a webhook or default_custom_message not found. """ clean_feed_url: str = feed_url.strip() @@ -1929,7 +1934,7 @@ def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None: try: reader.update_feed(clean_feed_url) except ReaderError as e: - raise HTTPException(status_code=404, detail=f"Error updating feed: {e}") from e + raise FeedUpdateError(status_code=404, detail=f"Error updating feed: {e}") from e # Mark every entry as read, so we don't send all the old entries to Discord. entries: Iterable[Entry] = reader.get_entries(feed=clean_feed_url, read=False) diff --git a/discord_rss_bot/main.py b/discord_rss_bot/main.py index a27199b..13b132b 100644 --- a/discord_rss_bot/main.py +++ b/discord_rss_bot/main.py @@ -52,6 +52,7 @@ from discord_rss_bot.custom_message import get_embed from discord_rss_bot.custom_message import get_first_image from discord_rss_bot.custom_message import replace_tags_in_text_message from discord_rss_bot.custom_message import save_embed +from discord_rss_bot.feeds import FeedUpdateError from discord_rss_bot.feeds import SentWebhookRecord from discord_rss_bot.feeds import coerce_media_gallery_image_limit from discord_rss_bot.feeds import create_feed @@ -122,6 +123,12 @@ class FilterPreviewContext(TypedDict): preview_helper_text: str +class AutodiscoverLink(TypedDict): + href: str + type: str | None + title: str | None + + LOGGING_CONFIG = { "version": 1, "disable_existing_loggers": False, @@ -262,6 +269,57 @@ templates.env.globals["get_backup_path"] = get_backup_path # pyright: ignore[re templates.env.globals["has_webhooks"] = has_webhooks # pyright: ignore[reportArgumentType] +def get_global_delivery_mode(reader: Reader) -> str: + """Return the normalized default delivery mode for new feeds. + + Args: + reader: The Reader instance. + + Returns: + The configured delivery mode, falling back to embed. + """ + global_delivery_mode: str = str(reader.get_tag((), "delivery_mode", "embed")).strip().lower() + return global_delivery_mode if global_delivery_mode in {"embed", "text"} else "embed" + + +def get_autodiscover_links(reader: Reader, feed_url: str) -> list[AutodiscoverLink]: + """Return valid autodiscovered links stored for a failed feed update. + + Args: + reader: The Reader instance. + feed_url: The URL that failed to parse as a feed. + + Returns: + Valid discovered feed link dictionaries. + """ + try: + stored_links = reader.get_tag(feed_url, ".reader.autodiscover", []) + except ReaderError: + return [] + + if not isinstance(stored_links, list): + return [] + + links: list[AutodiscoverLink] = [] + for stored_link in stored_links: + if not isinstance(stored_link, dict): + continue + + href = stored_link.get("href") + if not isinstance(href, str) or not href: + continue + + link_type = stored_link.get("type") + title = stored_link.get("title") + links.append({ + "href": href, + "type": link_type if isinstance(link_type, str) else None, + "title": title if isinstance(title, str) else None, + }) + + return links + + @app.post("/add_webhook") async def post_add_webhook( webhook_name: Annotated[str, Form()], @@ -357,24 +415,51 @@ async def post_delete_webhook( return RedirectResponse(url="/", status_code=303) -@app.post("/add") +@app.post("/add", response_model=None) async def post_create_feed( + request: Request, feed_url: Annotated[str, Form()], webhook_dropdown: Annotated[str, Form()], reader: Annotated[Reader, Depends(get_reader_dependency)], -) -> RedirectResponse: +) -> RedirectResponse | HTMLResponse: """Add a feed to the database. Args: + request: The request object. feed_url: The feed to add. webhook_dropdown: The webhook to use. reader: The Reader instance. Returns: RedirectResponse: Redirect to the feed page. + + Raises: + FeedUpdateError: If updating the feed fails without discovered feed links. """ clean_feed_url: str = feed_url.strip() - create_feed(reader, feed_url, webhook_dropdown) + try: + create_feed(reader, feed_url, webhook_dropdown) + except FeedUpdateError as exception: + autodiscover_links = get_autodiscover_links(reader, clean_feed_url) + if not autodiscover_links: + raise + + context = { + "request": request, + "webhooks": reader.get_tag((), "webhooks", []), + "global_delivery_mode": get_global_delivery_mode(reader), + "feed_url": clean_feed_url, + "selected_webhook": webhook_dropdown, + "messages": exception.detail, + "autodiscover_links": autodiscover_links, + } + return templates.TemplateResponse( + request=request, + name="add.html", + context=context, + status_code=exception.status_code, + ) + commit_state_change(reader, f"Add feed {clean_feed_url}") return RedirectResponse(url=f"/feed?feed_url={urllib.parse.quote(clean_feed_url)}", status_code=303) @@ -1609,14 +1694,10 @@ def get_add( Returns: HTMLResponse: The add feed page. """ - global_delivery_mode: str = str(reader.get_tag((), "delivery_mode", "embed")).strip().lower() - if global_delivery_mode not in {"embed", "text"}: - global_delivery_mode = "embed" - context = { "request": request, "webhooks": reader.get_tag((), "webhooks", []), - "global_delivery_mode": global_delivery_mode, + "global_delivery_mode": get_global_delivery_mode(reader), } return templates.TemplateResponse(request=request, name="add.html", context=context) diff --git a/discord_rss_bot/settings.py b/discord_rss_bot/settings.py index bb2b52a..2daf6d6 100644 --- a/discord_rss_bot/settings.py +++ b/discord_rss_bot/settings.py @@ -3,6 +3,7 @@ from __future__ import annotations import os import typing from functools import lru_cache +from importlib.util import find_spec from pathlib import Path from platformdirs import user_data_dir @@ -31,6 +32,31 @@ default_custom_embed: dict[str, str] = { } +def has_plugin(plugin_name: str) -> bool: + """Return whether the installed reader version provides a built-in plugin. + + We started using .autodiscover, but that is from Reader version 3.25. + """ + try: + return find_spec(f"reader.plugins.{plugin_name.removeprefix('.')}") is not None + except ModuleNotFoundError: + return False + + +def make_app_reader(db_location: Path) -> Reader: + """Create a reader with plugins supported by the installed reader version. + + Returns: + The configured reader. + """ + plugins_we_want = (".ua_fallback", ".autodiscover") + plugins: list[str] = [name for name in plugins_we_want if has_plugin(name)] + + if plugins: + return make_reader(url=str(db_location), plugins=plugins) + return make_reader(url=str(db_location)) + + @lru_cache(maxsize=1) def get_reader(custom_location: Path | None = None) -> Reader: """Get the reader. @@ -42,7 +68,7 @@ def get_reader(custom_location: Path | None = None) -> Reader: The reader. """ db_location: Path = custom_location or Path(data_dir) / "db.sqlite" - reader: Reader = make_reader(url=str(db_location)) + reader: Reader = make_app_reader(db_location) # https://reader.readthedocs.io/en/latest/api.html#reader.types.UpdateConfig # Set the default update interval to 15 minutes if not already configured diff --git a/discord_rss_bot/templates/add.html b/discord_rss_bot/templates/add.html index 760a228..d8b61dd 100644 --- a/discord_rss_bot/templates/add.html +++ b/discord_rss_bot/templates/add.html @@ -8,11 +8,6 @@ {% block content %}
-
- New feeds currently default to - {{ global_delivery_mode }} - delivery mode. -
@@ -20,7 +15,8 @@ + id="feed_url" + value="{{ feed_url }}" />
@@ -30,8 +26,11 @@ @@ -41,4 +40,28 @@ + {% if autodiscover_links %} +
+
+

Discovered feed links

+

The submitted URL is not a feed. Choose one of the feeds advertised by that page.

+
+ {% for link in autodiscover_links %} +
+ + +
+ {% if link.title %}
{{ link.title }}
{% endif %} + {{ link.href }} + {% if link.type %}
{{ link.type }}
{% endif %} +
+ +
+ {% endfor %} +
+
+
+ {% endif %} {% endblock content %} diff --git a/tests/test_main.py b/tests/test_main.py index d638da2..c8874d3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -188,6 +188,46 @@ def test_create_feed() -> None: assert encoded_feed_url(feed_url) in response.text, f"Feed not found in /: {response.text}" +def test_create_feed_suggests_autodiscovered_links() -> None: + """A page URL that fails to parse should render its advertised feed links.""" + submitted_url = "https://example.com/blog" + discovered_url = "https://example.com/rss.xml" + stub_reader = MagicMock() + + def get_tag(resource: str | tuple[()], key: str, default: TestTagValue = None) -> TestTagValue: + if resource == () and key == "webhooks": + return [{"name": webhook_name, "url": webhook_url}] + if resource == () and key == "delivery_mode": + return "embed" + if resource == submitted_url and key == ".reader.autodiscover": + return [{"href": discovered_url, "title": "Example feed", "type": "application/rss+xml"}] + return default + + stub_reader.get_tag.side_effect = get_tag + app.dependency_overrides[get_reader_dependency] = lambda: stub_reader + + try: + with patch.object( + main_module, + "create_feed", + side_effect=feeds.FeedUpdateError(status_code=404, detail="Error updating feed"), + ): + response: Response = client.post( + url="/add", + data={"feed_url": submitted_url, "webhook_dropdown": webhook_name}, + ) + finally: + app.dependency_overrides = {} + + assert response.status_code == 404 + assert "Discovered feed links" in response.text + assert "Example feed" in response.text + assert discovered_url in response.text + assert "application/rss+xml" in response.text + assert f'value="{submitted_url}"' in response.text + assert f'value="{webhook_name}"' in response.text + + def test_get() -> None: """Test the /create_feed page.""" # Ensure webhook exists for this test regardless of test order. @@ -572,7 +612,6 @@ def test_add_page_shows_global_default_delivery_mode_hint() -> None: response = client.get(url="/add") assert response.status_code == 200, f"/add failed: {response.text}" - assert "New feeds currently default to" in response.text assert "text" in response.text diff --git a/tests/test_settings.py b/tests/test_settings.py index fa03157..98f68a1 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -2,13 +2,62 @@ from __future__ import annotations import pathlib import tempfile +from contextlib import closing +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler +from http.server import ThreadingHTTPServer from pathlib import Path +from threading import Thread +from typing import TYPE_CHECKING +from unittest.mock import MagicMock +import pytest +from reader import ParseError from reader import Reader +import discord_rss_bot.settings as settings_module from discord_rss_bot.settings import data_dir from discord_rss_bot.settings import default_custom_message from discord_rss_bot.settings import get_reader +from discord_rss_bot.settings import has_plugin +from discord_rss_bot.settings import make_app_reader + +if TYPE_CHECKING: + from collections.abc import Iterator + + +class _AutodiscoverHandler(BaseHTTPRequestHandler): + """Serve an HTML page that advertises an RSS feed.""" + + def do_GET(self) -> None: + """Return HTML instead of a feed so reader attempts autodiscovery.""" + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.end_headers() + self.wfile.write( + b'' + ) + + def log_message(self, _format: str, *_args: object) -> None: + """Suppress HTTP request logging during tests.""" + + +@contextmanager +def _serve_autodiscover_html() -> Iterator[str]: + """Serve an HTML page URL while the context is active. + + Yields: + The URL of the HTML page. + """ + with ThreadingHTTPServer(("127.0.0.1", 0), _AutodiscoverHandler) as server: + server_thread = Thread(target=server.serve_forever, daemon=True) + server_thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}/" + finally: + server.shutdown() + server_thread.join() def test_reader() -> None: @@ -141,3 +190,89 @@ def test_get_reader_preserves_existing_global_delivery_mode() -> None: second_reader.close() get_reader.cache_clear() + + +def test_get_reader_enables_autodiscover_plugin() -> None: + """get_reader should store advertised feed links when HTML parsing fails.""" + get_reader.cache_clear() + + try: + with ( + tempfile.TemporaryDirectory() as temp_dir, + _serve_autodiscover_html() as feed_url, + closing(get_reader(custom_location=Path(temp_dir, "autodiscover_db.sqlite"))) as reader, + ): + reader.add_feed(feed_url) + + with pytest.raises(ParseError): + reader.update_feed(feed_url) + + assert reader.get_tag(feed_url, ".reader.autodiscover", None) == [ + { + "href": f"{feed_url}rss.xml", + "type": "application/rss+xml", + "title": "Example", + } + ] + finally: + get_reader.cache_clear() + + +def test_make_app_reader_enables_supported_builtin_plugins(monkeypatch: pytest.MonkeyPatch) -> None: + """Supported reader versions should load both built-in plugins explicitly.""" + reader = object() + make_reader = MagicMock(return_value=reader) + monkeypatch.setattr(settings_module, "has_plugin", lambda _plugin_name: True) + monkeypatch.setattr(settings_module, "make_reader", make_reader) + + assert make_app_reader(Path("db.sqlite")) is reader + make_reader.assert_called_once_with( + url="db.sqlite", + plugins=[".ua_fallback", ".autodiscover"], + ) + + +@pytest.mark.parametrize( + ("available_plugin", "expected_plugin"), + [ + (".ua_fallback", ".ua_fallback"), + (".autodiscover", ".autodiscover"), + ], +) +def test_make_app_reader_loads_only_available_builtin_plugin( + monkeypatch: pytest.MonkeyPatch, + available_plugin: str, + expected_plugin: str, +) -> None: + """Reader construction should not receive unavailable built-in plugins.""" + reader = object() + make_reader = MagicMock(return_value=reader) + monkeypatch.setattr(settings_module, "has_plugin", lambda plugin_name: plugin_name == available_plugin) + monkeypatch.setattr(settings_module, "make_reader", make_reader) + + assert make_app_reader(Path("db.sqlite")) is reader + make_reader.assert_called_once_with(url="db.sqlite", plugins=[expected_plugin]) + + +def test_make_app_reader_preserves_defaults_without_builtin_plugins(monkeypatch: pytest.MonkeyPatch) -> None: + """Reader versions without built-in plugins should start with their defaults.""" + reader = object() + make_reader = MagicMock(return_value=reader) + monkeypatch.setattr(settings_module, "has_plugin", lambda _plugin_name: False) + monkeypatch.setattr(settings_module, "make_reader", make_reader) + + assert make_app_reader(Path("db.sqlite")) is reader + make_reader.assert_called_once_with(url="db.sqlite") + + +def test_has_plugin_handles_reader_versions_without_plugins_package( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Older reader versions without a plugins package should be supported.""" + + def find_spec(_name: str) -> None: + raise ModuleNotFoundError + + monkeypatch.setattr(settings_module, "find_spec", find_spec) + + assert has_plugin(".autodiscover") is False