From 3425d4924e35b10f915f8f9a911307026c5d12f7 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 3 Feb 2020 22:21:12 -0800 Subject: [PATCH 1/8] render_template refactor, refs #577 --- datasette/app.py | 90 ++++++++++++++++++++++++++++++++++++++ datasette/views/base.py | 95 +++++------------------------------------ 2 files changed, 100 insertions(+), 85 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index f6b3e514..2ab9b1b3 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -27,6 +27,7 @@ from .utils import ( QueryInterrupted, escape_css_string, escape_sqlite, + format_bytes, get_plugins, module_from_path, sqlite3, @@ -526,6 +527,95 @@ class Datasette: for renderer in hook_renderers: self.renderers[renderer["extension"]] = renderer["callback"] + def _asset_urls(self, key, template, context): + # Flatten list-of-lists from plugins: + seen_urls = set() + for url_or_dict in itertools.chain( + itertools.chain.from_iterable( + getattr(pm.hook, key)( + template=template.name, + database=context.get("database"), + table=context.get("table"), + datasette=self.ds, + ) + ), + (self.ds.metadata(key) or []), + ): + if isinstance(url_or_dict, dict): + url = url_or_dict["url"] + sri = url_or_dict.get("sri") + else: + url = url_or_dict + sri = None + if url in seen_urls: + continue + seen_urls.add(url) + if sri: + yield {"url": url, "sri": sri} + else: + yield {"url": url} + + async def render_template( + self, templates, context=None, request=None, view_name=None + ): + if isinstance(templates, str): + templates = [templates] + template = self.jinja_env.select_template(templates) + select_templates = [ + "{}{}".format("*" if template_name == template.name else "", template_name) + for template_name in templates + ] + body_scripts = [] + # pylint: disable=no-member + for script in pm.hook.extra_body_script( + template=template.name, + database=context.get("database"), + table=context.get("table"), + view_name=view_name, + datasette=self, + ): + body_scripts.append(Markup(script)) + + extra_template_vars = {} + # pylint: disable=no-member + for extra_vars in pm.hook.extra_template_vars( + template=template.name, + database=context.get("database"), + table=context.get("table"), + view_name=view_name, + request=request, + datasette=self, + ): + if callable(extra_vars): + extra_vars = extra_vars() + if asyncio.iscoroutine(extra_vars): + extra_vars = await extra_vars + assert isinstance(extra_vars, dict), "extra_vars is of type {}".format( + type(extra_vars) + ) + extra_template_vars.update(extra_vars) + + template_context = { + **context, + **{ + "app_css_hash": self.app_css_hash(), + "select_templates": select_templates, + "zip": zip, + "body_scripts": body_scripts, + "extra_css_urls": self._asset_urls("extra_css_urls", template, context), + "extra_js_urls": self._asset_urls("extra_js_urls", template, context), + "format_bytes": format_bytes, + }, + **extra_template_vars, + } + if request.args.get("_context") and self.config("template_debug"): + return Response.html( + "
{}
".format( + escape(json.dumps(template_context, default=repr, indent=4)) + ) + ) + return Response.html(await template.render_async(template_context)) + def app(self): "Returns an ASGI app function that serves the whole of Datasette" default_templates = str(app_root / "datasette" / "templates") diff --git a/datasette/views/base.py b/datasette/views/base.py index 61561a32..a048fe3d 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -17,7 +17,6 @@ from datasette.utils import ( QueryInterrupted, InvalidSql, LimitedWriter, - format_bytes, is_url, path_with_added_args, path_with_removed_args, @@ -65,34 +64,6 @@ class BaseView(AsgiView): response.body = b"" return response - def _asset_urls(self, key, template, context): - # Flatten list-of-lists from plugins: - seen_urls = set() - for url_or_dict in itertools.chain( - itertools.chain.from_iterable( - getattr(pm.hook, key)( - template=template.name, - database=context.get("database"), - table=context.get("table"), - datasette=self.ds, - ) - ), - (self.ds.metadata(key) or []), - ): - if isinstance(url_or_dict, dict): - url = url_or_dict["url"] - sri = url_or_dict.get("sri") - else: - url = url_or_dict - sri = None - if url in seen_urls: - continue - seen_urls.add(url) - if sri: - yield {"url": url, "sri": sri} - else: - yield {"url": url} - def database_url(self, database): db = self.ds.databases[database] if self.ds.config("hash_urls") and db.hash: @@ -104,63 +75,17 @@ class BaseView(AsgiView): return "ff0000" async def render(self, templates, request, context): - template = self.ds.jinja_env.select_template(templates) - select_templates = [ - "{}{}".format("*" if template_name == template.name else "", template_name) - for template_name in templates - ] - body_scripts = [] - # pylint: disable=no-member - for script in pm.hook.extra_body_script( - template=template.name, - database=context.get("database"), - table=context.get("table"), - view_name=self.name, - datasette=self.ds, - ): - body_scripts.append(jinja2.Markup(script)) - - extra_template_vars = {} - # pylint: disable=no-member - for extra_vars in pm.hook.extra_template_vars( - template=template.name, - database=context.get("database"), - table=context.get("table"), - view_name=self.name, - request=request, - datasette=self.ds, - ): - if callable(extra_vars): - extra_vars = extra_vars() - if asyncio.iscoroutine(extra_vars): - extra_vars = await extra_vars - assert isinstance(extra_vars, dict), "extra_vars is of type {}".format( - type(extra_vars) - ) - extra_template_vars.update(extra_vars) - - template_context = { - **context, - **{ - "app_css_hash": self.ds.app_css_hash(), - "select_templates": select_templates, - "zip": zip, - "body_scripts": body_scripts, - "extra_css_urls": self._asset_urls("extra_css_urls", template, context), - "extra_js_urls": self._asset_urls("extra_js_urls", template, context), - "format_bytes": format_bytes, - "database_url": self.database_url, - "database_color": self.database_color, + return await self.ds.render_template( + templates, + { + **context, + **{ + "database_url": self.database_url, + "database_color": self.database_color, + }, }, - **extra_template_vars, - } - if request.args.get("_context") and self.ds.config("template_debug"): - return Response.html( - "
{}
".format( - escape(json.dumps(template_context, default=repr, indent=4)) - ) - ) - return Response.html(await template.render_async(template_context)) + request=request, + ) class DataView(BaseView): From 5019aa3c514ad8284dc70b150f0079d2e4d20800 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 3 Feb 2020 22:27:36 -0800 Subject: [PATCH 2/8] Fixed some imports --- datasette/app.py | 4 +++- datasette/views/base.py | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 2ab9b1b3..ba9e0234 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1,6 +1,7 @@ import asyncio import collections import hashlib +import json import os import re import sys @@ -12,7 +13,7 @@ from pathlib import Path import click from markupsafe import Markup -from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader +from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader, escape import uvicorn from .views.base import DatasetteError, ureg, AsgiRouter @@ -36,6 +37,7 @@ from .utils import ( from .utils.asgi import ( AsgiLifespan, NotFound, + Response, asgi_static, asgi_send, asgi_send_html, diff --git a/datasette/views/base.py b/datasette/views/base.py index a048fe3d..af3c5623 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -9,8 +9,6 @@ import urllib import jinja2 import pint -from html import escape - from datasette import __version__ from datasette.plugins import pm from datasette.utils import ( From 328ce115e39723e9f465f9aa6b31a3f3f839d391 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 3 Feb 2020 22:49:53 -0800 Subject: [PATCH 3/8] Continued refactor, refs #577 --- datasette/app.py | 51 +++++++++++------------------------------ datasette/views/base.py | 37 +++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index ba9e0234..48cb2f0e 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -14,6 +14,7 @@ from pathlib import Path import click from markupsafe import Markup from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader, escape +from jinja2.environment import Template import uvicorn from .views.base import DatasetteError, ureg, AsgiRouter @@ -529,44 +530,22 @@ class Datasette: for renderer in hook_renderers: self.renderers[renderer["extension"]] = renderer["callback"] - def _asset_urls(self, key, template, context): - # Flatten list-of-lists from plugins: - seen_urls = set() - for url_or_dict in itertools.chain( - itertools.chain.from_iterable( - getattr(pm.hook, key)( - template=template.name, - database=context.get("database"), - table=context.get("table"), - datasette=self.ds, - ) - ), - (self.ds.metadata(key) or []), - ): - if isinstance(url_or_dict, dict): - url = url_or_dict["url"] - sri = url_or_dict.get("sri") - else: - url = url_or_dict - sri = None - if url in seen_urls: - continue - seen_urls.add(url) - if sri: - yield {"url": url, "sri": sri} - else: - yield {"url": url} - async def render_template( self, templates, context=None, request=None, view_name=None ): - if isinstance(templates, str): - templates = [templates] - template = self.jinja_env.select_template(templates) - select_templates = [ - "{}{}".format("*" if template_name == template.name else "", template_name) - for template_name in templates - ] + if isinstance(templates, Template): + template = templates + select_templates = [] + else: + if isinstance(templates, str): + templates = [templates] + template = self.jinja_env.select_template(templates) + select_templates = [ + "{}{}".format( + "*" if template_name == template.name else "", template_name + ) + for template_name in templates + ] body_scripts = [] # pylint: disable=no-member for script in pm.hook.extra_body_script( @@ -604,8 +583,6 @@ class Datasette: "select_templates": select_templates, "zip": zip, "body_scripts": body_scripts, - "extra_css_urls": self._asset_urls("extra_css_urls", template, context), - "extra_js_urls": self._asset_urls("extra_js_urls", template, context), "format_bytes": format_bytes, }, **extra_template_vars, diff --git a/datasette/views/base.py b/datasette/views/base.py index af3c5623..7e579a30 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -72,14 +72,49 @@ class BaseView(AsgiView): def database_color(self, database): return "ff0000" + def _asset_urls(self, key, template, context): + # Flatten list-of-lists from plugins: + seen_urls = set() + for url_or_dict in itertools.chain( + itertools.chain.from_iterable( + getattr(pm.hook, key)( + template=template.name, + database=context.get("database"), + table=context.get("table"), + datasette=self.ds, + ) + ), + (self.ds.metadata(key) or []), + ): + if isinstance(url_or_dict, dict): + url = url_or_dict["url"] + sri = url_or_dict.get("sri") + else: + url = url_or_dict + sri = None + if url in seen_urls: + continue + seen_urls.add(url) + if sri: + yield {"url": url, "sri": sri} + else: + yield {"url": url} + async def render(self, templates, request, context): + template = self.ds.jinja_env.select_template(templates) return await self.ds.render_template( - templates, + template, { **context, **{ "database_url": self.database_url, "database_color": self.database_color, + "extra_css_urls": self._asset_urls( + "extra_css_urls", template, context + ), + "extra_js_urls": self._asset_urls( + "extra_js_urls", template, context + ), }, }, request=request, From 3dc5ee1cece11690cf6c2fb67ec7bf6624113dab Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 3 Feb 2020 23:02:00 -0800 Subject: [PATCH 4/8] Default context should be {} --- datasette/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/datasette/app.py b/datasette/app.py index 48cb2f0e..6152af21 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -533,6 +533,7 @@ class Datasette: async def render_template( self, templates, context=None, request=None, view_name=None ): + context = context or {} if isinstance(templates, Template): template = templates select_templates = [] From e05777d74f9e2ca0764fcd8081175a0326986889 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 4 Feb 2020 08:22:04 -0800 Subject: [PATCH 5/8] extra_css_urls and extra_js_urls for render_template() --- datasette/app.py | 31 +++++++++++++++++++++++++++++++ datasette/views/base.py | 34 ---------------------------------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 6152af21..4db64c35 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1,6 +1,7 @@ import asyncio import collections import hashlib +import itertools import json import os import re @@ -585,6 +586,8 @@ class Datasette: "zip": zip, "body_scripts": body_scripts, "format_bytes": format_bytes, + "extra_css_urls": self._asset_urls("extra_css_urls", template, context), + "extra_js_urls": self._asset_urls("extra_js_urls", template, context), }, **extra_template_vars, } @@ -596,6 +599,34 @@ class Datasette: ) return Response.html(await template.render_async(template_context)) + def _asset_urls(self, key, template, context): + # Flatten list-of-lists from plugins: + seen_urls = set() + for url_or_dict in itertools.chain( + itertools.chain.from_iterable( + getattr(pm.hook, key)( + template=template.name, + database=context.get("database"), + table=context.get("table"), + datasette=self, + ) + ), + (self.metadata(key) or []), + ): + if isinstance(url_or_dict, dict): + url = url_or_dict["url"] + sri = url_or_dict.get("sri") + else: + url = url_or_dict + sri = None + if url in seen_urls: + continue + seen_urls.add(url) + if sri: + yield {"url": url, "sri": sri} + else: + yield {"url": url} + def app(self): "Returns an ASGI app function that serves the whole of Datasette" default_templates = str(app_root / "datasette" / "templates") diff --git a/datasette/views/base.py b/datasette/views/base.py index 7e579a30..4ac11247 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -72,34 +72,6 @@ class BaseView(AsgiView): def database_color(self, database): return "ff0000" - def _asset_urls(self, key, template, context): - # Flatten list-of-lists from plugins: - seen_urls = set() - for url_or_dict in itertools.chain( - itertools.chain.from_iterable( - getattr(pm.hook, key)( - template=template.name, - database=context.get("database"), - table=context.get("table"), - datasette=self.ds, - ) - ), - (self.ds.metadata(key) or []), - ): - if isinstance(url_or_dict, dict): - url = url_or_dict["url"] - sri = url_or_dict.get("sri") - else: - url = url_or_dict - sri = None - if url in seen_urls: - continue - seen_urls.add(url) - if sri: - yield {"url": url, "sri": sri} - else: - yield {"url": url} - async def render(self, templates, request, context): template = self.ds.jinja_env.select_template(templates) return await self.ds.render_template( @@ -109,12 +81,6 @@ class BaseView(AsgiView): **{ "database_url": self.database_url, "database_color": self.database_color, - "extra_css_urls": self._asset_urls( - "extra_css_urls", template, context - ), - "extra_js_urls": self._asset_urls( - "extra_js_urls", template, context - ), }, }, request=request, From 5d1b424cfcfd1632e40e00cb98463ff5636695ff Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 4 Feb 2020 08:35:52 -0800 Subject: [PATCH 6/8] render_template() returns rendered string, not Response --- datasette/app.py | 8 +------- datasette/views/base.py | 17 +++++++++++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 4db64c35..4b09b7d1 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -591,13 +591,7 @@ class Datasette: }, **extra_template_vars, } - if request.args.get("_context") and self.config("template_debug"): - return Response.html( - "
{}
".format( - escape(json.dumps(template_context, default=repr, indent=4)) - ) - ) - return Response.html(await template.render_async(template_context)) + return await template.render_async(template_context) def _asset_urls(self, key, template, context): # Flatten list-of-lists from plugins: diff --git a/datasette/views/base.py b/datasette/views/base.py index 4ac11247..32e6fdda 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -74,17 +74,22 @@ class BaseView(AsgiView): async def render(self, templates, request, context): template = self.ds.jinja_env.select_template(templates) - return await self.ds.render_template( - template, - { + template_context = { **context, **{ "database_url": self.database_url, "database_color": self.database_color, }, - }, - request=request, - ) + } + if request and request.args.get("_context") and self.config("template_debug"): + return Response.html( + "
{}
".format( + escape(json.dumps(template_context, default=repr, indent=4)) + ) + ) + return Response.html(await self.ds.render_template( + template, template_context, request=request + )) class DataView(BaseView): From 6d672fe34c71408c440201796b9230a38699b0ef Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 4 Feb 2020 08:43:05 -0800 Subject: [PATCH 7/8] Fixed dumb bug --- datasette/views/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/views/base.py b/datasette/views/base.py index 32e6fdda..8481b9c4 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -81,7 +81,7 @@ class BaseView(AsgiView): "database_color": self.database_color, }, } - if request and request.args.get("_context") and self.config("template_debug"): + if request and request.args.get("_context") and self.ds.config("template_debug"): return Response.html( "
{}
".format( escape(json.dumps(template_context, default=repr, indent=4)) From 19d4406f562aca291fef04363802093672620220 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 4 Feb 2020 12:13:33 -0800 Subject: [PATCH 8/8] Fix for escape() error --- datasette/views/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/views/base.py b/datasette/views/base.py index 8481b9c4..673e6d7e 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -84,7 +84,7 @@ class BaseView(AsgiView): if request and request.args.get("_context") and self.ds.config("template_debug"): return Response.html( "
{}
".format( - escape(json.dumps(template_context, default=repr, indent=4)) + jinja2.escape(json.dumps(template_context, default=repr, indent=4)) ) ) return Response.html(await self.ds.render_template(