refactor: use component_id instead of Template as slot fill cache key

This commit is contained in:
Juro Oravec 2024-04-15 23:50:17 +02:00
parent 969f0bdc32
commit 1dd492314a
6 changed files with 398 additions and 75 deletions

View file

@ -26,11 +26,12 @@ from django_components.slots import (
FillContent,
FillNode,
SlotName,
SlotNode,
render_component_template_with_slots,
OUTER_CONTEXT_CONTEXT_KEY,
DEFAULT_SLOT_KEY,
)
from django_components.utils import search
from django_components.utils import search, walk_nodelist, gen_id
RENDERED_COMMENT_TEMPLATE = "<!-- _RENDERED {name} -->"
@ -185,12 +186,14 @@ class Component(View, metaclass=SimplifiedInterfaceMediaDefiningClass):
def __init__(
self,
registered_name: Optional[str] = None,
component_id: Optional[str] = None,
outer_context: Optional[Context] = None,
fill_content: dict[str, FillContent] = {}, # type: ignore
fill_content: Dict[str, FillContent] = {}, # type: ignore
):
self.registered_name: Optional[str] = registered_name
self.outer_context: Context = outer_context or Context()
self.fill_content = fill_content
self.component_id = component_id or gen_id()
def __init_subclass__(cls, **kwargs: Any) -> None:
cls.class_hash = hash(inspect.getfile(cls) + cls.__name__)
@ -255,10 +258,19 @@ class Component(View, metaclass=SimplifiedInterfaceMediaDefiningClass):
context = context_data if isinstance(context_data, Context) else Context(context_data)
template = self.get_template(context)
# Associate the slots with this component
def on_node(node: Node) -> None:
if isinstance(node, SlotNode):
node.component_id = self.component_id
walk_nodelist(template.nodelist, on_node)
if slots_data:
self._fill_slots(slots_data, escape_slots_content)
return render_component_template_with_slots(template, context, self.fill_content, self.registered_name)
return render_component_template_with_slots(
self.component_id, template, context, self.fill_content, self.registered_name
)
def render_to_response(
self,
@ -299,7 +311,9 @@ class ComponentNode(Node):
context_kwargs: Mapping[str, FilterExpression],
isolated_context: bool = False,
fill_nodes: Sequence[FillNode] = (),
component_id: Optional[str] = None,
) -> None:
self.component_id = component_id or gen_id()
self.name_fexp = name_fexp
self.context_args = context_args or []
self.context_kwargs = context_kwargs or {}
@ -329,7 +343,8 @@ class ComponentNode(Node):
resolved_context_args = [safe_resolve(arg, context) for arg in self.context_args]
resolved_context_kwargs = {key: safe_resolve(kwarg, context) for key, kwarg in self.context_kwargs.items()}
if len(self.fill_nodes) == 1 and self.fill_nodes[0].is_implicit:
is_default_slot = len(self.fill_nodes) == 1 and self.fill_nodes[0].is_implicit
if is_default_slot:
fill_content: Dict[str, FillContent] = {DEFAULT_SLOT_KEY: FillContent(self.fill_nodes[0].nodelist, None)}
else:
fill_content = {}
@ -344,6 +359,7 @@ class ComponentNode(Node):
registered_name=resolved_component_name,
outer_context=context,
fill_content=fill_content,
component_id=self.component_id,
)
component_context: dict = component.get_context_data(*resolved_context_args, **resolved_context_kwargs)

View file

