From 1ad3ebf2ed45e9e1f2caa7a91017b8899a6b4e46 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Thu, 14 Mar 2019 21:24:06 -0700
Subject: [PATCH] /dbname-hash now optional: turn on with --config hash_urls:1

---
 datasette/app.py                  |  3 +++
 datasette/templates/database.html | 14 ++++++------
 datasette/templates/query.html    |  6 +++---
 datasette/templates/row.html      |  6 +++---
 datasette/templates/table.html    |  8 +++----
 datasette/views/base.py           | 19 +++++++++++++---
 datasette/views/database.py       |  1 -
 datasette/views/index.py          |  2 +-
 datasette/views/table.py          |  2 --
 docs/config.rst                   | 14 ++++++++++++
 tests/fixtures.py                 |  7 ++++++
 tests/test_api.py                 | 12 ++++++-----
 tests/test_html.py                | 36 ++++++++++++-------------------
 13 files changed, 79 insertions(+), 51 deletions(-)

diff --git a/datasette/app.py b/datasette/app.py
index 37b199a4..f9268e44 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -69,6 +69,9 @@ CONFIG_OPTIONS = (
     ConfigOption("facet_suggest_time_limit_ms", 50, """
         Time limit for calculating a suggested facet
     """.strip()),
+    ConfigOption("hash_urls", False, """
+        Include DB file contents hash in URLs, for far-future caching
+    """.strip()),
     ConfigOption("allow_facet", True, """
         Allow users to specify columns to facet using ?_facet= parameter
     """.strip()),
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index f827e584..0e80c8b6 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -12,12 +12,12 @@
 {% block content %}
 <div class="hd"><a href="/">home</a></div>
 
-<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash[:6] }}">{{ metadata.title or database }}</h1>
+<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_color(database) }}">{{ metadata.title or database }}</h1>
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
 {% if config.allow_sql %}
-    <form class="sql" action="/{{ database }}-{{ database_hash }}" method="get">
+    <form class="sql" action="{{ database_url(database) }}" method="get">
         <h3>Custom SQL query</h3>
         <p><textarea name="sql">{% if tables %}select * from {{ tables[0].name|escape_sqlite }}{% else %}select sqlite_version(){% endif %}</textarea></p>
         <p><input type="submit" value="Run SQL"></p>
@@ -27,7 +27,7 @@
 {% for table in tables %}
 {% if show_hidden or not table.hidden %}
 <div class="db-table">
-    <h2><a href="/{{ database }}-{{ database_hash }}/{{ table.name|quote_plus }}">{{ table.name }}</a>{% if table.hidden %}<em> (hidden)</em>{% endif %}</h2>
+    <h2><a href="{{ database_url(database) }}/{{ table.name|quote_plus }}">{{ table.name }}</a>{% if table.hidden %}<em> (hidden)</em>{% endif %}</h2>
     <p><em>{% for column in table.columns[:9] %}{{ column }}{% if not loop.last %}, {% endif %}{% endfor %}{% if table.columns|length > 9 %}...{% endif %}</em></p>
     <p>{{ "{:,}".format(table.count) }} row{% if table.count == 1 %}{% else %}s{% endif %}</p>
 </div>
@@ -35,14 +35,14 @@
 {% endfor %}
 
 {% if hidden_count and not show_hidden %}
-    <p>... and <a href="/{{ database }}-{{ database_hash }}?_show_hidden=1">{{ "{:,}".format(hidden_count) }} hidden table{% if hidden_count == 1 %}{% else %}s{% endif %}</a></p>
+    <p>... and <a href="{{ database_url(database) }}?_show_hidden=1">{{ "{:,}".format(hidden_count) }} hidden table{% if hidden_count == 1 %}{% else %}s{% endif %}</a></p>
 {% endif %}
 
 {% if views %}
     <h2>Views</h2>
     <ul>
         {% for view in views %}
-            <li><a href="/{{ database }}-{{ database_hash }}/{{ view|urlencode }}">{{ view }}</a></li>
+            <li><a href="{{ database_url(database) }}/{{ view|urlencode }}">{{ view }}</a></li>
         {% endfor %}
     </ul>
 {% endif %}
@@ -51,13 +51,13 @@
     <h2>Queries</h2>
     <ul>
         {% for query in queries %}
