From 4f02529b152e9c5dda5d819b377d569b6e3cceaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= <fromme@dreipunktnull.com>
Date: Fri, 26 Sep 2025 08:21:57 +0200
Subject: [PATCH] feat: improved error handling of xml parsers and loaders

---
 src/BusProNet/XmlLoader/HotelLoader.php       | 19 ++++-
 src/BusProNet/XmlLoader/TravelLoader.php      | 37 ++++++++--
 .../Booking/CreateInitController.php          | 60 +++++++++++++++
 .../Booking/CreateStep1Controller.php         |  5 +-
 src/Exception/HotelNotFoundException.php      | 26 +++++++
 src/Exception/HotelNotInTravelException.php   | 27 +++++++
 src/Exception/TravelNotFoundException.php     | 26 +++++++
 src/Service/BookingService.php                | 73 ++++++++++++++-----
 src/Service/TravelDataService.php             | 26 ++++---
 9 files changed, 257 insertions(+), 42 deletions(-)
 create mode 100644 src/Controller/Booking/CreateInitController.php
 create mode 100644 src/Exception/HotelNotFoundException.php
 create mode 100644 src/Exception/HotelNotInTravelException.php
 create mode 100644 src/Exception/TravelNotFoundException.php

diff --git a/src/BusProNet/XmlLoader/HotelLoader.php b/src/BusProNet/XmlLoader/HotelLoader.php
index 28f6320..3ad1932 100644
--- a/src/BusProNet/XmlLoader/HotelLoader.php
+++ b/src/BusProNet/XmlLoader/HotelLoader.php
@@ -4,6 +4,7 @@ namespace App\BusProNet\XmlLoader;
 
 use App\BusProNet\Model\Hotel;
 use App\BusProNet\Model\Travel;
+use App\Exception\HotelNotFoundException;
 use Psr\Cache\InvalidArgumentException;
 use Symfony\Component\DomCrawler\Crawler;
 use Symfony\Contracts\Cache\ItemInterface;
@@ -47,11 +48,15 @@ class HotelLoader extends AbstractLoader
         return null;
     }
 
-    public function loadById(int $id, ?string $filename = 'hotel.xml'): ?Hotel
+    public function loadById(int $id, ?string $filename = 'hotel.xml'): Hotel
     {
         $hotels = $this->loadAll($filename);
 
-        return $hotels[$id] ?? null;
+        if (false === isset($hotels[$id])) {
+            throw new HotelNotFoundException($id);
+        }
+
+        return $hotels[$id];
     }
 
     private function loadXml(?string $filename = 'hotel.xml'): Crawler
@@ -78,7 +83,13 @@ class HotelLoader extends AbstractLoader
     public function patchHotelDetails(Travel $travel): void
     {
         $hotelId = $travel->hotelId;
-        $hotel = $this->loadById($hotelId);
-        $travel->hotel = $hotel;
+        try {
+            $hotel = $this->loadById($hotelId);
+            $travel->hotel = $hotel;
+        } catch (HotelNotFoundException $e) {
+            // If hotel details can't be loaded, leave travel->hotel as null
+            // This allows the travel to be processed even if hotel details are missing
+            $travel->hotel = null;
+        }
     }
 }
diff --git a/src/BusProNet/XmlLoader/TravelLoader.php b/src/BusProNet/XmlLoader/TravelLoader.php
index 8f7df37..cc62571 100644
--- a/src/BusProNet/XmlLoader/TravelLoader.php
+++ b/src/BusProNet/XmlLoader/TravelLoader.php
@@ -7,6 +7,8 @@ use App\BusProNet\Model\MutableData;
 use App\BusProNet\Model\Travel;
 use App\BusProNet\Utility\DateCodeUtility;
 use App\BusProNet\XmlParser\TravelParser;
+use App\Exception\HotelNotInTravelException;
+use App\Exception\TravelNotFoundException;
 use League\Flysystem\FilesystemException;
 use League\Flysystem\FilesystemOperator;
 use League\Flysystem\StorageAttributes;
