chore: code cleanup

This commit is contained in:
fromme committed 2026-09-17 16:10:34 +02:00
1 parent 1a3640b15c
commit 5a01054b0e
65 files changed
+1046 -795

No files matched your search

+187
View File
@@ -0,0 +1,187 @@
<?php
declare(strict_types=1);
namespace App\Service;
use App\Entity\Booking;
use App\Entity\BookingSeries;
use App\Entity\Resource;
use App\Entity\User;
use App\Exception\AlreadyHoldingResourceException;
use App\Exception\ResourceAlreadyBookedException;
use App\Model\SeriesResult;
use App\Model\TimeRange;
use App\Repository\BookingRepository;
use Doctrine\DBAL\LockMode;
use Doctrine\ORM\EntityManagerInterface;
/**
* Every write to a booking goes through here, because every write needs the same guard.
*
* **How double-booking is prevented, and why it changed.** While a booking was a whole day, a
* UNIQUE(resource_id, date) index did the job and the database rejected the loser of a race. A
* booking is now a time *range*, and an overlap is not something a unique index can express --
* nor does MariaDB have PostgreSQL's exclusion constraints. So the serialisation point moved
* into the application: each write opens a transaction, takes a row lock on the Resource
* (SELECT ... FOR UPDATE), and only then looks for overlaps. Two concurrent requests for the
* same resource queue up on that lock, so the second one sees the first one's row.
*
* This is still a real guard rather than the racy check-then-insert the plan warned about -- the
* check happens while nobody else can be inserting for that resource. The cost is that the
* protection now lives in code, so it belongs to this class alone: nothing else may persist a
* Booking.
*
* The per-user rule ("not two desks at once") is deliberately NOT covered by the lock: it spans
* resources, and locking every resource would serialise the whole app to protect against a user
* racing themselves in two browser tabs.
*/
final readonly class BookingManager
{
public function __construct(
private EntityManagerInterface $em,
private BookingRepository $bookings,
) {
}
/**
* @throws AlreadyHoldingResourceException if the user already holds a resource of this kind in that span
* @throws ResourceAlreadyBookedException if somebody else does
*/
public function book(User $user, Resource $resource, \DateTimeImmutable $date, ?TimeRange $time = null): Booking
{
// All day is the default, as it was before ranges existed.
$time ??= TimeRange::wholeDay();
return $this->em->wrapInTransaction(function () use ($user, $resource, $date, $time): Booking {
$this->em->lock($resource, LockMode::PESSIMISTIC_WRITE);
$this->assertFree($user, $resource, $date, $time);
$booking = new Booking($user, $resource, $date, $time);
$this->em->persist($booking);
$this->em->flush();
return $booking;
});
}
/**
* Materialises a series into concrete bookings, skipping days that are already taken.
*
* The resource is locked once for the whole series rather than per occurrence, so the set of
* days either all see the same world or none do. Occurrences fall on distinct days, so the
* not-yet-flushed ones cannot collide with each other.
*/
public function bookSeries(BookingSeries $series): SeriesResult
{
return $this->em->wrapInTransaction(function () use ($series): SeriesResult {
$resource = $series->getResource();
$user = $series->getUser();
$time = $series->getTimeRange();
$this->em->lock($resource, LockMode::PESSIMISTIC_WRITE);
$created = [];
$skipped = [];
foreach ($series->occurrences() as $date) {
try {
$this->assertFree($user, $resource, $date, $time);
} catch (AlreadyHoldingResourceException|ResourceAlreadyBookedException $e) {
$skipped[$date->format('Y-m-d')] = $e->getMessage();
continue;
}
$booking = new Booking($user, $resource, $date, $time, $series);
$this->em->persist($booking);
$created[] = $booking;
}
if ([] !== $created) {
$this->em->persist($series);
}
$this->em->flush();
return new SeriesResult($series, $created, $skipped);
});
}
/**
* Moves one occurrence to different times, leaving the rest of its series alone.
*/
public function reschedule(Booking $booking, TimeRange $time): void
{
$this->em->wrapInTransaction(function () use ($booking, $time): void {
$resource = $booking->getResource();
$this->em->lock($resource, LockMode::PESSIMISTIC_WRITE);
$this->assertFree($booking->getUser(), $resource, $booking->getDate(), $time, $booking);
$booking->changeTime($time);
$this->em->flush();
});
}
public function release(Booking $booking): void
{
$this->em->remove($booking);
$this->em->flush();
}
/**
* Ends a series from a day onwards: its remaining occurrences go, and the series itself is
* either shortened or dropped if nothing is left of it.
*
* @return int the number of occurrences removed
*/
public function endSeries(BookingSeries $series, \DateTimeImmutable $from): int
{
return $this->em->wrapInTransaction(function () use ($series, $from): int {
$from = $from->setTime(0, 0);
$removed = 0;
foreach ($this->bookings->findForSeriesFrom($series, $from) as $booking) {
$this->em->remove($booking);
++$removed;
}
if ($series->getStartDate() >= $from) {
$this->em->remove($series);
} else {
$series->setEndDate($from->modify('-1 day'));
}
$this->em->flush();
return $removed;
});
}
/**
* @throws AlreadyHoldingResourceException
* @throws ResourceAlreadyBookedException
*/
private function assertFree(
User $user,
Resource $resource,
\DateTimeImmutable $date,
TimeRange $time,
?Booking $ignore = null,
): void {
$mine = $this->bookings->findOverlappingForUser($user, $date, $time, $resource->getType(), $ignore);
if (null !== $mine) {
throw new AlreadyHoldingResourceException($mine);
}
foreach ($this->bookings->findOverlapping($resource, $date, $time) as $clash) {
if (null !== $ignore && $clash->getId() === $ignore->getId()) {
continue;
}
throw new ResourceAlreadyBookedException($resource, $date, $clash->getTimeRange());
}
}
}