Remove hashed URL mode

Also simplified how view class routing works.

Refs #1661

datasette/app.py

@@ -1097,7 +1097,7 @@ class Datasette:
         )
         add_route(
             TableView.as_view(self),
-            r"/(?P<db_name>[^/]+)/(?P<table_and_format>[^/]+?$)",
+            r"/(?P<db_name>[^/]+)/(?P<table>[^\/\.]+)(\.[a-zA-Z0-9_]+)?$",
         )
         add_route(
             RowView.as_view(self),

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(
+            return await self.dispatch_request(request)
-                request, **request.scope["url_route"]["kwargs"]
-            )
 
         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):
+    async def as_csv(self, request, database):
-        (
+        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):
         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(
+            response_or_template_contexts = await self.data(request)
-                request, database, hash, **kwargs
-            )
             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):
+    def get_format(self, request):
-        """Determine the format of the response from the request, from URL
+        # Format is the bit from the path following the ., if one exists
-        parameters or from a file extension.
+        last_path_component = request.path.split("/")[-1]
-
+        if "." in last_path_component:
-        `args` is a dict of the path components parsed from the URL by the router.
+            return last_path_component.split(".")[-1]
-        """
-        # 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(".")
         else:
-            args.pop("as_format", None)
+            return None
-        if "table_and_format" in args:
-            db = self.ds.databases[database]
 
-            async def async_table_exists(t):
+    async def get(self, request):
-                return await db.table_exists(t)
+        db_name = request.url_vars["db_name"]
-
+        database = tilde_decode(db_name)
-            table, _ext_format = await resolve_table_and_format(
+        _format = self.get_format(request)
-                table_and_format=tilde_decode(args["table_and_format"]),
+        data_kwargs = {}
-                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)
 
         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(
+            response_or_template_contexts = await self.data(request, **data_kwargs)
-                request, database, hash, **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")
-                ttl = self.ds.setting("default_cache_ttl_hashed")
-            else:
-                ttl = self.ds.setting("default_cache_ttl")
 
         return self.set_response_headers(r, ttl)
 

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")

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():

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)

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(
+        canned_query = await self.ds.get_canned_query(db_name, table, request.actor)
-            db_name, table_and_format, 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):
+    async def data(self, request, default_labels=False):
-        table = tilde_decode(table)
+        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)

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:

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"}'}
 ]

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 response.status == 200
-    assert "ABOUT! view_name:page" == response.text
+    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 response.status == 200
-    assert "path:/request" == response.text
+    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 response.status == 200
-    assert "path:/prefix/request" == response.text
+    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 response.status == 200
-    assert "Nest!" == response.text
+    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 response.status == 202
-    assert "202!" == response.text
+    assert response.text == "202!"
 
 
 def test_custom_headers(custom_pages_client):
     response = custom_pages_client.get("/headers")
-    assert 200 == response.status
+    assert response.status == 200
-    assert "foo" == response.headers["x-this-is-foo"]
+    assert response.headers["x-this-is-foo"] == "foo"
-    assert "bar" == response.headers["x-this-is-bar"]
+    assert response.headers["x-this-is-bar"] == "bar"
-    assert "FOOBAR" == response.text
+    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 "<?xml ...>" == response.text
+    assert response.text == "<?xml ...>"
 
 
 def test_redirect(custom_pages_client):
     response = custom_pages_client.get("/redirect")
-    assert 302 == response.status
+    assert response.status == 302
-    assert "/example" == response.headers["Location"]
+    assert response.headers["Location"] == "/example"
 
 
 def test_redirect2(custom_pages_client):
     response = custom_pages_client.get("/redirect2")
-    assert 301 == response.status
+    assert response.status == 301
-    assert "/example" == response.headers["Location"]
+    assert response.headers["Location"] == "/example"
 
 
 @pytest.mark.parametrize(

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")

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

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 [