CGM_Public/pretix_cgo
1
0
Fork 0
forked from CGM_Public/pretix_original
Code Pull Requests Actions Activity

Prevent some race conditions

Browse Source
This commit is contained in:
Raphael Michel
2018-10-29 17:27:12 +01:00
parent 7a945daefc
commit df2d8925ed
10 changed files with 209 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/pretix/api/views/order.py
View File

@@ -251,7 +251,7 @@ class OrderViewSet(CreateModelMixin, viewsets.ReadOnlyModelViewSet):
)
order.status = Order.STATUS_PENDING
order.save()
order.save(update_fields=['status'])
order.log_action(
'pretix.event.order.unpaid',
user=request.user if request.user.is_authenticated else None,
@@ -556,7 +556,7 @@ class PaymentViewSet(viewsets.ReadOnlyModelViewSet):
payment.order.event.subevents.filter(
id__in=payment.order.positions.values_list('subevent_id', flat=True))
)
payment.order.save()
payment.order.save(update_fields=['status', 'expires'])
return Response(OrderRefundSerializer(r).data, status=status.HTTP_200_OK)
@detail_route(methods=['POST'])
@@ -622,7 +622,7 @@ class RefundViewSet(CreateModelMixin, viewsets.ReadOnlyModelViewSet):
refund.order.event.subevents.filter(
id__in=refund.order.positions.values_list('subevent_id', flat=True))
)
refund.order.save()
refund.order.save(update_fields=['status', 'expires'])
return self.retrieve(request, [], **kwargs)
@detail_route(methods=['POST'])

2
src/pretix/base/migrations/0100_auto_20181023_2300.py
View File

@@ -1,7 +1,7 @@
# Generated by Django 2.1 on 2018-10-23 23:00
from django.db import migrations, models
import django_countries.fields
from django.db import migrations, models
class Migration(migrations.Migration):

47
src/pretix/base/models/base.py
View File

@@ -3,6 +3,7 @@ import uuid
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models.constants import LOOKUP_SEP
from django.db.models.signals import post_delete
from django.dispatch import receiver
from django.utils.crypto import get_random_string
@@ -100,3 +101,49 @@ class LoggedModel(models.Model, LoggingMixin):
return LogEntry.objects.filter(
content_type=ContentType.objects.get_for_model(type(self)), object_id=self.pk
).select_related('user', 'event', 'oauth_application', 'api_token', 'device')
class LockModel:
def refresh_for_update(self, fields=None, using=None, **kwargs):
"""
Like refresh_from_db(), but with select_for_update().
See also https://code.djangoproject.com/ticket/28344
"""
if fields is not None:
if not fields:
return
if any(LOOKUP_SEP in f for f in fields):
raise ValueError(
'Found "%s" in fields argument. Relations and transforms '
'are not allowed in fields.' % LOOKUP_SEP)
hints = {'instance': self}
db_instance_qs = self.__class__._base_manager.db_manager(using, hints=hints).filter(pk=self.pk).select_for_update(**kwargs)
# Use provided fields, if not set then reload all non-deferred fields.
deferred_fields = self.get_deferred_fields()
if fields is not None:
fields = list(fields)
db_instance_qs = db_instance_qs.only(*fields)
elif deferred_fields:
fields = [f.attname for f in self._meta.concrete_fields
if f.attname not in deferred_fields]
db_instance_qs = db_instance_qs.only(*fields)
db_instance = db_instance_qs.get()
non_loaded_fields = db_instance.get_deferred_fields()
for field in self._meta.concrete_fields:
if field.attname in non_loaded_fields:
# This field wasn't refreshed - skip ahead.
continue
setattr(self, field.attname, getattr(db_instance, field.attname))
# Clear cached foreign keys.
if field.is_relation and field.is_cached(self):
field.delete_cached_value(self)
# Clear cached relations.
for field in self._meta.related_objects:
if field.is_cached(self):
field.delete_cached_value(self)
self._state.db = db_instance._state.db

6
src/pretix/base/models/orders.py
View File

