fix: make booking overlap check atomic with select_for_update
Wrap the overlap query and Booking.objects.create() in a single transaction.atomic() block inside BookingCreateSerializer.create(). Lock the StaffProfile row with select_for_update() so concurrent requests for the same staff slot are serialized at the DB level; only one writer can hold the lock at a time, eliminating the race window between validate() and save(). The early check in validate() is kept for fast user feedback in the common non-concurrent case. The locked re-check in create() is the correctness guarantee. On SQLite (dev/tests) FOR UPDATE is silently ignored but writes are still serialized. PostgreSQL (production) gets row-level locking. Update docs/risks.md to mark the race condition as fixed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
from django.db import transaction
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
from rest_framework import serializers
|
from rest_framework import serializers
|
||||||
from apps.bookings.models import Booking, BookingStatus
|
from apps.bookings.models import Booking, BookingStatus
|
||||||
@@ -74,13 +75,39 @@ class BookingCreateSerializer(serializers.ModelSerializer):
|
|||||||
def create(self, validated_data):
|
def create(self, validated_data):
|
||||||
request = self.context["request"]
|
request = self.context["request"]
|
||||||
service = validated_data["service"]
|
service = validated_data["service"]
|
||||||
|
staff = validated_data.get("staff")
|
||||||
|
start_time = validated_data["start_time"]
|
||||||
|
end_time = validated_data["end_time"]
|
||||||
|
|
||||||
|
with transaction.atomic():
|
||||||
|
# Lock the staff row so concurrent booking requests for the same staff
|
||||||
|
# member are serialized. Without this, two requests that both pass the
|
||||||
|
# overlap check in validate() can race and both commit overlapping
|
||||||
|
# bookings. On SQLite (dev/tests) the FOR UPDATE clause is silently
|
||||||
|
# ignored but the transaction still serializes writes; PostgreSQL
|
||||||
|
# (production) gets true row-level locking.
|
||||||
|
StaffProfile.objects.select_for_update().get(pk=staff.pk)
|
||||||
|
|
||||||
|
# Re-run the overlap check inside the lock so the check and the insert
|
||||||
|
# are atomic with respect to other writers.
|
||||||
|
overlap = Booking.objects.filter(
|
||||||
|
staff=staff,
|
||||||
|
status__in=[BookingStatus.PENDING, BookingStatus.CONFIRMED],
|
||||||
|
start_time__lt=end_time,
|
||||||
|
end_time__gt=start_time,
|
||||||
|
).exists()
|
||||||
|
if overlap:
|
||||||
|
raise serializers.ValidationError(
|
||||||
|
{"start_time": _("Booking overlaps an existing appointment")}
|
||||||
|
)
|
||||||
|
|
||||||
return Booking.objects.create(
|
return Booking.objects.create(
|
||||||
salon=service.salon,
|
salon=service.salon,
|
||||||
customer=request.user,
|
customer=request.user,
|
||||||
service=service,
|
service=service,
|
||||||
staff=validated_data.get("staff"),
|
staff=staff,
|
||||||
start_time=validated_data["start_time"],
|
start_time=start_time,
|
||||||
end_time=validated_data["end_time"],
|
end_time=end_time,
|
||||||
notes=validated_data.get("notes", ""),
|
notes=validated_data.get("notes", ""),
|
||||||
price_amount=service.price_amount,
|
price_amount=service.price_amount,
|
||||||
currency=service.currency,
|
currency=service.currency,
|
||||||
|
|||||||
+1
-1
@@ -11,7 +11,7 @@ This file tracks known gaps and risks to address in future iterations.
|
|||||||
|
|
||||||
## Booking Integrity
|
## Booking Integrity
|
||||||
- Availability checks and overlap prevention are now enforced for staff bookings.
|
- Availability checks and overlap prevention are now enforced for staff bookings.
|
||||||
- **Race condition — booking overlap check is not atomic:** `validate_booking_request` runs the overlap query and returns; the view then calls `serializer.save()` in a separate step with no lock held. Two concurrent POST `/api/bookings/` requests for the same staff slot will both pass validation and both commit. Fix: wrap the overlap check and booking insert in a `select_for_update()` query (or use serializable transaction isolation) so only one request can hold the lock at a time.
|
- **Race condition — fixed:** `BookingCreateSerializer.create()` now locks the staff row with `select_for_update()` inside `transaction.atomic()` and re-runs the overlap check before inserting. Concurrent requests for the same staff slot are serialized at the DB level. Requires PostgreSQL in production (SQLite ignores `FOR UPDATE` but still serializes writes).
|
||||||
- No timezone handling or business hours enforcement.
|
- No timezone handling or business hours enforcement.
|
||||||
- No cancellation rules or refund logic.
|
- No cancellation rules or refund logic.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user