Only call now() once inside any event.lock() call

This commit is contained in:
Raphael Michel
2016-08-29 22:32:15 +02:00
parent 65c16bdc58
commit c30ff5e657
7 changed files with 62 additions and 56 deletions

View File

@@ -1,5 +1,6 @@
import sys import sys
import uuid import uuid
from datetime import datetime
from decimal import Decimal from decimal import Decimal
from typing import Tuple from typing import Tuple
@@ -204,16 +205,17 @@ class Item(LoggedModel):
if self.event: if self.event:
self.event.get_cache().clear() 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 Returns whether this item is available according to its ``active`` flag
and its ``available_from`` and ``available_until`` fields and its ``available_from`` and ``available_until`` fields
""" """
now_dt = now_dt or now()
if not self.active: if not self.active:
return False return False
if self.available_from and self.available_from > now(): if self.available_from and self.available_from > now_dt:
return False return False
if self.available_until and self.available_until < now(): if self.available_until and self.available_until < now_dt:
return False return False
return True return True
@@ -520,7 +522,7 @@ class Quota(LoggedModel):
if self.event: if self.event:
self.event.get_cache().clear() 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 This method is used to determine whether Items or ItemVariations belonging
to this quota should currently be available for sale. 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 :returns: a tuple where the first entry is one of the ``Quota.AVAILABILITY_`` constants
and the second is the number of available tickets. and the second is the number of available tickets.
""" """
now_dt = now_dt or now()
size_left = self.size size_left = self.size
if size_left is None: if size_left is None:
return Quota.AVAILABILITY_OK, None return Quota.AVAILABILITY_OK, None
@@ -541,33 +544,36 @@ class Quota(LoggedModel):
if size_left <= 0: if size_left <= 0:
return Quota.AVAILABILITY_ORDERED, 0 return Quota.AVAILABILITY_ORDERED, 0
size_left -= self.count_blocking_vouchers() size_left -= self.count_blocking_vouchers(now_dt)
if size_left <= 0: if size_left <= 0:
return Quota.AVAILABILITY_ORDERED, 0 return Quota.AVAILABILITY_ORDERED, 0
size_left -= self.count_in_cart() size_left -= self.count_in_cart(now_dt)
if size_left <= 0: if size_left <= 0:
return Quota.AVAILABILITY_RESERVED, 0 return Quota.AVAILABILITY_RESERVED, 0
return Quota.AVAILABILITY_OK, size_left 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 from pretix.base.models import Voucher
now_dt = now_dt or now()
return Voucher.objects.filter( return Voucher.objects.filter(
Q(block_quota=True) & Q(block_quota=True) &
Q(redeemed=False) & 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)) Q(Q(self._position_lookup) | Q(quota=self))
).distinct().count() ).distinct().count()
def count_in_cart(self) -> int: def count_in_cart(self, now_dt: datetime=None) -> int:
from pretix.base.models import CartPosition from pretix.base.models import CartPosition
now_dt = now_dt or now()
return CartPosition.objects.filter( return CartPosition.objects.filter(
Q(expires__gte=now()) & Q(expires__gte=now_dt) &
~Q( ~Q(
Q(voucher__isnull=False) & Q(voucher__block_quota=True) 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 self._position_lookup
).distinct().count() ).distinct().count()

View File

@@ -253,10 +253,11 @@ class Order(LoggedModel):
return self._is_still_available() 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 = { error_messages = {
'unavailable': _('Some of the ordered products are no longer available.'), 'unavailable': _('Some of the ordered products are no longer available.'),
} }
now_dt = now_dt or now()
positions = self.positions.all().select_related('item', 'variation') positions = self.positions.all().select_related('item', 'variation')
quota_cache = {} quota_cache = {}
try: try:
@@ -270,7 +271,7 @@ class Order(LoggedModel):
# quota while we're doing so. # quota while we're doing so.
if quota.id not in quota_cache: if quota.id not in quota_cache:
quota_cache[quota.id] = quota quota_cache[quota.id] = quota
quota.cached_availability = quota.availability()[1] quota.cached_availability = quota.availability(now_dt)[1]
else: else:
# Use cached version # Use cached version
quota = quota_cache[quota.id] quota = quota_cache[quota.id]

View File

@@ -4,7 +4,6 @@ from typing import List, Optional
from django.conf import settings from django.conf import settings
from django.db.models import Q from django.db.models import Q
from django.utils.timezone import now
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from pretix.base.i18n import LazyLocaleException 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 # Extend this user's cart session to 30 minutes from now to ensure all items in the
# cart expire at the same time # cart expire at the same time
# We can extend the reservation of items which are not yet expired without risk # We can extend the reservation of items which are not yet expired without risk
CartPosition.objects.filter( 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) ).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() positions = set()
# For items that are already expired, we have to delete and re-add them, as they might # 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! # be no longer available or prices might have changed. Sorry!
expired = CartPosition.objects.filter( 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: for cp in expired:
items.insert(0, { items.insert(0, {
@@ -66,21 +65,21 @@ def _re_add_expired_positions(items: List[dict], event: Event, cart_id: str) ->
return positions return positions
def _delete_expired(expired: List[CartPosition]) -> None: def _delete_expired(expired: List[CartPosition], now_dt: datetime) -> None:
for cp in expired: for cp in expired:
if cp.expires <= now(): if cp.expires <= now_dt:
cp.delete() cp.delete()
def _check_date(event: Event) -> None: def _check_date(event: Event, now_dt: datetime) -> None:
if event.presale_start and now() < event.presale_start: if event.presale_start and now_dt < event.presale_start:
raise CartError(error_messages['not_started']) 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']) raise CartError(error_messages['ended'])
def _add_new_items(event: Event, items: List[dict], 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 err = None
# Fetch items from the database # 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) voucher = Voucher.objects.get(code=i.get('voucher'), event=event)
if voucher.redeemed: if voucher.redeemed:
return error_messages['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'] return error_messages['voucher_expired']
if voucher.item and voucher.item.pk != item.pk: if voucher.item and voucher.item.pk != item.pk:
return error_messages['voucher_invalid_item'] 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: def _add_items_to_cart(event: Event, items: List[dict], cart_id: str=None) -> None:
with event.lock(): with event.lock() as now_dt:
_check_date(event) _check_date(event, now_dt)
existing = CartPosition.objects.filter(Q(cart_id=cart_id) & Q(event=event)).count() 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): if sum(i['count'] for i in items) + existing > int(event.settings.max_items_per_order):
# TODO: i18n plurals # TODO: i18n plurals
raise CartError(error_messages['max_items'], (event.settings.max_items_per_order,)) raise CartError(error_messages['max_items'], (event.settings.max_items_per_order,))
expiry = now() + timedelta(minutes=event.settings.get('reservation_time', as_type=int)) expiry = now_dt + timedelta(minutes=event.settings.get('reservation_time', as_type=int))
_extend_existing(event, cart_id, expiry) _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: if items:
err = _add_new_items(event, items, cart_id, expiry) err = _add_new_items(event, items, cart_id, expiry, now_dt)
_delete_expired(expired) _delete_expired(expired, now_dt)
if err: if err:
raise CartError(err) raise CartError(err)

View File

@@ -19,6 +19,7 @@ class LockManager:
def __enter__(self): def __enter__(self):
lock_event(self.event) lock_event(self.event)
return now()
def __exit__(self, exc_type, exc_val, exc_tb): def __exit__(self, exc_type, exc_val, exc_tb):
release_event(self.event) release_event(self.event)

View File

@@ -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 :param user: The user that performed the change
:raises Quota.QuotaExceededException: if the quota is exceeded and ``force`` is ``False`` :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() can_be_paid = order._can_be_paid()
if not force and can_be_paid is not True: if not force and can_be_paid is not True:
raise Quota.QuotaExceededException(can_be_paid) raise Quota.QuotaExceededException(can_be_paid)
order.payment_provider = provider or order.payment_provider order.payment_provider = provider or order.payment_provider
order.payment_info = info or order.payment_info 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: if manual is not None:
order.payment_manual = manual order.payment_manual = manual
order.status = Order.STATUS_PAID order.status = Order.STATUS_PAID
@@ -164,16 +164,16 @@ class OrderError(LazyLocaleException):
pass pass
def _check_date(event: Event): def _check_date(event: Event, now_dt: datetime):
if event.presale_start and now() < event.presale_start: if event.presale_start and now_dt < event.presale_start:
raise OrderError(error_messages['not_started']) 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']) 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 err = None
_check_date(event) _check_date(event, now_dt)
voucherids = set() voucherids = set()
for i, cp in enumerate(positions): for i, cp in enumerate(positions):
@@ -199,7 +199,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]):
cp.delete() cp.delete()
return error_messages['voucher_required'] 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 # Other checks are not necessary
continue continue
@@ -212,7 +212,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]):
continue continue
if cp.voucher: 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'] err = err or error_messages['voucher_expired']
continue continue
if cp.voucher.price is not None: if cp.voucher.price is not None:
@@ -227,14 +227,14 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]):
quota_ok = True 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))) cp.voucher and (cp.voucher.allow_ignore_quota or (cp.voucher.block_quota and cp.voucher.quota is None)))
if not ignore_all_quotas: if not ignore_all_quotas:
for quota in quotas: for quota in quotas:
if cp.voucher and cp.voucher.block_quota and cp.voucher.quota_id == quota.pk: if cp.voucher and cp.voucher.block_quota and cp.voucher.quota_id == quota.pk:
continue continue
avail = quota.availability() avail = quota.availability(now_dt)
if avail[0] != Quota.AVAILABILITY_OK: if avail[0] != Quota.AVAILABILITY_OK:
# This quota is sold out/currently unavailable, so do not sell this at all # This quota is sold out/currently unavailable, so do not sell this at all
err = err or error_messages['unavailable'] err = err or error_messages['unavailable']
@@ -243,7 +243,7 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]):
if quota_ok: if quota_ok:
positions[i] = cp positions[i] = cp
cp.expires = now() + timedelta( cp.expires = now_dt + timedelta(
minutes=event.settings.get('reservation_time', as_type=int)) minutes=event.settings.get('reservation_time', as_type=int))
cp.save() cp.save()
else: else:
@@ -253,19 +253,19 @@ def _check_positions(event: Event, dt: datetime, positions: List[CartPosition]):
@transaction.atomic() @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): payment_provider: BasePaymentProvider, locale: str=None):
total = sum([c.price for c in positions]) total = sum([c.price for c in positions])
payment_fee = payment_provider.calculate_fee(total) payment_fee = payment_provider.calculate_fee(total)
total += payment_fee 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'): if event.settings.get('payment_term_last'):
expires.append(event.settings.get('payment_term_last', as_type=datetime)) expires.append(event.settings.get('payment_term_last', as_type=datetime))
order = Order.objects.create( order = Order.objects.create(
status=Order.STATUS_PENDING, status=Order.STATUS_PENDING,
event=event, event=event,
email=email, email=email,
datetime=dt, datetime=now_dt,
expires=min(expires), expires=min(expires),
locale=locale, locale=locale,
total=total, total=total,
@@ -291,14 +291,13 @@ def _perform_order(event: str, payment_provider: str, position_ids: List[str],
if not pprov: if not pprov:
raise OrderError(error_messages['internal']) raise OrderError(error_messages['internal'])
dt = now() with event.lock() as now_dt:
with event.lock():
positions = list(CartPosition.objects.filter( positions = list(CartPosition.objects.filter(
id__in=position_ids).select_related('item', 'variation')) id__in=position_ids).select_related('item', 'variation'))
if len(position_ids) != len(positions): if len(position_ids) != len(positions):
raise OrderError(error_messages['internal']) raise OrderError(error_messages['internal'])
_check_positions(event, dt, positions) _check_positions(event, now_dt, positions)
order = _create_order(event, email, positions, dt, pprov, order = _create_order(event, email, positions, now_dt, pprov,
locale=locale) locale=locale)
if address is not None: if address is not None:

View File

@@ -417,8 +417,8 @@ class OrderExtend(OrderView):
self.form.save() self.form.save()
else: else:
try: try:
with self.order.event.lock(): with self.order.event.lock() as now_dt:
is_available = self.order._is_still_available() is_available = self.order._is_still_available(now_dt)
if is_available is True: if is_available is True:
self.form.save() self.form.save()
self.order.log_action('pretix.event.order.expirychanged', user=self.request.user, data={ self.order.log_action('pretix.event.order.expirychanged', user=self.request.user, data={

View File

@@ -23,7 +23,7 @@ def test_expiry_days(event):
today = now() today = now()
event.settings.set('payment_term_days', 5) event.settings.set('payment_term_days', 5)
order = _create_order(event, email='dummy@example.org', positions=[], order = _create_order(event, email='dummy@example.org', positions=[],
dt=today, payment_provider=FreeOrderProvider(event), now_dt=today, payment_provider=FreeOrderProvider(event),
locale='de') locale='de')
assert (order.expires - today).days == 5 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_days', 5)
event.settings.set('payment_term_last', now() + timedelta(days=3)) event.settings.set('payment_term_last', now() + timedelta(days=3))
order = _create_order(event, email='dummy@example.org', positions=[], order = _create_order(event, email='dummy@example.org', positions=[],
dt=today, payment_provider=FreeOrderProvider(event), now_dt=today, payment_provider=FreeOrderProvider(event),
locale='de') locale='de')
assert (order.expires - today).days == 3 assert (order.expires - today).days == 3
event.settings.set('payment_term_last', now() + timedelta(days=7)) event.settings.set('payment_term_last', now() + timedelta(days=7))
order = _create_order(event, email='dummy@example.org', positions=[], order = _create_order(event, email='dummy@example.org', positions=[],
dt=today, payment_provider=FreeOrderProvider(event), now_dt=today, payment_provider=FreeOrderProvider(event),
locale='de') locale='de')
assert (order.expires - today).days == 5 assert (order.expires - today).days == 5