fix issues in existing code

Author: bhatia2akshit

reflex/app.py

@@ -89,12 +89,7 @@
     replace_brackets_with_keywords,
     verify_route_validity,
 )
-from reflex.sitemap import (
-    check_sitemap_file_exists,
-    generate_links_for_sitemap,
-    generate_static_sitemap,
-    read_sitemap_file,
-)
+from reflex.sitemap import generate_sitemaps, read_sitemap_file
 from reflex.state import (
     BaseState,
     RouterData,
@@ -470,12 +465,6 @@ def __post_init__(self):
         # Set up the admin dash.
         self._setup_admin_dash()
 
-        # sitemap generation
-        links = generate_links_for_sitemap(
-            pages=self.get_pages(), sitemap_properties=self.get_sitemap_properties()
-        )
-        generate_static_sitemap(links)
-
         if sys.platform == "win32" and not is_prod_mode():
             # Hack to fix Windows hot reload issue.
             from reflex.utils.compat import windows_hot_reload_lifespan_hack
@@ -611,14 +600,7 @@ async def serve_sitemap(self) -> Response:
         Returns:
             Response: An HTTP response with the XML sitemap content and the media type set to "application/xml".
         """
-        if not check_sitemap_file_exists():
-            links_sitemaps = generate_links_for_sitemap(
-                pages=self.get_pages(), sitemap_properties=self.get_sitemap_properties()
-            )
-            generate_static_sitemap(links_sitemaps)
-
         sitemaps = read_sitemap_file()
-
         return Response(content=sitemaps, media_type="application/xml")
 
     def _add_optional_endpoints(self):
@@ -766,7 +748,7 @@ def add_page(
             )
 
         # Setup dynamic args for the route.
-        # this state assignment is only required for tests using the deprecated state kwarg for App
+        # This state assignment is only required for tests using the deprecated state kwarg for App
         state = self._state if self._state else State
         state.setup_dynamic_args(get_route_args(route))
 
@@ -815,14 +797,6 @@ def _compile_page(self, route: str, save_page: bool = True):
         if save_page:
             self._pages[route] = component
 
-    def get_pages(self) -> dict[str, Component]:
-        """Get the pages of the app.
-
-        Returns:
-            The pages of the app.
-        """
-        return self._pages
-
     def get_sitemap_properties(self) -> Dict[str, Dict]:
         """Get the sitemap properties."""
         return self._sitemap_properties
@@ -1108,6 +1082,9 @@ def _compile(self, export: bool = False):
 
         self._pages = {}
 
+        # generate sitemaps from sitemap properties
+        generate_sitemaps(self._sitemap_properties)
+
         def get_compilation_time() -> str:
             return str(datetime.now().time()).split(".")[0]
 

reflex/sitemap.py

@@ -5,7 +5,7 @@
 from xml.dom import minidom
 from xml.etree.ElementTree import Element, SubElement, tostring
 
-from reflex import Component, constants
+from reflex import constants
 from reflex.config import get_config
 from reflex.utils import prerequisites
 
@@ -61,46 +61,54 @@ def generate_xml(links: List[Dict[str, str]]) -> str:
     return reparsed.toprettyxml(indent="  ")
 
 
+def generate_sitemaps(sitemap_config: Dict[str, Dict[str, str]]) -> None:
+    """Generate the sitemap.xml file.
+
+    This function generates the sitemap.xml file by crawling through the available pages in the app and generating a list
+    of links with their respective sitemap properties such as location (URL), change frequency, and priority. Dynamic
+    routes and the 404 page are excluded from the sitemap.
+
+    Args:
+        sitemap_config: A dictionary containing the sitemap properties for each route.
+    """
+    links = generate_links_for_sitemap(sitemap_config)
+    generate_static_sitemap(links)
+
+
 def generate_links_for_sitemap(
-    pages: Dict[str, Component], sitemap_properties: Dict[str, Dict]
+    sitemap_config: Dict[str, Dict[str, str]],
 ) -> List[dict[str, str]]:
     """Generate a list of links for which sitemaps are generated.
 
-    This function loops through the registered routes in the app and generates a list of
-    links with their respective sitemap properties such as location (URL), change frequency,
-    and priority. Dynamic routes and the 404 page are excluded from the sitemap.
+    This function loops through sitemap_config and generates a list of links with their respective sitemap properties
+    such as location (URL), change frequency, and priority. Dynamic routes and the 404 page are excluded from the
+    sitemap.
 
     Args:
-        pages: dictionary contains the components for each registered route in the app.
-        sitemap_properties: dictionary containing the sitemap properties for each route.
+        sitemap_config: A dictionary containing the sitemap properties for each route.
 
     Returns:
-        List: A list of dictionaries where each dictionary contains the 'loc'  (URL of the page), 'priority' and
+        List: A list of dictionaries where each dictionary contains the 'loc' (URL of the page), 'priority' and
         'changefreq' of each route.
     """
     links = []
 
     # find link of pages that are not dynamicaly created.
-    for route in pages:
+    for route in sitemap_config:
         # Ignore dynamic routes and 404
         if ("[" in route and "]" in route) or route == "404":
             continue
 
+        sitemap_changefreq = sitemap_config[route]["changefreq"]
+        sitemap_priority = sitemap_config[route]["priority"]
+
         # Handle the index route
         if route == "index":
             route = "/"
 
         if not route.startswith("/"):
             route = f"/{route}"
 
-        sitemap_changefreq = constants.DefaultPage.SITEMAP_CHANGEFREQ  # default value
-        sitemap_priority = constants.DefaultPage.SITEMAP_PRIORITY  # default value
-
-        # extract sitemap properties from the app's property, route exist in the compiled pages.
-        if route in sitemap_properties:
-            sitemap_priority = sitemap_properties[route]["priority"]
-            sitemap_changefreq = sitemap_properties[route]["changefreq"]
-
         if (
             sitemap_priority == constants.DefaultPage.SITEMAP_PRIORITY
         ):  # indicates that user didn't set priority
@@ -136,13 +144,3 @@ def generate_static_sitemap(links: List[Dict[str, str]]) -> None:
     # file.
     with _sitemap_file_path.open("w") as f:
         f.write(sitemap)
-
-
-def remove_sitemap_file() -> None:
-    """Remove the sitemap file, if a regeneration is needed.
-
-    Generally for testing the generation of sitemap.xml file, we need to remove the automatically generated file
-    when the app is initialized.
-    """
-    if _sitemap_file_path.exists():
-        _sitemap_file_path.unlink()

tests/units/test_sitemap.py

@@ -1,7 +1,6 @@
 import unittest.mock
 from pathlib import Path
 
-import fastapi
 import pytest
 import reflex as rx
 from reflex import Component, constants
@@ -10,7 +9,6 @@
     generate_links_for_sitemap,
     generate_static_sitemap,
     generate_xml,
-    remove_sitemap_file,
 )
 from reflex.utils import prerequisites
 
@@ -90,7 +88,7 @@ def test_generate_xml():
 
 
 def test_generate_static_sitemaps(app_instance, index_page, about_page):
-    """Test the creation of sitemap.xml through generate_static_sitemaps function."""
+    """Test if the generated sitemap file is currently stored in static website or not."""
     pages = {"index": index_page, "about": about_page}
     # remove the sitemap.xml file if it exists.
     sitemap_file_path.unlink(missing_ok=True)
@@ -104,49 +102,17 @@ def test_generate_static_sitemaps(app_instance, index_page, about_page):
         assert sitemap_file_path.exists()  # check if the sitemap.xml file exists.
 
 
-# Unit test for `serve_sitemap`
-@pytest.mark.asyncio
-async def test_serve_sitemap(app_instance, index_page, about_page):
-    """Test if the correct response is returned when the `serve_sitemap` method is called."""
-    pages = {"index": index_page, "about": about_page}
-
-    # because app_instance automatically generates sitemap.xml from empty _pages dictionary, we need to remove this file.
-    remove_sitemap_file()
-
-    # mock self._pages to return the dictionary pages.
-    with unittest.mock.patch.object(app_instance, "_pages", pages):
-        # Call the `serve_sitemap` method
-        response: fastapi.Response = await app_instance.serve_sitemap()
-
-        # Assert that the response is of type `Response`
-        assert isinstance(response, fastapi.Response)
-        # Assert the content type is 'application/xml'
-        assert response.media_type == "application/xml"
-
-        # Convert memoryview to bytes explicitly (if it's a memoryview)
-        if isinstance(response.body, memoryview):
-            response_body = response.body.tobytes().decode("utf-8")
-        else:
-            # If it's already bytes, decode directly
-            response_body = response.body.decode("utf-8")
-
-        # Assert that the XML content is correct (mocked value)
-        assert response_body == mock_xml
-
-
-def test_generate_links_for_sitemap(app_instance, index_page, about_page):
-    # Add pages to the app
-
-    pages = {"index": index_page, "about": index_page}
+def test_generate_links_for_sitemap():
+    """Test if the links are generated correctly for the sitemap from the sitemap config file when no deploy url is
+    given.
+    """
     sitemap_properties = {
         "index": {"priority": 0.9, "changefreq": "weekly"},
         "about": {"priority": 0.9, "changefreq": "weekly"},
     }
 
-    # mock self._pages to return the dictionary pages.
-    # with unittest.mock.patch.object(app_instance, "get_pages", pages) and unittest.mock.patch.object(app_instance, "_sitemap_properties", sitemap_properties):
+    links = generate_links_for_sitemap(sitemap_properties)
 
-    links = generate_links_for_sitemap(pages, sitemap_properties)
     # Assert that the links are generated correctly
     assert links == [
         {"loc": "http://localhost:3000/", "changefreq": "weekly", "priority": 0.9},
@@ -156,3 +122,28 @@ def test_generate_links_for_sitemap(app_instance, index_page, about_page):
             "priority": 0.9,
         },
     ]
+
+
+def test_generate_links_for_sitemap_deploy_url():
+    """Test if the links are generated correctly for the sitemap from the sitemap config file when a deploy url is
+    given.
+    """
+    sitemap_properties = {
+        "index": {"priority": 0.9, "changefreq": "weekly"},
+        "about": {"priority": 0.9, "changefreq": "weekly"},
+    }
+
+    with unittest.mock.patch("reflex.sitemap.get_config") as mock_get_config:
+        mock_get_config().deploy_url = "http://www.google.com"
+
+        links = generate_links_for_sitemap(sitemap_properties)
+
+        # Assert that the links are generated correctly
+        assert links == [
+            {"loc": "http://www.google.com/", "changefreq": "weekly", "priority": 0.9},
+            {
+                "loc": "http://www.google.com/about",
+                "changefreq": "weekly",
+                "priority": 0.9,
+            },
+        ]