From e949682b6e90cb2b6ad4c6f3c42955534db24aa9 Mon Sep 17 00:00:00 2001
From: Joshua Tzucker <joshua.tz@gmail.com>
Date: Sun, 3 Nov 2024 22:05:49 -0800
Subject: [PATCH] Improve patt & asset path, SimpleStaticFileServer

---
 django_utils_lib/requests.py                  | 19 ++++++++++++++-----
 .../tests/test_simplestaticfileserver.py      |  7 ++-----
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/django_utils_lib/requests.py b/django_utils_lib/requests.py
index a3aa551..5f521b0 100644
--- a/django_utils_lib/requests.py
+++ b/django_utils_lib/requests.py
@@ -83,18 +83,25 @@ def guard_path(self, request: HttpRequest, url_path: str) -> Optional[HttpRespon
         return None
 
     def serve_static_path(
-        self, request: HttpRequest, asset_path: Optional[str] = None
+        self, request: HttpRequest, asset_path: str, url_path: Optional[str] = None
     ) -> Union[HttpResponse, FileResponse]:
         """
         This should be close to a drop-in replacement for `serve` (it wraps it),
         with config-based logic for how to handle the request
+
+        Note: The difference between `asset_path` and `url_path` is that `asset_path`
+        is the actual filesystem path (relative to staticfiles root) and `url_path`
+        is what the user sees as the path. They _can_ be different, but don't _have_
+        to be. A good use-case for having them different values is so that you can use
+        something like `/my_page/` as the `url_path`, but `/my_page/index.html` as the
+        `asset_path`.
         """
         if request.method not in ["GET", "HEAD", "OPTIONS"]:
             return HttpResponseNotAllowed(["GET", "HEAD", "OPTIONS"])
-        url_path = asset_path or request.path
+        url_path = url_path or request.path
         if (response := self.guard_path(request, url_path)) is not None:
             return response
-        return serve(request, document_root=str(settings.STATIC_ROOT), path=url_path)
+        return serve(request, document_root=str(settings.STATIC_ROOT), path=asset_path)
 
     def generate_url_patterns(self, ignore_start_strings: Optional[List[str]] = None):
         """
@@ -107,8 +114,10 @@ def generate_url_patterns(self, ignore_start_strings: Optional[List[str]] = None
             re_path(rf"^{negate_start_pattern}(?P<asset_path>.*\..*)$", self.serve_static_path),
             # For extension-less paths, try to map to an `index.html`
             re_path(
-                r"^(?P<asset_path>.+/$)",
-                lambda request, asset_path: self.serve_static_path(request, f"{asset_path}/index.html"),
+                r"^(?P<asset_path>[^?#]+).*$",
+                lambda request, asset_path: self.serve_static_path(
+                    request, f"{asset_path.removesuffix('/')}/index.html"
+                ),
             ),
         ]
 
diff --git a/django_utils_lib/tests/test_simplestaticfileserver.py b/django_utils_lib/tests/test_simplestaticfileserver.py
index fa05364..c2ed746 100644
--- a/django_utils_lib/tests/test_simplestaticfileserver.py
+++ b/django_utils_lib/tests/test_simplestaticfileserver.py
@@ -91,9 +91,6 @@ def check_response(
 
         check_response(server.serve_static_path(request=mock_request, asset_path=url_path), expected_response)
 
-        # Based on request alone
-        check_response(server.serve_static_path(request=mock_request), expected_response)
-
 
 @pytest.mark.parametrize(
     "ignore_start_strings, expected_patterns",
@@ -102,14 +99,14 @@ def check_response(
             None,
             [
                 re.compile(r"^(?!/static/)(?!/media/)(?P<asset_path>.*\..*)$"),
-                re.compile(r"^(?P<asset_path>.+/$)"),
+                re.compile(r"^(?P<asset_path>[^?#]+).*$"),
             ],
         ),
         (
             ["/assets/", "/files/"],
             [
                 re.compile(r"^(?!/assets/)(?!/files/)(?P<asset_path>.*\..*)$"),
-                re.compile(r"^(?P<asset_path>.+/$)"),
+                re.compile(r"^(?P<asset_path>[^?#]+).*$"),
             ],
         ),
     ],