From baf986c871708c01ca183be760995cf306ba21bf Mon Sep 17 00:00:00 2001 From: Brandon Roberts Date: Sat, 26 Jun 2021 15:24:54 -0700 Subject: [PATCH] New get_metadata() plugin hook for dynamic metadata The following hook is added: get_metadata( datasette=self, key=key, database=database, table=table, fallback=fallback ) This gets called when we're building our metdata for the rest of the system to use. We merge whatever the plugins return with any local metadata (from metadata.yml/yaml/json) allowing for a live-editable dynamic Datasette. As a security precation, local meta is *not* overwritable by plugin hooks. The workflow for transitioning to live-meta would be to load the plugin with the full metadata.yaml and save. Then remove the parts of the metadata that you want to be able to change from the file. * Avoid race condition: don't mutate databases list This avoids the nasty "RuntimeError: OrderedDict mutated during iteration" error that randomly happens when a plugin adds a new database to Datasette, using `add_database`. This change makes the add and remove database functions more expensive, but it prevents the random explosion race conditions that make for confusing user experience when importing live databases. Thanks, @brandonrobertz --- .gitignore | 1 + datasette/app.py | 47 ++++++++++++++++++++++++++++++++----- datasette/hookspecs.py | 5 ++++ datasette/utils/__init__.py | 1 - docs/plugin_hooks.rst | 35 +++++++++++++++++++++++++++ tests/test_permissions.py | 6 ++--- tests/test_plugins.py | 29 +++++++++++++++++++++++ 7 files changed, 114 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 29ac176f..066009f0 100644 --- a/.gitignore +++ b/.gitignore @@ -117,3 +117,4 @@ ENV/ # macOS files .DS_Store node_modules +.*.swp diff --git a/datasette/app.py b/datasette/app.py index e11c12eb..05ad5a8d 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -251,7 +251,7 @@ class Datasette: if config_dir and metadata_files and not metadata: with metadata_files[0].open() as fp: metadata = parse_metadata(fp.read()) - self._metadata = metadata or {} + self._metadata_local = metadata or {} self.sqlite_functions = [] self.sqlite_extensions = [] for extension in sqlite_extensions or []: @@ -380,6 +380,7 @@ class Datasette: return self.databases[name] def add_database(self, db, name=None): + new_databases = self.databases.copy() if name is None: # Pick a unique name for this database suggestion = db.suggest_name() @@ -391,14 +392,18 @@ class Datasette: name = "{}_{}".format(suggestion, i) i += 1 db.name = name - self.databases[name] = db + new_databases[name] = db + # don't mutate! that causes race conditions with live import + self.databases = new_databases return db def add_memory_database(self, memory_name): return self.add_database(Database(self, memory_name=memory_name)) def remove_database(self, name): - self.databases.pop(name) + new_databases = self.databases.copy() + new_databases.pop(name) + self.databases = new_databases def setting(self, key): return self._settings.get(key, None) @@ -407,6 +412,17 @@ class Datasette: # Returns a fully resolved config dictionary, useful for templates return {option.name: self.setting(option.name) for option in SETTINGS} + def _metadata_recursive_update(self, orig, updated): + if not isinstance(orig, dict) or not isinstance(updated, dict): + return orig + + for key, upd_value in updated.items(): + if isinstance(upd_value, dict) and isinstance(orig.get(key), dict): + orig[key] = self._metadata_recursive_update(orig[key], upd_value) + else: + orig[key] = upd_value + return orig + def metadata(self, key=None, database=None, table=None, fallback=True): """ Looks up metadata, cascading backwards from specified level. @@ -415,7 +431,21 @@ class Datasette: assert not ( database is None and table is not None ), "Cannot call metadata() with table= specified but not database=" - databases = self._metadata.get("databases") or {} + metadata = {} + + for hook_dbs in pm.hook.get_metadata( + datasette=self, key=key, database=database, table=table, fallback=fallback + ): + metadata = self._metadata_recursive_update(metadata, hook_dbs) + + # security precaution!! don't allow anything in the local config + # to be overwritten. this is a temporary measure, not sure if this + # is a good idea long term or maybe if it should just be a concern + # of the plugin's implemtnation + metadata = self._metadata_recursive_update(metadata, self._metadata_local) + + databases = metadata.get("databases") or {} + search_list = [] if database is not None: search_list.append(databases.get(database) or {}) @@ -424,7 +454,8 @@ class Datasette: table ) or {} search_list.insert(0, table_metadata) - search_list.append(self._metadata) + + search_list.append(metadata) if not fallback: # No fallback allowed, so just use the first one in the list search_list = search_list[:1] @@ -440,6 +471,10 @@ class Datasette: m.update(item) return m + @property + def _metadata(self): + return self.metadata() + def plugin_config(self, plugin_name, database=None, table=None, fallback=True): """Return config for plugin, falling back from specified database/table""" plugins = self.metadata( @@ -960,7 +995,7 @@ class Datasette: r"/:memory:(?P.*)$", ) add_route( - JsonDataView.as_view(self, "metadata.json", lambda: self._metadata), + JsonDataView.as_view(self, "metadata.json", lambda: self.metadata()), r"/-/metadata(?P(\.json)?)$", ) add_route( diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py index 63b06097..c40b3148 100644 --- a/datasette/hookspecs.py +++ b/datasette/hookspecs.py @@ -10,6 +10,11 @@ def startup(datasette): """Fires directly after Datasette first starts running""" +@hookspec +def get_metadata(datasette, key, database, table, fallback): + """Get configuration""" + + @hookspec def asgi_wrapper(datasette): """Returns an ASGI middleware callable to wrap our ASGI application with""" diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 73122976..1e193862 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -21,7 +21,6 @@ import numbers import yaml from .shutil_backport import copytree from .sqlite import sqlite3, sqlite_version, supports_table_xinfo -from ..plugins import pm # From https://www.sqlite.org/lang_keywords.html diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 5af601b4..9ec75f34 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -1129,3 +1129,38 @@ This example will disable CSRF protection for that specific URL path: return scope["path"] == "/submit-comment" If any of the currently active ``skip_csrf()`` plugin hooks return ``True``, CSRF protection will be skipped for the request. + +get_metadata(datasette, key, database, table, fallback) +------------------------------------------------------- + +``datasette`` - :ref:`internals_datasette` + You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``, or to execute SQL queries. + +``actor`` - dictionary or None + The currently authenticated :ref:`actor `. + +``database`` - string or None + The name of the database metadata is being asked for. + +``table`` - string or None + The name of the table. + +``key`` - string or None + The name of the key for which data is being asked for. + +This hook is responsible for returning a dictionary corresponding to Datasette :ref:`metadata`. This function is passed the `database`, `table` and `key` which were passed to the upstream internal request for metadata. Regardless, it is important to return a global metadata object, where `"databases": []` would be a top-level key. The dictionary returned here, will be merged with, and overwritten by, the contents of the physical `metadata.yaml` if one is present. + +.. code-block:: python + + @hookimpl + def get_metadata(datasette, key, database, table, fallback): + metadata = { + "title": "This will be the Datasette landing page title!", + "description": get_instance_description(datasette), + "databases": [], + } + for db_name, db_data_dict in get_my_database_meta(datasette, database, table, key): + metadata["databases"][db_name] = db_data_dict + # whatever we return here will be merged with any other plugins using this hook and + # will be overwritten by a local metadata.yaml if one exists! + return metadata diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 9317c0d9..788523b0 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -440,7 +440,7 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta """Test that e.g. having view-table but NOT view-database lets you view table page, etc""" allow = {"id": "*"} deny = {} - previous_metadata = cascade_app_client.ds._metadata + previous_metadata = cascade_app_client.ds.metadata() updated_metadata = copy.deepcopy(previous_metadata) actor = {"id": "test"} if "download" in permissions: @@ -457,11 +457,11 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta updated_metadata["databases"]["fixtures"]["queries"]["magic_parameters"][ "allow" ] = (allow if "query" in permissions else deny) - cascade_app_client.ds._metadata = updated_metadata + cascade_app_client.ds._metadata_local = updated_metadata response = cascade_app_client.get( path, cookies={"ds_actor": cascade_app_client.actor_cookie(actor)}, ) assert expected_status == response.status finally: - cascade_app_client.ds._metadata = previous_metadata + cascade_app_client.ds._metadata_local = previous_metadata diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 14273282..3b9c06b9 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -850,3 +850,32 @@ def test_hook_skip_csrf(app_client): "/skip-csrf-2", post_data={"this is": "post data"}, cookies={"ds_actor": cookie} ) assert second_missing_csrf_response.status == 403 + + +def test_hook_get_metadata(app_client): + app_client.ds._metadata_local = { + "title": "Testing get_metadata hook!", + "databases": { + "from-local": { + "title": "Hello from local metadata" + } + } + } + og_pm_hook_get_metadata = pm.hook.get_metadata + def get_metadata_mock(*args, **kwargs): + return [{ + "databases": { + "from-hook": { + "title": "Hello from the plugin hook" + }, + "from-local": { + "title": "This will be overwritten!" + } + } + }] + pm.hook.get_metadata = get_metadata_mock + meta = app_client.ds.metadata() + assert "Testing get_metadata hook!" == meta["title"] + assert "Hello from local metadata" == meta["databases"]["from-local"]["title"] + assert "Hello from the plugin hook" == meta["databases"]["from-hook"]["title"] + pm.hook.get_metadata = og_pm_hook_get_metadata