refactor: Fix template caching, expose cached_template, Component.template API changes (#647)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Juro Oravec 2024-09-06 22:40:39 +02:00 committed by GitHub
parent 589e802625
commit 841dd77e91
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 347 additions and 56 deletions

View file

@ -36,6 +36,7 @@ from django_components.tag_formatter import (
component_formatter as component_formatter,
component_shorthand_formatter as component_shorthand_formatter,
)
from django_components.template import cached_template as cached_template
import django_components.types as types
from django_components.types import (
EmptyTuple as EmptyTuple,

View file

@ -63,6 +63,7 @@ from django_components.slots import (
resolve_fill_nodes,
resolve_slots,
)
from django_components.template import cached_template
from django_components.utils import gen_id, validate_typed_dict, validate_typed_tuple
# TODO_REMOVE_IN_V1 - Users should use top-level import instead
@ -154,10 +155,42 @@ class Component(Generic[ArgsType, KwargsType, DataType, SlotsType], metaclass=Co
# non-null return.
_class_hash: ClassVar[int]
template_name: ClassVar[Optional[str]] = None
"""Relative filepath to the Django template associated with this component."""
template: Optional[str] = None
"""Inlined Django template associated with this component."""
template_name: Optional[str] = None
"""
Filepath to the Django template associated with this component.
The filepath must be relative to either the file where the component class was defined,
or one of the roots of `STATIFILES_DIRS`.
Only one of `template_name`, `get_template_name`, `template` or `get_template` must be defined.
"""
def get_template_name(self, context: Context) -> Optional[str]:
"""
Filepath to the Django template associated with this component.
The filepath must be relative to either the file where the component class was defined,
or one of the roots of `STATIFILES_DIRS`.
Only one of `template_name`, `get_template_name`, `template` or `get_template` must be defined.
"""
return None
template: Optional[Union[str, Template]] = None
"""
Inlined Django template associated with this component. Can be a plain string or a Template instance.
Only one of `template_name`, `get_template_name`, `template` or `get_template` must be defined.
"""
def get_template(self, context: Context) -> Optional[Union[str, Template]]:
"""
Inlined Django template associated with this component. Can be a plain string or a Template instance.
Only one of `template_name`, `get_template_name`, `template` or `get_template` must be defined.
"""
return None
js: Optional[str] = None
"""Inlined JS associated with this component."""
css: Optional[str] = None
@ -274,28 +307,55 @@ class Component(Generic[ArgsType, KwargsType, DataType, SlotsType], metaclass=Co
def get_context_data(self, *args: Any, **kwargs: Any) -> DataType:
return cast(DataType, {})
def get_template_name(self, context: Context) -> Optional[str]:
return self.template_name
def get_template_string(self, context: Context) -> Optional[str]:
return self.template
# NOTE: When the template is taken from a file (AKA specified via `template_name`),
# then we leverage Django's template caching. This means that the same instance
# of Template is reused. This is important to keep in mind, because the implication
# is that we should treat Templates AND their nodelists as IMMUTABLE.
def get_template(self, context: Context) -> Template:
template_string = self.get_template_string(context)
if template_string is not None:
return Template(template_string)
def _get_template(self, context: Context) -> Template:
# Resolve template name
template_name = self.template_name
if self.template_name is not None:
if self.get_template_name(context) is not None:
raise ImproperlyConfigured(
"Received non-null value from both 'template_name' and 'get_template_name' in"
f" Component {type(self).__name__}. Only one of the two must be set."
)
else:
template_name = self.get_template_name(context)
# Resolve template str
template_input = self.template
if self.template is not None:
if self.get_template(context) is not None:
raise ImproperlyConfigured(
"Received non-null value from both 'template' and 'get_template' in"
f" Component {type(self).__name__}. Only one of the two must be set."
)
else:
# TODO_REMOVE_IN_V1 - Remove `self.get_template_string` in v1
template_getter = getattr(self, "get_template_string", self.get_template)
template_input = template_getter(context)
if template_name is not None and template_input is not None:
raise ImproperlyConfigured(
f"Received both 'template_name' and 'template' in Component {type(self).__name__}."
" Only one of the two must be set."
)
template_name = self.get_template_name(context)
if template_name is not None:
return get_template(template_name).template
elif template_input is not None:
# We got template string, so we convert it to Template
if isinstance(template_input, str):
template: Template = cached_template(template_input)
else:
template = template_input
return template
raise ImproperlyConfigured(
f"Either 'template_name' or 'template' must be set for Component {type(self).__name__}."
f"Note: this attribute is not required if you are overriding the class's `get_template*()` methods."
)
def render_dependencies(self) -> SafeString:
@ -606,7 +666,6 @@ class Component(Generic[ArgsType, KwargsType, DataType, SlotsType], metaclass=Co
self.on_render_before(context, template)
rendered_component = template.render(context)
new_output = self.on_render_after(context, template, rendered_component)
rendered_component = new_output if new_output is not None else rendered_component
@ -884,7 +943,7 @@ def _prepare_template(
# an error when we try to use `{% include %}` tag inside the template?
# See https://github.com/EmilStenstrom/django-components/issues/580
# And https://github.com/EmilStenstrom/django-components/issues/634
template = component.get_template(context)
template = component._get_template(context)
_monkeypatch_template(template)
# Set `Template._dc_is_component_nested` based on whether we're currently INSIDE

View file

@ -3,6 +3,7 @@ import json
import re
from collections import deque
from dataclasses import dataclass
from functools import lru_cache
from typing import (
TYPE_CHECKING,
Any,
@ -27,7 +28,7 @@ from django.template.defaulttags import CommentNode
from django.template.exceptions import TemplateSyntaxError
from django.utils.safestring import SafeString, mark_safe
from django_components.app_settings import ContextBehavior
from django_components.app_settings import ContextBehavior, app_settings
from django_components.context import (
_FILLED_SLOTS_CONTENT_CONTEXT_KEY,
_INJECT_CONTEXT_KEY_PREFIX,
@ -37,6 +38,7 @@ from django_components.context import (
from django_components.expression import RuntimeKwargs, is_identifier
from django_components.logger import trace_msg
from django_components.node import BaseNode, NodeTraverse, nodelist_has_content, walk_nodelist
from django_components.utils import lazy_cache
if TYPE_CHECKING:
from django_components.component_registry import ComponentRegistry
@ -332,9 +334,13 @@ class FillNode(BaseNode):
return value
# NOTE: There may be more components per template, so using `app_settings.TEMPLATE_CACHE_SIZE`
# is not entirely correct. However, for now it's not worth it adding a separate setting
# to control this cache separately. So we use `TEMPLATE_CACHE_SIZE` so the cache is bounded.
@lazy_cache(lambda: lru_cache(maxsize=app_settings.TEMPLATE_CACHE_SIZE))
def parse_slot_fill_nodes_from_component_nodelist(
component_nodelist: NodeList,
ComponentNodeCls: Type[Node],
nodes: Tuple[Node, ...],
ignored_nodes: Tuple[Type[Node]],
) -> List[FillNode]:
"""
Given a component body (`django.template.NodeList`), find all slot fills,
@ -355,12 +361,12 @@ def parse_slot_fill_nodes_from_component_nodelist(
and `fill "second_fill"`.
"""
fill_nodes: List[FillNode] = []
if nodelist_has_content(component_nodelist):
if nodelist_has_content(nodes):
for parse_fn in (
_try_parse_as_default_fill,
_try_parse_as_named_fill_tag_set,
):
curr_fill_nodes = parse_fn(component_nodelist, ComponentNodeCls)
curr_fill_nodes = parse_fn(nodes, ignored_nodes)
if curr_fill_nodes:
fill_nodes = curr_fill_nodes
break
@ -375,12 +381,12 @@ def parse_slot_fill_nodes_from_component_nodelist(
def _try_parse_as_named_fill_tag_set(
nodelist: NodeList,
ComponentNodeCls: Type[Node],
nodes: Tuple[Node, ...],
ignored_nodes: Tuple[Type[Node]],
) -> List[FillNode]:
result = []
seen_names: Set[str] = set()
for node in nodelist:
for node in nodes:
if isinstance(node, FillNode):
# If the fill name was defined statically, then check for no duplicates.
maybe_fill_name = node.kwargs.kwargs.get(SLOT_NAME_KWARG)
@ -402,15 +408,15 @@ def _try_parse_as_named_fill_tag_set(
def _try_parse_as_default_fill(
nodelist: NodeList,
ComponentNodeCls: Type[Node],
nodes: Tuple[Node, ...],
ignored_nodes: Tuple[Type[Node]],
) -> List[FillNode]:
nodes_stack: List[Node] = list(nodelist)
nodes_stack: List[Node] = list(nodes)
while nodes_stack:
node = nodes_stack.pop()
if isinstance(node, FillNode):
return []
elif isinstance(node, ComponentNodeCls):
elif isinstance(node, ignored_nodes):
# Stop searching here, as fill tags are permitted inside component blocks
# embedded within a default fill node.
continue
@ -419,7 +425,7 @@ def _try_parse_as_default_fill(
else:
return [
FillNode(
nodelist=nodelist,
nodelist=NodeList(nodes),
kwargs=RuntimeKwargs(
{
# Wrap the default slot name in quotes so it's treated as FilterExpression

View file

@ -0,0 +1,42 @@
from functools import lru_cache
from typing import Any, Optional, Type, TypeVar
from django.template import Origin, Template
from django.template.base import UNKNOWN_SOURCE
from django_components.app_settings import app_settings
from django_components.utils import lazy_cache
TTemplate = TypeVar("TTemplate", bound=Template)
# Lazily initialize the cache. The cached function takes only the parts that can
# affect how the template string is processed - Template class, template string, and engine
@lazy_cache(lambda: lru_cache(maxsize=app_settings.TEMPLATE_CACHE_SIZE))
def _create_template(
template_cls: Type[TTemplate],
template_string: str,
engine: Optional[Any] = None,
) -> TTemplate:
return template_cls(template_string, engine=engine)
# Central logic for creating Templates from string, so we can cache the results
def cached_template(
template_string: str,
template_cls: Optional[Type[Template]] = None,
origin: Optional[Origin] = None,
name: Optional[str] = None,
engine: Optional[Any] = None,
) -> Template:
"""Create a Template instance that will be cached as per the `TEMPLATE_CACHE_SIZE` setting."""
template = _create_template(template_cls or Template, template_string, engine)
# Assign the origin and name separately, so the caching doesn't depend on them
# Since we might be accessing a template from cache, we want to define these only once
if not getattr(template, "_dc_cached", False):
template.origin = origin or Origin(UNKNOWN_SOURCE)
template.name = name
template._dc_cached = True
return template

View file

@ -246,7 +246,7 @@ def component(parser: Parser, token: Token, registry: ComponentRegistry, tag_nam
trace_msg("PARSE", "COMP", result.component_name, tag.id)
body = tag.parse_body()
fill_nodes = parse_slot_fill_nodes_from_component_nodelist(body, ComponentNode)
fill_nodes = parse_slot_fill_nodes_from_component_nodelist(tuple(body), ignored_nodes=(ComponentNode,))
# Tag all fill nodes as children of this particular component instance
for node in fill_nodes:

View file

@ -1,7 +1,8 @@
import functools
import sys
import typing
from pathlib import Path
from typing import Any, Callable, List, Mapping, Sequence, Tuple, Union, get_type_hints
from typing import Any, Callable, List, Mapping, Sequence, Tuple, TypeVar, Union, cast, get_type_hints
from django.utils.autoreload import autoreload_started
@ -166,3 +167,47 @@ def validate_typed_dict(value: Mapping[str, Any], dict_type: Any, prefix: str, k
# `Component 'name' got unexpected slot keys 'invalid_key'`
# `Component 'name' got unexpected data keys 'invalid_key'`
raise TypeError(f"{prefix} got unexpected {kind} keys {formatted_keys}")
TFunc = TypeVar("TFunc", bound=Callable)
def lazy_cache(
make_cache: Callable[[], Callable[[Callable], Callable]],
) -> Callable[[TFunc], TFunc]:
"""
Decorator that caches the given function similarly to `functools.lru_cache`.
But the cache is instantiated only at first invocation.
`cache` argument is a function that generates the cache function,
e.g. `functools.lru_cache()`.
"""
_cached_fn = None
def decorator(fn: TFunc) -> TFunc:
@functools.wraps(fn)
def wrapper(*args: Any, **kwargs: Any) -> Any:
# Lazily initialize the cache
nonlocal _cached_fn
if not _cached_fn:
# E.g. `lambda: functools.lru_cache(maxsize=app_settings.TEMPLATE_CACHE_SIZE)`
cache = make_cache()
_cached_fn = cache(fn)
return _cached_fn(*args, **kwargs)
# Allow to access the LRU cache methods
# See https://stackoverflow.com/a/37654201/9788634
wrapper.cache_info = lambda: _cached_fn.cache_info() # type: ignore
wrapper.cache_clear = lambda: _cached_fn.cache_clear() # type: ignore
# And allow to remove the cache instance (mostly for tests)
def cache_remove() -> None:
nonlocal _cached_fn
_cached_fn = None
wrapper.cache_remove = cache_remove # type: ignore
return cast(TFunc, wrapper)
return decorator