EP/myep
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: enforce submit-time immutability, scope booking cache per user

Browse Source
This commit is contained in:
Björn Fromme
2026-03-16 12:03:00 +01:00
parent 1f087b6c08
commit 49ea562322
6 changed files with 342 additions and 282 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Controller/Booking/Edit/IndexController.php
+36 -6
View File
@@ -21,6 +21,7 @@ use App\Service\BookingEditDataLoaderService;
use App\Service\BookingEditDraftService;
use App\Service\BookingFingerprintService;
use App\Service\BookingService;
use App\Service\BookingEditSubmitGuardService;
use App\Service\BookingSummaryDataService;
use App\Service\ParticipantCardDataService;
use App\Service\TravelDataService;
@@ -52,6 +53,7 @@ class IndexController extends AbstractController
private readonly BookingEditDraftService $draftService,
private readonly TravelDataService $travelDataService,
private readonly BookingService $bookingService,
private readonly BookingEditSubmitGuardService $submitGuard,
private readonly BookingFingerprintService $fingerprintService,
private readonly BookingSummaryDataService $summaryDataService,
private readonly ParticipantCardDataService $participantCardService,
@@ -69,8 +71,11 @@ class IndexController extends AbstractController
#[IsGranted('ROLE_USER')]
public function start(int $id, Request $request): Response
{
/** @var User $user */
$user = $this->getUser();
$this->bookingService->clearBookingDto($request, BookingDto::MODE_EDIT);
$this->dataLoader->invalidateBookingCache($id);
$this->dataLoader->invalidateBookingCache($id, $user);
return $this->redirectToRoute('app_booking_edit', ['id' => $id]);
}
@@ -103,10 +108,6 @@ class IndexController extends AbstractController
// Show flash message if draft was restored
if (true === $this->dataLoader->wasDraftRestored()) {
$this->addFlash('info', 'Dein zuvor gespeicherter Entwurf wurde wiederhergestellt.');
if (true === $this->dataLoader->draftHadImmutableSkips()) {
$this->addFlash('info', 'Einige Entwurfsänderungen wurden nicht übernommen, da diese aktuell nicht mehr änderbar sind.');
}
}
// Fetch booking data for display (surcharges, canceled status, etc.)
@@ -412,6 +413,35 @@ class IndexController extends AbstractController
'booking_id' => $id,
]);
$this->dataLoader->invalidateBookingCache($id, $user);
$freshBookingData = $this->dataLoader->fetchBookingData($id, $user);
if (null === $freshBookingData || $freshBookingData instanceof Notification) {
$this->addFlash('error', 'Buchungsdaten konnten vor dem Speichern nicht neu geladen werden');
$this->logger->warning('Failed to refresh booking before update submission', [
'email' => $email,
'booking_id' => $id,
'has_notification' => $freshBookingData instanceof Notification,
]);
return $this->redirectToRoute('app_booking_edit', ['id' => $id]);
}
$bookingDto->booking = $freshBookingData;
$mutableData = $this->travelDataService->getMutabilityData(
$freshBookingData->dateId,
forceRefresh: true
);
if (null !== $mutableData) {
$this->travelDataService->patchMutability($bookingDto->travel, $mutableData);
}
$immutableChangesReverted = $this->submitGuard->reconcileImmutableCategories($bookingDto, $freshBookingData);
if (true === $immutableChangesReverted) {
$this->bookingService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
$this->addFlash('info', 'Einige Änderungen wurden verworfen, da sie aktuell nicht mehr änderbar sind.');
}
try {
$response = $this->apiClient->updateBooking($bookingDto, true);
if ($response instanceof Notification) {
@@ -427,7 +457,7 @@ class IndexController extends AbstractController
]);
} elseif (true === $response->success) {
// Invalidate cache and clear session on success
$this->dataLoader->invalidateBookingCache($id);
$this->dataLoader->invalidateBookingCache($id, $user);
$this->bookingService->clearBookingDto($request, BookingDto::MODE_EDIT);
// Delete draft on successful submission
src/Service/BookingEditDataLoaderService.php
+13 -18
View File
@@ -53,14 +53,6 @@ class BookingEditDataLoaderService
return $this->draftWasRestored;
}
/**
* Returns whether restored draft data was partially skipped due to immutability.
*/
public function draftHadImmutableSkips(): bool
{
return $this->draftWasRestored && $this->draftService->hadImmutableSkips();
}
/**
* Loads booking data from session or initializes from API.
*
@@ -93,9 +85,8 @@ class BookingEditDataLoaderService
return $this->initializeFromApi($request, $bookingId, $user);
}
// Refresh availability and mutability data to ensure current state
// Availability changes frequently (real-time bookings), so always refresh
// Mutability uses 12h cache (changes daily at cutoff), no force refresh needed
// Refresh availability and mutability data to ensure current state.
// Availability is refreshed on every load; mutability is patched from cache.
$this->travelDataService->enrichWithFreshAvailabilities($formData->travel);
$bookingData = $this->fetchBookingData($bookingId, $user);
@@ -135,9 +126,7 @@ class BookingEditDataLoaderService
return null;
}
// Use cached mutability data (12h TTL ensures daily cutoffs are respected).
// Cache is shared across all users editing this travel date, reducing API load.
// The 12h TTL is shorter than typical mutability cutoff windows (days before travel).
// Use cached mutability data for form/UI state.
$mutableData = $this->travelDataService->getMutabilityData($bookingData->dateId, cached: true);
// Force refresh ensures fresh data at edit start, then populates cache for subsequent loads
$availabilities = $this->travelDataService->getAvailabilityData($bookingData->dateId, cached: true, forceRefresh: true);
@@ -187,7 +176,7 @@ class BookingEditDataLoaderService
*/
public function fetchBookingData(int $bookingId, User $user): Booking|Notification|null
{
$cacheKey = sprintf('bpn_booking_%d', $bookingId);
$cacheKey = $this->getBookingCacheKey($bookingId, $user);
$userTag = self::CACHE_TAG_USER_PREFIX.$user->getId();
$email = $user->getEmail();
$password = $this->crypt->decrypt($user->getPassword());
@@ -207,18 +196,24 @@ class BookingEditDataLoaderService
/**
* Invalidates the booking cache after successful update.
*
* @param int $bookingId The booking ID to invalidate
* @param int $bookingId The booking ID to invalidate
* @param User $user The authenticated user
*/
public function invalidateBookingCache(int $bookingId): void
public function invalidateBookingCache(int $bookingId, User $user): void
{
try {
$cacheKey = sprintf('bpn_booking_%d', $bookingId);
$cacheKey = $this->getBookingCacheKey($bookingId, $user);
$this->bpnCache->delete($cacheKey);
} catch (InvalidArgumentException) {
// Ignore cache deletion errors
}
}
private function getBookingCacheKey(int $bookingId, User $user): string
{
return sprintf('bpn_booking_%d_u_%d', $bookingId, $user->getId());
}
/**
* Invalidates all cached bookings for a user.
*
src/Service/BookingEditDraftService.php
-213
View File
@@ -25,28 +25,6 @@ use Psr\Log\LoggerInterface;
*/
class BookingEditDraftService
{
private const array ADDITIONAL_SERVICE_KEYS = [
'skiPass',
'courses',
'board',
'rentals',
'rentalInsurance',
'additionalServices',
];
private const array TRANSPORTATION_KEYS = [
'transportationOutbound',
'transportationInbound',
'parking',
];
private const array PICKUP_KEYS = [
'pickup',
'dropOff',
];
private bool $draftHadImmutableSkips = false;
public function __construct(
private readonly BookingEditDraftRepository $draftRepository,
private readonly EntityManagerInterface $entityManager,
@@ -134,14 +112,6 @@ class BookingEditDraftService
]);
}
/**
* Returns whether the last draft application skipped fields due to immutability.
*/
public function hadImmutableSkips(): bool
{
return $this->draftHadImmutableSkips;
}
/**
* Applies draft data to a fresh BookingDto loaded from the API.
*
@@ -158,8 +128,6 @@ class BookingEditDraftService
*/
public function applyDraftToDto(BookingEditDraft $draft, BookingDto $dto, Travel $travel): bool
{
$this->draftHadImmutableSkips = false;
try {
$formData = $draft->getFormData();
@@ -228,10 +196,6 @@ class BookingEditDraftService
$participant = $bookingDto->participants[$participantIndex];
$canApplyPersonalDataDraft = $this->canApplyPersonalDataDraft($bookingDto, $participantIndex, $participant);
if (false === $canApplyPersonalDataDraft && $this->hasBlockedPersonalOrAddressChanges($participant, $data)) {
$this->draftHadImmutableSkips = true;
}
// Personal data
if (true === $canApplyPersonalDataDraft && isset($data['personalData']) && true === is_array($data['personalData'])) {
$this->applyPersonalData($participant, $data['personalData']);
@@ -259,7 +223,6 @@ class BookingEditDraftService
// Services
if (true === isset($data['services']) && true === is_array($data['services'])) {
$this->markImmutableServiceSkips($data['services'], $participant, $travel);
$this->applyServiceSelections($participant, $data['services'], $travel);
}
@@ -290,182 +253,6 @@ class BookingEditDraftService
return $participant->mutable;
}
/**
* Checks whether draft payload contains personal or address data sections.
*/
private function hasBlockedPersonalOrAddressChanges(ParticipantDto $participant, array $data): bool
{
$personalData = $data['personalData'] ?? null;
$addressData = $data['address'] ?? null;
if (true === is_array($personalData)) {
if ($this->hasDifferences($personalData, $this->normalizePersonalData($participant), [
'firstName',
'lastName',
'dateOfBirth',
'email',
'mobile',
'gender',
'nationality',
])) {
return true;
}
}
if (true === is_array($addressData)) {
if ($this->hasDifferences($addressData, $this->normalizeAddressData($participant), [
'street',
'postCode',
'city',
'country',
])) {
return true;
}
}
return false;
}
/**
* Marks draft as partially skipped when immutable service categories had draft changes.
*/
private function markImmutableServiceSkips(array $servicesData, ParticipantDto $participant, Travel $travel): void
{
$currentServices = $this->normalizeServiceState($participant);
$this->markImmutableSkipForCategory(
false === $travel->additionalServicesMutable,
$servicesData,
$currentServices,
self::ADDITIONAL_SERVICE_KEYS
);
$this->markImmutableSkipForCategory(
false === $travel->transportationServicesMutable,
$servicesData,
$currentServices,
self::TRANSPORTATION_KEYS
);
$this->markImmutableSkipForCategory(
false === $travel->pickupsMutable,
$servicesData,
$currentServices,
self::PICKUP_KEYS
);
}
/**
* Marks immutable skip flag when category is locked and draft contains related keys.
*/
private function markImmutableSkipForCategory(bool $isImmutable, array $servicesData, array $currentServices, array $categoryKeys): void
{
if ($isImmutable && $this->hasDifferences($servicesData, $currentServices, $categoryKeys)) {
$this->draftHadImmutableSkips = true;
}
}
/**
* Normalizes participant personal data to draft payload shape.
*/
private function normalizePersonalData(ParticipantDto $participant): array
{
return [
'firstName' => $participant->firstName,
'lastName' => $participant->lastName,
'dateOfBirth' => $participant->dateOfBirth?->format('Y-m-d'),
'email' => $participant->email,
'mobile' => $participant->mobile,
'gender' => $participant->gender,
'nationality' => $participant->nationality,
];
}
/**
* Normalizes participant address data to draft payload shape.
*/
private function normalizeAddressData(ParticipantDto $participant): array
{
return [
'street' => $participant->address?->street,
'postCode' => $participant->address?->postCode,
'city' => $participant->address?->city,
'country' => $participant->address?->country,
];
}
/**
* Normalizes participant service state to draft payload shape.
*/
private function normalizeServiceState(ParticipantDto $participant): array
{
return [
'skiPass' => $participant->skiPass?->id,
'courses' => $this->normalizeServiceIds($participant->courses),
'board' => $this->normalizeServiceIds($participant->board),
'rentals' => $this->normalizeServiceIds($participant->rentals),
'rentalInsurance' => $participant->rentalInsurance?->id,
'additionalServices' => $this->normalizeServiceIds($participant->additionalServices),
'transportationOutbound' => $participant->transportationOutbound?->id,
'transportationInbound' => $participant->transportationInbound?->id,
'pickup' => $participant->pickup?->id,
'dropOff' => $participant->dropOff?->id,
'parking' => (bool) $participant->parking,
'insurance' => $participant->insurance?->id,
'bulkInsuranceBooking' => (bool) $participant->bulkInsuranceBooking,
];
}
/**
* Returns sorted unique IDs for service arrays.
*/
private function normalizeServiceIds(array $services): array
{
$ids = array_map(static fn ($service) => $service->id, $services);
$ids = array_values(array_unique($ids));
sort($ids);
return $ids;
}
/**
* Returns true if any of the given keys differ between draft and current values.
*/
private function hasDifferences(array $draftData, array $currentData, array $keys): bool
{
foreach ($keys as $key) {
if (false === array_key_exists($key, $draftData)) {
continue;
}
$draftValue = $this->normalizeComparableValue($draftData[$key]);
$currentValue = $this->normalizeComparableValue($currentData[$key] ?? null);
if ($draftValue !== $currentValue) {
return true;
}
}
return false;
}
/**
* Normalizes scalar/list values for strict comparisons.
*/
private function normalizeComparableValue(mixed $value): mixed
{
if (is_array($value)) {
$normalized = array_map(fn ($item) => $this->normalizeComparableValue($item), array_values($value));
sort($normalized);
return $normalized;
}
if ('' === $value) {
return null;
}
return $value;
}
/**
* Applies personal data fields to participant.
*/
src/Service/BookingEditSubmitGuardService.php
+185
View File
@@ -0,0 +1,185 @@
<?php
declare(strict_types=1);
namespace App\Service;
use App\BusProNet\DataProcessor\BookingDataProcessor;
use App\BusProNet\Model\Booking;
use App\BusProNet\Model\Pickup;
use App\BusProNet\Model\Service;
use App\Form\Model\BookingDto;
use App\Form\Model\ParticipantDto;
/**
* Enforces immutable edit categories before submitting booking updates.
*
* This guard reconciles the in-session edit DTO with a fresh booking snapshot.
* If a mutability cutoff passed during an edit session, stale user changes in
* immutable categories are reverted to the current booking state so update
* payloads cannot include blocked changes.
*/
class BookingEditSubmitGuardService
{
public function __construct(
private readonly BookingDataProcessor $bookingDataProcessor,
) {
}
/**
* Reverts immutable category changes to fresh booking values.
*
* @return bool True when at least one immutable field was reverted
*/
public function reconcileImmutableCategories(BookingDto $workingDto, Booking $freshBooking): bool
{
$baselineDto = $this->bookingDataProcessor->createBookingDtoFromBooking($freshBooking, $workingDto->travel);
$changed = false;
foreach ($workingDto->participants as $index => $participant) {
$baseline = $baselineDto->participants[$index] ?? null;
if (null === $baseline) {
continue;
}
if (false === $workingDto->travel->additionalServicesMutable) {
$changed = $this->reconcileAdditionalServices($participant, $baseline) || $changed;
}
if (false === $workingDto->travel->transportationServicesMutable) {
$changed = $this->reconcileTransportationServices($participant, $baseline) || $changed;
}
if (false === $workingDto->travel->pickupsMutable) {
$changed = $this->reconcilePickups($participant, $baseline) || $changed;
}
}
return $changed;
}
private function reconcileAdditionalServices(ParticipantDto $participant, ParticipantDto $baseline): bool
{
$changed = false;
if (false === $this->areServiceListsEqual($participant->courses, $baseline->courses)) {
$participant->courses = $baseline->courses;
$changed = true;
}
if (false === $this->areServiceListsEqual($participant->additionalServices, $baseline->additionalServices)) {
$participant->additionalServices = $baseline->additionalServices;
$changed = true;
}
if (false === $this->areServiceListsEqual($participant->board, $baseline->board)) {
$participant->board = $baseline->board;
$changed = true;
}
if (false === $this->areServiceListsEqual($participant->rentals, $baseline->rentals)) {
$participant->rentals = $baseline->rentals;
$changed = true;
}
if (false === $this->isSameService($participant->skiPass, $baseline->skiPass)) {
$participant->skiPass = $baseline->skiPass;
$changed = true;
}
if (false === $this->isSameService($participant->rentalInsurance, $baseline->rentalInsurance)) {
$participant->rentalInsurance = $baseline->rentalInsurance;
$changed = true;
}
if ($participant->rentalInsuranceSelected !== $baseline->rentalInsuranceSelected) {
$participant->rentalInsuranceSelected = $baseline->rentalInsuranceSelected;
$changed = true;
}
return $changed;
}
private function reconcileTransportationServices(ParticipantDto $participant, ParticipantDto $baseline): bool
{
$changed = false;
if (false === $this->isSameService($participant->transportationOutbound, $baseline->transportationOutbound)) {
$participant->transportationOutbound = $baseline->transportationOutbound;
$changed = true;
}
if (false === $this->isSameService($participant->transportationInbound, $baseline->transportationInbound)) {
$participant->transportationInbound = $baseline->transportationInbound;
$changed = true;
}
if ($participant->parking !== $baseline->parking) {
$participant->parking = $baseline->parking;
$changed = true;
}
if (false === $this->isSameService($participant->parkingService, $baseline->parkingService)) {
$participant->parkingService = $baseline->parkingService;
$changed = true;
}
return $changed;
}
private function reconcilePickups(ParticipantDto $participant, ParticipantDto $baseline): bool
{
$changed = false;
if (false === $this->isSamePickup($participant->pickup, $baseline->pickup)) {
$participant->pickup = $baseline->pickup;
$changed = true;
}
if (false === $this->isSamePickup($participant->dropOff, $baseline->dropOff)) {
$participant->dropOff = $baseline->dropOff;
$changed = true;
}
if ($participant->differentDropOff !== $baseline->differentDropOff) {
$participant->differentDropOff = $baseline->differentDropOff;
$changed = true;
}
return $changed;
}
private function areServiceListsEqual(array $left, array $right): bool
{
return $this->normalizeServiceIds($left) === $this->normalizeServiceIds($right);
}
/**
* @param Service[] $services
*
* @return int[]
*/
private function normalizeServiceIds(array $services): array
{
$ids = [];
foreach ($services as $service) {
$ids[] = $service->id;
}
$ids = array_values(array_unique($ids));
sort($ids);
return $ids;
}
private function isSameService(?Service $left, ?Service $right): bool
{
return $left?->id === $right?->id;
}
private function isSamePickup(?Pickup $left, ?Pickup $right): bool
{
return $left?->id === $right?->id;
}
}