Allow PDF variables to provide a bulk evaluation method (second try at #3517)

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:
+                if 'evaluate_bulk' in f:
-                    res[k] = f['evaluate'](instance, instance.order, ev)
+                    # Will be evaluated later by our list serializers
-                except:
+                    res[k] = (f['evaluate_bulk'], instance)
-                    logger.exception('Evaluating PDF variable failed')
+                else:
-                    res[k] = '(error)'
+                    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',

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"),

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.
 """
 
 

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
     ))