diff --git a/packages/reflex-base/src/reflex_base/plugins/sitemap.py b/packages/reflex-base/src/reflex_base/plugins/sitemap.py index 9af0e3ebf9a..4eea2e0293d 100644 --- a/packages/reflex-base/src/reflex_base/plugins/sitemap.py +++ b/packages/reflex-base/src/reflex_base/plugins/sitemap.py @@ -2,6 +2,7 @@ import datetime from collections.abc import Sequence +from dataclasses import dataclass from pathlib import Path from types import SimpleNamespace from typing import TYPE_CHECKING, Literal, TypedDict @@ -16,6 +17,8 @@ if TYPE_CHECKING: from reflex.app import UnevaluatedPage +TrailingSlashOption = Literal["always", "never", "preserve"] + Location = str LastModified = datetime.datetime ChangeFrequency = Literal[ @@ -49,7 +52,11 @@ class Constants(SimpleNamespace): def configuration_with_loc( - *, config: SitemapLinkConfiguration, deploy_url: str | None, loc: Location + *, + config: SitemapLinkConfiguration, + deploy_url: str | None, + loc: Location, + trailing_slash: TrailingSlashOption, ) -> SitemapLink: """Set the 'loc' field of the configuration. @@ -57,12 +64,18 @@ def configuration_with_loc( config: The configuration dictionary. deploy_url: The deployment URL, if any. loc: The location to set. + trailing_slash: Option for handling trailing slashes in URLs. Returns: A SitemapLink dictionary with the 'loc' field set. """ if deploy_url and not loc.startswith("http://") and not loc.startswith("https://"): loc = f"{deploy_url.rstrip('/')}/{loc.lstrip('/')}" + if trailing_slash == "always" and not loc.endswith("/"): + loc += "/" + elif trailing_slash == "never": + stripped = loc.rstrip("/") + loc = stripped or loc link: SitemapLink = {"loc": loc} if (lastmod := config.get("lastmod")) is not None: link["lastmod"] = lastmod @@ -121,11 +134,13 @@ def is_route_dynamic(route: str) -> bool: def generate_links_for_sitemap( unevaluated_pages: Sequence["UnevaluatedPage"], + trailing_slash: TrailingSlashOption, ) -> list[SitemapLink]: """Generate sitemap links from unevaluated pages. Args: unevaluated_pages: Sequence of unevaluated pages. + trailing_slash: Option for handling trailing slashes in URLs. Returns: A list of SitemapLink dictionaries. @@ -159,12 +174,18 @@ def generate_links_for_sitemap( continue sitemap_link = configuration_with_loc( - config=sitemap_config, deploy_url=deploy_url, loc=loc + config=sitemap_config, + deploy_url=deploy_url, + loc=loc, + trailing_slash=trailing_slash, ) elif (loc := sitemap_config.get("loc")) is not None: sitemap_link = configuration_with_loc( - config=sitemap_config, deploy_url=deploy_url, loc=loc + config=sitemap_config, + deploy_url=deploy_url, + loc=loc, + trailing_slash=trailing_slash, ) else: @@ -172,31 +193,40 @@ def generate_links_for_sitemap( if not loc.startswith("/"): loc = "/" + loc sitemap_link = configuration_with_loc( - config=sitemap_config, deploy_url=deploy_url, loc=loc + config=sitemap_config, + deploy_url=deploy_url, + loc=loc, + trailing_slash=trailing_slash, ) links.append(sitemap_link) return links -def sitemap_task(unevaluated_pages: Sequence["UnevaluatedPage"]) -> tuple[str, str]: +def sitemap_task( + unevaluated_pages: Sequence["UnevaluatedPage"], trailing_slash: TrailingSlashOption +) -> tuple[str, str]: """Task to generate the sitemap XML file. Args: unevaluated_pages: Sequence of unevaluated pages. + trailing_slash: Option for handling trailing slashes in URLs. Returns: A tuple containing the file path and the generated XML content. """ return ( str(Constants.FILE_PATH), - generate_xml(generate_links_for_sitemap(unevaluated_pages)), + generate_xml(generate_links_for_sitemap(unevaluated_pages, trailing_slash)), ) +@dataclass(kw_only=True, frozen=True) class SitemapPlugin(PluginBase): """Sitemap plugin for Reflex.""" + trailing_slash: TrailingSlashOption = "preserve" + def pre_compile(self, **context): """Generate the sitemap XML file before compilation. @@ -204,7 +234,7 @@ def pre_compile(self, **context): context: The context for the plugin. """ unevaluated_pages = context.get("unevaluated_pages", []) - context["add_save_task"](sitemap_task, unevaluated_pages) + context["add_save_task"](sitemap_task, unevaluated_pages, self.trailing_slash) Plugin = SitemapPlugin diff --git a/tests/units/plugins/test_sitemap.py b/tests/units/plugins/test_sitemap.py index 6f45f63ccb1..4ab7a87fe28 100644 --- a/tests/units/plugins/test_sitemap.py +++ b/tests/units/plugins/test_sitemap.py @@ -118,7 +118,7 @@ def mock_component(): context={"sitemap": {"priority": 0.7, "changefreq": "monthly"}}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 3 assert {"loc": "https://example.com/"} in links assert {"loc": "https://example.com/about"} in links @@ -185,7 +185,7 @@ def mock_component(): context={"sitemap": {"changefreq": "yearly"}}, ), # Has sitemap config but no loc ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 expected_link = { "loc": "https://sub.example.org/custom-blog-path", @@ -237,7 +237,7 @@ def mock_component(): context={"sitemap": {"priority": 0.2}}, ), # Has sitemap config but no loc ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 assert {"loc": "/custom-404", "priority": 0.1} in links mock_warn.assert_called_once_with( @@ -279,7 +279,7 @@ def mock_component(): context={}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 assert {"loc": "/listed"} in links @@ -318,7 +318,7 @@ def mock_component(): context={"sitemap": {"loc": "/custom_pricing"}}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 2 assert {"loc": "https://override.com/features_page"} in links assert {"loc": "http://localhost:3000/custom_pricing"} in links @@ -368,7 +368,7 @@ def mock_component(): context={"sitemap": {"priority": 0.5}}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") expected_links = [ {"loc": "https://example.com/high_prio", "priority": 1.0}, {"loc": "https://example.com/low_prio", "priority": 0.0}, @@ -422,7 +422,7 @@ def mock_component(): context={}, ), # Special case for index ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 3 expected_links = [{"loc": "/home"}, {"loc": "/about"}, {"loc": "/"}] for expected_link in expected_links: @@ -455,7 +455,7 @@ def mock_component(): context={}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 assert {"loc": "https://example.com/testpage"} in links @@ -484,7 +484,7 @@ def mock_component(): context={"sitemap": {"loc": "/another"}}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 assert {"loc": "https://example.com/another"} in links @@ -513,6 +513,194 @@ def mock_component(): context={"sitemap": {"loc": "http://othersite.com/page"}}, ), ] - links = generate_links_for_sitemap(pages) + links = generate_links_for_sitemap(pages, trailing_slash="preserve") assert len(links) == 1 assert {"loc": "http://othersite.com/page"} in links + + +@patch("reflex_base.config.get_config") +def test_generate_links_trailing_slash_always(mock_get_config: MagicMock): + """Test that trailing_slash='always' appends a slash to all URLs. + + Args: + mock_get_config: Mock for the get_config function. + """ + mock_get_config.return_value.deploy_url = "https://example.com" + + def mock_component(): + return rx.text("Test") + + pages = [ + UnevaluatedPage( + component=mock_component, + route="index", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="about", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="contact", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={"sitemap": {"loc": "https://override.com/contact"}}, + ), + ] + links = generate_links_for_sitemap(pages, trailing_slash="always") + assert len(links) == 3 + assert {"loc": "https://example.com/"} in links + assert {"loc": "https://example.com/about/"} in links + assert {"loc": "https://override.com/contact/"} in links + + +@patch("reflex_base.config.get_config") +def test_generate_links_trailing_slash_never(mock_get_config: MagicMock): + """Test that trailing_slash='never' strips trailing slashes from all URLs. + + Args: + mock_get_config: Mock for the get_config function. + """ + mock_get_config.return_value.deploy_url = "https://example.com" + + def mock_component(): + return rx.text("Test") + + pages = [ + UnevaluatedPage( + component=mock_component, + route="index", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="about", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="docs", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={"sitemap": {"loc": "https://override.com/docs/"}}, + ), + ] + links = generate_links_for_sitemap(pages, trailing_slash="never") + assert len(links) == 3 + # index route becomes "/" which stripped is "" — but rstrip("/") on "https://example.com/" strips the path slash + assert {"loc": "https://example.com"} in links + assert {"loc": "https://example.com/about"} in links + assert {"loc": "https://override.com/docs"} in links + + +@patch("reflex_base.config.get_config") +def test_generate_links_trailing_slash_never_no_deploy_url_index( + mock_get_config: MagicMock, +): + """Test that trailing_slash='never' with no deploy_url preserves '/' for index. + + Args: + mock_get_config: Mock for the get_config function. + """ + mock_get_config.return_value.deploy_url = None + + def mock_component(): + return rx.text("Test") + + pages = [ + UnevaluatedPage( + component=mock_component, + route="index", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="about", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + ] + links = generate_links_for_sitemap(pages, trailing_slash="never") + assert len(links) == 2 + # "/" should not become "" — it should be preserved as "/" + assert {"loc": "/"} in links + assert {"loc": "/about"} in links + + +@patch("reflex_base.config.get_config") +def test_generate_links_trailing_slash_preserve(mock_get_config: MagicMock): + """Test that trailing_slash='preserve' does not modify URLs. + + Args: + mock_get_config: Mock for the get_config function. + """ + mock_get_config.return_value.deploy_url = "https://example.com" + + def mock_component(): + return rx.text("Test") + + pages = [ + UnevaluatedPage( + component=mock_component, + route="about", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={}, + ), + UnevaluatedPage( + component=mock_component, + route="docs", + title=None, + description=None, + image="favicon.ico", + on_load=None, + meta=[], + context={"sitemap": {"loc": "/docs/"}}, + ), + ] + links = generate_links_for_sitemap(pages, trailing_slash="preserve") + assert len(links) == 2 + # No slash added to about + assert {"loc": "https://example.com/about"} in links + # Existing trailing slash preserved on docs + assert {"loc": "https://example.com/docs/"} in links