Skip to content

Commit

Permalink
Renamed /:memory: to /_memory, with redirects - closes #1205
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Jan 28, 2021
1 parent 382e9ec commit 1600d2a
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Now visiting http://localhost:8001/History/downloads will show you a web interfa
--plugins-dir DIRECTORY Path to directory containing custom plugins
--static STATIC MOUNT mountpoint:path-to-directory for serving static
files
--memory Make :memory: database available
--memory Make /_memory database available
--config CONFIG Set config option using configname:value
docs.datasette.io/en/stable/config.html
--version-note TEXT Additional note to show on /-/versions
Expand Down
23 changes: 19 additions & 4 deletions datasette/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def __init__(
self.immutables = set(immutables or [])
self.databases = collections.OrderedDict()
if memory or not self.files:
self.add_database(Database(self, is_memory=True), name=":memory:")
self.add_database(Database(self, is_memory=True), name="_memory")
# memory_name is a random string so that each Datasette instance gets its own
# unique in-memory named database - otherwise unit tests can fail with weird
# errors when different instances accidentally share an in-memory database
Expand Down Expand Up @@ -633,7 +633,7 @@ def _connected_databases(self):

def _versions(self):
conn = sqlite3.connect(":memory:")
self._prepare_connection(conn, ":memory:")
self._prepare_connection(conn, "_memory")
sqlite_version = conn.execute("select sqlite_version()").fetchone()[0]
sqlite_extensions = {}
for extension, testsql, hasversion in (
Expand Down Expand Up @@ -927,6 +927,12 @@ def add_route(view, regex):
plugin["name"].replace("-", "_")
),
)
add_route(
permanent_redirect(
"/_memory", forward_query_string=True, forward_rest=True
),
r"/:memory:(?P<rest>.*)$",
)
add_route(
JsonDataView.as_view(self, "metadata.json", lambda: self._metadata),
r"/-/metadata(?P<as_format>(\.json)?)$",
Expand Down Expand Up @@ -1290,9 +1296,18 @@ async def async_view_fn(request, send):
return async_view_fn


def permanent_redirect(path):
def permanent_redirect(path, forward_query_string=False, forward_rest=False):
return wrap_view(
lambda request, send: Response.redirect(path, status=301),
lambda request, send: Response.redirect(
path
+ (request.url_vars["rest"] if forward_rest else "")
+ (
("?" + request.query_string)
if forward_query_string and request.query_string
else ""
),
status=301,
),
datasette=None,
)

Expand Down
2 changes: 1 addition & 1 deletion datasette/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ def uninstall(packages, yes):
help="Serve static files from this directory at /MOUNT/...",
multiple=True,
)
@click.option("--memory", is_flag=True, help="Make :memory: database available")
@click.option("--memory", is_flag=True, help="Make /_memory database available")
@click.option(
"--config",
type=Config(),
Expand Down
4 changes: 2 additions & 2 deletions datasette/views/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ async def database_actions():
"metadata": metadata,
"allow_download": self.ds.setting("allow_download")
and not db.is_mutable
and database != ":memory:",
and not db.is_memory,
},
(f"database-{to_css_class(database)}.html", "database.html"),
)
Expand All @@ -160,7 +160,7 @@ async def view_get(self, request, database, hash, correct_hash_present, **kwargs
raise DatasetteError("Invalid database", status=404)
db = self.ds.databases[database]
if db.is_memory:
raise DatasetteError("Cannot download :memory: database", status=404)
raise DatasetteError("Cannot download in-memory databases", status=404)
if not self.ds.setting("allow_download") or db.is_mutable:
raise Forbidden("Database download is forbidden")
if not db.path:
Expand Down
2 changes: 1 addition & 1 deletion docs/datasette-serve-help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Options:
--template-dir DIRECTORY Path to directory containing custom templates
--plugins-dir DIRECTORY Path to directory containing custom plugins
--static MOUNT:DIRECTORY Serve static files from this directory at /MOUNT/...
--memory Make :memory: database available
--memory Make /_memory database available
--config CONFIG Deprecated: set config option using configname:value. Use
--setting instead.

Expand Down
26 changes: 21 additions & 5 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,11 +608,11 @@ def test_no_files_uses_memory_database(app_client_no_files):
response = app_client_no_files.get("/.json")
assert response.status == 200
assert {
":memory:": {
"name": ":memory:",
"_memory": {
"name": "_memory",
"hash": None,
"color": "f7935d",
"path": "/%3Amemory%3A",
"color": "a6c7b9",
"path": "/_memory",
"tables_and_views_truncated": [],
"tables_and_views_more": False,
"tables_count": 0,
Expand All @@ -626,12 +626,28 @@ def test_no_files_uses_memory_database(app_client_no_files):
} == response.json
# Try that SQL query
response = app_client_no_files.get(
"/:memory:.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())


@pytest.mark.parametrize(
"path,expected_redirect",
(
("/:memory:", "/_memory"),
("/:memory:.json", "/_memory.json"),
("/:memory:?sql=select+1", "/_memory?sql=select+1"),
("/:memory:.json?sql=select+1", "/_memory.json?sql=select+1"),
("/:memory:.csv?sql=select+1", "/_memory.csv?sql=select+1"),
),
)
def test_old_memory_urls_redirect(app_client_no_files, path, expected_redirect):
response = app_client_no_files.get(path, allow_redirects=False)
assert response.status == 301
assert response.headers["location"] == 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
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def test_config_deprecated(ensure_eventloop):

def test_sql_errors_logged_to_stderr(ensure_eventloop):
runner = CliRunner(mix_stderr=False)
result = runner.invoke(cli, ["--get", "/:memory:.json?sql=select+blah"])
result = runner.invoke(cli, ["--get", "/_memory.json?sql=select+blah"])
assert result.exit_code == 1
assert "sql = 'select blah', params = {}: no such column: blah\n" in result.stderr

Expand Down
4 changes: 2 additions & 2 deletions tests/test_cli_serve_get.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ def startup(datasette):
"--plugins-dir",
str(plugins_dir),
"--get",
"/:memory:.json?sql=select+sqlite_version()",
"/_memory.json?sql=select+sqlite_version()",
],
)
assert 0 == result.exit_code, result.output
assert {
"database": ":memory:",
"database": "_memory",
"truncated": False,
"columns": ["sqlite_version()"],
}.items() <= json.loads(result.output).items()
Expand Down
6 changes: 3 additions & 3 deletions tests/test_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def test_static_mounts():

def test_memory_database_page():
with make_app_client(memory=True) as client:
response = client.get("/:memory:")
response = client.get("/_memory")
assert response.status == 200


Expand Down Expand Up @@ -1056,10 +1056,10 @@ def test_database_download_disallowed_for_mutable(app_client):
def test_database_download_disallowed_for_memory():
with make_app_client(memory=True) as client:
# Memory page should NOT have a download link
response = client.get("/:memory:")
response = client.get("/_memory")
soup = Soup(response.body, "html.parser")
assert 0 == len(soup.findAll("a", {"href": re.compile(r"\.db$")}))
assert 404 == client.get("/:memory:.db").status
assert 404 == client.get("/_memory.db").status


def test_allow_download_off():
Expand Down
28 changes: 14 additions & 14 deletions tests/test_internals_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,48 +103,48 @@ def test_logout(ds, base_url, expected):
@pytest.mark.parametrize(
"base_url,format,expected",
[
("/", None, "/%3Amemory%3A"),
("/prefix/", None, "/prefix/%3Amemory%3A"),
("/", "json", "/%3Amemory%3A.json"),
("/", None, "/_memory"),
("/prefix/", None, "/prefix/_memory"),
("/", "json", "/_memory.json"),
],
)
def test_database(ds, base_url, format, expected):
ds._settings["base_url"] = base_url
actual = ds.urls.database(":memory:", format=format)
actual = ds.urls.database("_memory", format=format)
assert actual == expected
assert isinstance(actual, PrefixedUrlString)


@pytest.mark.parametrize(
"base_url,name,format,expected",
[
("/", "name", None, "/%3Amemory%3A/name"),
("/prefix/", "name", None, "/prefix/%3Amemory%3A/name"),
("/", "name", "json", "/%3Amemory%3A/name.json"),
("/", "name.json", "json", "/%3Amemory%3A/name.json?_format=json"),
("/", "name", None, "/_memory/name"),
("/prefix/", "name", None, "/prefix/_memory/name"),
("/", "name", "json", "/_memory/name.json"),
("/", "name.json", "json", "/_memory/name.json?_format=json"),
],
)
def test_table_and_query(ds, base_url, name, format, expected):
ds._settings["base_url"] = base_url
actual1 = ds.urls.table(":memory:", name, format=format)
actual1 = ds.urls.table("_memory", name, format=format)
assert actual1 == expected
assert isinstance(actual1, PrefixedUrlString)
actual2 = ds.urls.query(":memory:", name, format=format)
actual2 = ds.urls.query("_memory", name, format=format)
assert actual2 == expected
assert isinstance(actual2, PrefixedUrlString)


@pytest.mark.parametrize(
"base_url,format,expected",
[
("/", None, "/%3Amemory%3A/facetable/1"),
("/prefix/", None, "/prefix/%3Amemory%3A/facetable/1"),
("/", "json", "/%3Amemory%3A/facetable/1.json"),
("/", None, "/_memory/facetable/1"),
("/prefix/", None, "/prefix/_memory/facetable/1"),
("/", "json", "/_memory/facetable/1.json"),
],
)
def test_row(ds, base_url, format, expected):
ds._settings["base_url"] = base_url
actual = ds.urls.row(":memory:", "facetable", "1", format=format)
actual = ds.urls.row("_memory", "facetable", "1", format=format)
assert actual == expected
assert isinstance(actual, PrefixedUrlString)

Expand Down

0 comments on commit 1600d2a

Please sign in to comment.