diff --git a/datasette/app.py b/datasette/app.py index b39ef7cd..3099ada7 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1097,7 +1097,7 @@ class Datasette: ) add_route( TableView.as_view(self), - r"/(?P[^/]+)/(?P[^/]+?$)", + r"/(?P[^/]+)/(?P[^\/\.]+)(\.[a-zA-Z0-9_]+)?$", ) add_route( RowView.as_view(self), diff --git a/datasette/views/base.py b/datasette/views/base.py index 1c0c3f9b..e31beb19 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -122,11 +122,11 @@ class BaseView: async def delete(self, request, *args, **kwargs): return Response.text("Method not allowed", status=405) - async def dispatch_request(self, request, *args, **kwargs): + async def dispatch_request(self, request): if self.ds: await self.ds.refresh_schemas() handler = getattr(self, request.method.lower(), None) - return await handler(request, *args, **kwargs) + return await handler(request) async def render(self, templates, request, context=None): context = context or {} @@ -169,9 +169,7 @@ class BaseView: def as_view(cls, *class_args, **class_kwargs): async def view(request, send): self = view.view_class(*class_args, **class_kwargs) - return await self.dispatch_request( - request, **request.scope["url_route"]["kwargs"] - ) + return await self.dispatch_request(request) view.view_class = cls view.__doc__ = cls.__doc__ @@ -200,90 +198,14 @@ class DataView(BaseView): add_cors_headers(r.headers) return r - async def data(self, request, database, hash, **kwargs): + async def data(self, request): raise NotImplementedError - async def resolve_db_name(self, request, db_name, **kwargs): - hash = None - name = None - decoded_name = tilde_decode(db_name) - if decoded_name not in self.ds.databases and "-" in db_name: - # No matching DB found, maybe it's a name-hash? - name_bit, hash_bit = db_name.rsplit("-", 1) - if tilde_decode(name_bit) not in self.ds.databases: - raise NotFound(f"Database not found: {name}") - else: - name = tilde_decode(name_bit) - hash = hash_bit - else: - name = decoded_name - - try: - db = self.ds.databases[name] - except KeyError: - raise NotFound(f"Database not found: {name}") - - # Verify the hash - expected = "000" - if db.hash is not None: - expected = db.hash[:HASH_LENGTH] - correct_hash_provided = expected == hash - - if not correct_hash_provided: - if "table_and_format" in kwargs: - - async def async_table_exists(t): - return await db.table_exists(t) - - table, _format = await resolve_table_and_format( - table_and_format=tilde_decode(kwargs["table_and_format"]), - table_exists=async_table_exists, - allowed_formats=self.ds.renderers.keys(), - ) - kwargs["table"] = table - if _format: - kwargs["as_format"] = f".{_format}" - elif kwargs.get("table"): - kwargs["table"] = tilde_decode(kwargs["table"]) - - should_redirect = self.ds.urls.path(f"{name}-{expected}") - if kwargs.get("table"): - should_redirect += "/" + tilde_encode(kwargs["table"]) - if kwargs.get("pk_path"): - should_redirect += "/" + kwargs["pk_path"] - if kwargs.get("as_format"): - should_redirect += kwargs["as_format"] - if kwargs.get("as_db"): - should_redirect += kwargs["as_db"] - - if ( - (self.ds.setting("hash_urls") or "_hash" in request.args) - and - # Redirect only if database is immutable - not self.ds.databases[name].is_mutable - ): - return name, expected, correct_hash_provided, should_redirect - - return name, expected, correct_hash_provided, None - def get_templates(self, database, table=None): assert NotImplemented - async def get(self, request, db_name, **kwargs): - ( - database, - hash, - correct_hash_provided, - should_redirect, - ) = await self.resolve_db_name(request, db_name, **kwargs) - if should_redirect: - return self.redirect(request, should_redirect, remove_args={"_hash"}) - - return await self.view_get( - request, database, hash, correct_hash_provided, **kwargs - ) - - async def as_csv(self, request, database, hash, **kwargs): + async def as_csv(self, request, database): + kwargs = {} stream = request.args.get("_stream") # Do not calculate facets or counts: extra_parameters = [ @@ -313,9 +235,7 @@ class DataView(BaseView): kwargs["_size"] = "max" # Fetch the first page try: - response_or_template_contexts = await self.data( - request, database, hash, **kwargs - ) + response_or_template_contexts = await self.data(request) if isinstance(response_or_template_contexts, Response): return response_or_template_contexts elif len(response_or_template_contexts) == 4: @@ -367,10 +287,11 @@ class DataView(BaseView): next = None while first or (next and stream): try: + kwargs = {} if next: kwargs["_next"] = next if not first: - data, _, _ = await self.data(request, database, hash, **kwargs) + data, _, _ = await self.data(request, **kwargs) if first: if request.args.get("_header") != "off": await writer.writerow(headings) @@ -445,60 +366,39 @@ class DataView(BaseView): if not trace: content_type = "text/csv; charset=utf-8" disposition = 'attachment; filename="{}.csv"'.format( - kwargs.get("table", database) + request.url_vars.get("table", database) ) headers["content-disposition"] = disposition return AsgiStream(stream_fn, headers=headers, content_type=content_type) - async def get_format(self, request, database, args): - """Determine the format of the response from the request, from URL - parameters or from a file extension. - - `args` is a dict of the path components parsed from the URL by the router. - """ - # If ?_format= is provided, use that as the format - _format = request.args.get("_format", None) - if not _format: - _format = (args.pop("as_format", None) or "").lstrip(".") + def get_format(self, request): + # Format is the bit from the path following the ., if one exists + last_path_component = request.path.split("/")[-1] + if "." in last_path_component: + return last_path_component.split(".")[-1] else: - args.pop("as_format", None) - if "table_and_format" in args: - db = self.ds.databases[database] + return None - async def async_table_exists(t): - return await db.table_exists(t) - - table, _ext_format = await resolve_table_and_format( - table_and_format=tilde_decode(args["table_and_format"]), - table_exists=async_table_exists, - allowed_formats=self.ds.renderers.keys(), - ) - _format = _format or _ext_format - args["table"] = table - del args["table_and_format"] - elif "table" in args: - args["table"] = tilde_decode(args["table"]) - return _format, args - - async def view_get(self, request, database, hash, correct_hash_provided, **kwargs): - _format, kwargs = await self.get_format(request, database, kwargs) + async def get(self, request): + db_name = request.url_vars["db_name"] + database = tilde_decode(db_name) + _format = self.get_format(request) + data_kwargs = {} if _format == "csv": - return await self.as_csv(request, database, hash, **kwargs) + return await self.as_csv(request, database) if _format is None: # HTML views default to expanding all foreign key labels - kwargs["default_labels"] = True + data_kwargs["default_labels"] = True extra_template_data = {} start = time.perf_counter() status_code = None templates = [] try: - response_or_template_contexts = await self.data( - request, database, hash, **kwargs - ) + response_or_template_contexts = await self.data(request, **data_kwargs) if isinstance(response_or_template_contexts, Response): return response_or_template_contexts # If it has four items, it includes an HTTP status code @@ -650,10 +550,7 @@ class DataView(BaseView): ttl = request.args.get("_ttl", None) if ttl is None or not ttl.isdigit(): - if correct_hash_provided: - ttl = self.ds.setting("default_cache_ttl_hashed") - else: - ttl = self.ds.setting("default_cache_ttl") + ttl = self.ds.setting("default_cache_ttl") return self.set_response_headers(r, ttl) diff --git a/datasette/views/database.py b/datasette/views/database.py index e26706e7..48635e01 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -12,6 +12,7 @@ from datasette.utils import ( await_me_maybe, check_visibility, derive_named_parameters, + tilde_decode, to_css_class, validate_sql_select, is_url, @@ -21,7 +22,7 @@ from datasette.utils import ( sqlite3, InvalidSql, ) -from datasette.utils.asgi import AsgiFileDownload, Response, Forbidden +from datasette.utils.asgi import AsgiFileDownload, NotFound, Response, Forbidden from datasette.plugins import pm from .base import DatasetteError, DataView @@ -30,7 +31,8 @@ from .base import DatasetteError, DataView class DatabaseView(DataView): name = "database" - async def data(self, request, database, hash, default_labels=False, _size=None): + async def data(self, request, default_labels=False, _size=None): + database = tilde_decode(request.url_vars["db_name"]) await self.check_permissions( request, [ @@ -45,10 +47,13 @@ class DatabaseView(DataView): sql = request.args.get("sql") validate_sql_select(sql) return await QueryView(self.ds).data( - request, database, hash, sql, _size=_size, metadata=metadata + request, sql, _size=_size, metadata=metadata ) - db = self.ds.databases[database] + try: + db = self.ds.databases[database] + except KeyError: + raise NotFound("Database not found: {}".format(database)) table_counts = await db.table_counts(5) hidden_table_names = set(await db.hidden_table_names()) @@ -156,7 +161,8 @@ class DatabaseView(DataView): class DatabaseDownload(DataView): name = "database_download" - async def view_get(self, request, database, hash, correct_hash_present, **kwargs): + async def get(self, request): + database = tilde_decode(request.url_vars["db_name"]) await self.check_permissions( request, [ @@ -191,8 +197,6 @@ class QueryView(DataView): async def data( self, request, - database, - hash, sql, editable=True, canned_query=None, @@ -201,6 +205,7 @@ class QueryView(DataView): named_parameters=None, write=False, ): + database = tilde_decode(request.url_vars["db_name"]) params = {key: request.args.get(key) for key in request.args} if "sql" in params: params.pop("sql") diff --git a/datasette/views/index.py b/datasette/views/index.py index 18454759..311a49db 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -18,7 +18,8 @@ COUNT_DB_SIZE_LIMIT = 100 * 1024 * 1024 class IndexView(BaseView): name = "index" - async def get(self, request, as_format): + async def get(self, request): + as_format = request.url_vars["as_format"] await self.check_permission(request, "view-instance") databases = [] for name, db in self.ds.databases.items(): diff --git a/datasette/views/special.py b/datasette/views/special.py index cdd530f0..c7b5061f 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -14,7 +14,8 @@ class JsonDataView(BaseView): self.data_callback = data_callback self.needs_request = needs_request - async def get(self, request, as_format): + async def get(self, request): + as_format = request.url_vars["as_format"] await self.check_permission(request, "view-instance") if self.needs_request: data = self.data_callback(request) diff --git a/datasette/views/table.py b/datasette/views/table.py index 72b8e9a4..8bdc7417 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -271,20 +271,18 @@ class RowTableShared(DataView): class TableView(RowTableShared): name = "table" - async def post(self, request, db_name, table_and_format): + async def post(self, request): + db_name = tilde_decode(request.url_vars["db_name"]) + table = tilde_decode(request.url_vars["table"]) # Handle POST to a canned query - canned_query = await self.ds.get_canned_query( - db_name, table_and_format, request.actor - ) + canned_query = await self.ds.get_canned_query(db_name, table, request.actor) assert canned_query, "You may only POST to a canned query" return await QueryView(self.ds).data( request, - db_name, - None, canned_query["sql"], metadata=canned_query, editable=False, - canned_query=table_and_format, + canned_query=table, named_parameters=canned_query.get("params"), write=bool(canned_query.get("write")), ) @@ -325,20 +323,22 @@ class TableView(RowTableShared): async def data( self, request, - database, - hash, - table, default_labels=False, _next=None, _size=None, ): + database = tilde_decode(request.url_vars["db_name"]) + table = tilde_decode(request.url_vars["table"]) + try: + db = self.ds.databases[database] + except KeyError: + raise NotFound("Database not found: {}".format(database)) + # If this is a canned query, not a table, then dispatch to QueryView instead canned_query = await self.ds.get_canned_query(database, table, request.actor) if canned_query: return await QueryView(self.ds).data( request, - database, - hash, canned_query["sql"], metadata=canned_query, editable=False, @@ -347,9 +347,6 @@ class TableView(RowTableShared): write=bool(canned_query.get("write")), ) - table = tilde_decode(table) - - db = self.ds.databases[database] is_view = bool(await db.get_view_definition(table)) table_exists = bool(await db.table_exists(table)) @@ -940,8 +937,9 @@ async def _sql_params_pks(db, table, pk_values): class RowView(RowTableShared): name = "row" - async def data(self, request, database, hash, table, pk_path, default_labels=False): - table = tilde_decode(table) + async def data(self, request, default_labels=False): + database = tilde_decode(request.url_vars["db_name"]) + table = tilde_decode(request.url_vars["table"]) await self.check_permissions( request, [ @@ -950,7 +948,7 @@ class RowView(RowTableShared): "view-instance", ], ) - pk_values = urlsafe_components(pk_path) + pk_values = urlsafe_components(request.url_vars["pk_path"]) db = self.ds.databases[database] sql, params, pks = await _sql_params_pks(db, table, pk_values) results = await db.execute(sql, params, truncate=True) diff --git a/tests/fixtures.py b/tests/fixtures.py index 11f09c41..342a3020 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -214,12 +214,6 @@ def app_client_two_attached_databases_one_immutable(): yield client -@pytest.fixture(scope="session") -def app_client_with_hash(): - with make_app_client(settings={"hash_urls": True}, is_immutable=True) as client: - yield client - - @pytest.fixture(scope="session") def app_client_with_trace(): with make_app_client(settings={"trace_debug": True}, is_immutable=True) as client: diff --git a/tests/test_api.py b/tests/test_api.py index 87d91e56..46e41afb 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -825,35 +825,6 @@ def test_config_redirects_to_settings(app_client, path, expected_redirect): assert response.headers["Location"] == expected_redirect -@pytest.mark.parametrize( - "path,expected_redirect", - [ - ("/fixtures/facetable.json?_hash=1", "/fixtures-HASH/facetable.json"), - ( - "/fixtures/facetable.json?city_id=1&_hash=1", - "/fixtures-HASH/facetable.json?city_id=1", - ), - ], -) -def test_hash_parameter( - app_client_two_attached_databases_one_immutable, path, expected_redirect -): - # First get the current hash for the fixtures database - current_hash = app_client_two_attached_databases_one_immutable.ds.databases[ - "fixtures" - ].hash[:7] - response = app_client_two_attached_databases_one_immutable.get(path) - assert response.status == 302 - location = response.headers["Location"] - assert expected_redirect.replace("HASH", current_hash) == location - - -def test_hash_parameter_ignored_for_mutable_databases(app_client): - path = "/fixtures/facetable.json?_hash=1" - response = app_client.get(path) - assert response.status == 200 - - test_json_columns_default_expected = [ {"intval": 1, "strval": "s", "floatval": 0.5, "jsonval": '{"foo": "bar"}'} ] diff --git a/tests/test_custom_pages.py b/tests/test_custom_pages.py index 66b7437a..f2cfe394 100644 --- a/tests/test_custom_pages.py +++ b/tests/test_custom_pages.py @@ -21,61 +21,61 @@ def custom_pages_client_with_base_url(): def test_custom_pages_view_name(custom_pages_client): response = custom_pages_client.get("/about") - assert 200 == response.status - assert "ABOUT! view_name:page" == response.text + assert response.status == 200 + assert response.text == "ABOUT! view_name:page" def test_request_is_available(custom_pages_client): response = custom_pages_client.get("/request") - assert 200 == response.status - assert "path:/request" == response.text + assert response.status == 200 + assert response.text == "path:/request" def test_custom_pages_with_base_url(custom_pages_client_with_base_url): response = custom_pages_client_with_base_url.get("/prefix/request") - assert 200 == response.status - assert "path:/prefix/request" == response.text + assert response.status == 200 + assert response.text == "path:/prefix/request" def test_custom_pages_nested(custom_pages_client): response = custom_pages_client.get("/nested/nest") - assert 200 == response.status - assert "Nest!" == response.text + assert response.status == 200 + assert response.text == "Nest!" response = custom_pages_client.get("/nested/nest2") - assert 404 == response.status + assert response.status == 404 def test_custom_status(custom_pages_client): response = custom_pages_client.get("/202") - assert 202 == response.status - assert "202!" == response.text + assert response.status == 202 + assert response.text == "202!" def test_custom_headers(custom_pages_client): response = custom_pages_client.get("/headers") - assert 200 == response.status - assert "foo" == response.headers["x-this-is-foo"] - assert "bar" == response.headers["x-this-is-bar"] - assert "FOOBAR" == response.text + assert response.status == 200 + assert response.headers["x-this-is-foo"] == "foo" + assert response.headers["x-this-is-bar"] == "bar" + assert response.text == "FOOBAR" def test_custom_content_type(custom_pages_client): response = custom_pages_client.get("/atom") - assert 200 == response.status + assert response.status == 200 assert response.headers["content-type"] == "application/xml" - assert "" == response.text + assert response.text == "" def test_redirect(custom_pages_client): response = custom_pages_client.get("/redirect") - assert 302 == response.status - assert "/example" == response.headers["Location"] + assert response.status == 302 + assert response.headers["Location"] == "/example" def test_redirect2(custom_pages_client): response = custom_pages_client.get("/redirect2") - assert 301 == response.status - assert "/example" == response.headers["Location"] + assert response.status == 301 + assert response.headers["Location"] == "/example" @pytest.mark.parametrize( diff --git a/tests/test_html.py b/tests/test_html.py index 76a8423a..6e4c22b1 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -5,7 +5,6 @@ from .fixtures import ( # noqa app_client_base_url_prefix, app_client_shorter_time_limit, app_client_two_attached_databases, - app_client_with_hash, make_app_client, METADATA, ) @@ -101,13 +100,6 @@ def test_not_allowed_methods(): assert response.status == 405 -def test_database_page_redirects_with_url_hash(app_client_with_hash): - response = app_client_with_hash.get("/fixtures") - assert response.status == 302 - response = app_client_with_hash.get("/fixtures", follow_redirects=True) - assert "fixtures" in response.text - - def test_database_page(app_client): response = app_client.get("/fixtures") soup = Soup(response.body, "html.parser") @@ -182,26 +174,6 @@ def test_sql_time_limit(app_client_shorter_time_limit): assert expected_html_fragment in response.text -def test_row_redirects_with_url_hash(app_client_with_hash): - response = app_client_with_hash.get("/fixtures/simple_primary_key/1") - assert response.status == 302 - assert response.headers["Location"].endswith("/1") - response = app_client_with_hash.get( - "/fixtures/simple_primary_key/1", follow_redirects=True - ) - assert response.status == 200 - - -def test_row_strange_table_name_with_url_hash(app_client_with_hash): - response = app_client_with_hash.get("/fixtures/table~2Fwith~2Fslashes~2Ecsv/3") - assert response.status == 302 - assert response.headers["Location"].endswith("/table~2Fwith~2Fslashes~2Ecsv/3") - response = app_client_with_hash.get( - "/fixtures/table~2Fwith~2Fslashes~2Ecsv/3", follow_redirects=True - ) - assert response.status == 200 - - def test_row_page_does_not_truncate(): with make_app_client(settings={"truncate_cells_html": 5}) as client: response = client.get("/fixtures/facetable/1") diff --git a/tests/test_internals_urls.py b/tests/test_internals_urls.py index 4307789c..d60aafcf 100644 --- a/tests/test_internals_urls.py +++ b/tests/test_internals_urls.py @@ -1,6 +1,5 @@ from datasette.app import Datasette from datasette.utils import PrefixedUrlString -from .fixtures import app_client_with_hash import pytest @@ -147,20 +146,3 @@ def test_row(ds, base_url, format, expected): actual = ds.urls.row("_memory", "facetable", "1", format=format) assert actual == expected assert isinstance(actual, PrefixedUrlString) - - -@pytest.mark.parametrize("base_url", ["/", "/prefix/"]) -def test_database_hashed(app_client_with_hash, base_url): - ds = app_client_with_hash.ds - original_base_url = ds._settings["base_url"] - try: - ds._settings["base_url"] = base_url - db_hash = ds.get_database("fixtures").hash - assert len(db_hash) == 64 - expected = f"{base_url}fixtures-{db_hash[:7]}" - assert ds.urls.database("fixtures") == expected - assert ds.urls.table("fixtures", "name") == expected + "/name" - assert ds.urls.query("fixtures", "name") == expected + "/name" - finally: - # Reset this since fixture is shared with other tests - ds._settings["base_url"] = original_base_url diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 3ab369b3..3d0a7fbd 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -2,7 +2,6 @@ from datasette.utils import detect_json1 from datasette.utils.sqlite import sqlite_version from .fixtures import ( # noqa app_client, - app_client_with_hash, app_client_with_trace, app_client_returned_rows_matches_page_size, generate_compound_rows, @@ -41,13 +40,6 @@ def test_table_not_exists_json(app_client): } == app_client.get("/fixtures/blah.json").json -def test_jsono_redirects_to_shape_objects(app_client_with_hash): - response_1 = app_client_with_hash.get("/fixtures/simple_primary_key.jsono") - response = app_client_with_hash.get(response_1.headers["Location"]) - assert response.status == 302 - assert response.headers["Location"].endswith("?_shape=objects") - - def test_table_shape_arrays(app_client): response = app_client.get("/fixtures/simple_primary_key.json?_shape=arrays") assert [