From 37df7a631360c8d0ca804acdf59a1685a2870b10 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Mon, 21 Aug 2023 17:59:55 +0200 Subject: [PATCH] Allow PDF variables to provide a bulk evaluation method (second try at #3517) (#3535) --- src/pretix/api/serializers/order.py | 91 +++++++++++++++++++++++++++-- src/pretix/base/pdf.py | 5 +- src/pretix/base/signals.py | 6 +- src/tests/api/test_orders.py | 8 +++ 4 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/pretix/api/serializers/order.py b/src/pretix/api/serializers/order.py index b167fa5ec..a34321a4d 100644 --- a/src/pretix/api/serializers/order.py +++ b/src/pretix/api/serializers/order.py @@ -27,6 +27,7 @@ from decimal import Decimal import pycountry from django.conf import settings from django.core.files import File +from django.db import models from django.db.models import F, Q from django.utils.encoding import force_str from django.utils.timezone import now @@ -372,11 +373,15 @@ class PdfDataSerializer(serializers.Field): self.context['vars_images'] = get_images(self.context['event']) for k, f in self.context['vars'].items(): - try: - res[k] = f['evaluate'](instance, instance.order, ev) - except: - logger.exception('Evaluating PDF variable failed') - res[k] = '(error)' + if 'evaluate_bulk' in f: + # Will be evaluated later by our list serializers + res[k] = (f['evaluate_bulk'], instance) + else: + try: + res[k] = f['evaluate'](instance, instance.order, ev) + except: + logger.exception('Evaluating PDF variable failed') + res[k] = '(error)' if not hasattr(ev, '_cached_meta_data'): ev._cached_meta_data = ev.meta_data @@ -429,6 +434,38 @@ class PdfDataSerializer(serializers.Field): return res +class OrderPositionListSerializer(serializers.ListSerializer): + + def to_representation(self, data): + # We have a custom implementation of this method because PdfDataSerializer() might keep some elements unevaluated + # with a (callable, input) tuple. We'll loop over these entries and evaluate them bulk-wise to save on SQL queries. + + if isinstance(self.parent, OrderSerializer) and isinstance(self.parent.parent, OrderListSerializer): + # Do not execute our custom code because it will be executed by OrderListSerializer later for the + # full result set. + return super().to_representation(data) + + iterable = data.all() if isinstance(data, models.Manager) else data + + data = [] + evaluate_queue = defaultdict(list) + + for item in iterable: + entry = self.child.to_representation(item) + if "pdf_data" in entry: + for k, v in entry["pdf_data"].items(): + if isinstance(v, tuple) and callable(v[0]): + evaluate_queue[v[0]].append((v[1], entry, k)) + data.append(entry) + + for func, entries in evaluate_queue.items(): + results = func([item for (item, entry, k) in entries]) + for (item, entry, k), result in zip(entries, results): + entry["pdf_data"][k] = result + + return data + + class OrderPositionSerializer(I18nAwareModelSerializer): checkins = CheckinSerializer(many=True, read_only=True) answers = AnswerSerializer(many=True) @@ -440,6 +477,7 @@ class OrderPositionSerializer(I18nAwareModelSerializer): attendee_name = serializers.CharField(required=False) class Meta: + list_serializer_class = OrderPositionListSerializer model = OrderPosition fields = ('id', 'order', 'positionid', 'item', 'variation', 'price', 'attendee_name', 'attendee_name_parts', 'company', 'street', 'zipcode', 'city', 'country', 'state', 'discount', @@ -468,6 +506,20 @@ class OrderPositionSerializer(I18nAwareModelSerializer): def validate(self, data): raise TypeError("this serializer is readonly") + def to_representation(self, data): + if isinstance(self.parent, (OrderListSerializer, OrderPositionListSerializer)): + # Do not execute our custom code because it will be executed by OrderListSerializer later for the + # full result set. + return super().to_representation(data) + + entry = super().to_representation(data) + if "pdf_data" in entry: + for k, v in entry["pdf_data"].items(): + if isinstance(v, tuple) and callable(v[0]): + entry["pdf_data"][k] = v[0]([v[1]])[0] + + return entry + class RequireAttentionField(serializers.Field): def to_representation(self, instance: OrderPosition): @@ -613,6 +665,34 @@ class OrderURLField(serializers.URLField): }) +class OrderListSerializer(serializers.ListSerializer): + + def to_representation(self, data): + # We have a custom implementation of this method because PdfDataSerializer() might keep some elements + # unevaluated with a (callable, input) tuple. We'll loop over these entries and evaluate them bulk-wise to + # save on SQL queries. + iterable = data.all() if isinstance(data, models.Manager) else data + + data = [] + evaluate_queue = defaultdict(list) + + for item in iterable: + entry = self.child.to_representation(item) + for p in entry.get("positions", []): + if "pdf_data" in p: + for k, v in p["pdf_data"].items(): + if isinstance(v, tuple) and callable(v[0]): + evaluate_queue[v[0]].append((v[1], p, k)) + data.append(entry) + + for func, entries in evaluate_queue.items(): + results = func([item for (item, entry, k) in entries]) + for (item, entry, k), result in zip(entries, results): + entry["pdf_data"][k] = result + + return data + + class OrderSerializer(I18nAwareModelSerializer): invoice_address = InvoiceAddressSerializer(allow_null=True) positions = OrderPositionSerializer(many=True, read_only=True) @@ -627,6 +707,7 @@ class OrderSerializer(I18nAwareModelSerializer): class Meta: model = Order + list_serializer_class = OrderListSerializer fields = ( 'code', 'status', 'testmode', 'secret', 'email', 'phone', 'locale', 'datetime', 'expires', 'payment_date', 'payment_provider', 'fees', 'total', 'comment', 'custom_followup_at', 'invoice_address', 'positions', 'downloads', diff --git a/src/pretix/base/pdf.py b/src/pretix/base/pdf.py index 07349b48d..661b3e451 100644 --- a/src/pretix/base/pdf.py +++ b/src/pretix/base/pdf.py @@ -108,7 +108,10 @@ DEFAULT_VARIABLES = OrderedDict(( ("positionid", { "label": _("Order position number"), "editor_sample": "1", - "evaluate": lambda orderposition, order, event: str(orderposition.positionid) + "evaluate": lambda orderposition, order, event: str(orderposition.positionid), + # There is no performance gain in using evaluate_bulk here, but we want to make sure it is used somewhere + # in core to make sure we notice if the implementation of the API breaks. + "evaluate_bulk": lambda orderpositions: [str(p.positionid) for p in orderpositions], }), ("order_positionid", { "label": _("Order code and position number"), diff --git a/src/pretix/base/signals.py b/src/pretix/base/signals.py index 5c064b9c7..90789ed00 100644 --- a/src/pretix/base/signals.py +++ b/src/pretix/base/signals.py @@ -683,12 +683,16 @@ dictionaries as values that contain keys like in the following example:: "product": { "label": _("Product name"), "editor_sample": _("Sample product"), - "evaluate": lambda orderposition, order, event: str(orderposition.item) + "evaluate": lambda orderposition, order, event: str(orderposition.item), + "evaluate_bulk": lambda orderpositions: [str(op.item) for op in orderpositions], } } The ``evaluate`` member will be called with the order position, order and event as arguments. The event might also be a subevent, if applicable. + +The ``evaluate_bulk`` member is optional but can significantly improve performance in some situations because you +can perform database fetches in bulk instead of single queries for every position. """ diff --git a/src/tests/api/test_orders.py b/src/tests/api/test_orders.py index 331664880..ba1e5c06f 100644 --- a/src/tests/api/test_orders.py +++ b/src/tests/api/test_orders.py @@ -1794,6 +1794,8 @@ def test_pdf_data(token_client, organizer, event, order, django_assert_max_num_q )) assert resp.status_code == 200 assert resp.data['positions'][0].get('pdf_data') + assert resp.data['positions'][0]['pdf_data']['positionid'] == '1' + assert resp.data['positions'][0]['pdf_data']['order'] == order.code resp = token_client.get('/api/v1/organizers/{}/events/{}/orders/{}/'.format( organizer.slug, event.slug, order.code )) @@ -1807,6 +1809,8 @@ def test_pdf_data(token_client, organizer, event, order, django_assert_max_num_q )) assert resp.status_code == 200 assert resp.data['results'][0]['positions'][0].get('pdf_data') + assert resp.data['results'][0]['positions'][0]['pdf_data']['positionid'] == '1' + assert resp.data['results'][0]['positions'][0]['pdf_data']['order'] == order.code resp = token_client.get('/api/v1/organizers/{}/events/{}/orders/'.format( organizer.slug, event.slug )) @@ -1820,6 +1824,8 @@ def test_pdf_data(token_client, organizer, event, order, django_assert_max_num_q )) assert resp.status_code == 200 assert resp.data['results'][0].get('pdf_data') + assert resp.data['results'][0]['pdf_data']['positionid'] == '1' + assert resp.data['results'][0]['pdf_data']['order'] == order.code resp = token_client.get('/api/v1/organizers/{}/events/{}/orderpositions/'.format( organizer.slug, event.slug )) @@ -1834,6 +1840,8 @@ def test_pdf_data(token_client, organizer, event, order, django_assert_max_num_q )) assert resp.status_code == 200 assert resp.data.get('pdf_data') + assert resp.data['pdf_data']['positionid'] == '1' + assert resp.data['pdf_data']['order'] == order.code resp = token_client.get('/api/v1/organizers/{}/events/{}/orderpositions/{}/'.format( organizer.slug, event.slug, posid ))