From 310c3a3e059b89d05a38e373744928c1b54e54db Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Mon, 19 Oct 2020 17:33:59 -0700
Subject: [PATCH] New datasette.urls URL builders, refs #904

---
 datasette/app.py                  | 32 ++++++++++++++++++++++++++++++-
 datasette/templates/database.html | 14 +++++++-------
 datasette/templates/index.html    |  5 ++---
 datasette/templates/query.html    |  6 +++---
 datasette/templates/row.html      |  8 ++++----
 datasette/templates/table.html    | 10 +++++-----
 datasette/utils/__init__.py       |  2 ++
 datasette/views/base.py           | 18 +----------------
 datasette/views/database.py       |  2 +-
 datasette/views/index.py          |  2 +-
 tests/test_cli.py                 |  9 +++++++--
 11 files changed, 64 insertions(+), 44 deletions(-)

diff --git a/datasette/app.py b/datasette/app.py
index b768a298..b8e27f3c 100644
--- a/datasette/app.py
+++ b/datasette/app.py
@@ -56,7 +56,7 @@ from .utils import (
     resolve_env_secrets,
     sqlite3,
     to_css_class,
-    SpatialiteNotFound,
+    HASH_LENGTH,
 )
 from .utils.asgi import (
     AsgiLifespan,
@@ -321,6 +321,10 @@ class Datasette:
         self._root_token = secrets.token_hex(32)
         self.client = DatasetteClient(self)
 
+    @property
+    def urls(self):
+        return Urls(self)
+
     async def invoke_startup(self):
         for hook in pm.hook.startup(datasette=self):
             await await_me_maybe(hook)
@@ -748,6 +752,7 @@ class Datasette:
         template_context = {
             **context,
             **{
+                "urls": self.urls,
                 "actor": request.actor if request else None,
                 "display_actor": display_actor,
                 "show_logout": request is not None and "ds_actor" in request.cookies,
@@ -1259,3 +1264,28 @@ class DatasetteClient:
     async def request(self, method, path, **kwargs):
         async with httpx.AsyncClient(app=self.app) as client:
             return await client.request(method, self._fix(path), **kwargs)
+
+
+class Urls:
+    def __init__(self, ds):
+        self.ds = ds
+
+    def instance(self):
+        return self.ds.config("base_url")
+
+    def static(self, path):
+        return "{}-/static/{}".format(self.instance(), path)
+
+    def database(self, database):
+        db = self.ds.databases[database]
+        base_url = self.ds.config("base_url")
+        if self.ds.config("hash_urls") and db.hash:
+            return "{}{}-{}".format(base_url, database, db.hash[:HASH_LENGTH])
+        else:
+            return "{}{}".format(base_url, database)
+
+    def table(self, database, table):
+        return "{}/{}".format(self.database(database), urllib.parse.quote_plus(table))
+
+    def query(self, database, query):
+        return "{}/{}".format(self.database(database), urllib.parse.quote_plus(query))
diff --git a/datasette/templates/database.html b/datasette/templates/database.html
index 5ae51ef7..2f844b6a 100644
--- a/datasette/templates/database.html
+++ b/datasette/templates/database.html
@@ -11,7 +11,7 @@
 
 {% block nav %}
     <p class="crumbs">
-        <a href="{{ base_url }}">home</a>
+        <a href="{{ urls.instance() }}">home</a>
     </p>
     {{ super() }}
 {% endblock %}
@@ -23,7 +23,7 @@
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
 {% if allow_execute_sql %}
-    <form class="sql" action="{{ database_url(database) }}" method="get">
+    <form class="sql" action="{{ urls.database(database) }}" method="get">
         <h3>Custom SQL query</h3>
         <p><textarea id="sql-editor" name="sql">{% if tables %}select * from {{ tables[0].name|escape_sqlite }}{% else %}select sqlite_version(){% endif %}</textarea></p>
         <p>
@@ -36,7 +36,7 @@
 {% for table in tables %}
 {% if show_hidden or not table.hidden %}
 <div class="db-table">
-    <h2><a href="{{ database_url(database) }}/{{ table.name|quote_plus }}">{{ table.name }}</a>{% if table.private %} 🔒{% endif %}{% if table.hidden %}<em> (hidden)</em>{% endif %}</h2>
+    <h2><a href="{{ urls.table(database, table.name) }}">{{ table.name }}</a>{% if table.private %} 🔒{% endif %}{% 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>{% if table.count is none %}Many rows{% else %}{{ "{:,}".format(table.count) }} row{% if table.count == 1 %}{% else %}s{% endif %}{% endif %}</p>
 </div>
@@ -44,14 +44,14 @@
 {% endfor %}
 
 {% if hidden_count and not show_hidden %}
-    <p>... and <a href="{{ database_url(database) }}?_show_hidden=1">{{ "{:,}".format(hidden_count) }} hidden table{% if hidden_count == 1 %}{% else %}s{% endif %}</a></p>
+    <p>... and <a href="{{ urls.database(database) }}?_show_hidden=1">{{ "{:,}".format(hidden_count) }} hidden table{% if hidden_count == 1 %}{% else %}s{% endif %}</a></p>
 {% endif %}
 
 {% if views %}
     <h2 id="views">Views</h2>
     <ul>
         {% for view in views %}
-            <li><a href="{{ database_url(database) }}/{{ view.name|urlencode }}">{{ view.name }}</a>{% if view.private %} 🔒{% endif %}</li>
+            <li><a href="{{ urls.database(database) }}/{{ view.name|urlencode }}">{{ view.name }}</a>{% if view.private %} 🔒{% endif %}</li>
         {% endfor %}
     </ul>
 {% endif %}
@@ -60,13 +60,13 @@
     <h2 id="queries">Queries</h2>
     <ul>
         {% for query in queries %}
-            <li><a href="{{ database_url(database) }}/{{ query.name|urlencode }}{% if query.fragment %}#{{ query.fragment }}{% endif %}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a>{% if query.private %} 🔒{% endif %}</li>
+            <li><a href="{{ urls.query(database, query.name) }}{% if query.fragment %}#{{ query.fragment }}{% endif %}" title="{{ query.description or query.sql }}">{{ query.title or query.name }}</a>{% if query.private %} 🔒{% endif %}</li>
         {% endfor %}
     </ul>
 {% endif %}
 
 {% if allow_download %}
-    <p class="download-sqlite">Download SQLite DB: <a href="{{ database_url(database) }}.db">{{ database }}.db</a> <em>{{ format_bytes(size) }}</em></p>
+    <p class="download-sqlite">Download SQLite DB: <a href="{{ urls.database(database) }}.db">{{ database }}.db</a> <em>{{ format_bytes(size) }}</em></p>
 {% endif %}
 
 {% include "_codemirror_foot.html" %}
diff --git a/datasette/templates/index.html b/datasette/templates/index.html
index c1adfc59..06e09635 100644
--- a/datasette/templates/index.html
+++ b/datasette/templates/index.html
@@ -10,7 +10,7 @@
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
 {% for database in databases %}
-    <h2 style="padding-left: 10px; border-left: 10px solid #{{ database.color }}"><a href="{{ database.path }}">{{ database.name }}</a>{% if database.private %} 🔒{% endif %}</h2>
+    <h2 style="padding-left: 10px; border-left: 10px solid #{{ database.color }}"><a href="{{ urls.database(database.name) }}">{{ database.name }}</a>{% if database.private %} 🔒{% endif %}</h2>
     <p>
         {% if database.show_table_row_counts %}{{ "{:,}".format(database.table_rows_sum) }} rows in {% endif %}{{ database.tables_count }} table{% if database.tables_count != 1 %}s{% endif %}{% if database.tables_count and database.hidden_tables_count %}, {% endif -%}
         {% if database.hidden_tables_count -%}
@@ -21,8 +21,7 @@
             {{ "{:,}".format(database.views_count) }} view{% if database.views_count != 1 %}s{% endif %}
         {% endif %}
     </p>
-    <p>{% for table in database.tables_and_views_truncated %}<a href="{{ database.path }}/{{ table.name|quote_plus 
-    }}"{% if table.count %} title="{{ table.count }} rows"{% endif %}>{{ table.name }}</a>{% if table.private %} 🔒{% endif %}{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_and_views_more %}, <a href="{{ database.path }}">...</a>{% endif %}</p>
+    <p>{% for table in database.tables_and_views_truncated %}<a href="{{ urls.table(database.name, table.name) }}"{% if table.count %} title="{{ table.count }} rows"{% endif %}>{{ table.name }}</a>{% if table.private %} 🔒{% endif %}{% if not loop.last %}, {% endif %}{% endfor %}{% if database.tables_and_views_more %}, <a href="{{ urls.database(database.name) }}">...</a>{% endif %}</p>
 {% endfor %}
 
 {% endblock %}
diff --git a/datasette/templates/query.html b/datasette/templates/query.html
index be180f33..911119bb 100644
--- a/datasette/templates/query.html
+++ b/datasette/templates/query.html
@@ -20,8 +20,8 @@
 
 {% block nav %}
     <p class="crumbs">
-        <a href="/">home</a> /
-        <a href="{{ database_url(database) }}">{{ database }}</a>
+        <a href="{{ urls.instance() }}">home</a> /
+        <a href="{{ urls.database(database) }}">{{ database }}</a>
     </p>
     {{ super() }}
 {% endblock %}
@@ -32,7 +32,7 @@
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
-<form class="sql" action="{{ database_url(database) }}{% if canned_query %}/{{ canned_query }}{% endif %}" method="{% if canned_write %}post{% else %}get{% endif %}">
+<form class="sql" action="{{ urls.database(database) }}{% if canned_query %}/{{ canned_query }}{% endif %}" method="{% if canned_write %}post{% else %}get{% endif %}">
     <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 %} <span class="show-hide-sql">{% if hide_sql %}(<a href="{{ path_with_removed_args(request, {'_hide_sql': '1'}) }}">show</a>){% else %}(<a href="{{ path_with_added_args(request, {'_hide_sql': '1'}) }}">hide</a>){% endif %}</span></h3>
     {% if not hide_sql %}
         {% if editable and allow_execute_sql %}
diff --git a/datasette/templates/row.html b/datasette/templates/row.html
index cd49a497..916980b6 100644
--- a/datasette/templates/row.html
+++ b/datasette/templates/row.html
@@ -17,9 +17,9 @@
 
 {% block nav %}
     <p class="crumbs">
-        <a href="{{ base_url }}">home</a> /
-        <a href="{{ database_url(database) }}">{{ database }}</a> /
-        <a href="{{ database_url(database) }}/{{ table|quote_plus }}">{{ table }}</a>
+        <a href="{{ urls.instance() }}">home</a> /
+        <a href="{{ urls.database(database) }}">{{ database }}</a> /
+        <a href="{{ urls.table(database, table) }}">{{ table }}</a>
     </p>
     {{ super() }}
 {% endblock %}
@@ -38,7 +38,7 @@
     <ul>
         {% for other in foreign_key_tables %}
             <li>
-                <a href="{{ database_url(database) }}/{{ other.other_table|quote_plus }}?{{ other.other_column }}={{ ', '.join(primary_key_values) }}">
+                <a href="{{ urls.table(database, other.other_table) }}?{{ 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 ab2331c3..3f8c2fee 100644
--- a/datasette/templates/table.html
+++ b/datasette/templates/table.html
@@ -5,7 +5,7 @@
 
 {% block extra_head %}
 {{ super() }}
-<script src="{{ base_url }}-/static/table.js" defer></script>
+<script src="{{ urls.static('table.js') }}" defer></script>
 <style>
 @media only screen and (max-width: 576px) {
 {% for column in display_columns %}
@@ -18,8 +18,8 @@
 
 {% block nav %}
     <p class="crumbs">
-        <a href="{{ base_url }}">home</a> /
-        <a href="{{ database_url(database) }}">{{ database }}</a>
+        <a href="{{ urls.instance() }}">home</a> /
+        <a href="{{ urls.database(database) }}">{{ database }}</a>
     </p>
     {{ super() }}
 {% endblock %}
@@ -36,7 +36,7 @@
     </h3>
 {% endif %}
 
-<form class="filters" action="{{ database_url(database) }}/{{ table|quote_plus }}" method="get">
+<form class="filters" action="{{ urls.table(database, table) }}" 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 %}
@@ -107,7 +107,7 @@
 {% endif %}
 
 {% if query.sql and allow_execute_sql %}
-    <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>
+    <p><a class="not-underlined" title="{{ query.sql }}" href="{{ urls.database(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 {% for name, url in renderers.items() %}<a href="{{ url }}">{{ name }}</a>{{ ", " if not loop.last }}{% endfor %}{% if display_rows %}, <a href="{{ url_csv }}">CSV</a> (<a href="#export">advanced</a>){% endif %}</p>
diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py
index 7026bdf0..dea50861 100644
--- a/datasette/utils/__init__.py
+++ b/datasette/utils/__init__.py
@@ -59,6 +59,8 @@ SPATIALITE_PATHS = (
     "/usr/lib/x86_64-linux-gnu/mod_spatialite.so",
     "/usr/local/lib/mod_spatialite.dylib",
 )
+# Length of hash subset used in hashed URLs:
+HASH_LENGTH = 7
 
 # Can replace this with Column from sqlite_utils when I add that dependency
 Column = namedtuple(
diff --git a/datasette/views/base.py b/datasette/views/base.py
index 6cf0e8d9..92ec03ca 100644
--- a/datasette/views/base.py
+++ b/datasette/views/base.py
@@ -1,7 +1,5 @@
 import asyncio
 import csv
-import itertools
-import json
 import re
 import time
 import urllib
@@ -16,27 +14,22 @@ from datasette.utils import (
     InvalidSql,
     LimitedWriter,
     call_with_supported_arguments,
-    is_url,
     path_with_added_args,
     path_with_removed_args,
     path_with_format,
     resolve_table_and_format,
     sqlite3,
-    to_css_class,
+    HASH_LENGTH,
 )
 from datasette.utils.asgi import (
     AsgiStream,
-    AsgiWriter,
     Forbidden,
     NotFound,
-    Request,
     Response,
 )
 
 ureg = pint.UnitRegistry()
 
-HASH_LENGTH = 7
-
 
 class DatasetteError(Exception):
     def __init__(
@@ -99,14 +92,6 @@ class BaseView:
                 else:
                     raise Forbidden(action)
 
-    def database_url(self, database):
-        db = self.ds.databases[database]
-        base_url = self.ds.config("base_url")
-        if self.ds.config("hash_urls") and db.hash:
-            return "{}{}-{}".format(base_url, database, db.hash[:HASH_LENGTH])
-        else:
-            return "{}{}".format(base_url, database)
-
     def database_color(self, database):
         return "ff0000"
 
@@ -132,7 +117,6 @@ class BaseView:
         template_context = {
             **context,
             **{
-                "database_url": self.database_url,
                 "database_color": self.database_color,
                 "select_templates": [
                     "{}{}".format(
diff --git a/datasette/views/database.py b/datasette/views/database.py
index c06a6cea..772271c4 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -348,7 +348,7 @@ class QueryView(DataView):
                 pass
             if allow_execute_sql and is_validated_sql and ":_" not in sql:
                 edit_sql_url = (
-                    self.database_url(database)
+                    self.ds.urls.database(database)
                     + "?"
                     + urlencode(
                         {
diff --git a/datasette/views/index.py b/datasette/views/index.py
index ace4ff29..92f60855 100644
--- a/datasette/views/index.py
+++ b/datasette/views/index.py
@@ -112,7 +112,7 @@ class IndexView(BaseView):
                     "color": db.hash[:6]
                     if db.hash
                     else hashlib.md5(name.encode("utf8")).hexdigest()[:6],
-                    "path": self.database_url(name),
+                    "path": self.ds.urls.database(name),
                     "tables_and_views_truncated": tables_and_views_truncated,
                     "tables_and_views_more": (len(visible_tables) + len(views))
                     > TRUNCATE_AT,
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 76f94aa1..1aff8cd1 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -63,8 +63,13 @@ def test_spatialite_error_if_attempt_to_open_spatialite():
 def test_spatialite_error_if_cannot_find_load_extension_spatialite():
     runner = CliRunner()
     result = runner.invoke(
-        cli, ["serve", str(pathlib.Path(__file__).parent / "spatialite.db"),
-        "--load-extension", "spatialite"]
+        cli,
+        [
+            "serve",
+            str(pathlib.Path(__file__).parent / "spatialite.db"),
+            "--load-extension",
+            "spatialite",
+        ],
     )
     assert result.exit_code != 0
     assert "Could not find SpatiaLite extension" in result.output