diff --git a/config/services.yaml b/config/services.yaml index 520396d9..f4ea291e 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -223,6 +223,11 @@ services: arguments: $cache: '@bpn.cache' + App\Service\TravelExportBuilder: + arguments: + $cache: '@bpn.cache' + $logger: '@monolog.logger.bpn' + App\Service\VoucherValidator: arguments: $cache: '@cache.app' diff --git a/src/Controller/Api/TravelController.php b/src/Controller/Api/TravelController.php index 8b5a1128..49535e0e 100644 --- a/src/Controller/Api/TravelController.php +++ b/src/Controller/Api/TravelController.php @@ -6,12 +6,15 @@ use App\BusProNet\ApiClient; use App\BusProNet\Exception\ApiClientException; use App\BusProNet\Model\Travel; use App\BusProNet\Utility\DateCodeUtility; +use App\Model\TravelExportQuery; use App\Service\TravelDataProvider; +use App\Service\TravelExportBuilder; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\MapDateTime; +use Symfony\Component\HttpKernel\Attribute\MapQueryString; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Security\Http\Attribute\IsGranted; @@ -22,6 +25,7 @@ class TravelController extends AbstractController public function __construct( private readonly ApiClient $apiClient, private readonly TravelDataProvider $travelDataService, + private readonly TravelExportBuilder $travelExportBuilder, ) { } @@ -33,6 +37,45 @@ class TravelController extends AbstractController return $this->json($mapping); } + /** + * Bulk export of every travel date x hotel pair with its pickups resolved. + * + * Declared before api_travel_single_code on purpose: that route puts no requirement on + * {dateCode}, so /api/travels/export would otherwise resolve to byCode('export'), fail + * the code lookup and answer 404 -- a failure that reads like an endpoint with no data + * rather than a missing route. + * + * The counters in meta describe the whole export, not the page, so they repeat + * unchanged on every page and a consumer can abort after the first one. + */ + #[Route(path: '/travels/export', name: 'api_travel_export', methods: ['GET'])] + public function export( + #[MapQueryString(validationFailedStatusCode: Response::HTTP_BAD_REQUEST)] + TravelExportQuery $query = new TravelExportQuery(), + ): JsonResponse { + // Deliberately not wrapped in a catch that answers 200 with an error body, the way + // hotelAvailability() below does: the consumer's retry logic keys on status codes, + // so a failure here has to surface as a 5xx. + $export = $this->travelExportBuilder->build(); + + $total = count($export->rows); + $totalPages = (int) ceil($total / $query->perPage); + $offset = ($query->page - 1) * $query->perPage; + + return $this->json([ + 'data' => array_slice($export->rows, $offset, $query->perPage), + 'meta' => [ + 'page' => $query->page, + 'perPage' => $query->perPage, + 'total' => $total, + 'totalPages' => $totalPages, + 'skipped' => $export->skipped, + 'pickupsWithoutMasterData' => $export->pickupsWithoutMasterData, + 'generatedAt' => $export->generatedAt->format('c'), + ], + ]); + } + #[Route( path: '/travels/{dateId}/{hotelId}', name: 'api_travel_single_id', @@ -56,6 +99,7 @@ class TravelController extends AbstractController #[Route( path: '/travels/{dateCode}/{hotelCode}', name: 'api_travel_single_code', + requirements: ['dateCode' => '(?!export$).+'], defaults: ['hotelCode' => null], )] public function byCode(Request $request, string $dateCode, ?string $hotelCode = null): JsonResponse diff --git a/src/Model/TravelExport.php b/src/Model/TravelExport.php new file mode 100644 index 00000000..ab8a2758 --- /dev/null +++ b/src/Model/TravelExport.php @@ -0,0 +1,30 @@ +> $rows one row per (travel, hotel) pair + * @param int $skipped (travel, hotel) pairs dropped as unusable + * @param int $pickupsWithoutMasterData distinct pickup ids absent from zustiege.xml + */ + public function __construct( + public array $rows, + public int $skipped, + public int $pickupsWithoutMasterData, + public \DateTimeImmutable $generatedAt, + ) { + } +} diff --git a/src/Model/TravelExportQuery.php b/src/Model/TravelExportQuery.php new file mode 100644 index 00000000..65e12bd0 --- /dev/null +++ b/src/Model/TravelExportQuery.php @@ -0,0 +1,19 @@ +cache->get(self::CACHE_KEY, function (ItemInterface $item) { + $item->expiresAfter(3 * 60 * 60); + $item->tag(['xml-sync']); + + return $this->assemble(); + }); + } catch (InvalidArgumentException $e) { + throw new \RuntimeException('Cache key error in TravelExportBuilder', 0, $e); + } + } + + private function assemble(): TravelExport + { + $hotels = $this->hotelLoader->loadAll(); + $pickupMasterData = $this->pickupLoader->loadAll(); + + /** @var array> $rows keyed by travelId-hotelId */ + $rows = []; + $skipped = 0; + /** @var array $pickupsWithoutMasterData */ + $pickupsWithoutMasterData = []; + + $files = $this + ->xmlExport + ->listContents('.') + ->filter(fn (StorageAttributes $attributes) => $attributes->isFile() && str_starts_with($attributes->path(), 'Ziel_')); + + foreach ($files as $file) { + $crawler = XmlCrawlerFactory::create($this->xmlExport->read($file->path())); + + $crawler->filterXPath('//reisen/reise/termin')->each( + function (Crawler $node) use (&$rows, &$skipped, &$pickupsWithoutMasterData, $hotels, $pickupMasterData) { + $this->collectTravel($node, $rows, $skipped, $pickupsWithoutMasterData, $hotels, $pickupMasterData); + } + ); + } + + return new TravelExport( + array_values($rows), + $skipped, + count($pickupsWithoutMasterData), + new \DateTimeImmutable(), + ); + } + + /** + * @param array> $rows + * @param array $pickupsWithoutMasterData + * @param array $hotels + * @param array $pickupMasterData + */ + private function collectTravel( + Crawler $node, + array &$rows, + int &$skipped, + array &$pickupsWithoutMasterData, + array $hotels, + array $pickupMasterData, + ): void { + $hotelNodes = $node->filterXPath('.//hotel'); + + // Dates first: getPickups() needs a non-null default date for the time-only zeit + // shape, and parsing them up front is also what turns an unusable date into a + // dropped row rather than a row with a null date the consumer cannot key on. + $dateFrom = $this->stringToDate($node->attr('termin')); + $dateTo = $this->stringToDate($node->attr('bis')); + + if (null === $dateFrom || null === $dateTo) { + $this->logger->warning('Travel export: unparseable travel date', [ + 'travel_id' => $node->attr('idbuspro'), + 'termin' => $node->attr('termin'), + 'bis' => $node->attr('bis'), + ]); + $skipped += $hotelNodes->count(); + + return; + } + + $travelId = (int) $node->attr('idbuspro'); + + // getPickups() is what buys stringToDateTimeFuzzy() for both zeit shapes and + // adjustOvernightTimes(), which rolls an 02:15 listed after a 23:59 onto the next + // calendar day. Resolved once per travel date, then shared by its hotel rows. + $pickups = $this->buildPickups( + $this->travelParser->getPickups($node->filterXPath('.//zustiege/zustieg'), $dateFrom, true), + $pickupMasterData, + $pickupsWithoutMasterData, + ); + + $travel = [ + 'busProId' => $travelId, + 'code' => $node->attr('code'), + 'product' => $this->trimOrNull($this->getStringOrNullValue($node->filterXPath('.//text'))), + 'dateFrom' => $dateFrom->format('Y-m-d'), + 'dateTo' => $dateTo->format('Y-m-d'), + ]; + + $hotelNodes->each(function (Crawler $hotelNode) use (&$rows, &$skipped, $hotels, $travel, $travelId, $pickups) { + $hotelId = (int) $hotelNode->attr('idbuspro'); + + // Defensive: no travel in the current snapshot references a hotel that + // hotel.xml does not carry. A row without master data would reach the consumer + // with no name and no country, so it is dropped and counted instead. + if (false === isset($hotels[$hotelId])) { + $this->logger->warning('Travel export: hotel missing from master data', [ + 'travel_id' => $travelId, + 'hotel_id' => $hotelId, + ]); + ++$skipped; + + return; + } + + $hotel = $hotels[$hotelId]; + + // Keyed by the pair the consumer upserts on, so a date repeated across files + // collapses last-wins instead of reaching it as a duplicate. + $rows[$travelId.'-'.$hotelId] = $travel + [ + 'hotel' => [ + 'busProId' => $hotelId, + 'code' => $hotel->code, + 'name' => $hotel->name, + 'costUnit' => $hotel->costUnit, + 'country' => $hotel->country, + 'countryCode' => $hotel->countryCode, + ], + 'pickups' => $pickups, + ]; + }); + } + + /** + * Joins the travel XML's pickups against zustiege.xml. + * + * Deliberately not PickupLoader::patchPickupsDetails(), which sorts by time -- the + * contract is source order, which is the order the disposition lists them in. Nor + * enrichPickupDetails(), which is private and passes over a miss silently. + * + * @param array $pickups pickups indexed by id, in source order + * @param array $masterData + * @param array $pickupsWithoutMasterData + * + * @return list> + */ + private function buildPickups(array $pickups, array $masterData, array &$pickupsWithoutMasterData): array + { + $resolved = []; + + foreach ($pickups as $pickup) { + $pickupId = $pickup->id; + + // Defensive: none exist today. An entry with no id is not addressable by the + // consumer, so drop the entry -- never the row it belongs to. + if (null === $pickupId || 0 === $pickupId) { + continue; + } + + $details = $masterData[$pickupId] ?? null; + + // Around 21 ids referenced by travel XML have no entry in zustiege.xml. The + // entry is emitted anyway: dropping it would hide a real departure point from + // the disposition UI, where a nameless stop is still better than a missing one. + if (null === $details) { + $pickupsWithoutMasterData[$pickupId] = true; + } + + $resolved[] = [ + 'busProId' => $pickupId, + 'code' => $details?->code, + 'city' => $details?->city, + 'street' => $details?->street, + 'time' => $pickup->time?->format('H:i'), + // Emitted for time-only sources too: adjustOvernightTimes() has already + // worked out the real day, and a null here would throw that away. + 'datetime' => $pickup->time?->format('c'), + ]; + } + + return $resolved; + } + + private function getStringOrNullValue(Crawler $node): ?string + { + return 0 < $node->count() ? $node->text() : null; + } + + private function trimOrNull(?string $value): ?string + { + if (null === $value) { + return null; + } + + $trimmed = trim($value); + + return '' === $trimmed ? null : $trimmed; + } +} diff --git a/tests/Controller/Api/TravelExportControllerTest.php b/tests/Controller/Api/TravelExportControllerTest.php new file mode 100644 index 00000000..d6c6d41c --- /dev/null +++ b/tests/Controller/Api/TravelExportControllerTest.php @@ -0,0 +1,138 @@ +call(new TravelExportQuery(1, 5))['meta']; + + self::assertSame(1, $meta['page']); + self::assertSame(5, $meta['perPage']); + self::assertSame(12, $meta['total'], 'total is the whole export, not this page'); + self::assertSame(3, $meta['totalPages']); + } + + public function testCountersAreIdenticalOnEveryPage(): void + { + // Whole-export figures repeated per page, so a consumer can abort after page 1. + $first = $this->call(new TravelExportQuery(1, 5))['meta']; + $last = $this->call(new TravelExportQuery(3, 5))['meta']; + + foreach (['total', 'totalPages', 'skipped', 'pickupsWithoutMasterData', 'generatedAt'] as $key) { + self::assertSame($first[$key], $last[$key], $key.' must not vary by page'); + } + } + + public function testConsecutivePagesDoNotOverlap(): void + { + $first = array_column($this->call(new TravelExportQuery(1, 5))['data'], 'busProId'); + $second = array_column($this->call(new TravelExportQuery(2, 5))['data'], 'busProId'); + + self::assertSame([1, 2, 3, 4, 5], $first); + self::assertSame([6, 7, 8, 9, 10], $second); + self::assertSame([], array_intersect($first, $second)); + } + + public function testTheLastPageIsShort(): void + { + $data = $this->call(new TravelExportQuery(3, 5))['data']; + + self::assertCount(2, $data); + self::assertSame([11, 12], array_column($data, 'busProId')); + } + + public function testAPageBeyondTheEndIsEmptyRatherThanAnError(): void + { + $body = $this->call(new TravelExportQuery(99, 5)); + + self::assertSame([], $body['data']); + self::assertSame(12, $body['meta']['total']); + } + + public function testDefaultsToTheFirstPageOfFiveHundred(): void + { + $body = $this->call(); + + self::assertSame(1, $body['meta']['page']); + self::assertSame(500, $body['meta']['perPage']); + self::assertSame(1, $body['meta']['totalPages']); + self::assertCount(12, $body['data']); + } + + public function testCarriesTheBuilderCounters(): void + { + $meta = $this->call(null, 7, 21)['meta']; + + self::assertSame(7, $meta['skipped']); + self::assertSame(21, $meta['pickupsWithoutMasterData']); + self::assertSame('2026-09-29T03:20:00+02:00', $meta['generatedAt']); + } + + /** + * A builder failure must reach the consumer as a 5xx, never as the 200-with-an-error-body + * shape hotelAvailability() uses: the consumer's retry logic keys on the status code. + */ + public function testABuilderFailureIsNotDowngradedToATwoHundred(): void + { + $builder = $this->createStub(TravelExportBuilder::class); + $builder->method('build')->willThrowException(new \RuntimeException('cache exploded')); + + $this->expectException(\RuntimeException::class); + + $this->controller($builder)->export(new TravelExportQuery()); + } + + /** + * @return array + */ + private function call(?TravelExportQuery $query = null, int $skipped = 0, int $withoutMasterData = 0): array + { + $rows = []; + for ($i = 1; $i <= 12; ++$i) { + $rows[] = ['busProId' => $i, 'hotel' => ['busProId' => 1], 'pickups' => []]; + } + + $builder = $this->createStub(TravelExportBuilder::class); + $builder->method('build')->willReturn(new TravelExport( + $rows, + $skipped, + $withoutMasterData, + new \DateTimeImmutable('2026-09-29 03:20:00', new \DateTimeZone('Europe/Berlin')), + )); + + $response = $this->controller($builder)->export($query ?? new TravelExportQuery()); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + + return json_decode((string) $response->getContent(), true); + } + + private function controller(TravelExportBuilder $builder): TravelController + { + $controller = new TravelController( + $this->createStub(ApiClient::class), + $this->createStub(TravelDataProvider::class), + $builder, + ); + + // No 'serializer' service registered, so AbstractController::json() falls back to + // JsonResponse -- which is what this endpoint produces in production anyway. + $controller->setContainer(new Container()); + + return $controller; + } +} diff --git a/tests/Resources/travel_export_data.xml b/tests/Resources/travel_export_data.xml new file mode 100644 index 00000000..6be08c1b --- /dev/null +++ b/tests/Resources/travel_export_data.xml @@ -0,0 +1,41 @@ + + + + + + Semesterende - Portes du Soleil + + + + + + + + + + + + + + + Sommer + + + + + + + + + + Ohne Hotel + + + + + Kaputtes Datum + + + + + diff --git a/tests/Resources/travel_export_data_duplicate.xml b/tests/Resources/travel_export_data_duplicate.xml new file mode 100644 index 00000000..ad42f73f --- /dev/null +++ b/tests/Resources/travel_export_data_duplicate.xml @@ -0,0 +1,9 @@ + + + + + Zweiter Treffer + + + + diff --git a/tests/Service/TravelExportBuilderTest.php b/tests/Service/TravelExportBuilderTest.php new file mode 100644 index 00000000..d8774162 --- /dev/null +++ b/tests/Service/TravelExportBuilderTest.php @@ -0,0 +1,235 @@ +build()->rows; + + $keys = array_map( + static fn (array $row): string => $row['busProId'].'-'.$row['hotel']['busProId'], + $rows, + ); + + self::assertSame(['900-1', '900-2', '901-1'], $keys); + self::assertSame($keys, array_unique($keys), 'the pair the consumer upserts on must be unique export-wide'); + } + + public function testCarriesTravelAndHotelMasterData(): void + { + $row = $this->rowFor('900-1'); + + self::assertSame(900, $row['busProId']); + self::assertIsInt($row['busProId']); + self::assertSame('SBW261226', $row['code']); + self::assertSame('Semesterende - Portes du Soleil', $row['product'], 'the product label is trimmed'); + self::assertSame('2026-12-26', $row['dateFrom']); + self::assertSame('2026-12-30', $row['dateTo']); + + self::assertSame([ + 'busProId' => 1, + 'code' => 'H1', + 'name' => 'Hotel Eins', + 'costUnit' => '84 85', + 'country' => 'CH', + 'countryCode' => 'ch', + ], $row['hotel']); + } + + public function testResolvesBothZeitShapesAndRollsAnOvernightPickupOntoTheNextDay(): void + { + $pickups = array_column($this->rowFor('900-1')['pickups'], null, 'busProId'); + + self::assertSame('23:59', $pickups[10]['time']); + self::assertSame('2026-12-26T23:59:00+01:00', $pickups[10]['datetime']); + + // Time only, so it defaults to the departure date -- which would put it before the + // 23:59 above. The coach cannot board backwards, so it rolls onto the next day. + self::assertSame('02:15', $pickups[11]['time']); + self::assertSame('2026-12-27T02:15:00+01:00', $pickups[11]['datetime']); + } + + public function testEmitsTheSummerOffset(): void + { + $pickups = $this->rowFor('901-1')['pickups']; + + self::assertSame('2027-07-15T08:00:00+02:00', $pickups[0]['datetime']); + } + + public function testATimeOnlyPickupThatAlreadyFollowsTheLastDatedOneKeepsTheDepartureDate(): void + { + $pickups = array_column($this->rowFor('901-1')['pickups'], null, 'busProId'); + + self::assertSame('2027-07-15T09:30:00+02:00', $pickups[11]['datetime'], 'no roll when the order already holds'); + } + + public function testKeepsAPickupWithoutAZeitAndNullsBothTimeFields(): void + { + $pickups = array_column($this->rowFor('900-1')['pickups'], null, 'busProId'); + + self::assertArrayHasKey(12, $pickups); + self::assertNull($pickups[12]['time']); + self::assertNull($pickups[12]['datetime']); + self::assertSame('Z12', $pickups[12]['code'], 'master data still applies to an entry with no time'); + } + + public function testKeepsAPickupMissingFromMasterDataAndCountsIt(): void + { + $export = $this->build(); + $pickups = array_column($export->rows[0]['pickups'], null, 'busProId'); + + // Dropping it would hide a real departure point from the disposition UI. + self::assertArrayHasKey(99, $pickups); + self::assertNull($pickups[99]['code']); + self::assertNull($pickups[99]['city']); + self::assertNull($pickups[99]['street']); + self::assertSame('21:00', $pickups[99]['time'], 'the time is from the travel XML, not master data'); + self::assertSame('2026-12-26T21:00:00+01:00', $pickups[99]['datetime']); + + // Counted once, not once per row that references it. + self::assertSame(1, $export->pickupsWithoutMasterData); + } + + public function testPickupsKeepSourceOrder(): void + { + $order = array_column($this->rowFor('900-1')['pickups'], 'busProId'); + + self::assertSame([10, 11, 12, 99], $order, 'source order, never sorted by time'); + } + + public function testPickupsAreAnEmptyArrayRatherThanNullWhenThereAreNone(): void + { + $rows = $this->build('')->rows; + + self::assertSame([], $rows[0]['pickups']); + } + + public function testDropsAndCountsRowsWhoseHotelIsMissingFromMasterData(): void + { + $export = $this->build(); + + self::assertSame([], array_filter($export->rows, static fn (array $r): bool => 902 === $r['busProId'])); + // One hotel on travel 902, two on the travel with the unparseable date. + self::assertSame(3, $export->skipped); + } + + public function testDropsEveryRowOfATravelWhoseDateCannotBeParsed(): void + { + $export = $this->build(); + + self::assertSame([], array_filter($export->rows, static fn (array $r): bool => 903 === $r['busProId'])); + } + + public function testCollapsesADuplicatePairLastWins(): void + { + $export = $this->build(null, ['Ziel_1.xml', 'Ziel_2.xml']); + + $matching = array_values(array_filter( + $export->rows, + static fn (array $r): bool => 900 === $r['busProId'] && 1 === $r['hotel']['busProId'], + )); + + self::assertCount(1, $matching); + self::assertSame('Zweiter Treffer', $matching[0]['product']); + } + + /** + * @return array + */ + private function rowFor(string $key): array + { + foreach ($this->build()->rows as $row) { + if ($row['busProId'].'-'.$row['hotel']['busProId'] === $key) { + return $row; + } + } + + self::fail('No row for '.$key); + } + + /** + * @param list $files + */ + private function build(?string $xml = null, array $files = ['Ziel_1.xml']): TravelExport + { + $contents = [ + 'Ziel_1.xml' => $xml ?? (string) file_get_contents(__DIR__.'/../Resources/travel_export_data.xml'), + 'Ziel_2.xml' => (string) file_get_contents(__DIR__.'/../Resources/travel_export_data_duplicate.xml'), + ]; + + $filesystem = $this->createStub(FilesystemOperator::class); + $filesystem->method('listContents')->willReturn(new DirectoryListing(array_map( + static fn (string $path): FileAttributes => new FileAttributes($path), + $files, + ))); + $filesystem->method('read')->willReturnCallback( + static fn (string $path): string => $contents[$path] ?? '' + ); + + $hotelLoader = $this->createStub(HotelLoader::class); + $hotelLoader->method('loadAll')->willReturn([ + 1 => $this->hotel(1, 'H1', 'Hotel Eins', '84 85', 'CH', 'ch'), + 2 => $this->hotel(2, 'H2', 'Hotel Zwei', null, 'D', 'de'), + ]); + + $pickupLoader = $this->createStub(PickupLoader::class); + $pickupLoader->method('loadAll')->willReturn([ + 10 => $this->pickup(10), + 11 => $this->pickup(11), + 12 => $this->pickup(12), + ]); + + return (new TravelExportBuilder( + $hotelLoader, + $pickupLoader, + new TravelParser(new AgeConstraintParserRegistry([]), false), + $filesystem, + new TagAwareAdapter(new ArrayAdapter()), + new NullLogger(), + ))->build(); + } + + private function hotel(int $id, string $code, string $name, ?string $costUnit, string $country, string $countryCode): Hotel + { + $hotel = new Hotel(); + $hotel->id = $id; + $hotel->code = $code; + $hotel->name = $name; + $hotel->costUnit = $costUnit; + $hotel->country = $country; + $hotel->countryCode = $countryCode; + + return $hotel; + } + + private function pickup(int $id): Pickup + { + $pickup = new Pickup(); + $pickup->id = $id; + $pickup->code = 'Z'.$id; + $pickup->city = 'Stadt '.$id; + $pickup->street = 'Strasse '.$id; + + return $pickup; + } +}