Consistent treatment of format in route capturing, refs #1667

Also refs #1660
simonw · Mar 19, 2022 · b9c2b1c · b9c2b1c
1 parent 6141938
commit b9c2b1c
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 30 deletions.
diff --git a/datasette/app.py b/datasette/app.py
@@ -985,10 +985,7 @@ def _routes(self):
         def add_route(view, regex):
             routes.append((regex, view))
 
-        # Generate a regex snippet to match all registered renderer file extensions
-        renderer_regex = "|".join(r"\." + key for key in self.renderers.keys())
-
-        add_route(IndexView.as_view(self), r"/(?P<format>(\.jsono?)?$)")
+        add_route(IndexView.as_view(self), r"/(\.(?P<format>jsono?))?$")
         # TODO: /favicon.ico and /-/static/ deserve far-future cache expires
         add_route(favicon, "/favicon.ico")
 
@@ -1020,21 +1017,21 @@ def add_route(view, regex):
         )
         add_route(
             JsonDataView.as_view(self, "metadata.json", lambda: self.metadata()),
-            r"/-/metadata(?P<format>(\.json)?)$",
+            r"/-/metadata(\.(?P<format>json))?$",
         )
         add_route(
             JsonDataView.as_view(self, "versions.json", self._versions),
-            r"/-/versions(?P<format>(\.json)?)$",
+            r"/-/versions(\.(?P<format>json))?$",
         )
         add_route(
             JsonDataView.as_view(
                 self, "plugins.json", self._plugins, needs_request=True
             ),
-            r"/-/plugins(?P<format>(\.json)?)$",
+            r"/-/plugins(\.(?P<format>json))?$",
         )
         add_route(
             JsonDataView.as_view(self, "settings.json", lambda: self._settings),
-            r"/-/settings(?P<format>(\.json)?)$",
+            r"/-/settings(\.(?P<format>json))?$",
         )
         add_route(
             permanent_redirect("/-/settings.json"),
@@ -1046,15 +1043,15 @@ def add_route(view, regex):
         )
         add_route(
             JsonDataView.as_view(self, "threads.json", self._threads),
-            r"/-/threads(?P<format>(\.json)?)$",
+            r"/-/threads(\.(?P<format>json))?$",
         )
         add_route(
             JsonDataView.as_view(self, "databases.json", self._connected_databases),
-            r"/-/databases(?P<format>(\.json)?)$",
+            r"/-/databases(\.(?P<format>json))?$",
         )
         add_route(
             JsonDataView.as_view(self, "actor.json", self._actor, needs_request=True),
-            r"/-/actor(?P<format>(\.json)?)$",
+            r"/-/actor(\.(?P<format>json))?$",
         )
         add_route(
             AuthTokenView.as_view(self),
@@ -1080,20 +1077,17 @@ def add_route(view, regex):
             PatternPortfolioView.as_view(self),
             r"/-/patterns$",
         )
-        add_route(DatabaseDownload.as_view(self), r"/(?P<database>[^/]+?)\.db$")
+        add_route(DatabaseDownload.as_view(self), r"/(?P<database>[^\/\.]+)\.db$")
         add_route(
-            DatabaseView.as_view(self),
-            r"/(?P<database>[^/]+?)(?P<format>" + renderer_regex + r"|.jsono|\.csv)?$",
+            DatabaseView.as_view(self), r"/(?P<database>[^\/\.]+)(\.(?P<format>\w+))?$"
         )
         add_route(
             TableView.as_view(self),
-            r"/(?P<database>[^/]+)/(?P<table>[^\/\.]+)(\.[a-zA-Z0-9_]+)?$",
+            r"/(?P<database>[^\/\.]+)/(?P<table>[^\/\.]+)(\.(?P<format>\w+))?$",
         )
         add_route(
             RowView.as_view(self),
-            r"/(?P<database>[^/]+)/(?P<table>[^/]+?)/(?P<pks>[^/]+?)(?P<format>"
-            + renderer_regex
-            + r")?$",
+            r"/(?P<database>[^\/\.]+)/(?P<table>[^/]+?)/(?P<pks>[^/]+?)(\.(?P<format>\w+))?$",
         )
         return [
             # Compile any strings to regular expressions

diff --git a/tests/test_api.py b/tests/test_api.py
@@ -629,8 +629,8 @@ def test_old_memory_urls_redirect(app_client_no_files, path, expected_redirect):
 
 
 def test_database_page_for_database_with_dot_in_name(app_client_with_dot):
-    response = app_client_with_dot.get("/fixtures.dot.json")
-    assert 200 == response.status
+    response = app_client_with_dot.get("/fixtures~2Edot.json")
+    assert response.status == 200
 
 
 def test_custom_sql(app_client):

diff --git a/tests/test_routes.py b/tests/test_routes.py
@@ -12,14 +12,26 @@ def routes():
 @pytest.mark.parametrize(
     "path,expected_class,expected_matches",
     (
-        ("/", "IndexView", {"format": ""}),
+        ("/", "IndexView", {"format": None}),
         ("/foo", "DatabaseView", {"format": None, "database": "foo"}),
-        ("/foo.csv", "DatabaseView", {"format": ".csv", "database": "foo"}),
-        ("/foo.json", "DatabaseView", {"format": ".json", "database": "foo"}),
-        ("/foo.humbug", "DatabaseView", {"format": None, "database": "foo.humbug"}),
-        ("/foo/humbug", "TableView", {"database": "foo", "table": "humbug"}),
-        ("/foo/humbug.json", "TableView", {"database": "foo", "table": "humbug"}),
-        ("/foo/humbug.blah", "TableView", {"database": "foo", "table": "humbug"}),
+        ("/foo.csv", "DatabaseView", {"format": "csv", "database": "foo"}),
+        ("/foo.json", "DatabaseView", {"format": "json", "database": "foo"}),
+        ("/foo.humbug", "DatabaseView", {"format": "humbug", "database": "foo"}),
+        (
+            "/foo/humbug",
+            "TableView",
+            {"database": "foo", "table": "humbug", "format": None},
+        ),
+        (
+            "/foo/humbug.json",
+            "TableView",
+            {"database": "foo", "table": "humbug", "format": "json"},
+        ),
+        (
+            "/foo/humbug.blah",
+            "TableView",
+            {"database": "foo", "table": "humbug", "format": "blah"},
+        ),
         (
             "/foo/humbug/1",
             "RowView",
@@ -28,10 +40,10 @@ def routes():
         (
             "/foo/humbug/1.json",
             "RowView",
-            {"format": ".json", "database": "foo", "pks": "1", "table": "humbug"},
+            {"format": "json", "database": "foo", "pks": "1", "table": "humbug"},
         ),
-        ("/-/metadata.json", "JsonDataView", {"format": ".json"}),
-        ("/-/metadata", "JsonDataView", {"format": ""}),
+        ("/-/metadata.json", "JsonDataView", {"format": "json"}),
+        ("/-/metadata", "JsonDataView", {"format": None}),
     ),
 )
 def test_routes(routes, path, expected_class, expected_matches):