-            <li><a href="/{{ database }}-{{ database_hash }}/{{ query.name|urlencode }}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a></li>
+            <li><a href="{{ database_url(database) }}/{{ query.name|urlencode }}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a></li>
         {% endfor %}
     </ul>
 {% endif %}
 
 {% if config.allow_download and database != ":memory:" %}
-    <p class="download-sqlite">Download SQLite DB: <a href="/{{ database }}-{{ database_hash }}.db">{{ database }}.db</a> <em>{{ format_bytes(size) }}</em></p>
+    <p class="download-sqlite">Download SQLite DB: <a href="{{ database_url(database) }}.db">{{ database }}.db</a> <em>{{ format_bytes(size) }}</em></p>
 {% endif %}
 
 {% include "_codemirror_foot.html" %}
diff --git a/datasette/templates/query.html b/datasette/templates/query.html
index b23c67d8..7f49ace4 100644
--- a/datasette/templates/query.html
+++ b/datasette/templates/query.html
@@ -19,13 +19,13 @@
 {% block body_class %}query db-{{ database|to_css_class }}{% endblock %}
 
 {% block content %}
-<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a></div>
+<div class="hd"><a href="/">home</a> / <a href="{{ database_url(database) }}">{{ database }}</a></div>
 
-<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash[:6] }}">{{ metadata.title or database }}</h1>
+<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_color(database) }}">{{ metadata.title or database }}</h1>
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
-<form class="sql" action="/{{ database }}-{{ database_hash }}{% if canned_query %}/{{ canned_query }}{% endif %}" method="get">
+<form class="sql" action="{{ database_url(database) }}{% if canned_query %}/{{ canned_query }}{% endif %}" method="get">
     <h3>Custom SQL query{% if display_rows %} returning {% if truncated %}more than {% endif %}{{ "{:,}".format(display_rows|length) }} row{% if display_rows|length == 1 %}{% else %}s{% endif %}{% endif %}</h3>
     {% if editable and config.allow_sql %}
         <p><textarea name="sql">{% if query and query.sql %}{{ query.sql }}{% else %}select * from {{ tables[0].name|escape_sqlite }}{% endif %}</textarea></p>
diff --git a/datasette/templates/row.html b/datasette/templates/row.html
index 4ef0b758..389b16b2 100644
--- a/datasette/templates/row.html
+++ b/datasette/templates/row.html
@@ -16,9 +16,9 @@
 {% block body_class %}row db-{{ database|to_css_class }} table-{{ table|to_css_class }}{% endblock %}
 
 {% block content %}
-<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a> / <a href="/{{ database }}-{{ database_hash }}/{{ table|quote_plus }}">{{ table }}</a></div>
+<div class="hd"><a href="/">home</a> / <a href="{{ database_url(database) }}">{{ database }}</a> / <a href="{{ database_url(database) }}/{{ table|quote_plus }}">{{ table }}</a></div>
 
-<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash[:6] }}">{{ table }}: {{ ', '.join(primary_key_values) }}</a></h1>
+<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_color(database) }}">{{ table }}: {{ ', '.join(primary_key_values) }}</a></h1>
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
@@ -31,7 +31,7 @@
     <ul>
         {% for other in foreign_key_tables %}
             <li>
-                <a href="/{{ database }}-{{ database_hash }}/{{ other.other_table|quote_plus }}?{{ other.other_column }}={{ ', '.join(primary_key_values) }}">
+                <a href="{{ database_url(database) }}/{{ other.other_table|quote_plus }}?{{ other.other_column }}={{ ', '.join(primary_key_values) }}">
                     {{ "{:,}".format(other.count) }} row{% if other.count == 1 %}{% else %}s{% endif %}</a>
                 from {{ other.other_column }} in {{ other.other_table }}
             </li>
diff --git a/datasette/templates/table.html b/datasette/templates/table.html
index 7a9f9115..c9ec4aa1 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -17,9 +17,9 @@
 {% block body_class %}table db-{{ database|to_css_class }} table-{{ table|to_css_class }}{% endblock %}
 
 {% block content %}
-<div class="hd"><a href="/">home</a> / <a href="/{{ database }}-{{ database_hash }}">{{ database }}</a></div>
+<div class="hd"><a href="/">home</a> / <a href="{{ database_url(database) }}">{{ database }}</a></div>
 