@@ -31,7 +31,7 @@ from pretix.base.i18n import language
from pretix.base.models import User
from pretix.base.reldate import RelativeDateWrapper
from .base import LoggedModel
from .base import LockModel, LoggedModel
from .event import Event, SubEvent
from .items import Item, ItemVariation, Question, QuestionOption, Quota
@@ -47,7 +47,7 @@ def generate_position_secret():
return get_random_string(length=settings.ENTROPY['ticket_secret'], allowed_chars='abcdefghjkmnpqrstuvwxyz23456789')
class Order(LoggedModel):
class Order(LockModel, LoggedModel):
"""
An order is created when a user clicks 'buy' on his cart. It holds
several OrderPositions and is connected to a user. It has an
@@ -903,7 +903,7 @@ class OrderPayment(models.Model):
if not force and can_be_paid is not True:
raise Quota.QuotaExceededException(can_be_paid)
self.order.status = Order.STATUS_PAID
self.order.save()
self.order.save(update_fields=['status'])
self.order.log_action('pretix.event.order.paid', {
'provider': self.provider,

152
src/pretix/base/services/orders.py
View File

@@ -93,7 +93,7 @@ def extend_order(order: Order, new_date: datetime, force: bool=False, user: User
raise OrderError(_('The new expiry date needs to be in the future.'))
if order.status == Order.STATUS_PENDING:
order.expires = new_date
order.save()
order.save(update_fields=['expires'])
order.log_action(
'pretix.event.order.expirychanged',
user=user,
@@ -109,7 +109,7 @@ def extend_order(order: Order, new_date: datetime, force: bool=False, user: User
if is_available is True or force is True:
order.expires = new_date
order.status = Order.STATUS_PENDING
order.save()
order.save(update_fields=['expires', 'status'])
order.log_action(
'pretix.event.order.expirychanged',
user=user,
@@ -136,7 +136,7 @@ def mark_order_refunded(order, user=None, auth=None, api_token=None):
user = User.objects.get(pk=user)
with order.event.lock():
order.status = Order.STATUS_REFUNDED
order.save()
order.save(update_fields=['status'])
order.log_action('pretix.event.order.refunded', user=user, auth=auth or api_token)
i = order.invoices.filter(is_cancellation=False).last()
@@ -159,7 +159,7 @@ def mark_order_expired(order, user=None, auth=None):
user = User.objects.get(pk=user)
with order.event.lock():
order.status = Order.STATUS_EXPIRED
order.save()
order.save(update_fields=['status'])
order.log_action('pretix.event.order.expired', user=user, auth=auth)
i = order.invoices.filter(is_cancellation=False).last()
@@ -181,7 +181,7 @@ def approve_order(order, user=None, send_mail: bool=True, auth=None):
order.require_approval = False
order.set_expires(now(), order.event.subevents.filter(id__in=[p.subevent_id for p in order.positions.all()]))
order.save()
order.save(update_fields=['require_approval', 'expires'])
order.log_action('pretix.event.order.approved', user=user, auth=auth)
if order.total == Decimal('0.00'):
@@ -258,7 +258,7 @@ def deny_order(order, comment='', user=None, send_mail: bool=True, auth=None):
with order.event.lock():
order.status = Order.STATUS_CANCELED
order.save()
order.save(update_fields=['status'])
order.log_action('pretix.event.order.denied', user=user, auth=auth, data={
'comment': comment
@@ -327,7 +327,7 @@ def _cancel_order(order, user=None, send_mail: bool=True, api_token=None, device
if not order.cancel_allowed():
raise OrderError(_('You cannot cancel this order.'))
order.status = Order.STATUS_CANCELED
order.save()
order.save(update_fields=['status'])
order.log_action('pretix.event.order.canceled', user=user, auth=api_token or oauth_application or device)
i = order.invoices.filter(is_cancellation=False).last()
@@ -661,47 +661,52 @@ def send_expiry_warnings(sender, **kwargs):
eventcache = {}
today = now().replace(hour=0, minute=0, second=0)
for o in Order.objects.filter(expires__gte=today, expiry_reminder_sent=False, status=Order.STATUS_PENDING).select_related('event'):
eventsettings = eventcache.get(o.event.pk, None)
if eventsettings is None:
eventsettings = o.event.settings
eventcache[o.event.pk] = eventsettings
for o in Order.objects.filter(expires__gte=today, expiry_reminder_sent=False, status=Order.STATUS_PENDING).only('pk'):
with transaction.atomic():
o = Order.objects.select_related('event').select_for_update().get(pk=o.pk)
if o.status != Order.STATUS_PENDING or o.expiry_reminder_sent:
# Race condition
continue
eventsettings = eventcache.get(o.event.pk, None)
if eventsettings is None:
eventsettings = o.event.settings
eventcache[o.event.pk] = eventsettings
days = eventsettings.get('mail_days_order_expire_warning', as_type=int)
tz = pytz.timezone(eventsettings.get('timezone', settings.TIME_ZONE))
if days and (o.expires - today).days <= days:
with language(o.locale):
o.expiry_reminder_sent = True
o.save()
try:
invoice_name = o.invoice_address.name
invoice_company = o.invoice_address.company
except InvoiceAddress.DoesNotExist:
invoice_name = ""
invoice_company = ""
email_template = eventsettings.mail_text_order_expire_warning
email_context = {
'event': o.event.name,
'url': build_absolute_uri(o.event, 'presale:event.order', kwargs={
'order': o.code,
'secret': o.secret
}),
'expire_date': date_format(o.expires.astimezone(tz), 'SHORT_DATE_FORMAT'),
'invoice_name': invoice_name,
'invoice_company': invoice_company,
}
if eventsettings.payment_term_expire_automatically:
email_subject = _('Your order is about to expire: %(code)s') % {'code': o.code}
else:
email_subject = _('Your order is pending payment: %(code)s') % {'code': o.code}
days = eventsettings.get('mail_days_order_expire_warning', as_type=int)
tz = pytz.timezone(eventsettings.get('timezone', settings.TIME_ZONE))
if days and (o.expires - today).days <= days:
with language(o.locale):
o.expiry_reminder_sent = True
o.save(update_fields=['expiry_reminder_sent'])
try:
invoice_name = o.invoice_address.name
invoice_company = o.invoice_address.company
except InvoiceAddress.DoesNotExist:
invoice_name = ""
invoice_company = ""
email_template = eventsettings.mail_text_order_expire_warning
email_context = {
'event': o.event.name,
'url': build_absolute_uri(o.event, 'presale:event.order', kwargs={
'order': o.code,
'secret': o.secret
}),
'expire_date': date_format(o.expires.astimezone(tz), 'SHORT_DATE_FORMAT'),
'invoice_name': invoice_name,
'invoice_company': invoice_company,
}
if eventsettings.payment_term_expire_automatically:
email_subject = _('Your order is about to expire: %(code)s') % {'code': o.code}
else:
email_subject = _('Your order is pending payment: %(code)s') % {'code': o.code}
try:
o.send_mail(
email_subject, email_template, email_context,
'pretix.event.order.email.expire_warning_sent'
)
except SendMailException:
logger.exception('Reminder email could not be sent')
try:
o.send_mail(
email_subject, email_template, email_context,
'pretix.event.order.email.expire_warning_sent'
)
except SendMailException:
logger.exception('Reminder email could not be sent')
@receiver(signal=periodic_task)
@@ -709,6 +714,7 @@ def send_download_reminders(sender, **kwargs):
today = now().replace(hour=0, minute=0, second=0, microsecond=0)
for e in Event.objects.filter(date_from__gte=today):
days = e.settings.get('mail_days_download_reminder', as_type=int)
if days is None:
continue
@@ -717,30 +723,35 @@ def send_download_reminders(sender, **kwargs):
if now() < reminder_date:
continue
for o in e.orders.filter(status=Order.STATUS_PAID, download_reminder_sent=False):
if not all([r for rr, r in allow_ticket_download.send(e, order=o)]):
continue
for o in e.orders.filter(status=Order.STATUS_PAID, download_reminder_sent=False).only('pk'):
with transaction.atomic():
o = Order.objects.select_related('event').select_for_update().get(pk=o.pk)
if o.download_reminder_sent:
# Race condition
continue
if not all([r for rr, r in allow_ticket_download.send(e, order=o)]):
continue
with language(o.locale):
o.download_reminder_sent = True
o.save()
email_template = e.settings.mail_text_download_reminder
email_context = {
'event': o.event.name,
'url': build_absolute_uri(o.event, 'presale:event.order', kwargs={
'order': o.code,
'secret': o.secret
}),
}
email_subject = _('Your ticket is ready for download: %(code)s') % {'code': o.code}
try:
o.send_mail(
email_subject, email_template, email_context,
'pretix.event.order.email.download_reminder_sent',
attach_tickets=True
)
except SendMailException:
logger.exception('Reminder email could not be sent')
with language(o.locale):
o.download_reminder_sent = True
o.save(update_fields=['download_reminder_sent'])
email_template = e.settings.mail_text_download_reminder
email_context = {
'event': o.event.name,
'url': build_absolute_uri(o.event, 'presale:event.order', kwargs={
'order': o.code,
'secret': o.secret
}),
}
email_subject = _('Your ticket is ready for download: %(code)s') % {'code': o.code}
try:
o.send_mail(
email_subject, email_template, email_context,
'pretix.event.order.email.download_reminder_sent',
attach_tickets=True
)
except SendMailException:
logger.exception('Reminder email could not be sent')
class OrderChangeManager:
@@ -767,6 +778,7 @@ class OrderChangeManager:
def __init__(self, order: Order, user=None, auth=None, notify=True):
self.order = order
self.order.refresh_for_update()
self.user = user
self.auth = auth
self.split_order = None
@@ -1170,7 +1182,7 @@ class OrderChangeManager:
fee.save()
if not self.open_payment.fee:
self.open_payment.fee = fee
self.open_payment.save()
self.open_payment.save(update_fields=['fee'])
elif fee:
fee.delete()

12
src/pretix/control/views/orders.py
View File

@@ -218,7 +218,7 @@ class OrderComment(OrderView):
self.order.log_action('pretix.event.order.checkin_attention', user=self.request.user, data={
'new_value': form.cleaned_data.get('checkin_attention')
})
self.order.save()
self.order.save(update_fields=['checkin_attention', 'comment'])
messages.success(self.request, _('The comment has been updated.'))
else:
messages.error(self.request, _('Could not update the comment.'))
@@ -345,7 +345,7 @@ class OrderRefundProcess(OrderView):
self.order.event.subevents.filter(
id__in=self.order.positions.values_list('subevent_id', flat=True))
)
self.order.save()
self.order.save(update_fields=['status', 'expires'])
messages.success(self.request, _('The refund has been processed.'))
else:
@@ -507,7 +507,7 @@ class OrderRefundView(OrderView):
manual_value = formats.sanitize_separators(manual_value)
try:
manual_value = Decimal(manual_value)
except (DecimalException, TypeError) as e:
except (DecimalException, TypeError):
messages.error(self.request, _('You entered an invalid number.'))
is_valid = False
else:
@@ -530,7 +530,7 @@ class OrderRefundView(OrderView):
offsetting_value = formats.sanitize_separators(offsetting_value)
try:
offsetting_value = Decimal(offsetting_value)
except (DecimalException, TypeError) as e:
except (DecimalException, TypeError):
messages.error(self.request, _('You entered an invalid number.'))
is_valid = False
else:
@@ -561,7 +561,7 @@ class OrderRefundView(OrderView):
value = formats.sanitize_separators(value)
try:
value = Decimal(value)
except (DecimalException, TypeError) as e:
except (DecimalException, TypeError):
messages.error(self.request, _('You entered an invalid number.'))
is_valid = False
else:
@@ -630,7 +630,7 @@ class OrderRefundView(OrderView):
self.order.event.subevents.filter(
id__in=self.order.positions.values_list('subevent_id', flat=True))
)
self.order.save()
self.order.save(update_fields=['status', 'expires'])
return redirect(self.get_order_url())
else:

6
src/pretix/plugins/banktransfer/tasks.py
View File

@@ -142,12 +142,12 @@ def process_banktransfers(self, job: int, data: list) -> None:
code_len = settings.ENTROPY['order_code']
if job.event:
pattern = re.compile(job.event.slug.upper() + "[ \-_]*([A-Z0-9]{%s})" % code_len)
pattern = re.compile(job.event.slug.upper() + r"[ \-_]*([A-Z0-9]{%s})" % code_len)
else:
if not prefixes:
prefixes = [e.slug.upper().replace(".", r"\.").replace("-", r"\-")
prefixes = [e.slug.upper().replace(".", r"\.").replace("-", r"[\- ]*")
for e in job.organizer.events.all()]
pattern = re.compile("(%s)[ \-_]*([A-Z0-9]{%s})" % ("|".join(prefixes), code_len))
pattern = re.compile("(%s)[ \\-_]*([A-Z0-9]{%s})" % ("|".join(prefixes), code_len))
for trans in transactions:
match = pattern.search(trans.reference.replace(" ", "").replace("\n", "").upper())

2
src/setup.cfg
View File

@@ -1,5 +1,5 @@
[flake8]
ignore = N802,W503,E402,C901,E722
ignore = N802,W503,E402,C901,E722,W504,E252
max-line-length = 160
exclude = migrations,.ropeproject,static,mt940.py,_static,build,make_testdata.py,*/testutils/settings.py,tests/settings.py,pretix/base/models/__init__.py
max-complexity = 11

50
src/tests/base/test_orders.py
View File

@@ -19,7 +19,7 @@ from pretix.base.reldate import RelativeDate, RelativeDateWrapper
from pretix.base.services.invoices import generate_invoice
from pretix.base.services.orders import (
OrderChangeManager, OrderError, _create_order, approve_order, deny_order,
expire_orders, send_download_reminders,
expire_orders, send_download_reminders, send_expiry_warnings,
)
@@ -289,6 +289,54 @@ def test_deny(event):
assert 'denied' in djmail.outbox[0].subject
class PaymentReminderTests(TestCase):
def setUp(self):
super().setUp()
o = Organizer.objects.create(name='Dummy', slug='dummy')
self.event = Event.objects.create(
organizer=o, name='Dummy', slug='dummy',
date_from=now() + timedelta(days=2),
plugins='pretix.plugins.banktransfer'
)
self.order = Order.objects.create(
code='FOO', event=self.event, email='dummy@dummy.test',
status=Order.STATUS_PENDING, locale='en',
datetime=now(),
expires=now() + timedelta(days=10),
total=Decimal('46.00'),
)
self.ticket = Item.objects.create(event=self.event, name='Early-bird ticket',
default_price=Decimal('23.00'), admission=True)
self.op1 = OrderPosition.objects.create(
order=self.order, item=self.ticket, variation=None,
price=Decimal("23.00"), attendee_name="Peter", positionid=1
)
djmail.outbox = []
def test_disabled(self):
send_expiry_warnings(sender=self.event)
assert len(djmail.outbox) == 0
def test_sent_once(self):
self.event.settings.mail_days_order_expire_warning = 12
send_expiry_warnings(sender=self.event)
assert len(djmail.outbox) == 1
def test_paid(self):
self.order.status = Order.STATUS_PAID
self.order.save()
send_expiry_warnings(sender=self.event)
assert len(djmail.outbox) == 0
def test_sent_days(self):
self.event.settings.mail_days_order_expire_warning = 9
send_expiry_warnings(sender=self.event)
assert len(djmail.outbox) == 0
self.event.settings.mail_days_order_expire_warning = 10
send_expiry_warnings(sender=self.event)
assert len(djmail.outbox) == 1
class DownloadReminderTests(TestCase):
def setUp(self):
super().setUp()

14
src/tests/plugins/banktransfer/test_import.py
View File

@@ -248,6 +248,20 @@ def test_mark_paid_organizer(env, orga_job):
assert env[2].status == Order.STATUS_PAID
@pytest.mark.django_db
def test_mark_paid_organizer_dash_in_slug(env, orga_job):
env[0].slug = "foo-bar"
env[0].save()
process_banktransfers(orga_job, [{
'payer': 'Karla Kundin',
'reference': 'Bestellung FOO-BAR-1234S',
'date': '2016-01-26',
'amount': '23.00'
}])
env[2].refresh_from_db()
assert env[2].status == Order.STATUS_PAID
@pytest.mark.django_db
def test_mark_paid_organizer_weird_slug(env, orga_job):
env[0].slug = 'du.m-y'