diff --git a/src/pretix/base/exporter.py b/src/pretix/base/exporter.py index fb85dc3ab..bf0ae2ba2 100644 --- a/src/pretix/base/exporter.py +++ b/src/pretix/base/exporter.py @@ -105,6 +105,18 @@ class BaseExporter: """ return False + @property + def repeatable_read(self) -> bool: + """ + If ``True``, this exporter will be run in a REPEATABLE READ transaction. This ensures consistent results for + all queries performed by the exporter, but creates a performance burden on the database server. We recommend to + disable this for exporters that take very long to run and do not rely on this behavior, such as export of lists + to CSV files. + + Defaults to ``True`` for now, but default may change in future versions. + """ + return True + @property def identifier(self) -> str: """ diff --git a/src/pretix/base/exporters/invoices.py b/src/pretix/base/exporters/invoices.py index 0cc9b6903..0ae23e9b7 100644 --- a/src/pretix/base/exporters/invoices.py +++ b/src/pretix/base/exporters/invoices.py @@ -180,6 +180,7 @@ class InvoiceDataExporter(InvoiceExporterMixin, MultiSheetListExporter): 'includes two sheets, one with a line for every invoice, and one with a line for every position of ' 'every invoice.') featured = True + repeatable_read = False @property def additional_form_fields(self): diff --git a/src/pretix/base/exporters/orderlist.py b/src/pretix/base/exporters/orderlist.py index cd5453c6b..b756c247b 100644 --- a/src/pretix/base/exporters/orderlist.py +++ b/src/pretix/base/exporters/orderlist.py @@ -90,6 +90,7 @@ class OrderListExporter(MultiSheetListExporter): 'with a line for every order, one with a line for every order position, and one with ' 'a line for every additional fee charged in an order.') featured = True + repeatable_read = False @cached_property def providers(self): @@ -842,6 +843,7 @@ class TransactionListExporter(ListExporter): description = gettext_lazy('Download a spreadsheet of all substantial changes to orders, i.e. all changes to ' 'products, prices or tax rates. The information is only accurate for changes made with ' 'pretix versions released after October 2021.') + repeatable_read = False @cached_property def providers(self): @@ -1020,6 +1022,7 @@ class PaymentListExporter(ListExporter): category = pgettext_lazy('export_category', 'Order data') description = gettext_lazy('Download a spreadsheet of all payments or refunds of every order.') featured = True + repeatable_read = False @property def additional_form_fields(self): @@ -1159,7 +1162,7 @@ class QuotaListExporter(ListExporter): yield headers quotas = list(self.event.quotas.select_related('subevent')) - qa = QuotaAvailability(full_results=True) + qa = QuotaAvailability(full_results=True, allow_repeatable_read=False) qa.queue(*quotas) qa.compute() @@ -1200,6 +1203,7 @@ class GiftcardTransactionListExporter(OrganizerLevelExportMixin, ListExporter): organizer_required_permission = 'can_manage_gift_cards' category = pgettext_lazy('export_category', 'Gift cards') description = gettext_lazy('Download a spreadsheet of all gift card transactions.') + repeatable_read = False @property def additional_form_fields(self): @@ -1258,6 +1262,7 @@ class GiftcardRedemptionListExporter(ListExporter): verbose_name = gettext_lazy('Gift card redemptions') category = pgettext_lazy('export_category', 'Order data') description = gettext_lazy('Download a spreadsheet of all payments or refunds that involve gift cards.') + repeatable_read = False def iterate_list(self, form_data): payments = OrderPayment.objects.filter( diff --git a/src/pretix/base/exporters/reusablemedia.py b/src/pretix/base/exporters/reusablemedia.py index f73f1913a..14bde9b70 100644 --- a/src/pretix/base/exporters/reusablemedia.py +++ b/src/pretix/base/exporters/reusablemedia.py @@ -34,6 +34,7 @@ class ReusableMediaExporter(OrganizerLevelExportMixin, ListExporter): verbose_name = _('Reusable media') category = pgettext_lazy('export_category', 'Reusable media') description = _('Download a spread sheet with the data of all reusable medias on your account.') + repeatable_read = False def iterate_list(self, form_data): media = ReusableMedium.objects.filter( diff --git a/src/pretix/base/exporters/waitinglist.py b/src/pretix/base/exporters/waitinglist.py index 1b1cf1faf..acd2f1d5a 100644 --- a/src/pretix/base/exporters/waitinglist.py +++ b/src/pretix/base/exporters/waitinglist.py @@ -41,6 +41,7 @@ class WaitingListExporter(ListExporter): verbose_name = _('Waiting list') category = pgettext_lazy('export_category', 'Waiting list') description = _('Download a spread sheet with all your waiting list data.') + repeatable_read = False # map selected status to label and queryset-filter status_filters = [ diff --git a/src/pretix/base/services/export.py b/src/pretix/base/services/export.py index ee076cd8d..88fc327bb 100644 --- a/src/pretix/base/services/export.py +++ b/src/pretix/base/services/export.py @@ -49,7 +49,7 @@ from pretix.base.signals import ( periodic_task, register_data_exporters, register_multievent_data_exporters, ) from pretix.celery_app import app -from pretix.helpers import OF_SELF +from pretix.helpers import OF_SELF, repeatable_reads_transaction from pretix.helpers.urls import build_absolute_uri logger = logging.getLogger(__name__) @@ -80,7 +80,12 @@ def export(self, event: Event, fileid: str, provider: str, form_data: Dict[str, continue ex = response(event, event.organizer, set_progress) if ex.identifier == provider: - d = ex.render(form_data) + if ex.repeatable_read: + with repeatable_reads_transaction(): + d = ex.render(form_data) + else: + d = ex.render(form_data) + if d is None: raise ExportError( gettext('Your export did not contain any data.') @@ -151,7 +156,11 @@ def multiexport(self, organizer: Organizer, user: User, device: int, token: int, gettext('You do not have sufficient permission to perform this export.') ) - d = ex.render(form_data) + if ex.repeatable_read: + with repeatable_reads_transaction(): + d = ex.render(form_data) + else: + d = ex.render(form_data) if d is None: raise ExportError( gettext('Your export did not contain any data.') @@ -209,7 +218,11 @@ def _run_scheduled_export(schedule, context: Union[Event, Organizer], exporter, try: if not exporter: raise ExportError("Export type not found.") - d = exporter.render(schedule.export_form_data) + if exporter.repeatable_read: + with repeatable_reads_transaction(): + d = exporter.render(schedule.export_form_data) + else: + d = exporter.render(schedule.export_form_data) if d is None: raise ExportEmptyError( gettext('Your export did not contain any data.') diff --git a/src/pretix/base/services/quotas.py b/src/pretix/base/services/quotas.py index 4511309e7..8a2360e48 100644 --- a/src/pretix/base/services/quotas.py +++ b/src/pretix/base/services/quotas.py @@ -26,7 +26,7 @@ from itertools import zip_longest import django_redis from django.conf import settings -from django.db import models +from django.db import connection, models from django.db.models import ( Case, Count, F, Func, Max, OuterRef, Q, Subquery, Sum, Value, When, prefetch_related_objects, @@ -64,7 +64,8 @@ class QuotaAvailability: * count_cart (dict mapping quotas to ints) """ - def __init__(self, count_waitinglist=True, ignore_closed=False, full_results=False, early_out=True): + def __init__(self, count_waitinglist=True, ignore_closed=False, full_results=False, early_out=True, + allow_repeatable_read=False): """ Initialize a new quota availability calculator @@ -86,6 +87,8 @@ class QuotaAvailability: keep the database-level quota cache up to date so backend overviews render quickly. If you do not care about keeping the cache up to date, you can set this to ``False`` for further performance improvements. + + :param allow_repeatable_read: Allow to run this even in REPEATABLE READ mode, generally not advised. """ self._queue = [] self._count_waitinglist = count_waitinglist @@ -95,6 +98,7 @@ class QuotaAvailability: self._var_to_quotas = defaultdict(set) self._early_out = early_out self._quota_objects = {} + self._allow_repeatable_read = allow_repeatable_read self.results = {} self.count_paid_orders = defaultdict(int) self.count_pending_orders = defaultdict(int) @@ -119,6 +123,10 @@ class QuotaAvailability: Compute the queued quotas. If ``allow_cache`` is set, results may also be taken from a cache that might be a few minutes outdated. In this case, you may not rely on the results in the ``count_*`` properties. """ + if not self._allow_repeatable_read and getattr(connection, "tx_in_repeatable_read", False): + raise ValueError("You cannot compute quotas in REPEATABLE READ mode unless you explicitly opted in to " + "do so.") + now_dt = now_dt or now() quota_ids_set = {q.id for q in self._queue} if not quota_ids_set: diff --git a/src/pretix/helpers/database.py b/src/pretix/helpers/database.py index 5e5b4f009..4e537ff1c 100644 --- a/src/pretix/helpers/database.py +++ b/src/pretix/helpers/database.py @@ -21,7 +21,8 @@ # import contextlib -from django.core.exceptions import FieldDoesNotExist +from django.conf import settings +from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured from django.db import connection, transaction from django.db.models import ( Aggregate, Expression, F, Field, Lookup, OrderBy, Value, @@ -62,6 +63,43 @@ def casual_reads(): yield +@contextlib.contextmanager +def repeatable_reads_transaction(): + """ + pretix, and Django, operate in the transaction isolation level READ COMMITTED by default. This is not a strong level + of isolation, but we NEED to use it: Otherwise e.g. our quota logic breaks, because we need to be able to get the + *current* number of tickets sold at any time in a transaction, not the number of tickets sold *before* our transaction + started. + + However, this isolation mode has drawbacks, for example during reporting. When a user retrieves a report from the + system, it should return numbers that are consistent with each other. However, if the report makes multiple SQL + queries in READ COMMITTED mode, the results might be different for each query, causing numbers to be inconsistent + with each other. + + This context manager creates a transaction that is running in REPEATABLE READ mode to avoid this problem. + + **You should only make read-only queries during this transaction and not rely on quota calculations.** + """ + is_under_test = 'tests.testdummy' in settings.INSTALLED_APPS + try: + with transaction.atomic(durable=not is_under_test): + if not is_under_test: + # We're not running this in tests, where we can basically not use this since the test runner does its + # own transaction logic for efficiency + with connection.cursor() as cursor: + if 'postgresql' in settings.DATABASES['default']['ENGINE']: + cursor.execute('SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;') + elif 'sqlite' in settings.DATABASES['default']['ENGINE']: + pass # noop + else: + raise ImproperlyConfigured("Cannot set transaction isolation mode on this database backend") + + connection.tx_in_repeatable_read = True + yield + finally: + connection.tx_in_repeatable_read = False + + class GroupConcat(Aggregate): function = 'group_concat' template = '%(function)s(%(distinct)s%(field)s, "%(separator)s")' diff --git a/src/pretix/plugins/checkinlists/exporters.py b/src/pretix/plugins/checkinlists/exporters.py index 7f95a79cd..6001f1023 100644 --- a/src/pretix/plugins/checkinlists/exporters.py +++ b/src/pretix/plugins/checkinlists/exporters.py @@ -476,6 +476,7 @@ class CSVCheckinList(CheckInListMixin, ListExporter): category = pgettext_lazy('export_category', 'Check-in') description = gettext_lazy("Download a spreadsheet with all attendees that are included in a check-in list.") featured = True + repeatable_read = False @property def additional_form_fields(self): @@ -673,6 +674,7 @@ class CSVCheckinCodeList(CheckInListMixin, ListExporter): category = pgettext_lazy('export_category', 'Check-in') description = gettext_lazy("Download a spreadsheet with all valid check-in barcodes e.g. for import into a " "different system. Does not included blocked codes or personal data.") + repeatable_read = False @property def additional_form_fields(self): @@ -743,6 +745,7 @@ class CheckinLogList(ListExporter): category = pgettext_lazy('export_category', 'Check-in') description = gettext_lazy("Download a spreadsheet with one line for every scan that happened at your check-in " "stations.") + repeatable_read = False @property def additional_form_fields(self): diff --git a/src/pretix/plugins/reports/exporters.py b/src/pretix/plugins/reports/exporters.py index 13aac28a9..a39eb738b 100644 --- a/src/pretix/plugins/reports/exporters.py +++ b/src/pretix/plugins/reports/exporters.py @@ -661,6 +661,7 @@ class OrderTaxListReport(MultiSheetListExporter): verbose_name = gettext_lazy('Tax split list') category = pgettext_lazy('export_category', 'Order data') description = gettext_lazy("Download a spreadsheet with the tax amounts included in each order.") + repeatable_read = False @property def sheets(self): diff --git a/src/tests/base/test_models.py b/src/tests/base/test_models.py index a4091694a..1409a7a89 100644 --- a/src/tests/base/test_models.py +++ b/src/tests/base/test_models.py @@ -64,6 +64,7 @@ from pretix.base.models.items import ( from pretix.base.reldate import RelativeDate, RelativeDateWrapper from pretix.base.services.orders import OrderError, cancel_order, perform_order from pretix.base.services.quotas import QuotaAvailability +from pretix.helpers import repeatable_reads_transaction from pretix.testutils.scope import classscope @@ -99,6 +100,29 @@ class BaseQuotaTestCase(TestCase): self.var3 = ItemVariation.objects.create(item=self.item3, value='Fancy') +@pytest.mark.django_db(transaction=True) +@scopes_disabled() +def test_verify_repeatable_read_check(): + if 'sqlite' in settings.DATABASES['default']['ENGINE']: + pytest.skip('Not supported on SQLite') + + o = Organizer.objects.create(name='Dummy', slug='dummy') + event = Event.objects.create( + organizer=o, name='Dummy', slug='dummy', + date_from=now(), plugins='tests.testdummy' + ) + quota = Quota.objects.create(name="Test", size=2, event=event) + + with repeatable_reads_transaction(): + with pytest.raises(ValueError): + qa = QuotaAvailability(full_results=True) + qa.queue(quota) + qa.compute() + qa = QuotaAvailability(full_results=True, allow_repeatable_read=True) + qa.queue(quota) + qa.compute() + + @pytest.mark.usefixtures("fakeredis_client") class QuotaTestCase(BaseQuotaTestCase): @classscope(attr='o')