From b9c2b1cfc8692b9700416db98721fa3ec982f6be Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 19 Mar 2022 13:29:10 -0700 Subject: [PATCH] Consistent treatment of format in route capturing, refs #1667 Also refs #1660 --- datasette/app.py | 30 ++++++++++++------------------ tests/test_api.py | 4 ++-- tests/test_routes.py | 32 ++++++++++++++++++++++---------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 5259c50c25..edef34e916 100644 --- 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(\.jsono?)?$)") + add_route(IndexView.as_view(self), r"/(\.(?Pjsono?))?$") # 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(\.json)?)$", + r"/-/metadata(\.(?Pjson))?$", ) add_route( JsonDataView.as_view(self, "versions.json", self._versions), - r"/-/versions(?P(\.json)?)$", + r"/-/versions(\.(?Pjson))?$", ) add_route( JsonDataView.as_view( self, "plugins.json", self._plugins, needs_request=True ), - r"/-/plugins(?P(\.json)?)$", + r"/-/plugins(\.(?Pjson))?$", ) add_route( JsonDataView.as_view(self, "settings.json", lambda: self._settings), - r"/-/settings(?P(\.json)?)$", + r"/-/settings(\.(?Pjson))?$", ) 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(\.json)?)$", + r"/-/threads(\.(?Pjson))?$", ) add_route( JsonDataView.as_view(self, "databases.json", self._connected_databases), - r"/-/databases(?P(\.json)?)$", + r"/-/databases(\.(?Pjson))?$", ) add_route( JsonDataView.as_view(self, "actor.json", self._actor, needs_request=True), - r"/-/actor(?P(\.json)?)$", + r"/-/actor(\.(?Pjson))?$", ) 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[^/]+?)\.db$") + add_route(DatabaseDownload.as_view(self), r"/(?P[^\/\.]+)\.db$") add_route( - DatabaseView.as_view(self), - r"/(?P[^/]+?)(?P" + renderer_regex + r"|.jsono|\.csv)?$", + DatabaseView.as_view(self), r"/(?P[^\/\.]+)(\.(?P\w+))?$" ) add_route( TableView.as_view(self), - r"/(?P[^/]+)/(?P[^\/\.]+)(\.[a-zA-Z0-9_]+)?$", + r"/(?P[^\/\.]+)/(?P
[^\/\.]+)(\.(?P\w+))?$", ) add_route( RowView.as_view(self), - r"/(?P[^/]+)/(?P
[^/]+?)/(?P[^/]+?)(?P" - + renderer_regex - + r")?$", + r"/(?P[^\/\.]+)/(?P
[^/]+?)/(?P[^/]+?)(\.(?P\w+))?$", ) return [ # Compile any strings to regular expressions diff --git a/tests/test_api.py b/tests/test_api.py index 421bb1fe47..253c1718a7 100644 --- 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 index 349ac302d7..1fa550185e 100644 --- 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):