From c30ff5e65759a5a298d2cce53c29eeb47ba92a1f Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Mon, 29 Aug 2016 22:32:15 +0200 Subject: [PATCH] Only call now() once inside any event.lock() call --- src/pretix/base/models/items.py | 28 +++++++++++++--------- src/pretix/base/models/orders.py | 5 ++-- src/pretix/base/services/cart.py | 37 ++++++++++++++--------------- src/pretix/base/services/locking.py | 1 + src/pretix/base/services/orders.py | 37 ++++++++++++++--------------- src/pretix/control/views/orders.py | 4 ++-- src/tests/base/test_orders.py | 6 ++--- 7 files changed, 62 insertions(+), 56 deletions(-) diff --git a/src/pretix/base/models/items.py b/src/pretix/base/models/items.py index 8885315d05..e5b764ee0a 100644 --- a/src/pretix/base/models/items.py +++ b/src/pretix/base/models/items.py @@ -1,5 +1,6 @@ import sys import uuid +from datetime import datetime from decimal import Decimal from typing import Tuple @@ -204,16 +205,17 @@ class Item(LoggedModel): if self.event: self.event.get_cache().clear() - def is_available(self) -> bool: + def is_available(self, now_dt: datetime=None) -> bool: """ Returns whether this item is available according to its ``active`` flag and its ``available_from`` and ``available_until`` fields """ + now_dt = now_dt or now() if not self.active: return False - if self.available_from and self.available_from > now(): + if self.available_from and self.available_from > now_dt: return False - if self.available_until and self.available_until < now(): + if self.available_until and self.available_until < now_dt: return False return True @@ -520,7 +522,7 @@ class Quota(LoggedModel): if self.event: self.event.get_cache().clear() - def availability(self) -> Tuple[int, int]: + def availability(self, now_dt: datetime=None) -> Tuple[int, int]: """ This method is used to determine whether Items or ItemVariations belonging to this quota should currently be available for sale. @@ -528,6 +530,7 @@ class Quota(LoggedModel): :returns: a tuple where the first entry is one of the ``Quota.AVAILABILITY_`` constants and the second is the number of available tickets. """ + now_dt = now_dt or now() size_left = self.size if size_left is None: return Quota.AVAILABILITY_OK, None @@ -541,33 +544,36 @@ class Quota(LoggedModel): if size_left <= 0: return Quota.AVAILABILITY_ORDERED, 0 - size_left -= self.count_blocking_vouchers() + size_left -= self.count_blocking_vouchers(now_dt) if size_left <= 0: return Quota.AVAILABILITY_ORDERED, 0 - size_left -= self.count_in_cart() + size_left -= self.count_in_cart(now_dt) if size_left <= 0: return Quota.AVAILABILITY_RESERVED, 0 return Quota.AVAILABILITY_OK, size_left - def count_blocking_vouchers(self) -> int: + def count_blocking_vouchers(self, now_dt: datetime=None) -> int: from pretix.base.models import Voucher + + now_dt = now_dt or now() return Voucher.objects.filter( Q(block_quota=True) & Q(redeemed=False) & - Q(Q(valid_until__isnull=True) | Q(valid_until__gte=now())) & + Q(Q(valid_until__isnull=True) | Q(valid_until__gte=now_dt)) & Q(Q(self._position_lookup) | Q(quota=self)) ).distinct().count() - def count_in_cart(self) -> int: + def count_in_cart(self, now_dt: datetime=None) -> int: from pretix.base.models import CartPosition + now_dt = now_dt or now() return CartPosition.objects.filter( - Q(expires__gte=now()) & + Q(expires__gte=now_dt) & ~Q( Q(voucher__isnull=False) & Q(voucher__block_quota=True) - & Q(Q(voucher__valid_until__isnull=True) | Q(voucher__valid_until__gte=now())) + & Q(Q(voucher__valid_until__isnull=True) | Q(voucher__valid_until__gte=now_dt)) ) & self._position_lookup ).distinct().count() diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index 369fee3534..4baed38985 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -253,10 +253,11 @@ class Order(LoggedModel): return self._is_still_available() - def _is_still_available(self) -> Union[bool, str]: + def _is_still_available(self, now_dt: datetime=None) -> Union[bool, str]: error_messages = { 'unavailable': _('Some of the ordered products are no longer available.'), } + now_dt = now_dt or now() positions = self.positions.all().select_related('item', 'variation') quota_cache = {} try: @@ -270,7 +271,7 @@ class Order(LoggedModel): # quota while we're doing so. if quota.id not in quota_cache: quota_cache[quota.id] = quota - quota.cached_availability = quota.availability()[1] + quota.cached_availability = quota.availability(now_dt)[1] else: # Use cached version quota = quota_cache[quota.id] diff --git a/src/pretix/base/services/cart.py b/src/pretix/base/services/cart.py index e5c3ca337b..1061857e40 100644 --- a/src/pretix/base/services/cart.py +++ b/src/pretix/base/services/cart.py @@ -4,7 +4,6 @@ from typing import List, Optional from django.conf import settings from django.db.models import Q -from django.utils.timezone import now from django.utils.translation import ugettext as _ from pretix.base.i18n import LazyLocaleException @@ -37,21 +36,21 @@ error_messages = { } -def _extend_existing(event: Event, cart_id: str, expiry: datetime) -> None: +def _extend_existing(event: Event, cart_id: str, expiry: datetime, now_dt: datetime) -> None: # Extend this user's cart session to 30 minutes from now to ensure all items in the # cart expire at the same time # We can extend the reservation of items which are not yet expired without risk CartPosition.objects.filter( - Q(cart_id=cart_id) & Q(event=event) & Q(expires__gt=now()) + Q(cart_id=cart_id) & Q(event=event) & Q(expires__gt=now_dt) ).update(expires=expiry) -def _re_add_expired_positions(items: List[dict], event: Event, cart_id: str) -> List[CartPosition]: +def _re_add_expired_positions(items: List[dict], event: Event, cart_id: str, now_dt: datetime) -> List[CartPosition]: positions = set() # For items that are already expired, we have to delete and re-add them, as they might # be no longer available or prices might have changed. Sorry! expired = CartPosition.objects.filter( - Q(cart_id=cart_id) & Q(event=event) & Q(expires__lte=now()) + Q(cart_id=cart_id) & Q(event=event) & Q(expires__lte=now_dt) ) for cp in expired: items.insert(0, { @@ -66,21 +65,21 @@ def _re_add_expired_positions(items: List[dict], event: Event, cart_id: str) -> return positions -def _delete_expired(expired: List[CartPosition]) -> None: +def _delete_expired(expired: List[CartPosition], now_dt: datetime) -> None: for cp in expired: - if cp.expires <= now(): + if cp.expires <= now_dt: cp.delete() -def _check_date(event: Event) -> None: - if event.presale_start and now() < event.presale_start: +def _check_date(event: Event, now_dt: datetime) -> None: + if event.presale_start and now_dt < event.presale_start: raise CartError(error_messages['not_started']) - if event.presale_end and now() > event.presale_end: + if event.presale_end and now_dt > event.presale_end: raise CartError(error_messages['ended']) def _add_new_items(event: Event, items: List[dict], - cart_id: str, expiry: datetime) -> Optional[str]: + cart_id: str, expiry: datetime, now_dt: datetime) -> Optional[str]: err = None # Fetch items from the database @@ -111,7 +110,7 @@ def _add_new_items(event: Event, items: List[dict], voucher = Voucher.objects.get(code=i.get('voucher'), event=event) if voucher.redeemed: return error_messages['voucher_redeemed'] - if voucher.valid_until is not None and voucher.valid_until < now(): + if voucher.valid_until is not None and voucher.valid_until < now_dt: return error_messages['voucher_expired'] if voucher.item and voucher.item.pk != item.pk: return error_messages['voucher_invalid_item'] @@ -186,20 +185,20 @@ def _add_new_items(event: Event, items: List[dict], def _add_items_to_cart(event: Event, items: List[dict], cart_id: str=None) -> None: - with event.lock(): - _check_date(event) + with event.lock() as now_dt: + _check_date(event, now_dt) existing = CartPosition.objects.filter(Q(cart_id=cart_id) & Q(event=event)).count() if sum(i['count'] for i in items) + existing > int(event.settings.max_items_per_order): # TODO: i18n plurals raise CartError(error_messages['max_items'], (event.settings.max_items_per_order,)) - expiry = now() + timedelta(minutes=event.settings.get('reservation_time', as_type=int)) - _extend_existing(event, cart_id, expiry) + expiry = now_dt + timedelta(minutes=event.settings.get('reservation_time', as_type=int)) + _extend_existing(event, cart_id, expiry, now_dt) - expired = _re_add_expired_positions(items, event, cart_id) + expired = _re_add_expired_positions(items, event, cart_id, now_dt) if items: - err = _add_new_items(event, items, cart_id, expiry) - _delete_expired(expired) + err = _add_new_items(event, items, cart_id, expiry, now_dt) + _delete_expired(expired, now_dt) if err: raise CartError(err) diff --git a/src/pretix/base/services/locking.py b/src/pretix/base/services/locking.py index aa4bc02b6d..ebdf5ca5a3 100644 --- a/src/pretix/base/services/locking.py +++ b/src/pretix/base/services/locking.py @@ -19,6 +19,7 @@ class LockManager: def __enter__(self): lock_event(self.event) + return now() def __exit__(self, exc_type, exc_val, exc_tb): release_event(self.event) diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index ea6a5a3869..bae0c9fcae 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -68,13 +68,13 @@ def mark_order_paid(order: Order, provider: str=None, info: str=None, date: date :param user: The user that performed the change :raises Quota.QuotaExceededException: if the quota is exceeded and ``force`` is ``False`` """ - with order.event.lock(): + with order.event.lock() as now_dt: can_be_paid = order._can_be_paid() if not force and can_be_paid is not True: raise Quota.QuotaExceededException(can_be_paid) order.payment_provider = provider or order.payment_provider order.payment_info = info or order.payment_info - order.payment_date = date or now() + order.payment_date = date or now_dt if manual is not None: order.payment_manual = manual order.status = Order.STATUS_PAID @@ -164,16 +164,16 @@ class OrderError(LazyLocaleException): pass -def _check_date(event: Event): - if event.presale_start and now() < event.presale_start: +def _check_date(event: Event, now_dt: datetime): + if event.presale_start and now_dt < event.presale_start: raise OrderError(error_messages['not_started']) - if event.presale_end and now() > event.presale_end: + if event.presale_end and now_dt > event.presale_end: raise OrderError(error_messages['ended']) -def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): +def _check_positions(event: Event, now_dt: datetime, positions: List[CartPosition]): err = None - _check_date(event) + _check_date(event, now_dt) voucherids = set() for i, cp in enumerate(positions): @@ -199,7 +199,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): cp.delete() return error_messages['voucher_required'] - if cp.expires >= dt and not cp.voucher: + if cp.expires >= now_dt and not cp.voucher: # Other checks are not necessary continue @@ -212,7 +212,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): continue if cp.voucher: - if cp.voucher.valid_until and cp.voucher.valid_until < now(): + if cp.voucher.valid_until and cp.voucher.valid_until < now_dt: err = err or error_messages['voucher_expired'] continue if cp.voucher.price is not None: @@ -227,14 +227,14 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): quota_ok = True - ignore_all_quotas = cp.expires >= dt or ( + ignore_all_quotas = cp.expires >= now_dt or ( cp.voucher and (cp.voucher.allow_ignore_quota or (cp.voucher.block_quota and cp.voucher.quota is None))) if not ignore_all_quotas: for quota in quotas: if cp.voucher and cp.voucher.block_quota and cp.voucher.quota_id == quota.pk: continue - avail = quota.availability() + avail = quota.availability(now_dt) if avail[0] != Quota.AVAILABILITY_OK: # This quota is sold out/currently unavailable, so do not sell this at all err = err or error_messages['unavailable'] @@ -243,7 +243,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): if quota_ok: positions[i] = cp - cp.expires = now() + timedelta( + cp.expires = now_dt + timedelta( minutes=event.settings.get('reservation_time', as_type=int)) cp.save() else: @@ -253,19 +253,19 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]): @transaction.atomic() -def _create_order(event: Event, email: str, positions: List[CartPosition], dt: datetime, +def _create_order(event: Event, email: str, positions: List[CartPosition], now_dt: datetime, payment_provider: BasePaymentProvider, locale: str=None): total = sum([c.price for c in positions]) payment_fee = payment_provider.calculate_fee(total) total += payment_fee - expires = [dt + timedelta(days=event.settings.get('payment_term_days', as_type=int))] + expires = [now_dt + timedelta(days=event.settings.get('payment_term_days', as_type=int))] if event.settings.get('payment_term_last'): expires.append(event.settings.get('payment_term_last', as_type=datetime)) order = Order.objects.create( status=Order.STATUS_PENDING, event=event, email=email, - datetime=dt, + datetime=now_dt, expires=min(expires), locale=locale, total=total, @@ -291,14 +291,13 @@ def _perform_order(event: str, payment_provider: str, position_ids: List[str], if not pprov: raise OrderError(error_messages['internal']) - dt = now() - with event.lock(): + with event.lock() as now_dt: positions = list(CartPosition.objects.filter( id__in=position_ids).select_related('item', 'variation')) if len(position_ids) != len(positions): raise OrderError(error_messages['internal']) - _check_positions(event, dt, positions) - order = _create_order(event, email, positions, dt, pprov, + _check_positions(event, now_dt, positions) + order = _create_order(event, email, positions, now_dt, pprov, locale=locale) if address is not None: diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index bab5f61bd9..16477464ed 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -417,8 +417,8 @@ class OrderExtend(OrderView): self.form.save() else: try: - with self.order.event.lock(): - is_available = self.order._is_still_available() + with self.order.event.lock() as now_dt: + is_available = self.order._is_still_available(now_dt) if is_available is True: self.form.save() self.order.log_action('pretix.event.order.expirychanged', user=self.request.user, data={ diff --git a/src/tests/base/test_orders.py b/src/tests/base/test_orders.py index dde1962e6a..5c12f64106 100644 --- a/src/tests/base/test_orders.py +++ b/src/tests/base/test_orders.py @@ -23,7 +23,7 @@ def test_expiry_days(event): today = now() event.settings.set('payment_term_days', 5) order = _create_order(event, email='dummy@example.org', positions=[], - dt=today, payment_provider=FreeOrderProvider(event), + now_dt=today, payment_provider=FreeOrderProvider(event), locale='de') assert (order.expires - today).days == 5 @@ -34,12 +34,12 @@ def test_expiry_last(event): event.settings.set('payment_term_days', 5) event.settings.set('payment_term_last', now() + timedelta(days=3)) order = _create_order(event, email='dummy@example.org', positions=[], - dt=today, payment_provider=FreeOrderProvider(event), + now_dt=today, payment_provider=FreeOrderProvider(event), locale='de') assert (order.expires - today).days == 3 event.settings.set('payment_term_last', now() + timedelta(days=7)) order = _create_order(event, email='dummy@example.org', positions=[], - dt=today, payment_provider=FreeOrderProvider(event), + now_dt=today, payment_provider=FreeOrderProvider(event), locale='de') assert (order.expires - today).days == 5