chore: update documentation files
This commit is contained in:
Björn Fromme
+32
-155
@@ -1,162 +1,39 @@
|
|||||||
# Contingent API with XML Caching (Crawler-Based) — Plan (Updated)
|
# Contingent API Status
|
||||||
|
|
||||||
## Summary
|
## 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
|
||||||
- Implemented: parser, loader, data service, controller endpoints, XML sync for contingent dataset, and tests for parser/loader/service/controller.
|
- OAuth-protected endpoints:
|
||||||
- Implemented: hotel/date reference resolution (`hotelRef`/`dateRef`) with numeric passthrough and code mapping.
|
- `GET /api/contingents?hotelRef={hotelRef}&dateRef={dateRef}`
|
||||||
- Implemented: root-link one-hop behavior and BelKal control-room override semantics.
|
- `GET /api/contingents/rooms?hotelRef={hotelRef}&dateRef={dateRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}`
|
||||||
- Implemented: contingent caching via `bpn.cache` with `xml-sync` tagging strategy.
|
- `GET /api/contingents/calendar?hotelRef={hotelRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}`
|
||||||
- In progress: enrich rooms payload with booking URL and additional travel metadata fields.
|
- `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
|
## Config
|
||||||
- **New endpoints (OAuth-protected)**:
|
- `.env` defines `XML_EXPORT_CONTINGENTS_PATH` and `SFTP_XML_EXPORT_CONTINGENTS_*`.
|
||||||
1. `GET /api/contingents?hotelRef={hotelRef}&dateRef={dateRef}`
|
- `config/packages/flysystem.yaml` defines:
|
||||||
- Returns: object keyed by date with `{ date, minNights, total, capacity }`.
|
- `xml_export_contingents.storage`
|
||||||
2. `GET /api/contingents/rooms?hotelRef={hotelRef}&dateRef={dateRef}&dateFrom={Y-m-d}&dateTo={Y-m-d}`
|
- `xml_source_contingents.storage`
|
||||||
- Returns: **per date + room** rows with pricing and travel metadata.
|
- `config/services.yaml` wires contingent storage, `bpn.cache`, and `APP_BASE_URL` for booking URL generation.
|
||||||
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`.
|
|
||||||
|
|
||||||
## Key Decisions Locked
|
## Behavior Notes
|
||||||
- **Parsing approach**: DOM/Crawler (SimpleXML/Crawler), not XMLReader.
|
- Missing contingent XML returns empty data.
|
||||||
- **Cache contents**: DTO arrays (scalars + `\DateTimeImmutable`) only. No SimpleXML/Crawler objects in cache.
|
- Unknown travel/date or hotel/date mismatches return `404` from the API.
|
||||||
- **Rooms response shape**: **per date + room** (not aggregated).
|
- Invalid date ranges or formats return `400`.
|
||||||
- **BelKal override**:
|
- Dataset transfer failures are shown and logged as warnings, but `app:bpn:xml-sync` still completes with exit code `0`.
|
||||||
- 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.
|
|
||||||
|
|
||||||
## 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
|
## Verification
|
||||||
1. Add `XML_EXPORT_CONTINGENTS_PATH` to `.env`.
|
- Covered by parser, loader, data service, controller, booking URL, XML sync, and cache invalidation tests.
|
||||||
2. Add Flysystem storage in `config/packages/flysystem.yaml`:
|
- Current verification command: `php bin/phpunit`.
|
||||||
- `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: `<base>/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.
|
|
||||||
|
|||||||
@@ -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.
|
|
||||||
Reference in New Issue
Block a user