@@ -160,14 +162,18 @@ class TravelLoader extends AbstractLoader
      *
      * Retrieves travel data from XML exports. If filename is provided, loads directly
      * from that file. Otherwise, uses the cached mapping to find the appropriate file.
+     * Validates that both travel and hotel (if specified) exist before attempting to parse.
      *
      * @param int         $dateId   The travel date ID to load
      * @param int|null    $hotelId  Optional hotel ID for specific hotel data
      * @param string|null $filename Optional filename to load from directly
      *
-     * @return Travel|null The loaded travel object or null if not found
+     * @return Travel The loaded travel object
+     *
+     * @throws TravelNotFoundException      When travel ID is not found
+     * @throws HotelNotInTravelException    When hotel ID exists but not for this travel
      */
-    public function loadById(int $dateId, ?int $hotelId = null, ?string $filename = null): ?Travel
+    public function loadById(int $dateId, ?int $hotelId = null, ?string $filename = null): Travel
     {
         if (null !== $filename) {
             return $this->loadXml($dateId, $hotelId, $filename);
@@ -175,8 +181,14 @@ class TravelLoader extends AbstractLoader
 
         $mapping = $this->generateFilesMap();
 
+        // Validate travel exists
         if (false === isset($mapping[$dateId])) {
-            return null;
+            throw new TravelNotFoundException($dateId);
+        }
+
+        // Validate hotel exists in this travel if specified
+        if (null !== $hotelId && false === isset($mapping[$dateId]['hotels'][$hotelId])) {
+            throw new HotelNotInTravelException($dateId, $hotelId);
         }
 
         $filename = $mapping[$dateId]['file'];
@@ -194,9 +206,12 @@ class TravelLoader extends AbstractLoader
      * @param int|null $hotelId  Optional hotel ID for specific hotel data
      * @param string   $filename The XML filename to load from
      *
-     * @return Travel|null The loaded travel object or null if not found
+     * @return Travel The loaded travel object
+     *
+     * @throws TravelNotFoundException      When travel ID is not found in XML
+     * @throws HotelNotInTravelException    When hotel ID is not found in travel XML
      */
-    private function loadXml(int $dateId, ?int $hotelId, string $filename): ?Travel
+    private function loadXml(int $dateId, ?int $hotelId, string $filename): Travel
     {
         try {
             $xml = $this->xmlExport->read($filename);
@@ -205,12 +220,20 @@ class TravelLoader extends AbstractLoader
             $travelNode = $crawler->filterXPath(sprintf('//reise/termin[@idbuspro="%d"]', $dateId));
 
             if (0 === $travelNode->count()) {
-                return null;
+                throw new TravelNotFoundException($dateId);
+            }
+
+            // Validate hotel exists in travel XML if specified
+            if (null !== $hotelId) {
+                $hotelNode = $travelNode->filterXPath(sprintf('.//hotel[@idbuspro="%d"]', $hotelId));
+                if (0 === $hotelNode->count()) {
+                    throw new HotelNotInTravelException($dateId, $hotelId);
+                }
             }
 
             return $this->travelParser->parse($travelNode->first(), $hotelId);
         } catch (FilesystemException $e) {
-            return null;
+            throw new TravelNotFoundException($dateId, $e);
         }
     }
 
diff --git a/src/Controller/Booking/CreateInitController.php b/src/Controller/Booking/CreateInitController.php
new file mode 100644
index 0000000..80f9a5c
--- /dev/null
+++ b/src/Controller/Booking/CreateInitController.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Controller\Booking;
+
+use App\Exception\HotelNotFoundException;
+use App\Exception\HotelNotInTravelException;
+use App\Exception\NoRoomsAvailableException;
+use App\Exception\TravelNotFoundException;
+use App\Service\BookingService;
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Attribute\Route;
+
+/**
+ * Handles the initialization of new booking sessions.
+ *
+ * This controller provides a clean entry point for starting new booking flows
+ * without requiring random UID parameters. It creates fresh booking sessions
+ * and redirects to the first step of the booking process.
+ */
+class CreateInitController extends AbstractController
+{
+    public function __construct(
+        private readonly BookingService $bookingService,
+    ) {
+    }
+
+    /**
+     * Initializes a fresh booking session and redirects to step 1.
+     *
+     * This endpoint provides a clean way to start the booking flow with just
+     * dateId and hotelId parameters. It clears any existing booking session
+     * and creates a fresh BookingCreateDto before redirecting to step 1.
+     */
+    #[Route('/bookings/create/{dateId}/{hotelId}', name: 'app_booking_create_init', requirements: ['dateId' => '\d+', 'hotelId' => '\d+'])]
+    public function init(Request $request, int $dateId, int $hotelId): Response
+    {
+        try {
+            // Clear any existing booking session to ensure fresh start
+            $this->bookingService->clearBookingSession($request);
+
+            // Create fresh booking session with the provided parameters
+            $this->bookingService->startFreshBooking($request, $dateId, $hotelId);
+
+            // Redirect to step 1 of the booking flow
+            return $this->redirectToRoute('app_booking_create_step_1');
+        } catch (TravelNotFoundException $e) {
+            throw $this->createNotFoundException(sprintf('Travel not found for date ID %d', $dateId));
+        } catch (HotelNotFoundException $e) {
+            throw $this->createNotFoundException(sprintf('Hotel not found for hotel ID %d', $hotelId));
+        } catch (HotelNotInTravelException $e) {
+            throw $this->createNotFoundException(sprintf('Hotel ID %d is not available for travel ID %d', $hotelId, $dateId));
+        } catch (NoRoomsAvailableException $e) {
+            throw $this->createNotFoundException('No rooms available for this travel.');
+        }
+    }
+}
diff --git a/src/Controller/Booking/CreateStep1Controller.php b/src/Controller/Booking/CreateStep1Controller.php
index d9fb319..bc60751 100644
--- a/src/Controller/Booking/CreateStep1Controller.php
+++ b/src/Controller/Booking/CreateStep1Controller.php
@@ -36,8 +36,6 @@ class CreateStep1Controller extends AbstractController
         try {
             $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request);
         } catch (NoRoomsAvailableException $e) {
-            $this->addFlash('error', 'Leider sind für diese Reise aktuell keine Zimmer verfügbar.');
-            // TODO: Redirect to travel listing or hotel details page
             throw $this->createNotFoundException('No rooms available for this travel.');
         }
 
@@ -93,8 +91,7 @@ class CreateStep1Controller extends AbstractController
         try {
             $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request);
         } catch (NoRoomsAvailableException $e) {
-            // For HTMX requests, return a simple error message
-            return new Response('<div class="text-red-700 p-4">Keine Zimmer verfügbar</div>', 400);
+            throw $this->createNotFoundException('No rooms available for this travel.');
         }
 
         // Process the form to update the DTO with the latest room selection