@ -38,7 +38,8 @@ class FillContent(NamedTuple):
alias: Optional[AliasName]
FilledSlotsContext = ChainMap[Tuple[SlotName, Template], FillContent]
FilledSlotsKey = Tuple[SlotName, Template]
FilledSlotsContext = ChainMap[FilledSlotsKey, FillContent]
class UserSlotVar:
@ -61,25 +62,32 @@ class UserSlotVar:
return mark_safe(self._slot.nodelist.render(self._context))
class TemplateAwareNodeMixin:
_template: Template
class ComponentIdMixin:
"""
Mixin for classes use or pass through component ID.
We use component IDs to identify which slots should be
rendered with which fills for which components.
"""
_component_id: str
@property
def template(self) -> Template:
def component_id(self) -> str:
try:
return self._template
return self._component_id
except AttributeError:
raise RuntimeError(
f"Internal error: Instance of {type(self).__name__} was not "
"linked to Template before use in render() context."
"linked to Component before use in render() context. "
"Make sure that the 'component_id' field is set."
)
@template.setter
def template(self, value: Template) -> None:
self._template = value
@component_id.setter
def component_id(self, value: Template) -> None:
self._component_id = value
class SlotNode(Node, TemplateAwareNodeMixin):
class SlotNode(Node, ComponentIdMixin):
def __init__(
self,
name: str,
@ -105,15 +113,10 @@ class SlotNode(Node, TemplateAwareNodeMixin):
return f"<Slot Node: {self.name}. Contents: {repr(self.nodelist)}. Options: {self.active_flags}>"
def render(self, context: Context) -> SafeString:
try:
filled_slots_map: FilledSlotsContext = context[FILLED_SLOTS_CONTENT_CONTEXT_KEY]
except KeyError:
raise TemplateSyntaxError(f"Attempted to render SlotNode '{self.name}' outside a parent component.")
slot_fill_content = get_slot_fill(context, self.component_id, self.name, callee_node_name=f"SlotNode '{self.name}'")
extra_context = {}
try:
slot_fill_content = filled_slots_map[(self.name, self.template)]
except KeyError:
if slot_fill_content is None:
if self.is_required:
raise TemplateSyntaxError(
f"Slot '{self.name}' is marked as 'required' (i.e. non-optional), " f"yet no fill is provided. "
@ -136,7 +139,7 @@ class SlotNode(Node, TemplateAwareNodeMixin):
See SlotContextBehavior for the description of each option.
"""
root_ctx = context.get(OUTER_CONTEXT_CONTEXT_KEY, Context())
root_ctx: Context = context.get(OUTER_CONTEXT_CONTEXT_KEY, Context())
if app_settings.SLOT_CONTEXT_BEHAVIOR == SlotContextBehavior.ALLOW_OVERRIDE:
return context
@ -144,13 +147,13 @@ class SlotNode(Node, TemplateAwareNodeMixin):
return root_ctx
elif app_settings.SLOT_CONTEXT_BEHAVIOR == SlotContextBehavior.PREFER_ROOT:
new_context: Context = context.__copy__()
new_context.update(root_ctx)
new_context.update(root_ctx.flatten())
return new_context
else:
raise ValueError(f"Unknown value for SLOT_CONTEXT_BEHAVIOR: '{app_settings.SLOT_CONTEXT_BEHAVIOR}'")
class FillNode(Node):
class FillNode(Node, ComponentIdMixin):
is_implicit: bool
"""
Set when a `component` tag pair is passed template content that
@ -165,7 +168,7 @@ class FillNode(Node):
alias_fexp: Optional[FilterExpression] = None,
is_implicit: bool = False,
):
self.nodelist: NodeList = nodelist
self.nodelist = nodelist
self.name_fexp = name_fexp
self.alias_fexp = alias_fexp
self.is_implicit = is_implicit
@ -205,7 +208,7 @@ class _IfSlotFilledBranchNode(Node):
raise NotImplementedError
class IfSlotFilledConditionBranchNode(_IfSlotFilledBranchNode, TemplateAwareNodeMixin):
class IfSlotFilledConditionBranchNode(_IfSlotFilledBranchNode, ComponentIdMixin):
def __init__(
self,
slot_name: str,
@ -217,14 +220,8 @@ class IfSlotFilledConditionBranchNode(_IfSlotFilledBranchNode, TemplateAwareNode
super().__init__(nodelist)
def evaluate(self, context: Context) -> bool:
try:
filled_slots: FilledSlotsContext = context[FILLED_SLOTS_CONTENT_CONTEXT_KEY]
except KeyError:
raise TemplateSyntaxError(
f"Attempted to render {type(self).__name__} outside a Component rendering context."
)
slot_key = (self.slot_name, self.template)
is_filled = filled_slots.get(slot_key, None) is not None
slot_fill = get_slot_fill(context, self.component_id, self.slot_name, callee_node_name=type(self).__name__)
is_filled = slot_fill is not None
# Make polarity switchable.
# i.e. if slot name is NOT filled and is_positive=False,
# then False == False -> True
@ -260,6 +257,21 @@ class IfSlotFilledNode(Node):
return ""
def get_slot_fill(
context: Context,
component_id: str,
slot_name: str,
callee_node_name: str,
) -> Optional[FillContent]:
try:
filled_slots_map: FilledSlotsContext = context[FILLED_SLOTS_CONTENT_CONTEXT_KEY]
except KeyError:
raise TemplateSyntaxError(f"Attempted to render {callee_node_name} outside a parent component.")
slot_key = (component_id, slot_name)
return filled_slots_map.get(slot_key, None)
def parse_slot_fill_nodes_from_component_nodelist(
component_nodelist: NodeList,
ComponentNodeCls: Type[Node],
@ -363,6 +375,7 @@ def _block_has_content(nodelist: NodeList) -> bool:
def render_component_template_with_slots(
component_id: str,
template: Template,
context: Context,
fill_content: Dict[str, FillContent],
@ -377,21 +390,49 @@ def render_component_template_with_slots(
"""
prev_filled_slots_context: Optional[FilledSlotsContext] = context.get(FILLED_SLOTS_CONTENT_CONTEXT_KEY)
updated_filled_slots_context = _prepare_component_template_filled_slot_context(
component_id,
template,
fill_content,
prev_filled_slots_context,
registered_name,
)
with context.update({FILLED_SLOTS_CONTENT_CONTEXT_KEY: updated_filled_slots_context}):
return template.render(context)
def _prepare_component_template_filled_slot_context(
component_id: str,
template: Template,
fill_content: Dict[str, FillContent],
slots_context: Optional[FilledSlotsContext],
registered_name: Optional[str],
) -> FilledSlotsContext:
slot_name2fill_content = _collect_slot_fills_from_component_template(template, fill_content, registered_name)
# Give slot nodes knowledge of their parent component.
for node in template.nodelist.get_nodes_by_type((SlotNode, IfSlotFilledConditionBranchNode)):
if isinstance(node, (SlotNode, IfSlotFilledConditionBranchNode)):
node.component_id = component_id
# Return updated FILLED_SLOTS_CONTEXT map
filled_slots_map: Dict[FilledSlotsKey, FillContent] = {
(component_id, slot_name): content_data
for slot_name, content_data in slot_name2fill_content.items()
if content_data # Slots whose content is None (i.e. unfilled) are dropped.
}
if slots_context is not None:
return slots_context.new_child(filled_slots_map)
else:
return ChainMap(filled_slots_map)
def _collect_slot_fills_from_component_template(
template: Template,
fill_content: Dict[str, FillContent],
registered_name: Optional[str],
) -> Dict[SlotName, Optional[FillContent]]:
if DEFAULT_SLOT_KEY in fill_content:
named_fills_content = fill_content.copy()
default_fill_content = named_fills_content.pop(DEFAULT_SLOT_KEY)
@ -405,38 +446,40 @@ def _prepare_component_template_filled_slot_context(
required_slot_names: Set[str] = set()
# Collect fills and check for errors
for node in template.nodelist.get_nodes_by_type((SlotNode, IfSlotFilledConditionBranchNode)): # type: ignore
if isinstance(node, SlotNode):
# Give slot node knowledge of its parent template.
node.template = template
slot_name = node.name
if slot_name in slot_name2fill_content:
for node in template.nodelist.get_nodes_by_type(SlotNode):
# Type check so the rest of the logic has type of `node` is inferred
if not isinstance(node, SlotNode):
continue
slot_name = node.name
if slot_name in slot_name2fill_content:
raise TemplateSyntaxError(
f"Slot name '{slot_name}' re-used within the same template. "
f"Slot names must be unique."
f"To fix, check template '{template.name}' "
f"of component '{registered_name}'."
)
if node.is_required:
required_slot_names.add(node.name)
content_data: Optional[FillContent] = None # `None` -> unfilled
if node.is_default:
if default_slot_encountered:
raise TemplateSyntaxError(
f"Slot name '{slot_name}' re-used within the same template. "
f"Slot names must be unique."
"Only one component slot may be marked as 'default'. "
f"To fix, check template '{template.name}' "
f"of component '{registered_name}'."
)
content_data: Optional[FillContent] = None # `None` -> unfilled
if node.is_required:
required_slot_names.add(node.name)
if node.is_default:
if default_slot_encountered:
raise TemplateSyntaxError(
"Only one component slot may be marked as 'default'. "
f"To fix, check template '{template.name}' "
f"of component '{registered_name}'."
)
content_data = default_fill_content
default_slot_encountered = True
if not content_data:
content_data = named_fills_content.get(node.name)
slot_name2fill_content[slot_name] = content_data
elif isinstance(node, IfSlotFilledConditionBranchNode):
node.template = template
else:
raise RuntimeError(f"Node of {type(node).__name__} does not require linking.")
content_data = default_fill_content
default_slot_encountered = True
# If default fill was not found, try to fill it with named slot
# Effectively, this allows to fill in default slot as named ones.
if not content_data:
content_data = named_fills_content.get(node.name)
slot_name2fill_content[slot_name] = content_data
# Check: Only component templates that include a 'default' slot
# can be invoked with implicit filling.
if default_fill_content and not default_slot_encountered:
@ -449,6 +492,17 @@ def _prepare_component_template_filled_slot_context(
unfilled_slots: Set[str] = {k for k, v in slot_name2fill_content.items() if v is None}
unmatched_fills: Set[str] = named_fills_content.keys() - slot_name2fill_content.keys()
_report_slot_errors(unfilled_slots, unmatched_fills, registered_name, required_slot_names)
return slot_name2fill_content
def _report_slot_errors(
unfilled_slots: Set[str],
unmatched_fills: Set[str],
registered_name: Optional[str],
required_slot_names: Set[str],
) -> None:
# Check that 'required' slots are filled.
for slot_name in unfilled_slots:
if slot_name in required_slot_names:
@ -479,14 +533,3 @@ def _prepare_component_template_filled_slot_context(
if fuzzy_slot_name_matches:
msg += f"\nDid you mean '{fuzzy_slot_name_matches[0]}'?"
raise TemplateSyntaxError(msg)
# Return updated FILLED_SLOTS_CONTEXT map
filled_slots_map: Dict[Tuple[SlotName, Template], FillContent] = {
(slot_name, template): content_data
for slot_name, content_data in slot_name2fill_content.items()
if content_data # Slots whose content is None (i.e. unfilled) are dropped.
}
if slots_context is not None:
return slots_context.new_child(filled_slots_map)
else:
return ChainMap(filled_slots_map)

View file

@ -24,6 +24,7 @@ from django_components.slots import (
_IfSlotFilledBranchNode,
parse_slot_fill_nodes_from_component_nodelist,
)
from django_components.utils import gen_id
if TYPE_CHECKING:
from django_components.component import Component
@ -210,12 +211,22 @@ def do_component(parser: Parser, token: Token) -> ComponentNode:
body: NodeList = parser.parse(parse_until=["endcomponent"])
parser.delete_first_token()
fill_nodes = parse_slot_fill_nodes_from_component_nodelist(body, ComponentNode)
# Use a unique ID to be able to tie the fill nodes with this specific component
# and its slots
component_id = gen_id()
# Tag all fill nodes as children of this particular component instance
for node in fill_nodes:
node.component_id = component_id
component_node = ComponentNode(
FilterExpression(component_name, parser),
context_args,
context_kwargs,
isolated_context=isolated_context,
fill_nodes=fill_nodes,
component_id=component_id,
)
return component_node

View file

@ -1,7 +1,9 @@
import glob
import random
from pathlib import Path
from typing import List, NamedTuple, Optional
from typing import Callable, List, NamedTuple, Optional
from django.template.base import Node, NodeList
from django.template.engine import Engine
from django_components.template_loader import Loader
@ -35,3 +37,36 @@ def search(search_glob: Optional[str] = None, engine: Optional[Engine] = None) -
component_filenames.append(Path(path))
return SearchResult(searched_dirs=dirs, matched_files=component_filenames)
def walk_nodelist(nodes: NodeList, callback: Callable[[Node], None]) -> None:
"""Recursively walk a NodeList, calling `callback` for each Node."""
node_queue = [*nodes]
while len(node_queue):
node: Node = node_queue.pop()
callback(node)
node_queue.extend(get_node_children(node))
def get_node_children(node: Node) -> NodeList:
"""
Get child Nodes from Node's nodelist atribute.
This function is taken from `get_nodes_by_type` method of `django.template.base.Node`.
"""
nodes = NodeList()
for attr in node.child_nodelists:
nodelist = getattr(node, attr, [])
if nodelist:
nodes.extend(nodelist)
return nodes
def gen_id(length: int = 5) -> str:
# Generate random value
# See https://stackoverflow.com/questions/2782229
value = random.randrange(16**length)
# Signed hexadecimal (lowercase).
# See https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting
return f"{value:x}"

View file

@ -204,6 +204,60 @@ class ComponentTest(SimpleTestCase):
self.assertIn('<input type="text" name="variable" value="hello">', rendered, rendered)
def test_component_inside_slot(self):
class SlottedComponent(component.Component):
template_name = "slotted_template.html"
def get_context_data(self, name: str | None = None) -> component.Dict[str, component.Any]:
return {
"name": name,
}
component.registry.register("test", SlottedComponent)
self.template = Template(
"""
{% load component_tags %}
{% component "test" name='Igor' %}
{% fill "header" %}
Name: {{ name }}
{% endfill %}
{% fill "main" %}
Day: {{ day }}
{% endfill %}
{% fill "footer" %}
{% component "test" name='Joe2' %}
{% fill "header" %}
Name2: {{ name }}
{% endfill %}
{% fill "main" %}
Day2: {{ day }}
{% endfill %}
{% endcomponent %}
{% endfill %}
{% endcomponent %}
"""
)
# {{ name }} should be "Jannete" everywhere
rendered = self.template.render(Context({ "day": "Monday", "name": "Jannete" }))
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Name: Jannete</header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: Jannete</header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)
class InlineComponentTest(SimpleTestCase):
def test_inline_html_component(self):
@ -482,3 +536,152 @@ class ComponentIsolationTests(SimpleTestCase):
</custom-template>
""",
)
class SlotBehaviorTests(SimpleTestCase):
def setUp(self):
class SlottedComponent(component.Component):
template_name = "slotted_template.html"
def get_context_data(self, name: str | None = None) -> component.Dict[str, component.Any]:
return {
"name": name,
}
component.registry.register("test", SlottedComponent)
self.template = Template(
"""
{% load component_tags %}
{% component "test" name='Igor' %}
{% fill "header" %}
Name: {{ name }}
{% endfill %}
{% fill "main" %}
Day: {{ day }}
{% endfill %}
{% fill "footer" %}
{% component "test" name='Joe2' %}
{% fill "header" %}
Name2: {{ name }}
{% endfill %}
{% fill "main" %}
Day2: {{ day }}
{% endfill %}
{% endcomponent %}
{% endfill %}
{% endcomponent %}
"""
)
@override_settings(
COMPONENTS={"slot_context_behavior": "allow_override"},
)
def test_slot_context_allow_override(self):
# {{ name }} should be neither Jannete not empty, because overriden everywhere
rendered = self.template.render(Context({ "day": "Monday", "name": "Jannete" }))
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Name: Igor</header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: Joe2</header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)
# {{ name }} should be effectively the same as before, because overriden everywhere
rendered2 = self.template.render(Context({ "day": "Monday" }))
self.assertHTMLEqual(rendered2, rendered)
@override_settings(
COMPONENTS={"slot_context_behavior": "isolated"},
)
def test_slot_context_isolated(self):
# {{ name }} should be "Jannete" everywhere
rendered = self.template.render(Context({ "day": "Monday", "name": "Jannete" }))
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Name: Jannete</header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: Jannete</header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)
# {{ name }} should be empty everywhere
rendered2 = self.template.render(Context({ "day": "Monday" }))
self.assertHTMLEqual(
rendered2,
"""
<custom-template>
<header>Name: </header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: </header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)
@override_settings(
COMPONENTS={
"slot_context_behavior": "prefer_root",
},
)
def test_slot_context_prefer_root(self):
# {{ name }} should be "Jannete" everywhere
rendered = self.template.render(Context({ "day": "Monday", "name": "Jannete" }))
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Name: Jannete</header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: Jannete</header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)
# {{ name }} should be neither "Jannete" nor empty anywhere
rendered = self.template.render(Context({ "day": "Monday" }))
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Name: Igor</header>
<main>Day: Monday</main>
<footer>
<custom-template>
<header>Name2: Joe2</header>
<main>Day2: Monday</main>
<footer>Default footer</footer>
</custom-template>
</footer>
</custom-template>
""",
)

View file

@ -1,6 +1,7 @@
from unittest.mock import PropertyMock, patch
from django.template import Context, Template
from django.test import override_settings
from django_components import component
@ -65,7 +66,7 @@ class OuterContextComponent(component.Component):
template_name = "simple_template.html"
def get_context_data(self):
return self.outer_context
return self.outer_context.flatten()
component.registry.register(name="parent_component", component=ParentComponent)
@ -385,7 +386,21 @@ class IsolatedContextSettingTests(SimpleTestCase):
class OuterContextPropertyTests(SimpleTestCase):
def test_outer_context_property_with_component(self):
@override_settings(
COMPONENTS={"context_behavior": "global"},
)
def test_outer_context_property_with_component_global(self):
template = Template(
"{% load component_tags %}{% component_dependencies %}"
"{% component 'outer_context_component' only %}{% endcomponent %}"
)
rendered = template.render(Context({"variable": "outer_value"})).strip()
self.assertIn("outer_value", rendered, rendered)
@override_settings(
COMPONENTS={"context_behavior": "isolated"},
)
def test_outer_context_property_with_component_isolated(self):
template = Template(
"{% load component_tags %}{% component_dependencies %}"
"{% component 'outer_context_component' only %}{% endcomponent %}"