From 53bf875483d98861314db3a0cdcec8f5ce22ee96 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 6 Apr 2019 19:56:07 -0700 Subject: [PATCH] expand_foreign_keys() no longer uses inspect, refs #420 --- datasette/app.py | 29 +++++++++++++++++++++++++++++ datasette/inspect.py | 12 ------------ datasette/utils.py | 16 ++++++++++++++++ datasette/views/base.py | 8 -------- datasette/views/table.py | 24 +++++++----------------- docs/introspection.rst | 1 - tests/test_api.py | 25 ------------------------- 7 files changed, 52 insertions(+), 63 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 29dd91af46..0b992fbeef 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -29,6 +29,7 @@ Results, escape_css_string, escape_sqlite, + get_outbound_foreign_keys, get_plugins, module_from_path, sqlite3, @@ -464,11 +465,39 @@ def plugins(self, show_all=False): for p in ps ] + def table_metadata(self, database, table): + "Fetch table-specific metadata." + return (self.metadata("databases") or {}).get(database, {}).get( + "tables", {} + ).get( + table, {} + ) + async def table_columns(self, db_name, table): return await self.execute_against_connection_in_thread( db_name, lambda conn: table_columns(conn, table) ) + async def foreign_keys_for_table(self, database, table): + return await self.execute_against_connection_in_thread( + database, lambda conn: get_outbound_foreign_keys(conn, table) + ) + + async def label_column_for_table(self, db_name, table): + explicit_label_column = ( + self.table_metadata( + db_name, table + ).get("label_column") + ) + if explicit_label_column: + return explicit_label_column + # If a table has two columns, one of which is ID, then label_column is the other one + column_names = await self.table_columns(db_name, table) + if (column_names and len(column_names) == 2 and "id" in column_names): + return [c for c in column_names if c != "id"][0] + # Couldn't find a label: + return None + async def execute_against_connection_in_thread(self, db_name, fn): def in_thread(): conn = getattr(connections, db_name, None) diff --git a/datasette/inspect.py b/datasette/inspect.py index 12f67f0a01..3ff8b36745 100644 --- a/datasette/inspect.py +++ b/datasette/inspect.py @@ -31,17 +31,6 @@ def inspect_views(conn): return [v[0] for v in conn.execute('select name from sqlite_master where type = "view"')] -def detect_label_column(column_names): - """ Detect the label column - which we display as the label for a joined column. - - If a table has two columns, one of which is ID, then label_column is the other one. - """ - if (column_names and len(column_names) == 2 and "id" in column_names): - return [c for c in column_names if c != "id"][0] - - return None - - def detect_primary_keys(conn, table): " Figure out primary keys for a table. " table_info_rows = [ @@ -86,7 +75,6 @@ def inspect_tables(conn, database_metadata): "columns": column_names, "primary_keys": detect_primary_keys(conn, table), "count": count, - "label_column": detect_label_column(column_names), "hidden": table_metadata.get("hidden") or False, "fts_table": detect_fts(conn, table), } diff --git a/datasette/utils.py b/datasette/utils.py index 98f70592c2..50299ab752 100644 --- a/datasette/utils.py +++ b/datasette/utils.py @@ -475,6 +475,22 @@ def temporary_heroku_directory( os.chdir(saved_cwd) +def get_outbound_foreign_keys(conn, table): + infos = conn.execute( + 'PRAGMA foreign_key_list([{}])'.format(table) + ).fetchall() + fks = [] + for info in infos: + if info is not None: + id, seq, table_name, from_, to_, on_update, on_delete, match = info + fks.append({ + 'other_table': table_name, + 'column': from_, + 'other_column': to_ + }) + return fks + + def get_all_foreign_keys(conn): tables = [r[0] for r in conn.execute('select name from sqlite_master where type="table"')] table_to_foreign_keys = {} diff --git a/datasette/views/base.py b/datasette/views/base.py index 99bb175ff7..a0a1667970 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -131,14 +131,6 @@ class BaseView(RenderMixin): def __init__(self, datasette): self.ds = datasette - def table_metadata(self, database, table): - "Fetch table-specific metadata." - return (self.ds.metadata("databases") or {}).get(database, {}).get( - "tables", {} - ).get( - table, {} - ) - def options(self, request, *args, **kwargs): r = response.text("ok") if self.ds.cors: diff --git a/datasette/views/table.py b/datasette/views/table.py index 84ebec053b..49b7ac4b4e 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -33,7 +33,7 @@ class RowTableShared(BaseView): async def sortable_columns_for_table(self, database, table, use_rowid): - table_metadata = self.table_metadata(database, table) + table_metadata = self.ds.table_metadata(database, table) if "sortable_columns" in table_metadata: sortable_columns = set(table_metadata["sortable_columns"]) else: @@ -51,7 +51,7 @@ def expandable_columns(self, database, table): expandables = [] for fk in table_info["foreign_keys"]["outgoing"]: label_column = ( - self.table_metadata( + self.ds.table_metadata( database, fk["other_table"] ).get("label_column") or tables.get(fk["other_table"], {}).get("label_column") @@ -62,11 +62,7 @@ def expandable_columns(self, database, table): async def expand_foreign_keys(self, database, table, column, values): "Returns dict mapping (column, value) -> label" labeled_fks = {} - tables_info = self.ds.inspect()[database]["tables"] - table_info = tables_info.get(table) or {} - if not table_info: - return {} - foreign_keys = table_info["foreign_keys"]["outgoing"] + foreign_keys = await self.ds.foreign_keys_for_table(database, table) # Find the foreign_key for this column try: fk = [ @@ -75,13 +71,7 @@ async def expand_foreign_keys(self, database, table, column, values): ][0] except IndexError: return {} - label_column = ( - # First look in metadata.json for this foreign key table: - self.table_metadata( - database, fk["other_table"] - ).get("label_column") - or tables_info.get(fk["other_table"], {}).get("label_column") - ) + label_column = await self.ds.label_column_for_table(database, fk["other_table"]) if not label_column: return { (fk["column"], value): str(value) @@ -119,7 +109,7 @@ async def display_columns_and_rows( truncate_cells=0, ): "Returns columns, rows for specified table - including fancy foreign key treatment" - table_metadata = self.table_metadata(database, table) + table_metadata = self.ds.table_metadata(database, table) info = self.ds.inspect()[database] sortable_columns = await self.sortable_columns_for_table(database, table, True) columns = [ @@ -309,7 +299,7 @@ async def data(self, request, database, hash, table, default_labels=False, _nex forward_querystring=False, ) - table_metadata = self.table_metadata(database, table) + table_metadata = self.ds.table_metadata(database, table) units = table_metadata.get("units", {}) filters = Filters(sorted(other_args.items()), units, ureg) where_clauses, params = filters.build_where_clauses() @@ -880,7 +870,7 @@ async def template_data(): "columns": columns, "primary_keys": pks, "primary_key_values": pk_values, - "units": self.table_metadata(database, table).get("units", {}), + "units": self.ds.table_metadata(database, table).get("units", {}), } if "foreign_key_tables" in (request.raw_args.get("_extras") or "").split(","): diff --git a/docs/introspection.rst b/docs/introspection.rst index b4dbfc6e2e..6aeedc615e 100644 --- a/docs/introspection.rst +++ b/docs/introspection.rst @@ -50,7 +50,6 @@ This is an internal implementation detail of Datasette and the format should not }, "fts_table": null, "hidden": false, - "label_column": null, "name": "./index", "primary_keys": [] }, diff --git a/tests/test_api.py b/tests/test_api.py index 6fdcb115fb..a50148c161 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -35,7 +35,6 @@ def test_database_page(app_client): 'count': 0, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': [], }, { @@ -44,7 +43,6 @@ def test_database_page(app_client): 'count': 0, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -68,7 +66,6 @@ def test_database_page(app_client): }], }, 'hidden': False, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -77,7 +74,6 @@ def test_database_page(app_client): 'count': 1, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk1', 'pk2'], }, { @@ -86,7 +82,6 @@ def test_database_page(app_client): 'count': 1001, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk1', 'pk2', 'pk3'], }, { @@ -102,7 +97,6 @@ def test_database_page(app_client): 'other_table': 'primary_key_multiple_columns_explicit_label' }], }, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -119,7 +113,6 @@ def test_database_page(app_client): }, 'fts_table': None, 'hidden': False, - 'label_column': 'name', 'primary_keys': ['id'], }, { 'columns': ['pk', 'planet_int', 'on_earth', 'state', 'city_id', 'neighborhood'], @@ -135,7 +128,6 @@ def test_database_page(app_client): }, 'fts_table': None, 'hidden': False, - 'label_column': None, 'primary_keys': ['pk'], }, { 'columns': ['pk', 'foreign_key_with_label', 'foreign_key_with_no_label'], @@ -154,7 +146,6 @@ def test_database_page(app_client): 'other_table': 'simple_primary_key', }], }, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -162,7 +153,6 @@ def test_database_page(app_client): "columns": ["value"], "count": 3, "primary_keys": [], - "label_column": None, "hidden": False, "fts_table": None, "foreign_keys": {"incoming": [], "outgoing": []} @@ -179,7 +169,6 @@ def test_database_page(app_client): 'outgoing': [] }, 'hidden': False, - 'label_column': None, 'fts_table': None, 'primary_keys': ['id'] }, { @@ -195,7 +184,6 @@ def test_database_page(app_client): 'outgoing': [] }, 'hidden': False, - 'label_column': None, 'fts_table': None, 'primary_keys': ['id'] }, { @@ -209,14 +197,12 @@ def test_database_page(app_client): }], 'outgoing': []}, 'fts_table': 'searchable_fts', 'hidden': False, - 'label_column': None, 'primary_keys': ['pk'], }, { "name": "searchable_tags", "columns": ["searchable_id", "tag"], "primary_keys": ["searchable_id", "tag"], "count": 2, - "label_column": None, "hidden": False, "fts_table": None, "foreign_keys": { @@ -240,7 +226,6 @@ def test_database_page(app_client): 'count': 1, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': [], }, { @@ -268,7 +253,6 @@ def test_database_page(app_client): }], 'outgoing': [], }, - 'label_column': 'content', 'fts_table': None, 'primary_keys': ['id'], }, { @@ -280,7 +264,6 @@ def test_database_page(app_client): 'count': 201, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk1', 'pk2'], }, { @@ -289,7 +272,6 @@ def test_database_page(app_client): 'count': 1, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -297,7 +279,6 @@ def test_database_page(app_client): "columns": ["tag"], "primary_keys": ["tag"], "count": 2, - "label_column": None, "hidden": False, "fts_table": None, "foreign_keys": { @@ -316,7 +297,6 @@ def test_database_page(app_client): 'count': 3, 'hidden': False, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': ['pk'], }, { @@ -325,7 +305,6 @@ def test_database_page(app_client): 'count': 201, 'hidden': True, 'foreign_keys': {'incoming': [], 'outgoing': []}, - 'label_column': None, 'fts_table': None, 'primary_keys': [], }, { @@ -334,7 +313,6 @@ def test_database_page(app_client): 'foreign_keys': {'incoming': [], 'outgoing': []}, 'fts_table': 'searchable_fts', 'hidden': True, - 'label_column': None, 'name': 'searchable_fts', 'primary_keys': [] }, { @@ -343,7 +321,6 @@ def test_database_page(app_client): 'foreign_keys': {'incoming': [], 'outgoing': []}, 'fts_table': None, 'hidden': True, - 'label_column': None, 'name': 'searchable_fts_content', 'primary_keys': ['docid'] }, { @@ -355,7 +332,6 @@ def test_database_page(app_client): 'foreign_keys': {'incoming': [], 'outgoing': []}, 'fts_table': None, 'hidden': True, - 'label_column': None, 'name': 'searchable_fts_segdir', 'primary_keys': ['level', 'idx'] }, { @@ -364,7 +340,6 @@ def test_database_page(app_client): 'foreign_keys': {'incoming': [], 'outgoing': []}, 'fts_table': None, 'hidden': True, - 'label_column': None, 'name': 'searchable_fts_segments', 'primary_keys': ['blockid'] }] == data['tables']