diff --git a/src/Exception/HotelNotFoundException.php b/src/Exception/HotelNotFoundException.php
new file mode 100644
index 0000000..6d32785
--- /dev/null
+++ b/src/Exception/HotelNotFoundException.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Exception;
+
+use Symfony\Component\HttpKernel\Exception\HttpException;
+
+/**
+ * Exception thrown when attempting to load hotel data that does not exist.
+ *
+ * This exception is used when a hotel ID cannot be found in the
+ * available hotel data sources.
+ */
+class HotelNotFoundException extends HttpException
+{
+    public function __construct(int $hotelId, ?\Throwable $previous = null)
+    {
+        $message = sprintf(
+            'Hotel not found for hotel ID %d',
+            $hotelId
+        );
+
+        parent::__construct(404, $message, $previous);
+    }
+}
\ No newline at end of file
diff --git a/src/Exception/HotelNotInTravelException.php b/src/Exception/HotelNotInTravelException.php
new file mode 100644
index 0000000..b871d9c
--- /dev/null
+++ b/src/Exception/HotelNotInTravelException.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Exception;
+
+use Symfony\Component\HttpKernel\Exception\HttpException;
+
+/**
+ * Exception thrown when attempting to load a hotel that exists but is not available for the specified travel.
+ *
+ * This exception is used when a hotel ID exists in the system but is not
+ * associated with the requested travel/date ID.
+ */
+class HotelNotInTravelException extends HttpException
+{
+    public function __construct(int $dateId, int $hotelId, ?\Throwable $previous = null)
+    {
+        $message = sprintf(
+            'Hotel ID %d is not available for travel ID %d',
+            $hotelId,
+            $dateId
+        );
+
+        parent::__construct(404, $message, $previous);
+    }
+}
\ No newline at end of file
diff --git a/src/Exception/TravelNotFoundException.php b/src/Exception/TravelNotFoundException.php
new file mode 100644
index 0000000..acda097
--- /dev/null
+++ b/src/Exception/TravelNotFoundException.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Exception;
+
+use Symfony\Component\HttpKernel\Exception\HttpException;
+
+/**
+ * Exception thrown when attempting to load travel data that does not exist.
+ *
+ * This exception is used when a travel/date ID cannot be found in the
+ * available travel data sources (XML files or API).
+ */
+class TravelNotFoundException extends HttpException
+{
+    public function __construct(int $dateId, ?\Throwable $previous = null)
+    {
+        $message = sprintf(
+            'Travel data not found for date ID %d',
+            $dateId
+        );
+
+        parent::__construct(404, $message, $previous);
+    }
+}
\ No newline at end of file
diff --git a/src/Service/BookingService.php b/src/Service/BookingService.php
index 8335568..f2ef117 100644
--- a/src/Service/BookingService.php
+++ b/src/Service/BookingService.php
@@ -55,21 +55,20 @@ class BookingService
 
     public function getOrCreateBookingCreateDto(Request $request): BookingCreateDto
     {
-        $bookingCreateKey = self::BOOKING_CREATE_KEY;
-        $bookingUuid = $request->query->get('uid');
-        $bookingCreateDto = $request->getSession()->get($bookingCreateKey);
+        $bookingCreateDto = $request->getSession()->get(self::BOOKING_CREATE_KEY);
 
-        // No UID parameter - return existing DTO from session if available
-        if (null === $bookingUuid && null !== $bookingCreateDto) {
+        // Return existing DTO from session if available
+        if (null !== $bookingCreateDto) {
             return $bookingCreateDto;
         }
 
-        // Create a new DTO - we need date_id and hotel_id for this
+        // Legacy support: Create a new DTO if date_id and hotel_id are provided
+        // This maintains backward compatibility for existing URLs with UID parameters
         $dateId = $request->query->getInt('date_id');
         $hotelId = $request->query->getInt('hotel_id');
 
         if (0 === $dateId || 0 === $hotelId) {
-            throw new NotFoundHttpException('Missing date_id or hotel_id parameters');
+            throw new NotFoundHttpException('No booking session found. Please start a new booking.');
         }
 
         $travelData = $this->travelDataService->getTravelData($dateId, $hotelId);
@@ -102,6 +101,55 @@ class BookingService
         $request->getSession()->set(self::BOOKING_CREATE_KEY, $bookingCreateDto);
     }
 
+    /**
+     * Clears all booking-related session data.
+     *
+     * This method removes all booking session data including the main DTO
+     * and any cached snapshots to ensure a completely fresh start.
+     */
+    public function clearBookingSession(Request $request): void
+    {
+        $session = $request->getSession();
+        $session->remove(self::BOOKING_CREATE_KEY);
+        $session->remove(self::BOOKING_CREATE_BASELINE_KEY);
+    }
+
+    /**
+     * Creates a fresh booking session with the provided travel parameters.
+     *
+     * This method initializes a new BookingCreateDto with empty room selections
+     * and saves it to the session. It's designed to be called from the clean
+     * booking entry point without requiring UID parameters.
+     */
+    public function startFreshBooking(Request $request, int $dateId, int $hotelId): BookingCreateDto
+    {
+        $travelData = $this->travelDataService->getTravelData($dateId, $hotelId);
+        if (null === $travelData) {
+            throw new NotFoundHttpException(sprintf('Travel data not found for date ID %d and hotel ID %d', $dateId, $hotelId));
+        }
+
+        $availableRooms = $travelData->getAvailableRooms();
+
+        // Prevent booking flow entry when no rooms are available
+        if (empty($availableRooms)) {
+            throw new NoRoomsAvailableException($dateId, $hotelId);
+        }
+
+        // Create room selections with zero quantities (user will set these in step 1)
+        $roomSelections = array_map(
+            fn (Room $room) => $this->createRoomSelection($room, []),
+            $availableRooms
+        );
+
+        $bookingCreateDto = new BookingCreateDto($travelData, $hotelId);
+        $bookingCreateDto->roomSelections = $roomSelections;
+        $bookingCreateDto->currentStep = 1;
+
+        $this->saveBookingCreateDto($request, $bookingCreateDto);
+
+        return $bookingCreateDto;
+    }
+
     private function createRoomSelection(Room $room, array $roomsIdsAndQuantities): RoomSelectionDto
     {
         $selection = new RoomSelectionDto();
@@ -229,17 +277,6 @@ class BookingService
         return $groups;
     }
 
-    /**
-     * Determines if the room selection has changed between two DTOs.
-     */
-    public function shouldResetAssignments(BookingCreateDto $oldDto, BookingCreateDto $newDto): bool
-    {
-        $old = array_map(fn ($roomSelectionDto) => [$roomSelectionDto->roomId, $roomSelectionDto->quantity], $oldDto->roomSelections);
-        $new = array_map(fn ($roomSelectionDto) => [$roomSelectionDto->roomId, $roomSelectionDto->quantity], $newDto->roomSelections);
-
-        return $old !== $new;
-    }
-
     /**
      * Resets all participant room assignments in the DTO.
      */
diff --git a/src/Service/TravelDataService.php b/src/Service/TravelDataService.php
index 8864819..ede35c6 100644
--- a/src/Service/TravelDataService.php
+++ b/src/Service/TravelDataService.php
@@ -12,6 +12,9 @@ use App\BusProNet\Model\Travel;
 use App\BusProNet\XmlLoader\HotelLoader;
 use App\BusProNet\XmlLoader\PickupLoader;
 use App\BusProNet\XmlLoader\TravelLoader;
+use App\Exception\HotelNotFoundException;
+use App\Exception\HotelNotInTravelException;
+use App\Exception\TravelNotFoundException;
 use Psr\Cache\InvalidArgumentException;
 use Psr\Log\LoggerInterface;
 use Symfony\Contracts\Cache\CacheInterface;
@@ -101,15 +104,6 @@ class TravelDataService
         try {
             $travel = $this->travelLoader->loadById($dateId, $hotelId);
 
-            if (null === $travel) {
-                $this->logger->debug('Travel not found in XML', [
-                    'dateId' => $dateId,
-                    'hotelId' => $hotelId,
-                ]);
-
-                return null;
-            }
-
             $this->enrichTravelData($travel);
             $this->logger->debug('Travel data loaded from XML', [
                 'dateId' => $dateId,
@@ -118,6 +112,20 @@ class TravelDataService
             ]);
 
             return $travel;
+        } catch (TravelNotFoundException $e) {
+            $this->logger->debug('Travel not found in XML', [
+                'dateId' => $dateId,
+                'hotelId' => $hotelId,
+                'error' => $e->getMessage(),
+            ]);
+            throw $e;
+        } catch (HotelNotFoundException | HotelNotInTravelException $e) {
+            $this->logger->debug('Hotel not found in XML', [
+                'dateId' => $dateId,
+                'hotelId' => $hotelId,
+                'error' => $e->getMessage(),
+            ]);
+            throw $e;
         } catch (\Exception $e) {
             $this->logger->error('Failed to load travel data from XML', [
                 'dateId' => $dateId,