From c842ea597cf5a042e02655512989d6ad9ea9c5ea Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Mon, 11 Sep 2023 11:44:50 +0200 Subject: [PATCH] New locking mechanism (#2408) Co-authored-by: Richard Schreiber --- .github/workflows/tests.yml | 4 + doc/admin/config.rst | 7 + doc/development/implementation/index.rst | 1 + doc/development/implementation/locking.rst | 69 ++ pyproject.toml | 2 + src/pretix/api/serializers/order.py | 643 +++++++++--------- src/pretix/api/serializers/voucher.py | 9 +- src/pretix/api/views/cart.py | 22 +- src/pretix/api/views/voucher.py | 63 +- src/pretix/base/models/event.py | 7 +- src/pretix/base/models/orders.py | 52 +- src/pretix/base/models/vouchers.py | 23 +- src/pretix/base/services/cart.py | 55 +- src/pretix/base/services/locking.py | 235 +++---- src/pretix/base/services/orderimport.py | 19 +- src/pretix/base/services/orders.py | 405 ++++++----- src/pretix/base/services/quotas.py | 2 +- src/pretix/base/services/waitinglist.py | 23 +- src/pretix/base/views/tasks.py | 5 + src/pretix/control/views/vouchers.py | 55 +- src/pretix/settings.py | 2 + src/pretix/testutils/middleware.py | 58 ++ src/pretix/testutils/settings.py | 10 +- src/setup.cfg | 5 +- src/tests/api/test_idempotency.py | 17 +- src/tests/api/test_orders.py | 15 +- src/tests/base/test_locking.py | 77 --- src/tests/concurrency_tests/__init__.py | 21 + src/tests/concurrency_tests/conftest.py | 144 ++++ .../test_cart_creation_locking.py | 140 ++++ .../test_order_creation_locking.py | 189 +++++ .../test_order_paid_locking.py | 113 +++ src/tests/concurrency_tests/utils.py | 29 + 33 files changed, 1638 insertions(+), 883 deletions(-) create mode 100644 doc/development/implementation/locking.rst create mode 100644 src/pretix/testutils/middleware.py delete mode 100644 src/tests/base/test_locking.py create mode 100644 src/tests/concurrency_tests/__init__.py create mode 100644 src/tests/concurrency_tests/conftest.py create mode 100644 src/tests/concurrency_tests/test_cart_creation_locking.py create mode 100644 src/tests/concurrency_tests/test_order_creation_locking.py create mode 100644 src/tests/concurrency_tests/test_order_paid_locking.py create mode 100644 src/tests/concurrency_tests/utils.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0db3305fe..e1565b46e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -66,6 +66,10 @@ jobs: - name: Run tests working-directory: ./src run: PRETIX_CONFIG_FILE=tests/travis_${{ matrix.database }}.cfg py.test -n 3 -p no:sugar --cov=./ --cov-report=xml --reruns 3 tests --maxfail=100 + - name: Run concurrency tests + working-directory: ./src + run: PRETIX_CONFIG_FILE=tests/travis_${{ matrix.database }}.cfg py.test tests/concurrency_tests/ --reruns 0 --reuse-db + if: matrix.database == 'postgres' - name: Upload coverage uses: codecov/codecov-action@v1 with: diff --git a/doc/admin/config.rst b/doc/admin/config.rst index 964b852f9..a6773aa0f 100644 --- a/doc/admin/config.rst +++ b/doc/admin/config.rst @@ -152,6 +152,7 @@ Example:: password=abcd host=localhost port=3306 + advisory_lock_index=1 sslmode=require sslrootcert=/etc/pretix/postgresql-ca.crt sslcert=/etc/pretix/postgresql-client-crt.crt @@ -167,11 +168,17 @@ Example:: ``user``, ``password``, ``host``, ``port`` Connection details for the database connection. Empty by default. +``advisory_lock_index`` + On PostgreSQL, pretix uses the "advisory lock" feature. However, advisory locks use a server-wide name space and + and are not scoped to a specific database. If you run multiple pretix applications with the same PostgreSQL server, + you should set separate values for this setting (integers up to 256). + ``sslmode``, ``sslrootcert`` Connection TLS details for the PostgreSQL database connection. Possible values of ``sslmode`` are ``disable``, ``allow``, ``prefer``, ``require``, ``verify-ca``, and ``verify-full``. ``sslrootcert`` should be the accessible path of the ca certificate. Both values are empty by default. ``sslcert``, ``sslkey`` Connection mTLS details for the PostgreSQL database connection. It's also necessary to specify ``sslmode`` and ``sslrootcert`` parameters, please check the correct values from the TLS part. ``sslcert`` should be the accessible path of the client certificate. ``sslkey`` should be the accessible path of the client key. All values are empty by default. + .. _`config-replica`: Database replica settings diff --git a/doc/development/implementation/index.rst b/doc/development/implementation/index.rst index a9c70e0c8..9a050c02b 100644 --- a/doc/development/implementation/index.rst +++ b/doc/development/implementation/index.rst @@ -18,3 +18,4 @@ Contents: email permissions logging + locking diff --git a/doc/development/implementation/locking.rst b/doc/development/implementation/locking.rst new file mode 100644 index 000000000..337481d35 --- /dev/null +++ b/doc/development/implementation/locking.rst @@ -0,0 +1,69 @@ +.. highlight:: python + +Resource locking +================ + +.. versionchanged:: 2023.8 + + Our locking mechanism changed heavily in version 2023.8. Read `this PR`_ for background information. + +One of pretix's core objectives as a ticketing system could be described as the management of scarce resources. +Specifically, the following types of scarce-ness exist in pretix: + +- Quotas can limit the number of tickets available +- Seats can only be booked once +- Vouchers can only be used a limited number of times +- Some memberships can only be used a limited number of times + +For all of these, it is critical that we prevent race conditions. +While for some events it wouldn't be a big deal to sell a ticket more or less, for some it would be problematic and selling the same seat twice would always be catastrophic. + +We therefore implement a standardized locking approach across the system to limit concurrency in cases where it could +be problematic. + +To acquire a lock on a set of quotas to create a new order that uses that quota, you should follow the following pattern:: + + with transaction.atomic(durable=True): + quotas = Quota.objects.filter(...) + lock_objects(quotas, shared_lock_objects=[event]) + check_quota(quotas) + create_ticket() + +The lock will automatically be released at the end of your database transaction. + +Generally, follow the following guidelines during your development: + +- **Always** acquire a lock on every **quota**, **voucher** or **seat** that you "use" during your transaction. "Use" + here means any action after which the quota, voucher or seat will be **less available**, such as creating a cart + position, creating an order, creating a blocking voucher, etc. + + - There is **no need** to acquire a lock if you **free up** capacity, e.g. by canceling an order, deleting a voucher, etc. + +- **Always** acquire a shared lock on the ``event`` you are working in whenever you acquire a lock on a quota, voucher, + or seat. + +- Only call ``lock_objects`` **once** per transaction. If you violate this rule, `deadlocks`_ become possible. + +- For best performance, call ``lock_objects`` as **late** in your transaction as possible, but always before you check + if the desired resource is still available in sufficient quantity. + +Behind the scenes, the locking is implemented through `PostgreSQL advisory locks`_. You should also be aware of the following +properties of our system: + +- In some situations, an exclusive lock on the ``event`` is used, such as when the system can't determine for sure which + seats will become unavailable after the transaction. + +- An exclusive lock on the event is also used if you pass more than 20 objects to ``lock_objects``. This is a performance + trade-off because it would take long to acquire all of the individual locks. + +- If ``lock_objects`` is unable to acquire a lock within 3 seconds, a ``LockTimeoutException`` will be thrown. + +.. note:: + + We currently do not use ``lock_objects`` for memberships. Instead, we use ``select_for_update()`` on the membership + model. This might change in the future, but you should usually not be concerned about it since + ``validate_memberships_in_order(lock=True)`` will handle it for you. + +.. _this PR: https://github.com/pretix/pretix/pull/2408 +.. _deadlocks: https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-DEADLOCKS +.. _PostgreSQL advisory locks: https://www.postgresql.org/docs/11/explicit-locking.html#ADVISORY-LOCKS \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 97f3049f1..6ef549c09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,6 +110,7 @@ dependencies = [ [project.optional-dependencies] memcached = ["pylibmc"] dev = [ + "aiohttp==3.8.*", "coverage", "coveralls", "fakeredis==2.18.*", @@ -120,6 +121,7 @@ dev = [ "potypo", "pycodestyle==2.10.*", "pyflakes==3.0.*", + "pytest-asyncio", "pytest-cache", "pytest-cov", "pytest-django==4.*", diff --git a/src/pretix/api/serializers/order.py b/src/pretix/api/serializers/order.py index 7f3cc2723..4a7809d1a 100644 --- a/src/pretix/api/serializers/order.py +++ b/src/pretix/api/serializers/order.py @@ -22,6 +22,7 @@ import logging import os from collections import Counter, defaultdict +from datetime import timedelta from decimal import Decimal import pycountry @@ -59,10 +60,11 @@ from pretix.base.models.orders import ( ) from pretix.base.pdf import get_images, get_variables from pretix.base.services.cart import error_messages -from pretix.base.services.locking import NoLockManager +from pretix.base.services.locking import LOCK_TRUST_WINDOW, lock_objects from pretix.base.services.pricing import ( apply_discounts, get_line_price, get_listed_price, is_included_for_free, ) +from pretix.base.services.quotas import QuotaAvailability from pretix.base.settings import COUNTRIES_WITH_STATE_IN_ADDRESS from pretix.base.signals import register_ticket_outputs from pretix.helpers.countries import CachedCountries @@ -1144,338 +1146,367 @@ class OrderCreateSerializer(I18nAwareModelSerializer): else: ia = None - lock_required = False + quotas_by_item = {} + quota_diff_for_locking = Counter() + voucher_diff_for_locking = Counter() + seat_diff_for_locking = Counter() + quota_usage = Counter() + voucher_usage = Counter() + seat_usage = Counter() + v_budget = {} + now_dt = now() + delete_cps = [] + consume_carts = validated_data.pop('consume_carts', []) + for pos_data in positions_data: - pos_data['_quotas'] = list( - pos_data.get('variation').quotas.filter(subevent=pos_data.get('subevent')) - if pos_data.get('variation') - else pos_data.get('item').quotas.filter(subevent=pos_data.get('subevent')) - ) - if pos_data.get('voucher') or pos_data.get('seat') or any(q.size is not None for q in pos_data['_quotas']): - lock_required = True + if (pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')) not in quotas_by_item: + quotas_by_item[pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')] = list( + pos_data.get('variation').quotas.filter(subevent=pos_data.get('subevent')) + if pos_data.get('variation') + else pos_data.get('item').quotas.filter(subevent=pos_data.get('subevent')) + ) + for q in quotas_by_item[pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')]: + quota_diff_for_locking[q] += 1 + if pos_data.get('voucher'): + voucher_diff_for_locking[pos_data['voucher']] += 1 + if pos_data.get('seat'): + try: + seat = self.context['event'].seats.get(seat_guid=pos_data['seat'], subevent=pos_data.get('subevent')) + except Seat.DoesNotExist: + pos_data['seat'] = Seat.DoesNotExist + else: + pos_data['seat'] = seat + seat_diff_for_locking[pos_data['seat']] += 1 - lockfn = self.context['event'].lock - if simulate or not lock_required: - lockfn = NoLockManager - with lockfn() as now_dt: - free_seats = set() - seats_seen = set() - consume_carts = validated_data.pop('consume_carts', []) - delete_cps = [] - quota_avail_cache = {} - v_budget = {} - voucher_usage = Counter() - if consume_carts: - for cp in CartPosition.objects.filter( - event=self.context['event'], cart_id__in=consume_carts, expires__gt=now() - ): - quotas = (cp.variation.quotas.filter(subevent=cp.subevent) - if cp.variation else cp.item.quotas.filter(subevent=cp.subevent)) - for quota in quotas: - if quota not in quota_avail_cache: - quota_avail_cache[quota] = list(quota.availability()) - if quota_avail_cache[quota][1] is not None: - quota_avail_cache[quota][1] += 1 - if cp.voucher: - voucher_usage[cp.voucher] -= 1 - if cp.expires > now_dt: - if cp.seat: - free_seats.add(cp.seat) - delete_cps.append(cp) + if consume_carts: + offset = now() + timedelta(seconds=LOCK_TRUST_WINDOW) + for cp in CartPosition.objects.filter( + event=self.context['event'], cart_id__in=consume_carts, expires__gt=now_dt + ): + quotas = (cp.variation.quotas.filter(subevent=cp.subevent) + if cp.variation else cp.item.quotas.filter(subevent=cp.subevent)) + for quota in quotas: + if cp.expires > offset: + quota_diff_for_locking[quota] -= 1 + quota_usage[quota] -= 1 + if cp.voucher: + if cp.expires > offset: + voucher_diff_for_locking[cp.voucher] -= 1 + voucher_usage[cp.voucher] -= 1 + if cp.seat: + if cp.expires > offset: + seat_diff_for_locking[cp.seat] -= 1 + seat_usage[cp.seat] -= 1 + delete_cps.append(cp) - errs = [{} for p in positions_data] + if not simulate: + full_lock_required = seat_diff_for_locking and self.context['event'].settings.seating_minimal_distance > 0 + if full_lock_required: + # We lock the entire event in this case since we don't want to deal with fine-granular locking + # in the case of seating distance enforcement + lock_objects([self.context['event']]) + else: + lock_objects( + [q for q, d in quota_diff_for_locking.items() if d > 0 and q.size is not None and not force] + + [v for v, d in voucher_diff_for_locking.items() if d > 0 and not force] + + [s for s, d in seat_diff_for_locking.items() if d > 0], + shared_lock_objects=[self.context['event']] + ) - for i, pos_data in enumerate(positions_data): + qa = QuotaAvailability() + qa.queue(*[q for q, d in quota_diff_for_locking.items() if d > 0]) + qa.compute() - if pos_data.get('voucher'): - v = pos_data['voucher'] + # These are not technically correct as diff use due to the time offset applied above, so let's prevent accidental + # use further down + del quota_diff_for_locking, voucher_diff_for_locking, seat_diff_for_locking - if pos_data.get('addon_to'): - errs[i]['voucher'] = ['Vouchers are currently not supported for add-on products.'] - continue + errs = [{} for p in positions_data] - if not v.applies_to(pos_data['item'], pos_data.get('variation')): - errs[i]['voucher'] = [error_messages['voucher_invalid_item']] - continue + for i, pos_data in enumerate(positions_data): + if pos_data.get('voucher'): + v = pos_data['voucher'] - if v.subevent_id and pos_data.get('subevent').pk != v.subevent_id: - errs[i]['voucher'] = [error_messages['voucher_invalid_subevent']] - continue + if pos_data.get('addon_to'): + errs[i]['voucher'] = ['Vouchers are currently not supported for add-on products.'] + continue - if v.valid_until is not None and v.valid_until < now_dt: - errs[i]['voucher'] = [error_messages['voucher_expired']] - continue + if not v.applies_to(pos_data['item'], pos_data.get('variation')): + errs[i]['voucher'] = [error_messages['voucher_invalid_item']] + continue - voucher_usage[v] += 1 - if voucher_usage[v] > 0: - redeemed_in_carts = CartPosition.objects.filter( - Q(voucher=pos_data['voucher']) & Q(event=self.context['event']) & Q(expires__gte=now_dt) - ).exclude(pk__in=[cp.pk for cp in delete_cps]) - v_avail = v.max_usages - v.redeemed - redeemed_in_carts.count() - if v_avail < voucher_usage[v]: - errs[i]['voucher'] = [ - 'The voucher has already been used the maximum number of times.' - ] + if v.subevent_id and pos_data.get('subevent').pk != v.subevent_id: + errs[i]['voucher'] = [error_messages['voucher_invalid_subevent']] + continue - if v.budget is not None: - price = pos_data.get('price') - listed_price = get_listed_price(pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')) + if v.valid_until is not None and v.valid_until < now_dt: + errs[i]['voucher'] = [error_messages['voucher_expired']] + continue - if pos_data.get('voucher'): - price_after_voucher = pos_data.get('voucher').calculate_price(listed_price) - else: - price_after_voucher = listed_price - if price is None: - price = price_after_voucher + voucher_usage[v] += 1 + if voucher_usage[v] > 0: + redeemed_in_carts = CartPosition.objects.filter( + Q(voucher=pos_data['voucher']) & Q(event=self.context['event']) & Q(expires__gte=now_dt) + ).exclude(pk__in=[cp.pk for cp in delete_cps]) + v_avail = v.max_usages - v.redeemed - redeemed_in_carts.count() + if v_avail < voucher_usage[v]: + errs[i]['voucher'] = [ + 'The voucher has already been used the maximum number of times.' + ] - if v not in v_budget: - v_budget[v] = v.budget - v.budget_used() - disc = max(listed_price - price, 0) - if disc > v_budget[v]: - new_disc = v_budget[v] - v_budget[v] -= new_disc - if new_disc == Decimal('0.00') or pos_data.get('price') is not None: - errs[i]['voucher'] = [ - 'The voucher has a remaining budget of {}, therefore a discount of {} can not be ' - 'given.'.format(v_budget[v] + new_disc, disc) - ] - continue - pos_data['price'] = price + (disc - new_disc) - else: - v_budget[v] -= disc + if v.budget is not None: + price = pos_data.get('price') + listed_price = get_listed_price(pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')) - seated = pos_data.get('item').seat_category_mappings.filter(subevent=pos_data.get('subevent')).exists() - if pos_data.get('seat'): - if pos_data.get('addon_to'): - errs[i]['seat'] = ['Seats are currently not supported for add-on products.'] - continue - - if not seated: - errs[i]['seat'] = ['The specified product does not allow to choose a seat.'] - try: - seat = self.context['event'].seats.get(seat_guid=pos_data['seat'], subevent=pos_data.get('subevent')) - except Seat.DoesNotExist: - errs[i]['seat'] = ['The specified seat does not exist.'] - else: - pos_data['seat'] = seat - if (seat not in free_seats and not seat.is_available(sales_channel=validated_data.get('sales_channel', 'web'))) or seat in seats_seen: - errs[i]['seat'] = [gettext_lazy('The selected seat "{seat}" is not available.').format(seat=seat.name)] - seats_seen.add(seat) - elif seated: - errs[i]['seat'] = ['The specified product requires to choose a seat.'] - - requested_valid_from = pos_data.pop('requested_valid_from', None) - if 'valid_from' not in pos_data and 'valid_until' not in pos_data: - valid_from, valid_until = pos_data['item'].compute_validity( - requested_start=( - max(requested_valid_from, now()) - if requested_valid_from and pos_data['item'].validity_dynamic_start_choice - else now() - ), - enforce_start_limit=True, - override_tz=self.context['event'].timezone, - ) - pos_data['valid_from'] = valid_from - pos_data['valid_until'] = valid_until - - if not force: - for i, pos_data in enumerate(positions_data): if pos_data.get('voucher'): - if pos_data['voucher'].allow_ignore_quota or pos_data['voucher'].block_quota: + price_after_voucher = pos_data.get('voucher').calculate_price(listed_price) + else: + price_after_voucher = listed_price + if price is None: + price = price_after_voucher + + if v not in v_budget: + v_budget[v] = v.budget - v.budget_used() + disc = max(listed_price - price, 0) + if disc > v_budget[v]: + new_disc = v_budget[v] + v_budget[v] -= new_disc + if new_disc == Decimal('0.00') or pos_data.get('price') is not None: + errs[i]['voucher'] = [ + 'The voucher has a remaining budget of {}, therefore a discount of {} can not be ' + 'given.'.format(v_budget[v] + new_disc, disc) + ] continue + pos_data['price'] = price + (disc - new_disc) + else: + v_budget[v] -= disc - if pos_data.get('subevent'): - if pos_data.get('item').pk in pos_data['subevent'].item_overrides and pos_data['subevent'].item_overrides[pos_data['item'].pk].disabled: - errs[i]['item'] = [gettext_lazy('The product "{}" is not available on this date.').format( - str(pos_data.get('item')) - )] - if ( - pos_data.get('variation') and pos_data['variation'].pk in pos_data['subevent'].var_overrides and - pos_data['subevent'].var_overrides[pos_data['variation'].pk].disabled - ): - errs[i]['item'] = [gettext_lazy('The product "{}" is not available on this date.').format( - str(pos_data.get('item')) - )] + seated = pos_data.get('item').seat_category_mappings.filter(subevent=pos_data.get('subevent')).exists() + if pos_data.get('seat'): + if pos_data.get('addon_to'): + errs[i]['seat'] = ['Seats are currently not supported for add-on products.'] + continue + if not seated: + errs[i]['seat'] = ['The specified product does not allow to choose a seat.'] + seat = pos_data['seat'] + if seat is Seat.DoesNotExist: + errs[i]['seat'] = ['The specified seat does not exist.'] + else: + seat_usage[seat] += 1 + if (seat_usage[seat] > 0 and not seat.is_available(sales_channel=validated_data.get('sales_channel', 'web'))) or seat_usage[seat] > 1: + errs[i]['seat'] = [gettext_lazy('The selected seat "{seat}" is not available.').format(seat=seat.name)] + elif seated: + errs[i]['seat'] = ['The specified product requires to choose a seat.'] - new_quotas = pos_data['_quotas'] - if len(new_quotas) == 0: - errs[i]['item'] = [gettext_lazy('The product "{}" is not assigned to a quota.').format( + requested_valid_from = pos_data.pop('requested_valid_from', None) + if 'valid_from' not in pos_data and 'valid_until' not in pos_data: + valid_from, valid_until = pos_data['item'].compute_validity( + requested_start=( + max(requested_valid_from, now()) + if requested_valid_from and pos_data['item'].validity_dynamic_start_choice + else now() + ), + enforce_start_limit=True, + override_tz=self.context['event'].timezone, + ) + pos_data['valid_from'] = valid_from + pos_data['valid_until'] = valid_until + + if not force: + for i, pos_data in enumerate(positions_data): + if pos_data.get('voucher'): + if pos_data['voucher'].allow_ignore_quota or pos_data['voucher'].block_quota: + continue + + if pos_data.get('subevent'): + if pos_data.get('item').pk in pos_data['subevent'].item_overrides and pos_data['subevent'].item_overrides[pos_data['item'].pk].disabled: + errs[i]['item'] = [gettext_lazy('The product "{}" is not available on this date.').format( + str(pos_data.get('item')) + )] + if ( + pos_data.get('variation') and pos_data['variation'].pk in pos_data['subevent'].var_overrides and + pos_data['subevent'].var_overrides[pos_data['variation'].pk].disabled + ): + errs[i]['item'] = [gettext_lazy('The product "{}" is not available on this date.').format( str(pos_data.get('item')) )] - else: - for quota in new_quotas: - if quota not in quota_avail_cache: - quota_avail_cache[quota] = list(quota.availability()) - if quota_avail_cache[quota][1] is not None: - quota_avail_cache[quota][1] -= 1 - if quota_avail_cache[quota][1] < 0: - errs[i]['item'] = [ - gettext_lazy('There is not enough quota available on quota "{}" to perform the operation.').format( - quota.name - ) - ] - - if any(errs): - raise ValidationError({'positions': errs}) - - if validated_data.get('locale', None) is None: - validated_data['locale'] = self.context['event'].settings.locale - order = Order(event=self.context['event'], **validated_data) - order.set_expires(subevents=[p.get('subevent') for p in positions_data]) - order.meta_info = "{}" - order.total = Decimal('0.00') - if validated_data.get('require_approval') is not None: - order.require_approval = validated_data['require_approval'] - if simulate: - order = WrappedModel(order) - order.last_modified = now() - order.code = 'PREVIEW' - else: - order.save() - - if ia: - if not simulate: - ia.order = order - ia.save() + new_quotas = quotas_by_item[pos_data.get('item'), pos_data.get('variation'), pos_data.get('subevent')] + if len(new_quotas) == 0: + errs[i]['item'] = [gettext_lazy('The product "{}" is not assigned to a quota.').format( + str(pos_data.get('item')) + )] else: - order.invoice_address = ia - ia.last_modified = now() + for quota in new_quotas: + quota_usage[quota] += 1 + if quota_usage[quota] > 0 and qa.results[quota][1] is not None: + if qa.results[quota][1] < quota_usage[quota]: + errs[i]['item'] = [ + gettext_lazy('There is not enough quota available on quota "{}" to perform the operation.').format( + quota.name + ) + ] - # Generate position objects - pos_map = {} - for pos_data in positions_data: - addon_to = pos_data.pop('addon_to', None) - attendee_name = pos_data.pop('attendee_name', '') - if attendee_name and not pos_data.get('attendee_name_parts'): - pos_data['attendee_name_parts'] = { - '_legacy': attendee_name - } - pos = OrderPosition(**{k: v for k, v in pos_data.items() if k != 'answers' and k != '_quotas' and k != 'use_reusable_medium'}) - if simulate: - pos.order = order._wrapped - else: - pos.order = order - if addon_to: - if simulate: - pos.addon_to = pos_map[addon_to] - else: - pos.addon_to = pos_map[addon_to] + if any(errs): + raise ValidationError({'positions': errs}) - pos_map[pos.positionid] = pos - pos_data['__instance'] = pos - - # Calculate prices if not set - for pos_data in positions_data: - pos = pos_data['__instance'] - if pos.addon_to_id and is_included_for_free(pos.item, pos.addon_to): - listed_price = Decimal('0.00') - else: - listed_price = get_listed_price(pos.item, pos.variation, pos.subevent) - - if pos.price is None: - if pos.voucher: - price_after_voucher = pos.voucher.calculate_price(listed_price) - else: - price_after_voucher = listed_price - - line_price = get_line_price( - price_after_voucher=price_after_voucher, - custom_price_input=None, - custom_price_input_is_net=False, - tax_rule=pos.item.tax_rule, - invoice_address=ia, - bundled_sum=Decimal('0.00'), - ) - pos.price = line_price.gross - pos._auto_generated_price = True - else: - if pos.voucher: - if not pos.item.tax_rule or pos.item.tax_rule.price_includes_tax: - price_after_voucher = max(pos.price, pos.voucher.calculate_price(listed_price)) - else: - price_after_voucher = max(pos.price - pos.tax_value, pos.voucher.calculate_price(listed_price)) - else: - price_after_voucher = listed_price - pos._auto_generated_price = False - pos._voucher_discount = listed_price - price_after_voucher - if pos.voucher: - pos.voucher_budget_use = max(listed_price - price_after_voucher, Decimal('0.00')) - - order_positions = [pos_data['__instance'] for pos_data in positions_data] - discount_results = apply_discounts( - self.context['event'], - order.sales_channel, - [ - (cp.item_id, cp.subevent_id, cp.price, bool(cp.addon_to), cp.is_bundled, pos._voucher_discount) - for cp in order_positions - ] - ) - for cp, (new_price, discount) in zip(order_positions, discount_results): - if new_price != pos.price and pos._auto_generated_price: - pos.price = new_price - pos.discount = discount - - # Save instances - for pos_data in positions_data: - answers_data = pos_data.pop('answers', []) - use_reusable_medium = pos_data.pop('use_reusable_medium', None) - pos = pos_data['__instance'] - pos._calculate_tax() - - if simulate: - pos = WrappedModel(pos) - pos.id = 0 - answers = [] - for answ_data in answers_data: - options = answ_data.pop('options', []) - answ = WrappedModel(QuestionAnswer(**answ_data)) - answ.options = WrappedList(options) - answers.append(answ) - pos.answers = answers - pos.pseudonymization_id = "PREVIEW" - pos.checkins = [] - pos_map[pos.positionid] = pos - else: - if pos.voucher: - Voucher.objects.filter(pk=pos.voucher.pk).update(redeemed=F('redeemed') + 1) - pos.save() - seen_answers = set() - for answ_data in answers_data: - # Workaround for a pretixPOS bug :-( - if answ_data.get('question') in seen_answers: - continue - seen_answers.add(answ_data.get('question')) - - options = answ_data.pop('options', []) - - if isinstance(answ_data['answer'], File): - an = answ_data.pop('answer') - answ = pos.answers.create(**answ_data, answer='') - answ.file.save(os.path.basename(an.name), an, save=False) - answ.answer = 'file://' + answ.file.name - answ.save() - else: - answ = pos.answers.create(**answ_data) - answ.options.add(*options) - - if use_reusable_medium: - use_reusable_medium.linked_orderposition = pos - use_reusable_medium.save(update_fields=['linked_orderposition']) - use_reusable_medium.log_action( - 'pretix.reusable_medium.linked_orderposition.changed', - data={ - 'by_order': order.code, - 'linked_orderposition': pos.pk, - } - ) + if validated_data.get('locale', None) is None: + validated_data['locale'] = self.context['event'].settings.locale + order = Order(event=self.context['event'], **validated_data) + order.set_expires(subevents=[p.get('subevent') for p in positions_data]) + order.meta_info = "{}" + order.total = Decimal('0.00') + if validated_data.get('require_approval') is not None: + order.require_approval = validated_data['require_approval'] + if simulate: + order = WrappedModel(order) + order.last_modified = now() + order.code = 'PREVIEW' + else: + order.save() + if ia: if not simulate: - for cp in delete_cps: - if cp.addon_to_id: + ia.order = order + ia.save() + else: + order.invoice_address = ia + ia.last_modified = now() + + # Generate position objects + pos_map = {} + for pos_data in positions_data: + addon_to = pos_data.pop('addon_to', None) + attendee_name = pos_data.pop('attendee_name', '') + if attendee_name and not pos_data.get('attendee_name_parts'): + pos_data['attendee_name_parts'] = { + '_legacy': attendee_name + } + pos = OrderPosition(**{k: v for k, v in pos_data.items() if k != 'answers' and k != '_quotas' and k != 'use_reusable_medium'}) + if simulate: + pos.order = order._wrapped + else: + pos.order = order + if addon_to: + if simulate: + pos.addon_to = pos_map[addon_to] + else: + pos.addon_to = pos_map[addon_to] + + pos_map[pos.positionid] = pos + pos_data['__instance'] = pos + + # Calculate prices if not set + for pos_data in positions_data: + pos = pos_data['__instance'] + if pos.addon_to_id and is_included_for_free(pos.item, pos.addon_to): + listed_price = Decimal('0.00') + else: + listed_price = get_listed_price(pos.item, pos.variation, pos.subevent) + + if pos.price is None: + if pos.voucher: + price_after_voucher = pos.voucher.calculate_price(listed_price) + else: + price_after_voucher = listed_price + + line_price = get_line_price( + price_after_voucher=price_after_voucher, + custom_price_input=None, + custom_price_input_is_net=False, + tax_rule=pos.item.tax_rule, + invoice_address=ia, + bundled_sum=Decimal('0.00'), + ) + pos.price = line_price.gross + pos._auto_generated_price = True + else: + if pos.voucher: + if not pos.item.tax_rule or pos.item.tax_rule.price_includes_tax: + price_after_voucher = max(pos.price, pos.voucher.calculate_price(listed_price)) + else: + price_after_voucher = max(pos.price - pos.tax_value, pos.voucher.calculate_price(listed_price)) + else: + price_after_voucher = listed_price + pos._auto_generated_price = False + pos._voucher_discount = listed_price - price_after_voucher + if pos.voucher: + pos.voucher_budget_use = max(listed_price - price_after_voucher, Decimal('0.00')) + + order_positions = [pos_data['__instance'] for pos_data in positions_data] + discount_results = apply_discounts( + self.context['event'], + order.sales_channel, + [ + (cp.item_id, cp.subevent_id, cp.price, bool(cp.addon_to), cp.is_bundled, pos._voucher_discount) + for cp in order_positions + ] + ) + for cp, (new_price, discount) in zip(order_positions, discount_results): + if new_price != pos.price and pos._auto_generated_price: + pos.price = new_price + pos.discount = discount + + # Save instances + for pos_data in positions_data: + answers_data = pos_data.pop('answers', []) + use_reusable_medium = pos_data.pop('use_reusable_medium', None) + pos = pos_data['__instance'] + pos._calculate_tax() + + if simulate: + pos = WrappedModel(pos) + pos.id = 0 + answers = [] + for answ_data in answers_data: + options = answ_data.pop('options', []) + answ = WrappedModel(QuestionAnswer(**answ_data)) + answ.options = WrappedList(options) + answers.append(answ) + pos.answers = answers + pos.pseudonymization_id = "PREVIEW" + pos.checkins = [] + pos_map[pos.positionid] = pos + else: + if pos.voucher: + Voucher.objects.filter(pk=pos.voucher.pk).update(redeemed=F('redeemed') + 1) + pos.save() + seen_answers = set() + for answ_data in answers_data: + # Workaround for a pretixPOS bug :-( + if answ_data.get('question') in seen_answers: continue - cp.addons.all().delete() - cp.delete() + seen_answers.add(answ_data.get('question')) + + options = answ_data.pop('options', []) + + if isinstance(answ_data['answer'], File): + an = answ_data.pop('answer') + answ = pos.answers.create(**answ_data, answer='') + answ.file.save(os.path.basename(an.name), an, save=False) + answ.answer = 'file://' + answ.file.name + answ.save() + else: + answ = pos.answers.create(**answ_data) + answ.options.add(*options) + + if use_reusable_medium: + use_reusable_medium.linked_orderposition = pos + use_reusable_medium.save(update_fields=['linked_orderposition']) + use_reusable_medium.log_action( + 'pretix.reusable_medium.linked_orderposition.changed', + data={ + 'by_order': order.code, + 'linked_orderposition': pos.pk, + } + ) + + if not simulate: + for cp in delete_cps: + if cp.addon_to_id: + continue + cp.addons.all().delete() + cp.delete() order.total = sum([p.price for p in pos_map.values()]) fees = [] diff --git a/src/pretix/api/serializers/voucher.py b/src/pretix/api/serializers/voucher.py index c081f99aa..219f420fc 100644 --- a/src/pretix/api/serializers/voucher.py +++ b/src/pretix/api/serializers/voucher.py @@ -94,8 +94,13 @@ class VoucherSerializer(I18nAwareModelSerializer): ) if check_quota: Voucher.clean_quota_check( - full_data, 1, self.instance, self.context.get('event'), - full_data.get('quota'), full_data.get('item'), full_data.get('variation') + full_data, + full_data.get('max_usages', 1) - (self.instance.redeemed if self.instance else 0), + self.instance, + self.context.get('event'), + full_data.get('quota'), + full_data.get('item'), + full_data.get('variation') ) Voucher.clean_voucher_code(full_data, self.context.get('event'), self.instance.pk if self.instance else None) diff --git a/src/pretix/api/views/cart.py b/src/pretix/api/views/cart.py index 1c13c41ed..342ad0a1d 100644 --- a/src/pretix/api/views/cart.py +++ b/src/pretix/api/views/cart.py @@ -25,6 +25,7 @@ from typing import List from django.db import transaction from django.utils.crypto import get_random_string from django.utils.functional import cached_property +from django.utils.timezone import now from django.utils.translation import gettext as _ from rest_framework import status, viewsets from rest_framework.decorators import action @@ -41,7 +42,7 @@ from pretix.base.models import CartPosition from pretix.base.services.cart import ( _get_quota_availability, _get_voucher_availability, error_messages, ) -from pretix.base.services.locking import NoLockManager +from pretix.base.services.locking import lock_objects class CartPositionViewSet(CreateModelMixin, DestroyModelMixin, viewsets.ReadOnlyModelViewSet): @@ -150,12 +151,21 @@ class CartPositionViewSet(CreateModelMixin, DestroyModelMixin, viewsets.ReadOnly quota_diff[q] += 1 seats_seen = set() + now_dt = now() + with transaction.atomic(): + full_lock_required = seat_diff and self.request.event.settings.seating_minimal_distance > 0 + if full_lock_required: + # We lock the entire event in this case since we don't want to deal with fine-granular locking + # in the case of seating distance enforcement + lock_objects([self.request.event]) + else: + lock_objects( + [q for q, d in quota_diff.items() if q.size is not None and d > 0] + + [v for v, d in voucher_use_diff.items() if d > 0] + + [s for s, d in seat_diff.items() if d > 0], + shared_lock_objects=[self.request.event] + ) - lockfn = NoLockManager - if self._require_locking(quota_diff, voucher_use_diff, seat_diff): - lockfn = self.request.event.lock - - with lockfn() as now_dt, transaction.atomic(): vouchers_ok, vouchers_depend_on_cart = _get_voucher_availability( self.request.event, voucher_use_diff, diff --git a/src/pretix/api/views/voucher.py b/src/pretix/api/views/voucher.py index 8c119f74a..3fcb235e2 100644 --- a/src/pretix/api/views/voucher.py +++ b/src/pretix/api/views/voucher.py @@ -19,8 +19,6 @@ # You should have received a copy of the GNU Affero General Public License along with this program. If not, see # . # -import contextlib - from django.db import transaction from django.db.models import F, Q from django.utils.timezone import now @@ -69,30 +67,9 @@ class VoucherViewSet(viewsets.ModelViewSet): def get_queryset(self): return self.request.event.vouchers.select_related('seat').all() - def _predict_quota_check(self, data, instance): - # This method predicts if Voucher.clean_quota_needs_checking - # *migh* later require a quota check. It is only approximate - # and returns True a little too often. The point is to avoid - # locks when we know we won't need them. - if 'allow_ignore_quota' in data and data.get('allow_ignore_quota'): - return False - if instance and 'allow_ignore_quota' not in data and instance.allow_ignore_quota: - return False - - if 'block_quota' in data and not data.get('block_quota'): - return False - if instance and 'block_quota' not in data and not instance.block_quota: - return False - - return True - + @transaction.atomic() def create(self, request, *args, **kwargs): - if self._predict_quota_check(request.data, None): - lockfn = request.event.lock - else: - lockfn = contextlib.suppress # noop context manager - with lockfn(): - return super().create(request, *args, **kwargs) + return super().create(request, *args, **kwargs) def perform_create(self, serializer): serializer.save(event=self.request.event) @@ -108,13 +85,9 @@ class VoucherViewSet(viewsets.ModelViewSet): ctx['event'] = self.request.event return ctx + @transaction.atomic() def update(self, request, *args, **kwargs): - if self._predict_quota_check(request.data, self.get_object()): - lockfn = request.event.lock - else: - lockfn = contextlib.suppress # noop context manager - with lockfn(): - return super().update(request, *args, **kwargs) + return super().update(request, *args, **kwargs) def perform_update(self, serializer): serializer.save(event=self.request.event) @@ -140,22 +113,18 @@ class VoucherViewSet(viewsets.ModelViewSet): super().perform_destroy(instance) @action(detail=False, methods=['POST']) + @transaction.atomic() def batch_create(self, request, *args, **kwargs): - if any(self._predict_quota_check(d, None) for d in request.data): - lockfn = request.event.lock - else: - lockfn = contextlib.suppress # noop context manager - with lockfn(): - serializer = self.get_serializer(data=request.data, many=True) - serializer.is_valid(raise_exception=True) - with transaction.atomic(): - serializer.save(event=self.request.event) - for i, v in enumerate(serializer.instance): - v.log_action( - 'pretix.voucher.added', - user=self.request.user, - auth=self.request.auth, - data=self.request.data[i] - ) + serializer = self.get_serializer(data=request.data, many=True) + serializer.is_valid(raise_exception=True) + with transaction.atomic(): + serializer.save(event=self.request.event) + for i, v in enumerate(serializer.instance): + v.log_action( + 'pretix.voucher.added', + user=self.request.user, + auth=self.request.auth, + data=self.request.data[i] + ) headers = self.get_success_headers(serializer.data) return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) diff --git a/src/pretix/base/models/event.py b/src/pretix/base/models/event.py index 9d88f2b1d..be2d67397 100644 --- a/src/pretix/base/models/event.py +++ b/src/pretix/base/models/event.py @@ -743,12 +743,7 @@ class Event(EventMixin, LoggedModel): return ObjectRelatedCache(self) def lock(self): - """ - Returns a contextmanager that can be used to lock an event for bookings. - """ - from pretix.base.services import locking - - return locking.LockManager(self) + raise NotImplementedError("this method has been removed") def get_mail_backend(self, timeout=None): """ diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index d2eaee486..e134a4f52 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -37,10 +37,13 @@ import copy import hashlib import json import logging +import operator import string from collections import Counter from datetime import datetime, time, timedelta from decimal import Decimal +from functools import reduce +from time import sleep from typing import Any, Dict, List, Union from zoneinfo import ZoneInfo @@ -75,7 +78,6 @@ from pretix.base.email import get_email_context from pretix.base.i18n import language from pretix.base.models import Customer, User from pretix.base.reldate import RelativeDateWrapper -from pretix.base.services.locking import LOCK_TIMEOUT, NoLockManager from pretix.base.settings import PERSON_NAME_SCHEMES from pretix.base.signals import order_gracefully_delete @@ -83,6 +85,7 @@ from ...helpers import OF_SELF from ...helpers.countries import CachedCountries, FastCountryField from ...helpers.format import format_map from ...helpers.names import build_name +from ...testutils.middleware import debugflags_var from ._transactions import ( _fail, _transactions_mark_order_clean, _transactions_mark_order_dirty, ) @@ -923,7 +926,7 @@ class Order(LockModel, LoggedModel): else: return expires - def _can_be_paid(self, count_waitinglist=True, ignore_date=False, force=False) -> Union[bool, str]: + def _can_be_paid(self, count_waitinglist=True, ignore_date=False, force=False, lock=False) -> Union[bool, str]: error_messages = { 'late_lastdate': _("The payment can not be accepted as the last date of payments configured in the " "payment settings is over."), @@ -944,10 +947,11 @@ class Order(LockModel, LoggedModel): if not self.event.settings.get('payment_term_accept_late') and not ignore_date and not force: return error_messages['late'] - return self._is_still_available(count_waitinglist=count_waitinglist, force=force) + return self._is_still_available(count_waitinglist=count_waitinglist, force=force, lock=lock) - def _is_still_available(self, now_dt: datetime=None, count_waitinglist=True, force=False, + def _is_still_available(self, now_dt: datetime=None, count_waitinglist=True, lock=False, force=False, check_voucher_usage=False, check_memberships=False) -> Union[bool, str]: + from pretix.base.services.locking import lock_objects from pretix.base.services.memberships import ( validate_memberships_in_order, ) @@ -966,10 +970,21 @@ class Order(LockModel, LoggedModel): try: if check_memberships: try: - validate_memberships_in_order(self.customer, positions, self.event, lock=False, testmode=self.testmode) + validate_memberships_in_order(self.customer, positions, self.event, lock=lock, testmode=self.testmode) except ValidationError as e: raise Quota.QuotaExceededException(e.message) + for cp in positions: + cp._cached_quotas = list(cp.quotas) if not force else [] + + if lock: + lock_objects( + [q for q in reduce(operator.or_, (set(cp._cached_quotas) for cp in positions), set()) if q.size is not None] + + [op.voucher for op in positions if op.voucher and not force] + + [op.seat for op in positions if op.seat], + shared_lock_objects=[self.event] + ) + for i, op in enumerate(positions): if op.seat: if not op.seat.is_available(ignore_orderpos=op): @@ -994,7 +1009,7 @@ class Order(LockModel, LoggedModel): voucher=op.voucher.code )) - quotas = list(op.quotas) + quotas = op._cached_quotas if len(quotas) == 0: raise Quota.QuotaExceededException(error_messages['unavailable'].format( item=str(op.item) + (' - ' + str(op.variation) if op.variation else '') @@ -1016,6 +1031,9 @@ class Order(LockModel, LoggedModel): )) except Quota.QuotaExceededException as e: return str(e) + + if 'sleep-after-quota-check' in debugflags_var.get(): + sleep(2) return True def send_mail(self, subject: Union[str, LazyI18nString], template: Union[str, LazyI18nString], @@ -1647,9 +1665,10 @@ class OrderPayment(models.Model): return self.order.event.get_payment_providers(cached=True).get(self.provider) @transaction.atomic() - def _mark_paid_inner(self, force, count_waitinglist, user, auth, ignore_date=False, overpaid=False): + def _mark_paid_inner(self, force, count_waitinglist, user, auth, ignore_date=False, overpaid=False, lock=False): from pretix.base.signals import order_paid - can_be_paid = self.order._can_be_paid(count_waitinglist=count_waitinglist, ignore_date=ignore_date, force=force) + can_be_paid = self.order._can_be_paid(count_waitinglist=count_waitinglist, ignore_date=ignore_date, force=force, + lock=lock) if can_be_paid is not True: self.order.log_action('pretix.event.order.quotaexceeded', { 'message': can_be_paid @@ -1780,25 +1799,24 @@ class OrderPayment(models.Model): )) return - self._mark_order_paid(count_waitinglist, send_mail, force, user, auth, mail_text, ignore_date, lock, payment_sum - refund_sum, - generate_invoice) + with transaction.atomic(): + self._mark_order_paid(count_waitinglist, send_mail, force, user, auth, mail_text, ignore_date, lock, payment_sum - refund_sum, + generate_invoice) def _mark_order_paid(self, count_waitinglist=True, send_mail=True, force=False, user=None, auth=None, mail_text='', ignore_date=False, lock=True, payment_refund_sum=0, allow_generate_invoice=True): from pretix.base.services.invoices import ( generate_invoice, invoice_qualified, ) + from pretix.base.services.locking import LOCK_TRUST_WINDOW - if (self.order.status == Order.STATUS_PENDING and self.order.expires > now() + timedelta(seconds=LOCK_TIMEOUT * 2)) or not lock: + if lock and self.order.status == Order.STATUS_PENDING and self.order.expires > now() + timedelta(seconds=LOCK_TRUST_WINDOW): # Performance optimization. In this case, there's really no reason to lock everything and an atomic # database transaction is more than enough. - lockfn = NoLockManager - else: - lockfn = self.order.event.lock + lock = False - with lockfn(): - self._mark_paid_inner(force, count_waitinglist, user, auth, overpaid=payment_refund_sum > self.order.total, - ignore_date=ignore_date) + self._mark_paid_inner(force, count_waitinglist, user, auth, overpaid=payment_refund_sum > self.order.total, + ignore_date=ignore_date, lock=lock) invoice = None if invoice_qualified(self.order) and allow_generate_invoice: diff --git a/src/pretix/base/models/vouchers.py b/src/pretix/base/models/vouchers.py index 33dc705cc..7ad46d8f0 100644 --- a/src/pretix/base/models/vouchers.py +++ b/src/pretix/base/models/vouchers.py @@ -435,28 +435,37 @@ class Voucher(LoggedModel): @staticmethod def clean_quota_check(data, cnt, old_instance, event, quota, item, variation): + from ..services.locking import lock_objects + from ..services.quotas import QuotaAvailability + old_quotas = Voucher.clean_quota_get_ignored(old_instance) if event.has_subevents and data.get('block_quota') and not data.get('subevent'): raise ValidationError(_('If you want this voucher to block quota, you need to select a specific date.')) if quota: - if quota in old_quotas: - return - else: - avail = quota.availability(count_waitinglist=False) + new_quotas = {quota} elif item and item.has_variations and not variation: raise ValidationError(_('You can only block quota if you specify a specific product variation. ' 'Otherwise it might be unclear which quotas to block.')) elif item and variation: - avail = variation.check_quotas(ignored_quotas=old_quotas, subevent=data.get('subevent')) + new_quotas = set(variation.quotas.filter(subevent=data.get('subevent'))) elif item and not item.has_variations: - avail = item.check_quotas(ignored_quotas=old_quotas, subevent=data.get('subevent')) + new_quotas = set(item.quotas.filter(subevent=data.get('subevent'))) else: raise ValidationError(_('You need to select a specific product or quota if this voucher should reserve ' 'tickets.')) - if avail[0] != Quota.AVAILABILITY_OK or (avail[1] is not None and avail[1] < cnt): + if not (new_quotas - old_quotas): + return + + lock_objects([q for q in (new_quotas - old_quotas) if q.size is not None], shared_lock_objects=[event]) + + qa = QuotaAvailability(count_waitinglist=False) + qa.queue(*(new_quotas - old_quotas)) + qa.compute() + + if any(r[0] != Quota.AVAILABILITY_OK or (r[1] is not None and r[1] < cnt) for r in qa.results.values()): raise ValidationError(_('You cannot create a voucher that blocks quota as the selected product or ' 'quota is currently sold out or completely reserved.')) diff --git a/src/pretix/base/services/cart.py b/src/pretix/base/services/cart.py index 569377731..169fb1991 100644 --- a/src/pretix/base/services/cart.py +++ b/src/pretix/base/services/cart.py @@ -36,6 +36,7 @@ import uuid from collections import Counter, defaultdict, namedtuple from datetime import datetime, time, timedelta from decimal import Decimal +from time import sleep from typing import List, Optional from celery.exceptions import MaxRetriesExceededError @@ -62,7 +63,7 @@ from pretix.base.models.orders import OrderFee from pretix.base.models.tax import TaxRule from pretix.base.reldate import RelativeDateWrapper from pretix.base.services.checkin import _save_answers -from pretix.base.services.locking import LockTimeoutException, NoLockManager +from pretix.base.services.locking import LockTimeoutException, lock_objects from pretix.base.services.pricing import ( apply_discounts, get_line_price, get_listed_price, get_price, is_included_for_free, @@ -76,6 +77,7 @@ from pretix.celery_app import app from pretix.presale.signals import ( checkout_confirm_messages, fee_calculation_for_cart, ) +from pretix.testutils.middleware import debugflags_var class CartError(Exception): @@ -1073,7 +1075,20 @@ class CartManager: ) return err + @transaction.atomic(durable=True) def _perform_operations(self): + full_lock_required = any(getattr(o, 'seat', False) for o in self._operations) and self.event.settings.seating_minimal_distance > 0 + if full_lock_required: + # We lock the entire event in this case since we don't want to deal with fine-granular locking + # in the case of seating distance enforcement + lock_objects([self.event]) + else: + lock_objects( + [q for q, d in self._quota_diff.items() if q.size is not None and d > 0] + + [v for v, d in self._voucher_use_diff.items() if d > 0] + + [getattr(o, 'seat', False) for o in self._operations if getattr(o, 'seat', False)], + shared_lock_objects=[self.event] + ) vouchers_ok = self._get_voucher_availability() quotas_ok = _get_quota_availability(self._quota_diff, self.now_dt) err = None @@ -1085,6 +1100,9 @@ class CartManager: self._operations.sort(key=lambda a: self.order[type(a)]) seats_seen = set() + if 'sleep-after-quota-check' in debugflags_var.get(): + sleep(2) + for iop, op in enumerate(self._operations): if isinstance(op, self.RemoveOperation): if op.position.expires > self.now_dt: @@ -1289,22 +1307,11 @@ class CartManager: p.save() _save_answers(p, {}, p._answers) CartPosition.objects.bulk_create([p for p in new_cart_positions if not getattr(p, '_answers', None) and not p.pk]) + + if 'sleep-before-commit' in debugflags_var.get(): + sleep(2) return err - def _require_locking(self): - if self._voucher_use_diff: - # If any vouchers are used, we lock to make sure we don't redeem them to often - return True - - if self._quota_diff and any(q.size is not None for q in self._quota_diff): - # If any quotas are affected that are not unlimited, we lock - return True - - if any(getattr(o, 'seat', False) for o in self._operations): - return True - - return False - def recompute_final_prices_and_taxes(self): positions = sorted(list(self.positions), key=lambda op: -(op.addon_to_id or 0)) diff = Decimal('0.00') @@ -1343,18 +1350,14 @@ class CartManager: err = self.extend_expired_positions() or err err = err or self._check_min_per_voucher() - lockfn = NoLockManager - if self._require_locking(): - lockfn = self.event.lock + self.now_dt = now() - with lockfn() as now_dt: - with transaction.atomic(): - self.now_dt = now_dt - self._extend_expiry_of_valid_existing_positions() - err = self._perform_operations() or err - self.recompute_final_prices_and_taxes() - if err: - raise CartError(err) + self._extend_expiry_of_valid_existing_positions() + err = self._perform_operations() or err + self.recompute_final_prices_and_taxes() + + if err: + raise CartError(err) def add_payment_to_cart(request, provider, min_value: Decimal=None, max_value: Decimal=None, info_data: dict=None): diff --git a/src/pretix/base/services/locking.py b/src/pretix/base/services/locking.py index 4b9eeacd0..74d59b899 100644 --- a/src/pretix/base/services/locking.py +++ b/src/pretix/base/services/locking.py @@ -20,31 +20,105 @@ # . # -# This file is based on an earlier version of pretix which was released under the Apache License 2.0. The full text of -# the Apache License 2.0 can be obtained at . -# -# This file may have since been changed and any changes are released under the terms of AGPLv3 as described above. A -# full history of changes and contributors is available at . -# -# This file contains Apache-licensed contributions copyrighted by: Tobias Kunze -# -# Unless required by applicable law or agreed to in writing, software distributed under the Apache License 2.0 is -# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations under the License. - import logging -import time -import uuid -from datetime import timedelta +from itertools import groupby from django.conf import settings -from django.db import transaction +from django.db import DatabaseError, connection from django.utils.timezone import now -from pretix.base.models import EventLock +from pretix.base.models import Event, Membership, Quota, Seat, Voucher +from pretix.testutils.middleware import debugflags_var logger = logging.getLogger('pretix.base.locking') -LOCK_TIMEOUT = 120 + +# A lock acquisition is aborted if it takes longer than LOCK_ACQUISITION_TIMEOUT to prevent connection starvation +LOCK_ACQUISITION_TIMEOUT = 3 + +# We make the assumption that it is safe to e.g. transform an order into a cart if the order has a lifetime of more than +# LOCK_TRUST_WINDOW into the future. In other words, we assume that a lock is never held longer than LOCK_TRUST_WINDOW. +# This assumption holds true for all in-request locks, since our gunicorn default settings kill a worker that takes +# longer than 60 seconds to process a request. It however does not hold true for celery tasks, especially long-running +# ones, so this does introduce *some* risk of incorrect locking. +LOCK_TRUST_WINDOW = 120 + +# These are different offsets for the different types of keys we want to lock +KEY_SPACES = { + Event: 1, + Quota: 2, + Seat: 3, + Voucher: 4, + Membership: 5 +} + + +def pg_lock_key(obj): + """ + This maps the primary key space of multiple tables to a single bigint key space within postgres. It is not + an injective function, which is fine, as long as collisions are rare. + """ + keyspace = KEY_SPACES.get(type(obj)) + objectid = obj.pk + if not keyspace: + raise ValueError(f"No key space defined for locking objects of type {type(obj)}") + assert isinstance(objectid, int) + # 64bit int: xxxxxxxx xxxxxxx xxxxxxx xxxxxxx xxxxxx xxxxxxx xxxxxxx xxxxxxx + # | objectid mod 2**48 | |index| |keysp.| + key = ((objectid % 281474976710656) << 16) | ((settings.DATABASE_ADVISORY_LOCK_INDEX % 256) << 8) | (keyspace % 256) + return key + + +class LockTimeoutException(Exception): + pass + + +def lock_objects(objects, *, shared_lock_objects=None, replace_exclusive_with_shared_when_exclusive_are_more_than=20): + """ + Create an exclusive lock on the objects passed in `objects`. This function MUST be called within an atomic + transaction and SHOULD be called only once per transaction to prevent deadlocks. + + A shared lock will be created on objects passed in `shared_lock_objects`. + + If `objects` contains more than `replace_exclusive_with_shared_when_exclusive_are_more_than` objects, `objects` + will be ignored and `shared_lock_objects` will be used in its place and receive an exclusive lock. + + The idea behind it is this: Usually we create a lock on every quota, voucher, or seat contained in an order. + However, this has a large performance penalty in case we have hundreds of locks required. Therefore, we always + place a shared lock in the event, and if we have too many affected objects, we fall back to event-level locks. + """ + if (not objects and not shared_lock_objects) or 'skip-locking' in debugflags_var.get(): + return + + if 'fail-locking' in debugflags_var.get(): + raise LockTimeoutException() + + if not connection.in_atomic_block: + raise RuntimeError( + "You cannot create locks outside of an transaction" + ) + + if 'postgresql' in settings.DATABASES['default']['ENGINE']: + shared_keys = set(pg_lock_key(obj) for obj in shared_lock_objects) if shared_lock_objects else set() + exclusive_keys = set(pg_lock_key(obj) for obj in objects) + if replace_exclusive_with_shared_when_exclusive_are_more_than and len(exclusive_keys) > replace_exclusive_with_shared_when_exclusive_are_more_than: + exclusive_keys = shared_keys + keys = sorted(list(shared_keys | exclusive_keys)) + calls = ", ".join([ + (f"pg_advisory_xact_lock({k})" if k in exclusive_keys else f"pg_advisory_xact_lock_shared({k})") for k in keys + ]) + + try: + with connection.cursor() as cursor: + cursor.execute(f"SET LOCAL lock_timeout = '{LOCK_ACQUISITION_TIMEOUT}s';") + cursor.execute(f"SELECT {calls};") + cursor.execute("SET LOCAL lock_timeout = '0';") # back to default + except DatabaseError as e: + logger.warning(f"Waiting for locks timed out: {e} on SELECT {calls};") + raise LockTimeoutException() + + else: + for model, instances in groupby(objects, key=lambda o: type(o)): + model.objects.select_for_update().filter(pk__in=[o.pk for o in instances]) class NoLockManager: @@ -57,128 +131,3 @@ class NoLockManager: def __exit__(self, exc_type, exc_val, exc_tb): if exc_type is not None: return False - - -class LockManager: - def __init__(self, event): - self.event = event - - def __enter__(self): - lock_event(self.event) - return now() - - def __exit__(self, exc_type, exc_val, exc_tb): - release_event(self.event) - if exc_type is not None: - return False - - -class LockTimeoutException(Exception): - pass - - -class LockReleaseException(LockTimeoutException): - pass - - -def lock_event(event): - """ - Issue a lock on this event so nobody can book tickets for this event until - you release the lock. Will retry 5 times on failure. - - :raises LockTimeoutException: if the event is locked every time we try - to obtain the lock - """ - if hasattr(event, '_lock') and event._lock: - return True - - if settings.HAS_REDIS: - return lock_event_redis(event) - else: - return lock_event_db(event) - - -def release_event(event): - """ - Release a lock placed by :py:meth:`lock()`. If the parameter force is not set to ``True``, - the lock will only be released if it was issued in _this_ python - representation of the database object. - - :raises LockReleaseException: if we do not own the lock - """ - if not hasattr(event, '_lock') or not event._lock: - raise LockReleaseException('Lock is not owned by this thread') - if settings.HAS_REDIS: - return release_event_redis(event) - else: - return release_event_db(event) - - -def lock_event_db(event): - retries = 5 - for i in range(retries): - with transaction.atomic(): - dt = now() - l, created = EventLock.objects.get_or_create(event=event.id) - if created: - event._lock = l - return True - elif l.date < now() - timedelta(seconds=LOCK_TIMEOUT): - newtoken = str(uuid.uuid4()) - updated = EventLock.objects.filter(event=event.id, token=l.token).update(date=dt, token=newtoken) - if updated: - l.token = newtoken - event._lock = l - return True - time.sleep(2 ** i / 100) - raise LockTimeoutException() - - -@transaction.atomic -def release_event_db(event): - if not hasattr(event, '_lock') or not event._lock: - raise LockReleaseException('Lock is not owned by this thread') - try: - lock = EventLock.objects.get(event=event.id, token=event._lock.token) - lock.delete() - event._lock = None - except EventLock.DoesNotExist: - raise LockReleaseException('Lock is no longer owned by this thread') - - -def redis_lock_from_event(event): - from django_redis import get_redis_connection - from redis.lock import Lock - - if not hasattr(event, '_lock') or not event._lock: - rc = get_redis_connection("redis") - event._lock = Lock(redis=rc, name='pretix_event_%s' % event.id, timeout=LOCK_TIMEOUT) - return event._lock - - -def lock_event_redis(event): - from redis.exceptions import RedisError - - lock = redis_lock_from_event(event) - retries = 5 - for i in range(retries): - try: - if lock.acquire(blocking=False): - return True - except RedisError: - logger.exception('Error locking an event') - raise LockTimeoutException() - time.sleep(2 ** i / 100) - raise LockTimeoutException() - - -def release_event_redis(event): - from redis import RedisError - - lock = redis_lock_from_event(event) - try: - lock.release() - except RedisError: - logger.exception('Error releasing an event lock') - raise LockReleaseException() - event._lock = None diff --git a/src/pretix/base/services/orderimport.py b/src/pretix/base/services/orderimport.py index 65773c924..8d74b09d0 100644 --- a/src/pretix/base/services/orderimport.py +++ b/src/pretix/base/services/orderimport.py @@ -36,7 +36,7 @@ from pretix.base.models import ( from pretix.base.models.orders import Transaction from pretix.base.orderimport import get_all_columns from pretix.base.services.invoices import generate_invoice, invoice_qualified -from pretix.base.services.locking import NoLockManager +from pretix.base.services.locking import lock_objects from pretix.base.services.tasks import ProfiledEventTask from pretix.base.signals import order_paid, order_placed from pretix.celery_app import app @@ -88,7 +88,6 @@ def setif(record, obj, attr, setting): def import_orders(event: Event, fileid: str, settings: dict, locale: str, user) -> None: cf = CachedFile.objects.get(id=fileid) user = User.objects.get(pk=user) - seats_used = False with language(locale, event.settings.region): cols = get_all_columns(event) parsed = parse_csv(cf.file) @@ -118,6 +117,7 @@ def import_orders(event: Event, fileid: str, settings: dict, locale: str, user) # Prepare model objects. Yes, this might consume lots of RAM, but allows us to make the actual SQL transaction # shorter. We'll see what works better in reality… + lock_seats = [] for i, record in enumerate(data): try: if order is None or settings['orders'] == 'many': @@ -135,7 +135,7 @@ def import_orders(event: Event, fileid: str, settings: dict, locale: str, user) position.attendee_name_parts = {'_scheme': event.settings.name_scheme} position.meta_info = {} if position.seat is not None: - seats_used = True + lock_seats.append(position.seat) order._positions.append(position) position.assign_pseudonymization_id() @@ -147,12 +147,15 @@ def import_orders(event: Event, fileid: str, settings: dict, locale: str, user) _('Invalid data in row {row}: {message}').format(row=i, message=str(e)) ) - # We don't support vouchers, quotas, or memberships here, so we only need to lock if seats - # are in use - lockfn = event.lock if seats_used else NoLockManager - try: - with lockfn(), transaction.atomic(): + with transaction.atomic(): + # We don't support vouchers, quotas, or memberships here, so we only need to lock if seats are in use + if lock_seats: + lock_objects(lock_seats, shared_lock_objects=[event]) + for s in lock_seats: + if not s.is_available(): + raise ImportError(_('The seat you selected has already been taken. Please select a different seat.')) + save_transactions = [] for o in orders: o.total = sum([c.price for c in o._positions]) # currently no support for fees diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 093eefbd9..11802001a 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -35,10 +35,13 @@ import json import logging +import operator import sys from collections import Counter, defaultdict, namedtuple from datetime import datetime, time, timedelta from decimal import Decimal +from functools import reduce +from time import sleep from typing import List, Optional from celery.exceptions import MaxRetriesExceededError @@ -82,7 +85,9 @@ from pretix.base.services import tickets from pretix.base.services.invoices import ( generate_cancellation, generate_invoice, invoice_qualified, ) -from pretix.base.services.locking import LockTimeoutException, NoLockManager +from pretix.base.services.locking import ( + LOCK_TRUST_WINDOW, LockTimeoutException, lock_objects, +) from pretix.base.services.mail import SendMailException from pretix.base.services.memberships import ( create_membership, validate_memberships_in_order, @@ -102,6 +107,7 @@ from pretix.celery_app import app from pretix.helpers import OF_SELF from pretix.helpers.models import modelcopy from pretix.helpers.periodic import minimum_interval +from pretix.testutils.middleware import debugflags_var class OrderError(Exception): @@ -209,9 +215,9 @@ def reactivate_order(order: Order, force: bool=False, user: User=None, auth=None if order.status != Order.STATUS_CANCELED: raise OrderError(_('The order was not canceled.')) - with order.event.lock() as now_dt: - is_available = order._is_still_available(now_dt, count_waitinglist=False, check_voucher_usage=True, - check_memberships=True, force=force) + with transaction.atomic(): + is_available = order._is_still_available(now(), count_waitinglist=False, check_voucher_usage=True, + check_memberships=True, lock=True, force=force) if is_available is True: if order.payment_refund_sum >= order.total: order.status = Order.STATUS_PAID @@ -220,29 +226,28 @@ def reactivate_order(order: Order, force: bool=False, user: User=None, auth=None order.cancellation_date = None order.set_expires(now(), order.event.subevents.filter(id__in=[p.subevent_id for p in order.positions.all()])) - with transaction.atomic(): - order.save(update_fields=['expires', 'status', 'cancellation_date']) - order.log_action( - 'pretix.event.order.reactivated', - user=user, - auth=auth, - data={ - 'expires': order.expires, - } - ) - for position in order.positions.all(): - if position.voucher: - Voucher.objects.filter(pk=position.voucher.pk).update(redeemed=Greatest(0, F('redeemed') + 1)) + order.save(update_fields=['expires', 'status', 'cancellation_date']) + order.log_action( + 'pretix.event.order.reactivated', + user=user, + auth=auth, + data={ + 'expires': order.expires, + } + ) + for position in order.positions.all(): + if position.voucher: + Voucher.objects.filter(pk=position.voucher.pk).update(redeemed=Greatest(0, F('redeemed') + 1)) - for gc in position.issued_gift_cards.all(): - gc = GiftCard.objects.select_for_update(of=OF_SELF).get(pk=gc.pk) - gc.transactions.create(value=position.price, order=order, acceptor=order.event.organizer) - break + for gc in position.issued_gift_cards.all(): + gc = GiftCard.objects.select_for_update(of=OF_SELF).get(pk=gc.pk) + gc.transactions.create(value=position.price, order=order, acceptor=order.event.organizer) + break - for m in position.granted_memberships.all(): - m.canceled = False - m.save() - order.create_transactions() + for m in position.granted_memberships.all(): + m.canceled = False + m.save() + order.create_transactions() else: raise OrderError(is_available) @@ -264,7 +269,6 @@ def extend_order(order: Order, new_date: datetime, force: bool=False, valid_if_p if new_date < now(): raise OrderError(_('The new expiry date needs to be in the future.')) - @transaction.atomic def change(was_expired=True): old_date = order.expires order.expires = new_date @@ -302,11 +306,11 @@ def extend_order(order: Order, new_date: datetime, force: bool=False, valid_if_p generate_invoice(order) order.create_transactions() - if order.status == Order.STATUS_PENDING: - change(was_expired=False) - else: - with order.event.lock() as now_dt: - is_available = order._is_still_available(now_dt, count_waitinglist=False, force=force) + with transaction.atomic(): + if order.status == Order.STATUS_PENDING: + change(was_expired=False) + else: + is_available = order._is_still_available(now(), count_waitinglist=False, lock=True, force=force) if is_available is True: change(was_expired=True) else: @@ -334,9 +338,8 @@ def mark_order_expired(order, user=None, auth=None): order = Order.objects.get(pk=order) if isinstance(user, int): user = User.objects.get(pk=user) - with order.event.lock(): - order.status = Order.STATUS_EXPIRED - order.save(update_fields=['status']) + order.status = Order.STATUS_EXPIRED + order.save(update_fields=['status']) order.log_action('pretix.event.order.expired', user=user, auth=auth) i = order.invoices.filter(is_cancellation=False).last() @@ -439,9 +442,8 @@ def deny_order(order, comment='', user=None, send_mail: bool=True, auth=None): if not order.require_approval or not order.status == Order.STATUS_PENDING: raise OrderError(_('This order is not pending approval.')) - with order.event.lock(): - order.status = Order.STATUS_CANCELED - order.save(update_fields=['status']) + order.status = Order.STATUS_CANCELED + order.save(update_fields=['status']) order.log_action('pretix.event.order.denied', user=user, auth=auth, data={ 'comment': comment @@ -521,51 +523,49 @@ def _cancel_order(order, user=None, send_mail: bool=True, api_token=None, device m.save() if cancellation_fee: - with order.event.lock(): - for position in order.positions.all(): - if position.voucher: - Voucher.objects.filter(pk=position.voucher.pk).update(redeemed=Greatest(0, F('redeemed') - 1)) - position.canceled = True - assign_ticket_secret( - event=order.event, position=position, force_invalidate_if_revokation_list_used=True, force_invalidate=False, save=False - ) - position.save(update_fields=['canceled', 'secret']) - new_fee = cancellation_fee - for fee in order.fees.all(): - if keep_fees and fee in keep_fees: - new_fee -= fee.value - else: - fee.canceled = True - fee.save(update_fields=['canceled']) - - if new_fee: - f = OrderFee( - fee_type=OrderFee.FEE_TYPE_CANCELLATION, - value=new_fee, - tax_rule=order.event.settings.tax_rate_default, - order=order, - ) - f._calculate_tax() - f.save() - - if cancellation_fee > order.total: - raise OrderError(_('The cancellation fee cannot be higher than the total amount of this order.')) - elif order.payment_refund_sum < cancellation_fee: - order.status = Order.STATUS_PENDING - order.set_expires() + for position in order.positions.all(): + if position.voucher: + Voucher.objects.filter(pk=position.voucher.pk).update(redeemed=Greatest(0, F('redeemed') - 1)) + position.canceled = True + assign_ticket_secret( + event=order.event, position=position, force_invalidate_if_revokation_list_used=True, force_invalidate=False, save=False + ) + position.save(update_fields=['canceled', 'secret']) + new_fee = cancellation_fee + for fee in order.fees.all(): + if keep_fees and fee in keep_fees: + new_fee -= fee.value else: - order.status = Order.STATUS_PAID - order.total = cancellation_fee - order.cancellation_date = now() - order.save(update_fields=['status', 'cancellation_date', 'total']) + fee.canceled = True + fee.save(update_fields=['canceled']) + + if new_fee: + f = OrderFee( + fee_type=OrderFee.FEE_TYPE_CANCELLATION, + value=new_fee, + tax_rule=order.event.settings.tax_rate_default, + order=order, + ) + f._calculate_tax() + f.save() + + if cancellation_fee > order.total: + raise OrderError(_('The cancellation fee cannot be higher than the total amount of this order.')) + elif order.payment_refund_sum < cancellation_fee: + order.status = Order.STATUS_PENDING + order.set_expires() + else: + order.status = Order.STATUS_PAID + order.total = cancellation_fee + order.cancellation_date = now() + order.save(update_fields=['status', 'cancellation_date', 'total']) if cancel_invoice and i: invoices.append(generate_invoice(order)) else: - with order.event.lock(): - order.status = Order.STATUS_CANCELED - order.cancellation_date = now() - order.save(update_fields=['status', 'cancellation_date']) + order.status = Order.STATUS_CANCELED + order.cancellation_date = now() + order.save(update_fields=['status', 'cancellation_date']) for position in order.positions.all(): assign_ticket_secret( @@ -666,6 +666,27 @@ def _check_positions(event: Event, now_dt: datetime, positions: List[CartPositio sorted_positions = sorted(positions, key=lambda s: -int(s.is_bundled)) + for cp in sorted_positions: + cp._cached_quotas = list(cp.quotas) + + # Create locks + if any(cp.expires < now() + timedelta(seconds=LOCK_TRUST_WINDOW) for cp in sorted_positions): + # No need to perform any locking if the cart positions still guarantee everything long enough. + full_lock_required = any( + getattr(o, 'seat', False) for o in sorted_positions + ) and event.settings.seating_minimal_distance > 0 + if full_lock_required: + # We lock the entire event in this case since we don't want to deal with fine-granular locking + # in the case of seating distance enforcement + lock_objects([event]) + else: + lock_objects( + [q for q in reduce(operator.or_, (set(cp._cached_quotas) for cp in sorted_positions), set()) if q.size is not None] + + [op.voucher for op in sorted_positions if op.voucher] + + [op.seat for op in sorted_positions if op.seat], + shared_lock_objects=[event] + ) + # Check availability for i, cp in enumerate(sorted_positions): if cp.pk in deleted_positions: @@ -675,7 +696,7 @@ def _check_positions(event: Event, now_dt: datetime, positions: List[CartPositio err = err or error_messages['unavailable'] delete(cp) continue - quotas = list(cp.quotas) + quotas = cp._cached_quotas products_seen[cp.item] += 1 if cp.item.max_per_order and products_seen[cp.item] > cp.item.max_per_order: @@ -689,6 +710,7 @@ def _check_positions(event: Event, now_dt: datetime, positions: List[CartPositio if cp.voucher: v_usages[cp.voucher] += 1 if cp.voucher not in v_avail: + cp.voucher.refresh_from_db(fields=['redeemed']) redeemed_in_carts = CartPosition.objects.filter( Q(voucher=cp.voucher) & Q(event=event) & Q(expires__gte=now_dt) ).exclude(cart_id=cp.cart_id) @@ -927,91 +949,87 @@ def _create_order(event: Event, email: str, positions: List[CartPosition], now_d payments = [] sales_channel = get_all_sales_channels()[sales_channel] - with transaction.atomic(): + try: + validate_memberships_in_order(customer, positions, event, lock=True, testmode=event.testmode) + except ValidationError as e: + raise OrderError(e.message) - try: - validate_memberships_in_order(customer, positions, event, lock=True, testmode=event.testmode) - except ValidationError as e: - raise OrderError(e.message) + require_approval = any(p.requires_approval(invoice_address=address) for p in positions) + try: + fees = _get_fees(positions, payment_requests, address, meta_info, event, require_approval=require_approval) + except TaxRule.SaleNotAllowed: + raise OrderError(error_messages['country_blocked']) + total = pending_sum = sum([c.price for c in positions]) + sum([c.value for c in fees]) - require_approval = any(p.requires_approval(invoice_address=address) for p in positions) + order = Order( + status=Order.STATUS_PENDING, + event=event, + email=email, + phone=(meta_info or {}).get('contact_form_data', {}).get('phone'), + datetime=now_dt, + locale=get_language_without_region(locale), + total=total, + testmode=True if sales_channel.testmode_supported and event.testmode else False, + meta_info=json.dumps(meta_info or {}), + require_approval=require_approval, + sales_channel=sales_channel.identifier, + customer=customer, + valid_if_pending=valid_if_pending, + ) + if customer: + order.email_known_to_work = customer.is_verified + order.set_expires(now_dt, event.subevents.filter(id__in=[p.subevent_id for p in positions])) + order.save() + + if address: + if address.order is not None: + address.pk = None + address.order = order + address.save() + + for fee in fees: + fee.order = order try: - fees = _get_fees(positions, payment_requests, address, meta_info, event, require_approval=require_approval) + fee._calculate_tax() except TaxRule.SaleNotAllowed: raise OrderError(error_messages['country_blocked']) - total = pending_sum = sum([c.price for c in positions]) + sum([c.value for c in fees]) + if fee.tax_rule and not fee.tax_rule.pk: + fee.tax_rule = None # TODO: deprecate + fee.save() - order = Order( - status=Order.STATUS_PENDING, - event=event, - email=email, - phone=(meta_info or {}).get('contact_form_data', {}).get('phone'), - datetime=now_dt, - locale=get_language_without_region(locale), - total=total, - testmode=True if sales_channel.testmode_supported and event.testmode else False, - meta_info=json.dumps(meta_info or {}), - require_approval=require_approval, - sales_channel=sales_channel.identifier, - customer=customer, - valid_if_pending=valid_if_pending, - ) - if customer: - order.email_known_to_work = customer.is_verified - order.set_expires(now_dt, event.subevents.filter(id__in=[p.subevent_id for p in positions])) - order.save() + # Safety check: Is the amount we're now going to charge the same amount the user has been shown when they + # pressed "Confirm purchase"? If not, we should better warn the user and show the confirmation page again. + # We used to have a *known* case where this happened is if a gift card is used in two concurrent sessions, + # but this is now a payment error instead. So currently this code branch is usually only triggered by bugs + # in other places (e.g. tax calculation). + if shown_total is not None: + if Decimal(shown_total) != pending_sum: + raise OrderError( + _('While trying to place your order, we noticed that the order total has changed. Either one of ' + 'the prices changed just now, or a gift card you used has been used in the meantime. Please ' + 'check the prices below and try again.') + ) - if address: - if address.order is not None: - address.pk = None - address.order = order - address.save() + if payment_requests and not order.require_approval: + for p in payment_requests: + if not p.get('multi_use_supported') or p['payment_amount'] > Decimal('0.00'): + payments.append(order.payments.create( + state=OrderPayment.PAYMENT_STATE_CREATED, + provider=p['provider'], + amount=p['payment_amount'], + fee=p.get('fee'), + info=json.dumps(p['info_data']), + process_initiated=False, + )) - order.save() - - for fee in fees: - fee.order = order - try: - fee._calculate_tax() - except TaxRule.SaleNotAllowed: - raise OrderError(error_messages['country_blocked']) - if fee.tax_rule and not fee.tax_rule.pk: - fee.tax_rule = None # TODO: deprecate - fee.save() - - # Safety check: Is the amount we're now going to charge the same amount the user has been shown when they - # pressed "Confirm purchase"? If not, we should better warn the user and show the confirmation page again. - # We used to have a *known* case where this happened is if a gift card is used in two concurrent sessions, - # but this is now a payment error instead. So currently this code branch is usually only triggered by bugs - # in other places (e.g. tax calculation). - if shown_total is not None: - if Decimal(shown_total) != pending_sum: - raise OrderError( - _('While trying to place your order, we noticed that the order total has changed. Either one of ' - 'the prices changed just now, or a gift card you used has been used in the meantime. Please ' - 'check the prices below and try again.') - ) - - if payment_requests and not order.require_approval: - for p in payment_requests: - if not p.get('multi_use_supported') or p['payment_amount'] > Decimal('0.00'): - payments.append(order.payments.create( - state=OrderPayment.PAYMENT_STATE_CREATED, - provider=p['provider'], - amount=p['payment_amount'], - fee=p.get('fee'), - info=json.dumps(p['info_data']), - process_initiated=False, - )) - - orderpositions = OrderPosition.transform_cart_positions(positions, order) - order.create_transactions(positions=orderpositions, fees=fees, is_new=True) - order.log_action('pretix.event.order.placed') - if order.require_approval: - order.log_action('pretix.event.order.placed.require_approval') - if meta_info: - for msg in meta_info.get('confirm_messages', []): - order.log_action('pretix.event.order.consent', data={'msg': msg}) + orderpositions = OrderPosition.transform_cart_positions(positions, order) + order.create_transactions(positions=orderpositions, fees=fees, is_new=True) + order.log_action('pretix.event.order.placed') + if order.require_approval: + order.log_action('pretix.event.order.placed.require_approval') + if meta_info: + for msg in meta_info.get('confirm_messages', []): + order.log_action('pretix.event.order.consent', data={'msg': msg}) order_placed.send(event, order=order) return order, payments @@ -1116,18 +1134,12 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis if result: valid_if_pending = True - lockfn = NoLockManager - locked = False - if positions.filter(Q(voucher__isnull=False) | Q(expires__lt=now() + timedelta(minutes=2)) | Q(seat__isnull=False)).exists(): - # Performance optimization: If no voucher is used and no cart position is dangerously close to its expiry date, - # creating this order shouldn't be prone to any race conditions and we don't need to lock the event. - locked = True - lockfn = event.lock - warnings = [] any_payment_failed = False - with lockfn() as now_dt: + now_dt = now() + err_out = None + with transaction.atomic(durable=True): positions = list( positions.select_related('item', 'variation', 'subevent', 'seat', 'addon_to').prefetch_related('addons') ) @@ -1136,16 +1148,28 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis raise OrderError(error_messages['empty']) if len(position_ids) != len(positions): raise OrderError(error_messages['internal']) - _check_positions(event, now_dt, positions, address=addr, sales_channel=sales_channel, customer=customer) - order, payment_objs = _create_order(event, email, positions, now_dt, payment_requests, - locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel, - shown_total=shown_total, customer=customer, valid_if_pending=valid_if_pending) try: - for p in payment_objs: - if p.provider == 'free': - p.confirm(send_mail=False, lock=not locked, generate_invoice=False) - except Quota.QuotaExceededException: - pass + _check_positions(event, now_dt, positions, address=addr, sales_channel=sales_channel, customer=customer) + except OrderError as e: + err_out = e # Don't raise directly to make sure transaction is committed, as it might have deleted things + else: + if 'sleep-after-quota-check' in debugflags_var.get(): + sleep(2) + + order, payment_objs = _create_order(event, email, positions, now_dt, payment_requests, + locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel, + shown_total=shown_total, customer=customer, valid_if_pending=valid_if_pending) + + try: + for p in payment_objs: + if p.provider == 'free': + # Passing lock=False is safe here because it's absolutely impossible for the order to be expired + # here before it is even committed. + p.confirm(send_mail=False, lock=False, generate_invoice=False) + except Quota.QuotaExceededException: + pass + if err_out: + raise err_out # We give special treatment to GiftCardPayment here because our invoice renderer expects gift cards to already be # processed, and because we historically treat gift card orders like free orders with regards to email texts. @@ -2658,6 +2682,19 @@ class OrderChangeManager: except ValidationError as e: raise OrderError(e.message) + def _create_locks(self): + full_lock_required = any(diff > 0 for diff in self._seatdiff.values()) and self.event.settings.seating_minimal_distance > 0 + if full_lock_required: + # We lock the entire event in this case since we don't want to deal with fine-granular locking + # in the case of seating distance enforcement + lock_objects([self.event]) + else: + lock_objects( + [q for q, d in self._quotadiff.items() if q.size is not None and d > 0] + + [s for s, d in self._seatdiff.items() if d > 0], + shared_lock_objects=[self.event] + ) + def commit(self, check_quotas=True): if self._committed: # an order change can only be committed once @@ -2677,17 +2714,17 @@ class OrderChangeManager: self._payment_fee_diff() with transaction.atomic(): - with self.order.event.lock(): - if self.order.status in (Order.STATUS_PENDING, Order.STATUS_PAID): - if check_quotas: - self._check_quotas() - self._check_seats() - self._check_complete_cancel() - self._check_and_lock_memberships() - try: - self._perform_operations() - except TaxRule.SaleNotAllowed: - raise OrderError(self.error_messages['tax_rule_country_blocked']) + if self.order.status in (Order.STATUS_PENDING, Order.STATUS_PAID): + if check_quotas: + self._check_quotas() + self._check_seats() + self._create_locks() + self._check_complete_cancel() + self._check_and_lock_memberships() + try: + self._perform_operations() + except TaxRule.SaleNotAllowed: + raise OrderError(self.error_messages['tax_rule_country_blocked']) self._recalculate_total_and_payment_fee() self._check_paid_price_change() self._check_paid_to_free() diff --git a/src/pretix/base/services/quotas.py b/src/pretix/base/services/quotas.py index cd1e2dfe8..cc454a235 100644 --- a/src/pretix/base/services/quotas.py +++ b/src/pretix/base/services/quotas.py @@ -191,7 +191,7 @@ class QuotaAvailability: update[q.event_id].append(q) for eventid, quotas in update.items(): - rc.hmset(f'quotas:{eventid}:availabilitycache{self._cache_key_suffix}', { + rc.hset(f'quotas:{eventid}:availabilitycache{self._cache_key_suffix}', mapping={ str(q.id): ",".join( [str(i) for i in self.results[q]] + [str(int(time.time()))] diff --git a/src/pretix/base/services/waitinglist.py b/src/pretix/base/services/waitinglist.py index c9677e3f0..ab95f4c94 100644 --- a/src/pretix/base/services/waitinglist.py +++ b/src/pretix/base/services/waitinglist.py @@ -22,6 +22,7 @@ import sys from datetime import timedelta +from django.db import transaction from django.db.models import ( Exists, F, OuterRef, Prefetch, Q, Sum, prefetch_related_objects, ) @@ -33,6 +34,7 @@ from pretix.base.models import ( Event, EventMetaValue, SeatCategoryMapping, User, WaitingListEntry, ) from pretix.base.models.waitinglist import WaitingListException +from pretix.base.services.locking import lock_objects from pretix.base.services.tasks import EventTask from pretix.base.signals import periodic_task from pretix.celery_app import app @@ -86,11 +88,23 @@ def assign_automatically(event: Event, user_id: int=None, subevent_id: int=None) sent = 0 - with event.lock(): + with transaction.atomic(durable=True): + quotas_by_item = {} + quotas = set() + for wle in qs: + if (wle.item_id, wle.variation_id, wle.subevent_id) not in quotas_by_item: + quotas_by_item[wle.item_id, wle.variation_id, wle.subevent_id] = list( + wle.variation.quotas.filter(subevent=wle.subevent) + if wle.variation + else wle.item.quotas.filter(subevent=wle.subevent) + ) + wle._quotas = quotas_by_item[wle.item_id, wle.variation_id, wle.subevent_id] + quotas |= set(wle._quotas) + + lock_objects(quotas, shared_lock_objects=[event]) for wle in qs: if (wle.item, wle.variation, wle.subevent) in gone: continue - ev = (wle.subevent or event) if not ev.presale_is_running or (wle.subevent and not wle.subevent.active): continue @@ -105,9 +119,6 @@ def assign_automatically(event: Event, user_id: int=None, subevent_id: int=None) gone.add((wle.item, wle.variation, wle.subevent)) continue - quotas = (wle.variation.quotas.filter(subevent=wle.subevent) - if wle.variation - else wle.item.quotas.filter(subevent=wle.subevent)) availability = ( wle.variation.check_quotas(count_waitinglist=False, _cache=quota_cache, subevent=wle.subevent) if wle.variation @@ -121,7 +132,7 @@ def assign_automatically(event: Event, user_id: int=None, subevent_id: int=None) continue # Reduce affected quotas in cache - for q in quotas: + for q in wle._quotas: quota_cache[q.pk] = ( quota_cache[q.pk][0] if quota_cache[q.pk][0] > 1 else 0, quota_cache[q.pk][1] - 1 if quota_cache[q.pk][1] is not None else sys.maxsize diff --git a/src/pretix/base/views/tasks.py b/src/pretix/base/views/tasks.py index 4b74cdb39..4e4f28389 100644 --- a/src/pretix/base/views/tasks.py +++ b/src/pretix/base/views/tasks.py @@ -29,6 +29,7 @@ from celery.result import AsyncResult from django.conf import settings from django.contrib import messages from django.core.exceptions import PermissionDenied, ValidationError +from django.db import transaction from django.http import HttpResponse, JsonResponse, QueryDict from django.shortcuts import redirect, render from django.test import RequestFactory @@ -217,6 +218,7 @@ class AsyncFormView(AsyncMixin, FormView): known_errortypes = ['ValidationError'] expected_exceptions = (ValidationError,) task_base = ProfiledEventTask + atomic_execute = False def async_set_progress(self, percentage): if not self._task_self.request.called_directly: @@ -263,6 +265,9 @@ class AsyncFormView(AsyncMixin, FormView): form.is_valid() return view_instance.async_form_valid(self, form) + if cls.atomic_execute: + async_execute = transaction.atomic(async_execute) + cls.async_execute = app.task( base=cls.task_base, bind=True, diff --git a/src/pretix/control/views/vouchers.py b/src/pretix/control/views/vouchers.py index 1574a5558..0dd3c0e7c 100644 --- a/src/pretix/control/views/vouchers.py +++ b/src/pretix/control/views/vouchers.py @@ -64,7 +64,6 @@ from pretix.base.models import ( CartPosition, LogEntry, Voucher, WaitingListEntry, ) from pretix.base.models.vouchers import generate_codes -from pretix.base.services.locking import NoLockManager from pretix.base.services.vouchers import vouchers_send from pretix.base.templatetags.rich_text import markdown_compile_email from pretix.base.views.tasks import AsyncFormView @@ -293,7 +292,6 @@ class VoucherUpdate(EventPermissionRequiredMixin, UpdateView): except Voucher.DoesNotExist: raise Http404(_("The requested voucher does not exist.")) - @transaction.atomic def form_valid(self, form): messages.success(self.request, _('Your changes have been saved.')) if form.has_changed(): @@ -304,6 +302,10 @@ class VoucherUpdate(EventPermissionRequiredMixin, UpdateView): ) return super().form_valid(form) + @transaction.atomic + def post(self, request, *args, **kwargs): + return super().post(request, *args, **kwargs) + def get_success_url(self) -> str: return reverse('control:event.vouchers', kwargs={ 'organizer': self.request.event.organizer.slug, @@ -355,7 +357,6 @@ class VoucherCreate(EventPermissionRequiredMixin, CreateView): kwargs['instance'] = Voucher(event=self.request.event) return kwargs - @transaction.atomic def form_valid(self, form): form.instance.event = self.request.event ret = super().form_valid(form) @@ -370,10 +371,9 @@ class VoucherCreate(EventPermissionRequiredMixin, CreateView): form.instance.log_action('pretix.voucher.added', data=dict(form.cleaned_data), user=self.request.user) return ret + @transaction.atomic def post(self, request, *args, **kwargs): - # TODO: Transform this into an asynchronous call? - with request.event.lock(): - return super().post(request, *args, **kwargs) + return super().post(request, *args, **kwargs) class VoucherGo(EventPermissionRequiredMixin, View): @@ -398,6 +398,7 @@ class VoucherBulkCreate(EventPermissionRequiredMixin, AsyncFormView): template_name = 'pretixcontrol/vouchers/bulk.html' permission = 'can_change_vouchers' context_object_name = 'voucher' + atomic_execute = True def get_success_url(self, value) -> str: return reverse('control:event.vouchers', kwargs={ @@ -437,9 +438,6 @@ class VoucherBulkCreate(EventPermissionRequiredMixin, AsyncFormView): return form_kwargs def async_form_valid(self, task, form): - lockfn = NoLockManager - if form.data.get('block_quota'): - lockfn = self.request.event.lock batch_size = 500 total_num = 1 # will be set later @@ -473,27 +471,26 @@ class VoucherBulkCreate(EventPermissionRequiredMixin, AsyncFormView): set_progress(len(voucherids) / total_num * (50. if form.cleaned_data['send'] else 100.)) voucherids = [] - with lockfn(), transaction.atomic(): - if not form.is_valid(): - raise ValidationError(form.errors) - total_num = len(form.cleaned_data['codes']) + if not form.is_valid(): + raise ValidationError(form.errors) + total_num = len(form.cleaned_data['codes']) - batch_vouchers = [] - for code in form.cleaned_data['codes']: - if len(batch_vouchers) >= batch_size: - process_batch(batch_vouchers, voucherids) + batch_vouchers = [] + for code in form.cleaned_data['codes']: + if len(batch_vouchers) >= batch_size: + process_batch(batch_vouchers, voucherids) - obj = modelcopy(form.instance, code=None) - obj.event = self.request.event - obj.code = code - try: - obj.seat = form.cleaned_data['seats'].pop() - obj.item = obj.seat.product - except IndexError: - pass - batch_vouchers.append(obj) + obj = modelcopy(form.instance, code=None) + obj.event = self.request.event + obj.code = code + try: + obj.seat = form.cleaned_data['seats'].pop() + obj.item = obj.seat.product + except IndexError: + pass + batch_vouchers.append(obj) - process_batch(batch_vouchers, voucherids) + process_batch(batch_vouchers, voucherids) if form.cleaned_data['send']: vouchers_send( @@ -525,6 +522,10 @@ class VoucherBulkCreate(EventPermissionRequiredMixin, AsyncFormView): messages.error(self.request, _('We could not save your changes. See below for details.')) return super().form_invalid(form) + @transaction.atomic() + def post(self, request, *args, **kwargs): + return super().post(request, *args, **kwargs) + class VoucherBulkMailPreview(EventPermissionRequiredMixin, View): permission = 'can_change_vouchers' diff --git a/src/pretix/settings.py b/src/pretix/settings.py index a0123c9f5..c12fd68c4 100644 --- a/src/pretix/settings.py +++ b/src/pretix/settings.py @@ -114,6 +114,8 @@ elif 'mysql' in db_backend: print("pretix does no longer support running on MySQL/MariaDB") sys.exit(1) +DATABASE_ADVISORY_LOCK_INDEX = config.getint('database', 'advisory_lock_index', fallback=0) + db_options = {} postgresql_sslmode = config.get('database', 'sslmode', fallback='disable') diff --git a/src/pretix/testutils/middleware.py b/src/pretix/testutils/middleware.py new file mode 100644 index 000000000..9fe07857c --- /dev/null +++ b/src/pretix/testutils/middleware.py @@ -0,0 +1,58 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import contextvars + +from django.conf import settings +from django.db import connection + +debugflags_var = contextvars.ContextVar('debugflags', default=frozenset()) + + +class DebugFlagMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + if '_debug_flag' in request.GET: + debugflags_var.set(frozenset(request.GET.getlist('_debug_flag'))) + else: + debugflags_var.set(frozenset()) + + if 'skip-csrf' in debugflags_var.get(): + request.csrf_processing_done = True + + if 'repeatable-read' in debugflags_var.get(): + with connection.cursor() as cursor: + if 'postgresql' in settings.DATABASES['default']['ENGINE']: + cursor.execute('SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ;') + elif 'mysql' in settings.DATABASES['default']['ENGINE']: + cursor.execute('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;') + + try: + return self.get_response(request) + finally: + if 'repeatable-read' in debugflags_var.get(): + with connection.cursor() as cursor: + if 'postgresql' in settings.DATABASES['default']['ENGINE']: + cursor.execute('SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED;') + elif 'mysql' in settings.DATABASES['default']['ENGINE']: + cursor.execute('SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;') diff --git a/src/pretix/testutils/settings.py b/src/pretix/testutils/settings.py index 09924d52d..daeb557ec 100644 --- a/src/pretix/testutils/settings.py +++ b/src/pretix/testutils/settings.py @@ -70,16 +70,22 @@ CELERY_TASK_ALWAYS_EAGER = True # Don't use redis SESSION_ENGINE = "django.contrib.sessions.backends.db" HAS_REDIS = False +ORIGINAL_CACHES = CACHES CACHES = { 'default': { 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', } } + +# Set databases DATABASE_REPLICA = 'default' +DATABASES['default']['CONN_MAX_AGE'] = 0 +DATABASES.pop('replica', None) + +MIDDLEWARE.insert(0, 'pretix.testutils.middleware.DebugFlagMiddleware') + # Don't run migrations - - class DisableMigrations(object): def __contains__(self, item): diff --git a/src/setup.cfg b/src/setup.cfg index e7851bdab..3023ba9cf 100644 --- a/src/setup.cfg +++ b/src/setup.cfg @@ -39,7 +39,10 @@ filterwarnings = ignore::DeprecationWarning:compressor ignore:.*FakeStrictRedis.hmset.*:DeprecationWarning: ignore:pkg_resources is deprecated as an API: - ignore:.*pkg_resources.declare_namespace.*: + ignore:.*declare_namespace.*: + ignore:.*PdfMerger.*: + ignore:teardown:pytest.PytestWarning + ignore:avoid running initialization queries:RuntimeWarning [coverage:run] diff --git a/src/tests/api/test_idempotency.py b/src/tests/api/test_idempotency.py index 9f961184e..e5602599e 100644 --- a/src/tests/api/test_idempotency.py +++ b/src/tests/api/test_idempotency.py @@ -168,15 +168,14 @@ def order(event): def test_allow_retry_409(token_client, organizer, event, order): order.status = Order.STATUS_EXPIRED order.save() - with event.lock(): - resp = token_client.post( - '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/'.format( - organizer.slug, event.slug, order.code - ), format='json', HTTP_X_IDEMPOTENCY_KEY='foo' - ) - assert resp.status_code == 409 - order.refresh_from_db() - assert order.status == Order.STATUS_EXPIRED + resp = token_client.post( + '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/?_debug_flag=fail-locking'.format( + organizer.slug, event.slug, order.code + ), format='json', HTTP_X_IDEMPOTENCY_KEY='foo' + ) + assert resp.status_code == 409 + order.refresh_from_db() + assert order.status == Order.STATUS_EXPIRED resp = token_client.post( '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/'.format( organizer.slug, event.slug, order.code diff --git a/src/tests/api/test_orders.py b/src/tests/api/test_orders.py index 488274e6f..3e0dca4b5 100644 --- a/src/tests/api/test_orders.py +++ b/src/tests/api/test_orders.py @@ -1188,15 +1188,14 @@ def test_order_mark_paid_expired_quota_fill(token_client, organizer, event, orde def test_order_mark_paid_locked(token_client, organizer, event, order): order.status = Order.STATUS_EXPIRED order.save() - with event.lock(): - resp = token_client.post( - '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/'.format( - organizer.slug, event.slug, order.code - ) + resp = token_client.post( + '/api/v1/organizers/{}/events/{}/orders/{}/mark_paid/?_debug_flag=fail-locking'.format( + organizer.slug, event.slug, order.code ) - assert resp.status_code == 409 - order.refresh_from_db() - assert order.status == Order.STATUS_EXPIRED + ) + assert resp.status_code == 409 + order.refresh_from_db() + assert order.status == Order.STATUS_EXPIRED @pytest.mark.django_db diff --git a/src/tests/base/test_locking.py b/src/tests/base/test_locking.py deleted file mode 100644 index 8083fc367..000000000 --- a/src/tests/base/test_locking.py +++ /dev/null @@ -1,77 +0,0 @@ -# -# This file is part of pretix (Community Edition). -# -# Copyright (C) 2014-2020 Raphael Michel and contributors -# Copyright (C) 2020-2021 rami.io GmbH and contributors -# -# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General -# Public License as published by the Free Software Foundation in version 3 of the License. -# -# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are -# applicable granting you additional permissions and placing additional restrictions on your usage of this software. -# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive -# this file, see . -# -# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied -# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more -# details. -# -# You should have received a copy of the GNU Affero General Public License along with this program. If not, see -# . -# -import time - -import pytest -from django.utils.timezone import now -from django_scopes import scope, scopes_disabled - -from pretix.base.models import Event, Organizer -from pretix.base.services import locking -from pretix.base.services.locking import ( - LockReleaseException, LockTimeoutException, -) - - -@pytest.fixture -def event(): - o = Organizer.objects.create(name='Dummy', slug='dummy') - event = Event.objects.create( - organizer=o, name='Dummy', slug='dummy', - date_from=now() - ) - with scope(organizer=o): - yield event - - -@pytest.mark.django_db -def test_locking_exclusive(event): - with event.lock(): - with pytest.raises(LockTimeoutException): - with scopes_disabled(): - ev = Event.objects.get(id=event.id) - with ev.lock(): - pass - - -@pytest.mark.django_db -def test_locking_different_events(event): - other = Event.objects.create( - organizer=event.organizer, name='Dummy', slug='dummy2', - date_from=now() - ) - with event.lock(): - with other.lock(): - pass - - -@pytest.mark.django_db -def test_lock_timeout_steal(event): - locking.LOCK_TIMEOUT = 1 - locking.lock_event(event) - with pytest.raises(LockTimeoutException): - ev = Event.objects.get(id=event.id) - locking.lock_event(ev) - time.sleep(1.5) - locking.lock_event(ev) - with pytest.raises(LockReleaseException): - locking.release_event(event) diff --git a/src/tests/concurrency_tests/__init__.py b/src/tests/concurrency_tests/__init__.py new file mode 100644 index 000000000..9fd5bdc50 --- /dev/null +++ b/src/tests/concurrency_tests/__init__.py @@ -0,0 +1,21 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# diff --git a/src/tests/concurrency_tests/conftest.py b/src/tests/concurrency_tests/conftest.py new file mode 100644 index 000000000..9983b60fc --- /dev/null +++ b/src/tests/concurrency_tests/conftest.py @@ -0,0 +1,144 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +from datetime import datetime, timedelta + +import aiohttp +import pytest +import pytest_asyncio +from django.utils.timezone import now +from django_scopes import scopes_disabled +from pytz import UTC + +from pretix.base.models import ( + Device, Event, Item, Organizer, Quota, SeatingPlan, +) +from pretix.base.models.devices import generate_api_token + + +@pytest.fixture(autouse=True) +def autoskip(request, settings): + if 'sqlite3' in settings.DATABASES['default']['ENGINE']: + pytest.skip("cannot be run on sqlite") + if not request.config.getvalue("reuse_db"): + pytest.skip("only works with --reuse-db due to some weird connection handling bug") + + +@pytest.fixture +@scopes_disabled() +def organizer(): + return Organizer.objects.create(name='Dummy', slug='dummy') + + +@pytest.fixture +@scopes_disabled() +def event(organizer): + e = Event.objects.create( + organizer=organizer, name='Dummy', slug='dummy', + date_from=datetime(2017, 12, 27, 10, 0, 0, tzinfo=UTC), + presale_end=now() + timedelta(days=300), + plugins='pretix.plugins.banktransfer,pretix.plugins.ticketoutputpdf', + is_public=True, live=True + ) + e.item_meta_properties.create(name="day", default="Monday") + e.settings.timezone = 'Europe/Berlin' + e.settings.payment_banktransfer__enabled = True + return e + + +@pytest.fixture +@scopes_disabled() +def item(event): + return Item.objects.create( + event=event, + name='Regular ticket', + default_price=0, + ) + + +@pytest.fixture +@scopes_disabled() +def quota(event, item): + q = Quota.objects.create( + event=event, + size=10, + name='Regular tickets' + ) + q.items.add(item) + return q + + +@pytest.fixture +@scopes_disabled() +def voucher(event, item): + return event.vouchers.create(code="Foo", max_usages=1) + + +@pytest.fixture +@scopes_disabled() +def membership_type(event): + return event.organizer.membership_types.create(name="foo", allow_parallel_usage=False) + + +@pytest.fixture +@scopes_disabled() +def customer(event, membership_type): + return event.organizer.customers.create(email="admin@localhost", is_active=True, is_verified=True) + + +@pytest.fixture +@scopes_disabled() +def membership(event, membership_type, customer): + return customer.memberships.create( + membership_type=membership_type, + date_start=datetime(2017, 1, 1, 0, 0, tzinfo=UTC), + date_end=datetime(2099, 1, 1, 0, 0, tzinfo=UTC), + ) + + +@pytest.fixture +@scopes_disabled() +def seat(event, organizer, item): + SeatingPlan.objects.create( + name="Plan", organizer=organizer, layout="{}" + ) + event.seat_category_mappings.create( + layout_category='Stalls', product=item + ) + return event.seats.create(seat_number="A1", product=item, seat_guid="A1") + + +@pytest.fixture +@scopes_disabled() +def device(organizer): + return Device.objects.create( + organizer=organizer, + all_events=True, + name='Foo', + initialized=now(), + api_token=generate_api_token() + ) + + +@pytest_asyncio.fixture +async def session(live_server, event): + async with aiohttp.ClientSession() as session: + yield session diff --git a/src/tests/concurrency_tests/test_cart_creation_locking.py b/src/tests/concurrency_tests/test_cart_creation_locking.py new file mode 100644 index 000000000..a18f75348 --- /dev/null +++ b/src/tests/concurrency_tests/test_cart_creation_locking.py @@ -0,0 +1,140 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import asyncio + +import pytest +from asgiref.sync import sync_to_async +from django_scopes import scopes_disabled +from tests.concurrency_tests.utils import post + +from pretix.base.models import CartPosition + + +@pytest.mark.asyncio +async def test_quota_race_condition_happens_if_we_disable_locks(live_server, session, event, item, quota): + # This test exists to ensure that our test setup makes sense. If it fails, all tests down below + # might be useless. + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=skip-locking&_debug_flag=sleep-after-quota-check" + payload = { + f'item_{item.pk}': '1', + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 2 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 2 + + +@pytest.mark.asyncio +async def test_cart_race_condition_prevented_by_locks(live_server, session, event, item, quota): + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = { + f'item_{item.pk}': '1', + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 1 + + +@pytest.mark.asyncio +async def test_cart_race_condition_possible_in_repeatable_read(live_server, session, event, item, quota): + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=repeatable-read&_debug_flag=sleep-before-commit" + payload = { + f'item_{item.pk}': '1', + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 2 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 2 + + +@pytest.mark.asyncio +async def test_cart_race_condition_prevented_by_read_committed(live_server, session, event, item, quota): + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=sleep-before-commit" + payload = { + f'item_{item.pk}': '1', + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 1 + + +@pytest.mark.asyncio +async def test_cart_voucher_race_condition_prevented_by_locks(live_server, session, event, item, quota, voucher): + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = { + f'item_{item.pk}': '1', + '_voucher_code': voucher.code, + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item, voucher=voucher).count)() == 1 + + +@pytest.mark.asyncio +async def test_cart_seat_race_condition_prevented_by_locks(live_server, session, event, item, quota, seat): + url = f"/{event.organizer.slug}/{event.slug}/cart/add?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = { + f'seat_{item.pk}': seat.seat_guid, + } + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload), + post(session, f"{live_server}{url}", data=payload) + ) + assert ['alert-success' in r1, 'alert-success' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item, seat=seat).count)() == 1 diff --git a/src/tests/concurrency_tests/test_order_creation_locking.py b/src/tests/concurrency_tests/test_order_creation_locking.py new file mode 100644 index 000000000..f67f4ed21 --- /dev/null +++ b/src/tests/concurrency_tests/test_order_creation_locking.py @@ -0,0 +1,189 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import asyncio +from datetime import timedelta +from importlib import import_module + +import pytest +from asgiref.sync import sync_to_async +from django.conf import settings +from django.utils.timezone import now +from django_scopes import scopes_disabled +from tests.concurrency_tests.utils import post + +from pretix.base.models import CartPosition, OrderPosition + +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore + + +@pytest.fixture +@scopes_disabled() +def cart1_expired(event, organizer, item, customer): + cp = CartPosition.objects.create( + event=event, + item=item, + datetime=now(), + expires=now() - timedelta(days=1), + price=item.default_price, + cart_id="cart1" + ) + session = SessionStore("cart1") + session['current_cart_event_{}'.format(event.pk)] = "cart1" + session['carts'] = { + 'cart1': { + 'payment': 'banktransfer', + 'email': 'admin@localhost', + 'customer': customer.pk, + } + } + session[f'customer_auth_id:{event.organizer.pk}'] = customer.pk + session[f'customer_auth_hash:{event.organizer.pk}'] = customer.get_session_auth_hash() + session.save() + return cp, session + + +@pytest.fixture +@scopes_disabled() +def cart2_expired(event, organizer, item, customer): + cp = CartPosition.objects.create( + event=event, + item=item, + datetime=now(), + expires=now() - timedelta(days=1), + price=item.default_price, + cart_id="cart2" + ) + session = SessionStore("cart2") + session['current_cart_event_{}'.format(event.pk)] = "cart2" + session['carts'] = { + 'cart2': { + 'payment': 'banktransfer', + 'email': 'admin@localhost', + 'customer': customer.pk, + } + } + session[f'customer_auth_id:{event.organizer.pk}'] = customer.pk + session[f'customer_auth_hash:{event.organizer.pk}'] = customer.get_session_auth_hash() + session.save() + return cp, session + + +@pytest.mark.asyncio +async def test_quota_race_condition_happens_if_we_disable_locks(live_server, session, event, item, quota, + cart1_expired, cart2_expired): + # This test exists to ensure that our test setup makes sense. If it fails, all tests down below + # might be useless. + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/checkout/confirm/?_debug_flag=skip-csrf&_debug_flag=skip-locking&_debug_flag=sleep-after-quota-check" + payload = {} + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart1_expired[1].session_key}), + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart2_expired[1].session_key}), + ) + assert ['thank-you' in r1, 'thank-you' in r2].count(True) == 2 + with scopes_disabled(): + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 0 + assert await sync_to_async(OrderPosition.objects.filter(item=item).count)() == 2 + + +@pytest.mark.asyncio +async def test_quota_race_condition_prevented_by_locks(live_server, session, event, item, quota, cart1_expired, cart2_expired): + quota.size = 1 + await sync_to_async(quota.save)() + + url = f"/{event.organizer.slug}/{event.slug}/checkout/confirm/?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = {} + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart1_expired[1].session_key}), + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart2_expired[1].session_key}), + ) + assert ['thank-you' in r1, 'thank-you' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(OrderPosition.objects.filter(item=item).count)() == 1 + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 0 + + +@pytest.mark.asyncio +async def test_voucher_race_condition_prevented_by_locks(live_server, session, event, item, quota, cart1_expired, cart2_expired, voucher): + cart1_expired[0].voucher = voucher + await sync_to_async(cart1_expired[0].save)() + cart2_expired[0].voucher = voucher + await sync_to_async(cart2_expired[0].save)() + + url = f"/{event.organizer.slug}/{event.slug}/checkout/confirm/?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = {} + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart1_expired[1].session_key}), + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart2_expired[1].session_key}), + ) + assert ['thank-you' in r1, 'thank-you' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(OrderPosition.objects.filter(item=item, voucher=voucher).count)() == 1 + assert await sync_to_async(CartPosition.objects.filter(item=item, voucher=voucher).count)() == 0 + + +@pytest.mark.asyncio +async def test_seat_race_condition_prevented_by_locks(live_server, session, event, item, quota, cart1_expired, cart2_expired, seat): + cart1_expired[0].seat = seat + await sync_to_async(cart1_expired[0].save)() + cart2_expired[0].seat = seat + await sync_to_async(cart2_expired[0].save)() + + url = f"/{event.organizer.slug}/{event.slug}/checkout/confirm/?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = {} + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart1_expired[1].session_key}), + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart2_expired[1].session_key}), + ) + assert ['thank-you' in r1, 'thank-you' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(OrderPosition.objects.filter(item=item, seat=seat).count)() == 1 + assert await sync_to_async(CartPosition.objects.filter(item=item, seat=seat).count)() == 0 + + +@pytest.mark.asyncio +async def test_membership_race_condition_prevented_by_locks(live_server, session, event, item, quota, cart1_expired, cart2_expired, membership): + cart1_expired[0].used_membership = membership + await sync_to_async(cart1_expired[0].save)() + cart2_expired[0].used_membership = membership + await sync_to_async(cart2_expired[0].save)() + item.require_membership = True + await sync_to_async(item.save)() + await sync_to_async(item.require_membership_types.set)([membership.membership_type]) + + url = f"/{event.organizer.slug}/{event.slug}/checkout/confirm/?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + payload = {} + + r1, r2 = await asyncio.gather( + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart1_expired[1].session_key}), + post(session, f"{live_server}{url}", data=payload, cookies={settings.SESSION_COOKIE_NAME: cart2_expired[1].session_key}), + ) + assert ['thank-you' in r1, 'thank-you' in r2].count(True) == 1 + with scopes_disabled(): + assert await sync_to_async(OrderPosition.objects.filter(item=item).count)() == 1 + assert await sync_to_async(CartPosition.objects.filter(item=item).count)() == 1 diff --git a/src/tests/concurrency_tests/test_order_paid_locking.py b/src/tests/concurrency_tests/test_order_paid_locking.py new file mode 100644 index 000000000..18c5b41a5 --- /dev/null +++ b/src/tests/concurrency_tests/test_order_paid_locking.py @@ -0,0 +1,113 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import asyncio +from datetime import timedelta +from decimal import Decimal +from importlib import import_module + +import pytest +from asgiref.sync import sync_to_async +from django.conf import settings +from django.utils.timezone import now +from django_scopes import scopes_disabled +from tests.concurrency_tests.utils import get + +from pretix.base.models import Order, OrderPayment, OrderPosition + +SessionStore = import_module(settings.SESSION_ENGINE).SessionStore + + +@pytest.fixture +@scopes_disabled() +def order1_expired(event, organizer, item): + order = Order.objects.create( + code='FOO', event=event, email='dummy@dummy.test', + status=Order.STATUS_EXPIRED, locale='en', + datetime=now(), expires=now() - timedelta(days=10), + total=Decimal('0.00'), + ) + OrderPosition.objects.create( + order=order, item=item, variation=None, + price=Decimal("0.00"), attendee_name_parts={'full_name': "Peter"}, positionid=1 + ) + p = OrderPayment.objects.create( + order=order, amount=Decimal("0.00"), provider="free", state=OrderPayment.PAYMENT_STATE_CREATED, + ) + return order, p + + +@pytest.fixture +@scopes_disabled() +def order2_expired(event, organizer, item, customer): + order = Order.objects.create( + code='BAR', event=event, email='dummy@dummy.test', + status=Order.STATUS_EXPIRED, locale='en', + datetime=now(), expires=now() - timedelta(days=10), + total=Decimal('0.00'), + ) + OrderPosition.objects.create( + order=order, item=item, variation=None, + price=Decimal("0.00"), attendee_name_parts={'full_name': "Peter"}, positionid=1 + ) + p = OrderPayment.objects.create( + order=order, amount=Decimal("0.00"), provider="free", state=OrderPayment.PAYMENT_STATE_CREATED, + ) + return order, p + + +@pytest.mark.asyncio +async def test_quota_race_condition_happens_if_we_disable_locks(live_server, session, event, item, quota, + order1_expired, order2_expired): + # This test exists to ensure that our test setup makes sense. If it fails, all tests down below + # might be useless. + quota.size = 1 + await sync_to_async(quota.save)() + + url1 = f"/{event.organizer.slug}/{event.slug}/order/{order1_expired[0].code}/{order1_expired[0].secret}/" \ + f"pay/{order1_expired[1].pk}/complete?_debug_flag=skip-csrf&_debug_flag=skip-locking&_debug_flag=sleep-after-quota-check" + url2 = f"/{event.organizer.slug}/{event.slug}/order/{order2_expired[0].code}/{order2_expired[0].secret}/" \ + f"pay/{order2_expired[1].pk}/complete?_debug_flag=skip-csrf&_debug_flag=skip-locking&_debug_flag=sleep-after-quota-check" + await asyncio.gather( + get(session, f"{live_server}{url1}"), + get(session, f"{live_server}{url2}"), + ) + await sync_to_async(order1_expired[0].refresh_from_db)() + await sync_to_async(order2_expired[0].refresh_from_db)() + assert {order1_expired[0].status, order2_expired[0].status} == {Order.STATUS_PAID} + + +@pytest.mark.asyncio +async def test_quota_race_condition_happens_prevented_by_lock(live_server, session, event, item, quota, order1_expired, order2_expired): + quota.size = 1 + await sync_to_async(quota.save)() + + url1 = f"/{event.organizer.slug}/{event.slug}/order/{order1_expired[0].code}/{order1_expired[0].secret}/" \ + f"pay/{order1_expired[1].pk}/complete?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + url2 = f"/{event.organizer.slug}/{event.slug}/order/{order2_expired[0].code}/{order2_expired[0].secret}/" \ + f"pay/{order2_expired[1].pk}/complete?_debug_flag=skip-csrf&_debug_flag=sleep-after-quota-check" + await asyncio.gather( + get(session, f"{live_server}{url1}"), + get(session, f"{live_server}{url2}"), + ) + await sync_to_async(order1_expired[0].refresh_from_db)() + await sync_to_async(order2_expired[0].refresh_from_db)() + assert {order1_expired[0].status, order2_expired[0].status} == {Order.STATUS_PAID, Order.STATUS_EXPIRED} diff --git a/src/tests/concurrency_tests/utils.py b/src/tests/concurrency_tests/utils.py new file mode 100644 index 000000000..59f65add6 --- /dev/null +++ b/src/tests/concurrency_tests/utils.py @@ -0,0 +1,29 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +async def post(session, url, data, **kwargs): + async with session.post(url, data=data, **kwargs) as response: + return await response.text() + + +async def get(session, url, **kwargs): + async with session.get(url, **kwargs) as response: + return await response.text()