-<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_hash[:6] }}">{{ metadata.title or table }}{% if is_view %} (view){% endif %}</h1>
+<h1 style="padding-left: 10px; border-left: 10px solid #{{ database_color(database) }}">{{ metadata.title or table }}{% if is_view %} (view){% endif %}</h1>
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
@@ -29,7 +29,7 @@
     </h3>
 {% endif %}
 
-<form class="filters" action="/{{ database }}-{{ database_hash }}/{{ table|quote_plus }}" method="get">
+<form class="filters" action="{{ database_url(database) }}/{{ table|quote_plus }}" method="get">
     {% if supports_search %}
         <div class="search-row"><label for="_search">Search:</label><input id="_search" type="search" name="_search" value="{{ search }}"></div>
     {% endif %}
@@ -89,7 +89,7 @@
 </form>
 
 {% if query.sql and config.allow_sql %}
-    <p><a class="not-underlined" title="{{ query.sql }}" href="/{{ database }}-{{ database_hash }}?{{ {'sql': query.sql}|urlencode|safe }}{% if query.params %}&amp;{{ query.params|urlencode|safe }}{% endif %}">&#x270e; <span class="underlined">View and edit SQL</span></a></p>
+    <p><a class="not-underlined" title="{{ query.sql }}" href="{{ database_url(database) }}?{{ {'sql': query.sql}|urlencode|safe }}{% if query.params %}&amp;{{ query.params|urlencode|safe }}{% endif %}">&#x270e; <span class="underlined">View and edit SQL</span></a></p>
 {% endif %}
 
 <p class="export-links">This data as <a href="{{ url_json }}">JSON</a>{% if display_rows %}, <a href="{{ url_csv }}">CSV</a> (<a href="#export">advanced</a>){% endif %}</p>
diff --git a/datasette/views/base.py b/datasette/views/base.py
index e98762b7..6053fa95 100644
--- a/datasette/views/base.py
+++ b/datasette/views/base.py
@@ -74,6 +74,17 @@ class RenderMixin(HTTPMethodView):
             else:
                 yield {"url": url}
 
+    def database_url(self, database):
+        if not self.ds.config("hash_urls"):
+            return "/{}".format(database)
+        else:
+            return "/{}-{}".format(
+                database, self.ds.inspect()[database]["hash"][:HASH_LENGTH]
+            )
+
+    def database_color(self, database):
+        return 'ff0000'
+
     def render(self, templates, **context):
         template = self.ds.jinja_env.select_template(templates)
         select_templates = [
@@ -104,6 +115,8 @@ class RenderMixin(HTTPMethodView):
                             "extra_js_urls", template, context
                         ),
                         "format_bytes": format_bytes,
+                        "database_url": self.database_url,
+                        "database_color": self.database_color,
                     }
                 }
             )
@@ -187,7 +200,9 @@ class BaseView(RenderMixin):
                 should_redirect += kwargs["as_format"]
             if "as_db" in kwargs:
                 should_redirect += kwargs["as_db"]
-            return name, expected, should_redirect
+
+            if self.ds.config("hash_urls"):
+                return name, expected, should_redirect
 
         return name, expected, None
 
@@ -417,7 +432,6 @@ class BaseView(RenderMixin):
                             "ok": False,
                             "error": error,
                             "database": database,
-                            "database_hash": hash,
                         }
                 elif shape == "array":
                     data = data["rows"]
