From 46ed6fa2aa0621a95ef0a102c6033c855eb80673 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 7 Feb 2023 17:17:15 +0100 Subject: [PATCH] wcs: resolve card_ids lazily (#74306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The class LazyValue is a thunk[1] like in lazy evaluated language it is initialized with a function needing no argument and store its result on first call. Direct access to the context to get the ids is replaced by calls to new method WcsCardCell.get_card_ids(context). Original get_card_ids() is renamed resolve_card_ids() and used with LazyValue to implement the lazy evaluation of card_ids. This line was modified as it does not really work : if·','·in·first_cell.card_ids: since card_ids can contain templates like {{ ...|join:"," }} which will evaluate to True even if the template returns only one card (one test was broken because of this, the bug was hidden by eager evaluation of card_ids previously). [1]: https://en.wikipedia.org/wiki/Thunk --- combo/apps/wcs/models.py | 38 +++++++++++++++++++++++++++++--------- tests/wcs/test_card.py | 10 +++++++--- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/combo/apps/wcs/models.py b/combo/apps/wcs/models.py index 0a0682c4..f6389500 100644 --- a/combo/apps/wcs/models.py +++ b/combo/apps/wcs/models.py @@ -822,6 +822,19 @@ class CardMixin: return escape(self.custom_title) or self.cached_title or None +class LazyValue: + sentinel = object() + + def __init__(self, getter): + self.value = self.sentinel + self.getter = getter + + def __call__(self): + if self.value is self.sentinel: + self.value = self.getter() + return self.value + + @register_cell_class class WcsCardCell(CardMixin, CellBase): carddef_reference = models.CharField(_('Card Model'), max_length=150) @@ -872,6 +885,12 @@ class WcsCardCell(CardMixin, CellBase): class Meta: verbose_name = _('Card(s)') + def get_card_ids(self, context): + lazy_value = context.get(self.global_context_key) + if lazy_value is not None: + return lazy_value() + return [] + def save(self, *args, **kwargs): super().save(*args, **kwargs) @@ -960,14 +979,14 @@ class WcsCardCell(CardMixin, CellBase): # don't call wcs on page loading return if self.carddef_reference and self.global_context_key not in context: - card_ids = self.get_card_ids(context, request) - context[self.global_context_key] = card_ids + # self.resolve_card_ids(context, request) + context[self.global_context_key] = LazyValue(lambda: self.resolve_card_ids(context, request)) def get_repeat_template(self, context): if self.display_mode == 'table': # don't repeat cell if table display mode return [] - return len(context.get(self.global_context_key) or []) + return len(self.get_card_ids(context)) def get_related_card_path(self): if self.related_card_path == '__all__': @@ -1026,7 +1045,7 @@ class WcsCardCell(CardMixin, CellBase): def get_card_data_from_ids(self, card_id, context): # get the correct card from all known cards for ids in context - card_ids = context.get(self.global_context_key) + card_ids = self.get_card_ids(context) if not card_ids: return None if len(card_ids) == 1: @@ -1287,7 +1306,8 @@ class WcsCardCell(CardMixin, CellBase): if first_cell.related_card_path: # no explicit ids return [] - if ',' in first_cell.card_ids: + card_ids = first_cell.get_card_ids(context) + if len(card_ids) > 1: # multiple ids, can not follow relations return [] first_cell.repeat_index = 0 @@ -1324,7 +1344,7 @@ class WcsCardCell(CardMixin, CellBase): except (VariableDoesNotExist, TemplateSyntaxError): return [] - def get_card_ids(self, original_context, request): + def resolve_card_ids(self, original_context, request): if not self.carddef_reference: # not configured return [] @@ -1367,10 +1387,10 @@ class WcsCardCell(CardMixin, CellBase): return [] def get_card_id(self, context): - repeat_index = getattr(self, 'repeat_index', context.get('repeat_index')) + repeat_index = getattr(self, 'repeat_index', context.get('repeat_index')) or 0 if repeat_index is not None: try: - return context.get(self.global_context_key)[repeat_index] + return self.get_card_ids(context)[repeat_index] except (IndexError, TypeError): return None @@ -1475,7 +1495,7 @@ class WcsCardCell(CardMixin, CellBase): card_page = matching_pages[0] extra_context['card_page_base_url'] = card_page.get_online_url() - card_ids = context.get(self.global_context_key) + card_ids = self.get_card_ids(context) if not card_ids and self.related_card_path != '__all__': extra_context['card_objects'] = [] else: diff --git a/tests/wcs/test_card.py b/tests/wcs/test_card.py index 19041f47..3e83b63a 100644 --- a/tests/wcs/test_card.py +++ b/tests/wcs/test_card.py @@ -1360,7 +1360,7 @@ def test_card_cell_table_mode_render_identifier(mock_send, nocache, app): @mock.patch('requests.Session.send', side_effect=mocked_requests_send) -def test_card_cell_table_mode_render_identifier_from_related(mock_send, nocache, app): +def test_card_cell_table_mode_render_identifier_from_related(mock_send, app): page = Page.objects.create(title='xxx', slug='foo', template_name='standard') WcsCardCell.objects.create( page=page, @@ -1407,6 +1407,10 @@ def test_card_cell_table_mode_render_identifier_from_related(mock_send, nocache, check(urls) # direct and multiple relation (items) + # clear cache to see call to /api/cards/card_a/1/ + from django.core.cache import cache + + cache.clear() cell2.carddef_reference = 'default:card_b' # reset cell2.related_card_path = 'sluga/cardsb' cell2.save() @@ -2275,10 +2279,10 @@ def test_card_cell_card_mode_render_custom_schema_link_entry(mock_send, context, assert PyQuery(result).find('.value a').attr['class'] is None # empty label or no value/no file field/unknown field: no link in output - context[cell.global_context_key] = [12] + context[cell.global_context_key] = lambda: [12] result = cell.render(context) assert PyQuery(result).find('.value a') == [] - context[cell.global_context_key] = [11] + context[cell.global_context_key] = lambda: [11] cell.modify_global_context(context, request) cell.repeat_index = 0 cell.custom_schema['cells'][0]['link_field'] = 'fielda' -- 2.37.2