diff --git a/docs/contingent-api-plan.md b/docs/contingent-api-plan.md index 346795d..120c85c 100644 --- a/docs/contingent-api-plan.md +++ b/docs/contingent-api-plan.md @@ -1,162 +1,39 @@ -# Contingent API with XML Caching (Crawler-Based) — Plan (Updated) +# Contingent API Status ## Summary -Implement a cache-only contingent API backed by XML files in `var/xmlexportzimmer`, using Symfony cache (tag-aware) and Flysystem. Parsing uses DOM/Crawler and caches DTO arrays only. Add loader/parser/services/controllers and integrate cache invalidation into the existing XML sync flow. Provide endpoints for contingents, rooms, and calendar events. +The contingent API is implemented on `master`. It serves cached availability data from `HotelZimmer_*.xml` files in `var/xmlexportzimmer`, using Flysystem and the tag-aware `bpn.cache` pool. XML sync now covers both travel and contingent datasets, invalidates `xml-sync` tagged cache entries, and logs dataset transfer errors without aborting the command. -## Current Status (feature/contingents-api) -- Implemented: parser, loader, data service, controller endpoints, XML sync for contingent dataset, and tests for parser/loader/service/controller. -- Implemented: hotel/date reference resolution (`hotelRef`/`dateRef`) with numeric passthrough and code mapping. -- Implemented: root-link one-hop behavior and BelKal control-room override semantics. -- Implemented: contingent caching via `bpn.cache` with `xml-sync` tagging strategy. -- In progress: enrich rooms payload with booking URL and additional travel metadata fields. +## Implemented +- OAuth-protected endpoints: + - `GET /api/contingents?hotelRef={hotelRef}&dateRef={dateRef}` + - `GET /api/contingents/rooms?hotelRef={hotelRef}&dateRef={dateRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}` + - `GET /api/contingents/calendar?hotelRef={hotelRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}` +- `hotelRef` and `dateRef` accept numeric IDs or business codes; date codes are sanitized before lookup. +- Parser/loader/service/controller flow is implemented via `ContingentParser`, `ContingentLoader`, `ContingentDataService`, and `ContingentController`. +- Room responses include per date + room availability, pricing, status, and `bookingUrl`. +- BelKal control-room rows override date status and set room availability to `0` for affected dates. +- Room-level contingent links and one-hop root-level file links are supported. +- `app:bpn:xml-sync` syncs travel and contingents, invalidates `xml-sync` cache tags, and refreshes travel snapshots only when travel data synced. +- `app:bpn:xml-cache-invalidate` checks both travel and contingent local XML exports. -## Public API / Interface Changes -- **New endpoints (OAuth-protected)**: - 1. `GET /api/contingents?hotelRef={hotelRef}&dateRef={dateRef}` - - Returns: object keyed by date with `{ date, minNights, total, capacity }`. - 2. `GET /api/contingents/rooms?hotelRef={hotelRef}&dateRef={dateRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}` - - Returns: **per date + room** rows with pricing and travel metadata. - 3. `GET /api/contingents/calendar?hotelRef={hotelRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}` - - Returns per date `{ date, status, pax, available }` (status is worst-case across rooms). -- **New config**: - - `.env`: `XML_EXPORT_CONTINGENTS_PATH="%kernel.project_dir%/var/xmlexportzimmer"` - - `config/packages/flysystem.yaml`: `xml_export_contingents.storage` and `xml_source_contingents.storage`. -- **New cache pool usage**: - - `bpn.cache` used for contingent file map + hotel cache entries, tagged with `xml-sync`. +## Config +- `.env` defines `XML_EXPORT_CONTINGENTS_PATH` and `SFTP_XML_EXPORT_CONTINGENTS_*`. +- `config/packages/flysystem.yaml` defines: + - `xml_export_contingents.storage` + - `xml_source_contingents.storage` +- `config/services.yaml` wires contingent storage, `bpn.cache`, and `APP_BASE_URL` for booking URL generation. -## Key Decisions Locked -- **Parsing approach**: DOM/Crawler (SimpleXML/Crawler), not XMLReader. -- **Cache contents**: DTO arrays (scalars + `\DateTimeImmutable`) only. No SimpleXML/Crawler objects in cache. -- **Rooms response shape**: **per date + room** (not aggregated). -- **BelKal override**: - - BelKal is a control room that can override availability. - - If BelKal is A/S on a date, **all rooms** on that date are treated as on-request/blocked. -- **Root-level link**: one hop only; do not chain if linked file also has link attribute. -- **Room-level link**: link only affects which `zimmer[@idbuspro]` node is used for capacity. +## Behavior Notes +- Missing contingent XML returns empty data. +- Unknown travel/date or hotel/date mismatches return `404` from the API. +- Invalid date ranges or formats return `400`. +- Dataset transfer failures are shown and logged as warnings, but `app:bpn:xml-sync` still completes with exit code `0`. -## Detailed Implementation Plan +## Remaining TODOs +- Decide whether room responses still need travel metadata fields such as `summer`, `servicesIncluded`, and `servicesOptional`; these are not currently implemented. +- If the XML files grow materially or cold-cache parsing becomes slow, revisit whether `ContingentParser` should move from DOM/Crawler parsing to a streaming parser. +- Validate the deployed contingent SFTP environment variables before enabling scheduled sync in production. -### 1) Configuration -1. Add `XML_EXPORT_CONTINGENTS_PATH` to `.env`. -2. Add Flysystem storage in `config/packages/flysystem.yaml`: - - `xml_export_contingents.storage` with `directory: '%env(resolve:XML_EXPORT_CONTINGENTS_PATH)%'`. - - `xml_source_contingents.storage` for SFTP contingent source. -3. Wire service injection in `config/services.yaml`: - - Bind `xml_export_contingents.storage` into `ContingentLoader`. - - Inject `@bpn.cache` explicitly into `ContingentLoader` and `BpnXmlSyncCommand`. - -### 2) Parser (Crawler) -**New** `src/BusProNet/XmlParser/ContingentParser.php` -Responsibilities: -- Parse a single `HotelZimmer_*.xml` (string or Crawler root) and return: - - Room types: `{ idBusPro, code, pax, label, controlRoom, link }` - - Contingent rows (DTO arrays) with: - - `date` (`\DateTimeImmutable`) - - `roomCode`, `roomLabel` - - `pax`, `available`, `status` (`OK|ON_REQUEST|BLOCKED`) - - `minPrice`, `minNights` - - `additionalNightMinPrice`, `additionalNightMinNights` -- Logic: - - Room types from `unterbringungen/unterbringung`. - - Skip `code=PDGS`. - - Control room is `code=BelKal`. - - For each date node (`kapazitaeten/kapazitaet @termin d.m.Y`) and room type, resolve `zimmerliste/zimmer` by id: - - Use `roomType.link ?? roomType.idBusPro`. - - BelKal rows: - - status `A` => ON_REQUEST - - status `S` => BLOCKED - - Non-BelKal rows: - - `pax = kontingent * pax_max` - - `available = frei * pax_max` -- Return arrays (no XML objects) to be cache-safe. - - ### 3) Loader (Cache + Flysystem) -**New** `src/BusProNet/XmlLoader/ContingentLoader.php` -Responsibilities: -- Extend `AbstractLoader`. -- `generateFilesMap()`: - - List `HotelZimmer_*.xml` in `xml_export_zimmer.storage`. - - Build mapping `hotelId => filename`. - - Cache as `bpn_contingent_files` for 3h on `bpn.cache`. -- `loadByHotelId(int $hotelId): array`: - - Use file map to find filename; return `[]` if missing. - - Read XML content. - - **Root-level link**: if root has `idbuspro_kontingent_aus`, load that file **instead** (one hop only). - - Parse using `ContingentParser`. - - Cache result in `bpn.cache` as `contingent_hotel_{hotelId}` TTL 3600, tagged `xml-sync`. - -### 4) Data Service (No DB) -**New** `src/Service/ContingentDataService.php` -Dependencies: `ContingentLoader`, `TravelLoader`, optional booking URL base. - -Functions: -- `getAvailableContingents(int $hotelId, int $dateId): array` - - Load travel via `TravelLoader->loadById($dateId)` to get `dateFrom/dateTo`. - - Load contingents by hotel. - - Filter within range (inclusive). - - Aggregate per date: `minNights` (min), `total` (sum of available), `capacity` (sum pax). - - **BelKal override**: if BelKal is A/S on a date, mark all rooms on that date as on-request/blocked for outputs. -- `getAvailableRooms(string $dateFrom, string $dateTo, int $hotelId, int $dateId, string $myEpUrl = ''): array` - - Filter contingents by range; build **per date + room** rows. - - Compute `priceForSelection = minPrice + max(0, nights - minNights) * additionalNightMinPrice`. - - Add bookingUrl (see below). - - Add `summer/servicesIncluded/servicesOptional` if travel metadata available; otherwise empty. - - **BelKal override**: if BelKal is A/S on a date, force each row’s `status` to ON_REQUEST/BLOCKED (and optionally available to 0 if desired by frontend). -- `getCalendarEvents(int $hotelId, string $dateFrom, string $dateTo): array` - - Aggregate per date: - - Exclude BelKal rows from `pax` and `available`. - - Status is worst-case across **all** rows (BelKal included). - - Return `{ date, status, pax, available }`. - -### 5) Booking URL Utility -- Add `src/BusProNet/Utility/BookingUrlUtility.php`. -- URL format: `/bookings/create?date_id={dateId}&hotel_id={hotelId}`. -- Base resolution: use optional `my_ep_url` when provided, otherwise `APP_BASE_URL` (default `https://my.ep-reisen.de`). - -### 6) Controller + Routes -**New** `src/Controller/Api/ContingentController.php` -- Base: `#[Route('/api')]` and `#[IsGranted('ROLE_OAUTH2_API')]`. -- Canonical query routes: - 1. `/api/contingents/calendar` with query `hotelRef`, `dateFrom`, `dateTo` - 2. `/api/contingents` with query `hotelRef`, `dateRef` - 3. `/api/contingents/rooms` with query `hotelRef`, `dateRef`, `dateFrom`, `dateTo` -- Reference resolution: - - Numeric `hotelRef` / `dateRef` values are treated as IDs. - - Non-numeric `hotelRef` is mapped as hotel code. - - Non-numeric `dateRef` is sanitized (`-` and `/` removed, uppercase) and mapped as date code. - -### 7) Cache Invalidation on Sync -Update `src/Command/BpnXmlSyncCommand.php`: -- Inject `TagAwareCacheInterface $bpnCache` (bind to `@bpn.cache`). -- In `invalidateCaches()`: - - Invalidate `xml-sync` tag after successful sync. - - Sync now covers two datasets (`travel` and `contingents`) in one command run. - -## Edge Cases & Failure Modes -- Missing `HotelZimmer_{hotelId}.xml`: loader returns empty array; API returns empty result or 404 (match existing pattern). -- Root-level link missing target file: return empty array (optionally log), no fallback to original. -- Travel not found for `dateId`: return 404. -- Hotel not in travel: optionally validate using TravelLoader mapping and return 404. -- Date parsing: `d.m.Y` -> `\DateTimeImmutable` with server timezone. - -## Tests / Scenarios -1. **Parser – happy path**: room types, capacities, prices parsed correctly. -2. **Parser – room link**: `idbuspro_kontingent_aus` on room type uses linked id for `zimmerliste/zimmer`. -3. **Parser – root link**: root `idbuspro_kontingent_aus` loads alternate file exactly once. -4. **Parser – BelKal override**: - - BelKal A/S forces all rooms on that date to on-request/blocked. - - Calendar status uses BelKal and sums exclude BelKal. -5. **Service – getAvailableContingents**: correct per-date aggregation. -6. **Service – getAvailableRooms**: per date + room rows with correct pricing. -7. **Calendar**: worst-case status across rooms, with correct sums excluding BelKal. -8. **Cache**: entries tagged and invalidated after `app:bpn:xml-sync`. - -## Performance Notes -- DOM/Crawler parsing should be acceptable for ~7MB XMLs, especially with cached DTOs. -- If XML size grows materially (>20-30MB) or cold-cache concurrency spikes, consider adding an XMLReader streaming parser as a future enhancement. - -## Assumptions / Defaults -- `hotelRef` accepts either numeric `hotelId` (`idbuspro`) or hotel code. -- `dateRef` accepts either numeric `dateId` or date code. -- Rooms endpoint returns per date + room rows. -- Return structure follows existing API patterns (`TravelController`, `PickupController`) for HTTP status and JSON formatting. +## Verification +- Covered by parser, loader, data service, controller, booking URL, XML sync, and cache invalidation tests. +- Current verification command: `php bin/phpunit`. diff --git a/docs/service-simplification-plan.md b/docs/service-simplification-plan.md deleted file mode 100644 index d22b5b2..0000000 --- a/docs/service-simplification-plan.md +++ /dev/null @@ -1,122 +0,0 @@ -# Service Simplification Plan - -Status: active -Last updated: 2026-04-13 - -## Purpose - -This document tracks the next pass of service simplification work. The goal is to reduce orchestration density, make the booking flow easier to follow for a human reader, and keep responsibilities aligned with the actual boundaries in the code. - -The emphasis is not on deleting services for its own sake. The emphasis is on: -- keeping one clear owner for each meaningful boundary -- removing thin wrappers and pass-through helpers -- avoiding services that mostly shuffle data between layers -- keeping presentation concerns out of calculation and orchestration code - -## Current Read - -The codebase is already in a better place than it was at the start of the refactor, but a few services still carry more than one responsibility: - -- `BookingService` now mostly covers booking bootstrap, service preselection, and booking status rules. -- `BookingParticipantCountService` was removed — its logic lives as a private method in the one controller that needed it (`Step2Controller`). -- `BookingRoomSelectionService` was removed — its logic lives as a private method in `BookingCreateContextFactory`. -- `BookingSummaryParticipantCountService` was removed — its logic lives as a private method in `BookingSummaryDataService`. -- `ParticipantPrepopulationService` now owns applicant prefill plus the create-mode dummy-data shortcut. -- `BookingEditParticipantContextFactory` now prepares the edit participant page context directly, replacing the older pass-through participant form service. -- `BookingPriceCalculatorService` is focused on pricing, but it still sits close to display-oriented behavior in adjacent code paths. -- `TravelDataService` remains broad and is likely the next larger boundary after booking orchestration is reduced. - -One registry stands out as a real orchestration boundary and should be left alone for now: -- `ParticipantFieldHandlerRegistry` - -It is not just a lookup table. It owns execution order, edit-mode mutability gating, and synchronization of submitted form data back into the DTO state. - -## Next Pass - -### 1. Keep pricing calculation focused - -Primary goal: keep pricing code about pricing, not rendering. - -Concrete next steps: -- keep `BookingPriceCalculatorService` as the pricing boundary -- continue removing display formatting from pricing code paths -- keep any remaining view-specific formatting in the presentation layer or a dedicated UI helper -- avoid introducing another service that only formats values already known to the view - -Decision rule: -- if a value is only needed for display, prefer exposing the raw numeric/domain value and formatting it as close to the UI as possible - -### 2. Leave the field-handler registry in place - -Primary goal: avoid unnecessary churn in a class that is already a meaningful orchestration layer. - -Concrete next steps: -- do not refactor `ParticipantFieldHandlerRegistry` in this pass -- revisit only if a later change can split ordering, mutability, and synchronization into clear collaborators without making the flow harder to trace - -Decision rule: -- if a registry owns actual workflow behavior, treat it as a boundary rather than a smell - -## Follow-Up Queue - -After the booking service pass, the next likely candidates are: - -### `TravelDataService` - -This is the largest broad service still in the codebase. It likely needs a later pass if the application should become easier to follow end to end. - -Likely directions: -- separate runtime travel loading from cache/snapshot maintenance if the public API still feels too wide -- keep the read path explicit and avoid hiding maintenance work behind one large method surface - -### `BookingPriceCalculatorService` - -This service should stay focused on pricing logic, but it may still have room for further internal cleanup if more display or transport aggregation concerns surface. - -Likely directions: -- keep calculation responsibilities together -- avoid dragging presentation behavior back into the calculator -- split only if a sub-boundary becomes obvious and reusable - -### `ParticipantFieldHandlerRegistry` - -This stays on the list only as a future optional refactor, not as an immediate target. - -Likely directions, only if justified later: -- isolate ordering/sorting logic if it becomes independently meaningful -- split synchronization code if a clearer DTO/form boundary emerges -- otherwise leave it as the central orchestration point for participant field processing - -## Current Notes - -- `templates/booking/_summary_travel_info.html.twig` already renders the summary participant count directly, so the remaining work here is naming and contract clarity rather than Twig branching. -- The summary count still needs a clear name if the code should distinguish the display-oriented count from the canonical participant total in the DTO/service layer. -- `ParticipantFormSupportService` was reviewed and kept: it is shared between two controllers (`Step2ParticipantController` and `Edit/ParticipantController`) with real shared logic, not just delegation. - -## Progress Tracker - -| Item | Status | Notes | -|------|--------|-------| -| Participant card DTO cleanup | Done | Card data now uses typed DTOs instead of nested array payloads | -| Room label formatting cleanup | Done | Pricing labels now have a dedicated presentation helper | -| Booking service split | Done | Session lifecycle, hydration, baseline snapshot, return URL handling, room grouping, and participant count shaping moved out of `BookingService` | -| Dummy prefill extraction | Done | Moved into `ParticipantPrepopulationService` | -| Micro-service consolidation | Done | `BookingRoomSelectionService`, `BookingParticipantCountService`, `BookingSummaryParticipantCountService` inlined into their single callers | -| Pricing service review | Pending | Keep focused on calculation, not rendering | -| Travel data service review | Pending | Broad boundary, likely later pass | -| Participant field registry review | Deferred | Real orchestration boundary, intentionally left alone for now | - -## Acceptance Criteria - -The next booking-service pass is only worth keeping if it: -- reduces the number of unrelated responsibilities in `BookingService` -- makes the booking flow easier to trace from controller to session/DTO state -- preserves existing booking behavior and test coverage -- does not replace one large service with several generic "manager" classes - -## Working Agreement - -- Update this document as decisions are made. -- Record rejected simplification ideas here with a short reason. -- Keep the plan aligned with actual code, not with an abstract architecture ideal. -- If a future simplification does not clearly reduce cognitive load, do not add it.