@@ -571,7 +585,6 @@ class BaseView(RenderMixin):
                 display_rows.append(display_row)
             return {
                 "display_rows": display_rows,
-                "database_hash": hash,
                 "custom_sql": True,
                 "named_parameter_values": named_parameter_values,
                 "editable": editable,
diff --git a/datasette/views/database.py b/datasette/views/database.py
index 9c44a800..5ab10b36 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -30,7 +30,6 @@ class DatabaseView(BaseView):
             "views": info["views"],
             "queries": self.ds.get_canned_queries(database),
         }, {
-            "database_hash": hash,
             "show_hidden": request.args.get("_show_hidden"),
             "editable": True,
             "metadata": metadata,
diff --git a/datasette/views/index.py b/datasette/views/index.py
index 32c04585..70f7e943 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -21,7 +21,7 @@ class IndexView(RenderMixin):
             database = {
                 "name": key,
                 "hash": info["hash"],
-                "path": "{}-{}".format(key, info["hash"][:HASH_LENGTH]),
+                "path": self.database_url(key),
                 "tables_truncated": sorted(
                     tables, key=lambda t: t["count"], reverse=True
                 )[
diff --git a/datasette/views/table.py b/datasette/views/table.py
index cb744708..14f3be6f 100644
--- a/datasette/views/table.py
+++ b/datasette/views/table.py
@@ -750,7 +750,6 @@ class TableView(RowTableShared):
             )
             self.ds.update_with_inherited_metadata(metadata)
             return {
-                "database_hash": hash,
                 "supports_search": bool(fts_table),
                 "search": search or "",
                 "use_rowid": use_rowid,
@@ -851,7 +850,6 @@ class RowView(RowTableShared):
             for column in display_columns:
                 column["sortable"] = False
             return {
-                "database_hash": hash,
                 "foreign_key_tables": await self.foreign_key_tables(
                     database, table, pk_values
                 ),
diff --git a/docs/config.rst b/docs/config.rst
index b934ef2a..c2aca5c1 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -179,3 +179,17 @@ HTTP but is served to the outside world via a proxy that enables HTTPS.
 ::
 
     datasette mydatabase.db --config force_https_urls:1
+
+hash_urls
+---------
+
+When enabled, this setting causes Datasette to append a content hash of the
+database file to the URL path for every table and query within that database.
+
+When combined with far-future  expire headers this ensures that queries can be
+cached forever, safe in the knowledge that any modifications to the database
+itself will result in new, uncachcacheed URL paths.
+
+::
+
+    datasette mydatabase.db --config hash_urls:1
diff --git a/tests/fixtures.py b/tests/fixtures.py
index efd85fab..81432e30 100644
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@@ -73,6 +73,13 @@ def app_client_no_files():
     yield client
 
 
+@pytest.fixture(scope="session")
+def app_client_with_hash():
+    yield from make_app_client(config={
+        'hash_urls': True
+    })
+
+
 @pytest.fixture(scope='session')
 def app_client_shorter_time_limit():
     yield from make_app_client(20)
diff --git a/tests/test_api.py b/tests/test_api.py
index a6ba3f37..00951dca 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1,6 +1,7 @@
 from .fixtures import ( # noqa
     app_client,
     app_client_no_files,
+    app_client_with_hash,
     app_client_shorter_time_limit,
     app_client_larger_cache_size,
     app_client_returned_rows_matches_page_size,
@@ -378,7 +379,7 @@ def test_no_files_uses_memory_database(app_client_no_files):
             "hidden_table_rows_sum": 0,
             "hidden_tables_count": 0,
             "name": ":memory:",
-            "path": ":memory:-000",
+            "path": "/:memory:",
             "table_rows_sum": 0,
             "tables_count": 0,
             "tables_more": False,
@@ -388,7 +389,7 @@ def test_no_files_uses_memory_database(app_client_no_files):
     } == response.json
     # Try that SQL query
     response = app_client_no_files.get(
-        "/:memory:-0.json?sql=select+sqlite_version()&_shape=array"
+        "/:memory:.json?sql=select+sqlite_version()&_shape=array"
     )
     assert 1 == len(response.json)
     assert ["sqlite_version()"] == list(response.json[0].keys())
@@ -501,12 +502,12 @@ def test_table_not_exists_json(app_client):
     } == app_client.get('/fixtures/blah.json').json
 
 
-def test_jsono_redirects_to_shape_objects(app_client):
-    response_1 = app_client.get(
+def test_jsono_redirects_to_shape_objects(app_client_with_hash):
+    response_1 = app_client_with_hash.get(
         '/fixtures/simple_primary_key.jsono',
         allow_redirects=False
     )
-    response = app_client.get(
+    response = app_client_with_hash.get(
         response_1.headers['Location'],
         allow_redirects=False
     )
@@ -1056,6 +1057,7 @@ def test_config_json(app_client):
         "max_csv_mb": 100,
         "truncate_cells_html": 2048,
         "force_https_urls": False,
+        "hash_urls": False,
     } == response.json
 
 
diff --git a/tests/test_html.py b/tests/test_html.py
index ca6d62aa..04f2ea7b 100644
--- a/tests/test_html.py
+++ b/tests/test_html.py
@@ -2,6 +2,7 @@ from bs4 import BeautifulSoup as Soup
 from .fixtures import ( # noqa
     app_client,
     app_client_shorter_time_limit,
+    app_client_with_hash,
     make_app_client,
 )
 import pytest
@@ -15,10 +16,10 @@ def test_homepage(app_client):
     assert 'fixtures' in response.text
 
 
-def test_database_page(app_client):
-    response = app_client.get('/fixtures', allow_redirects=False)
+def test_database_page_redirects_with_url_hash(app_client_with_hash):
+    response = app_client_with_hash.get('/fixtures', allow_redirects=False)
     assert response.status == 302
-    response = app_client.get('/fixtures')
+    response = app_client_with_hash.get('/fixtures')
     assert 'fixtures' in response.text
 
 
@@ -41,19 +42,19 @@ def test_sql_time_limit(app_client_shorter_time_limit):
     assert expected_html_fragment in response.text
 
 
-def test_row(app_client):
-    response = app_client.get(
+def test_row_redirects_with_url_hash(app_client_with_hash):
+    response = app_client_with_hash.get(
         '/fixtures/simple_primary_key/1',
         allow_redirects=False
     )
     assert response.status == 302
     assert response.headers['Location'].endswith('/1')
-    response = app_client.get('/fixtures/simple_primary_key/1')
+    response = app_client_with_hash.get('/fixtures/simple_primary_key/1')
     assert response.status == 200
 
 
-def test_row_strange_table_name(app_client):
-    response = app_client.get(
+def test_row_strange_table_name_with_url_hash(app_client_with_hash):
+    response = app_client_with_hash.get(
         '/fixtures/table%2Fwith%2Fslashes.csv/3',
         allow_redirects=False
     )
@@ -61,7 +62,7 @@ def test_row_strange_table_name(app_client):
     assert response.headers['Location'].endswith(
         '/table%2Fwith%2Fslashes.csv/3'
     )
-    response = app_client.get('/fixtures/table%2Fwith%2Fslashes.csv/3')
+    response = app_client_with_hash.get('/fixtures/table%2Fwith%2Fslashes.csv/3')
     assert response.status == 200
 
 
@@ -105,10 +106,7 @@ def test_add_filter_redirects(app_client):
         '_filter_op': 'startswith',
         '_filter_value': 'x'
     })
-    # First we need to resolve the correct path before testing more redirects
-    path_base = app_client.get(
-        '/fixtures/simple_primary_key', allow_redirects=False
-    ).headers['Location']
+    path_base = '/fixtures/simple_primary_key'
     path = path_base + '?' + filter_args
     response = app_client.get(path, allow_redirects=False)
     assert response.status == 302
@@ -146,9 +144,7 @@ def test_existing_filter_redirects(app_client):
         '_filter_op_4': 'contains',
         '_filter_value_4': 'world',
     }
-    path_base = app_client.get(
-        '/fixtures/simple_primary_key', allow_redirects=False
-    ).headers['Location']
+    path_base = '/fixtures/simple_primary_key'
     path = path_base + '?' + urllib.parse.urlencode(filter_args)
     response = app_client.get(path, allow_redirects=False)
     assert response.status == 302
@@ -174,9 +170,7 @@ def test_existing_filter_redirects(app_client):
 
 
 def test_empty_search_parameter_gets_removed(app_client):
-    path_base = app_client.get(
-        '/fixtures/simple_primary_key', allow_redirects=False
-    ).headers['Location']
+    path_base = '/fixtures/simple_primary_key'
     path = path_base + '?' + urllib.parse.urlencode({
         '_search': '',
         '_filter_column': 'name',
@@ -191,9 +185,7 @@ def test_empty_search_parameter_gets_removed(app_client):
 
 
 def test_sort_by_desc_redirects(app_client):
-    path_base = app_client.get(
-        '/fixtures/sortable', allow_redirects=False
-    ).headers['Location']
+    path_base = '/fixtures/sortable'
     path = path_base + '?' + urllib.parse.urlencode({
         '_sort': 'sortable',
         '_sort_by_desc': '1',