diff --git a/.env b/.env index 5dd84cf..ae623cb 100644 --- a/.env +++ b/.env @@ -78,11 +78,6 @@ APP_DEFAULT_EMAIL_TO=team@ep-reisen.de # Example: APP_MAIL_RECIPIENT_WHITELIST='.*@ep-reisen\.de,qa@example\.com' APP_MAIL_RECIPIENT_WHITELIST= -SFTP_XML_EXPORT_HOST= -SFTP_XML_EXPORT_PORT=22 -SFTP_XML_EXPORT_USER= -SFTP_XML_EXPORT_PASSWORD= -SFTP_XML_EXPORT_ROOT= # Machine to machine access to the MyE&P API. This is a separate confidential client # from the login one below, so it can be rotated without locking anyone out. The base diff --git a/config/packages/flysystem.yaml b/config/packages/flysystem.yaml index 7242945..e0dc750 100644 --- a/config/packages/flysystem.yaml +++ b/config/packages/flysystem.yaml @@ -1,13 +1,6 @@ # Read the documentation at https://github.com/thephpleague/flysystem-bundle/blob/master/docs/1-getting-started.md flysystem: storages: - xml_export.storage: - sftp: - host: '%env(SFTP_XML_EXPORT_HOST)%' - port: '%env(int:SFTP_XML_EXPORT_PORT)%' - username: '%env(SFTP_XML_EXPORT_USER)%' - password: '%env(SFTP_XML_EXPORT_PASSWORD)%' - root: '%env(SFTP_XML_EXPORT_ROOT)%' xml_dump.storage: local: directory: '%kernel.project_dir%/var/bpn' diff --git a/config/packages/zenstruck_schedule.yaml b/config/packages/zenstruck_schedule.yaml index 5c0b652..902a103 100644 --- a/config/packages/zenstruck_schedule.yaml +++ b/config/packages/zenstruck_schedule.yaml @@ -17,9 +17,9 @@ when@prod: tasks: # 03:20 rather than 03:00: MyE&P mirrors from BusProNet hourly overnight, on the # hour, so an import at 03:00 can read a half written export. - - task: app:bpn-import + - task: app:myep-import frequency: '20 3 * * *' - description: "Imports dates, products and hotels from BusProNet" + description: "Imports travel dates, products and hotels from the MyE&P API" - task: app:cron frequency: '0 1 * * *' @@ -47,9 +47,9 @@ when@staging: subject_prefix: "[E&P-Team STAGING]" tasks: - - task: app:bpn-import + - task: app:myep-import frequency: '50 3 * * *' - description: "Imports dates, products and hotels from BusProNet" + description: "Imports travel dates, products and hotels from the MyE&P API" - task: app:cron frequency: '30 2 * * *' diff --git a/config/services.yaml b/config/services.yaml index eeb5517..9a5deb9 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -130,7 +130,6 @@ services: # it drops its records from that table. $logger: '@monolog.logger.audit' $houses: '%houses%' - $xmlExport: '@xml_export.storage' $xmlDump: '@xml_dump.storage' $teamerInactivePeriod: '%teamer_inactive_period%' $contractUploadDeadlineDays: '%contract_upload_deadline_days%' @@ -165,7 +164,7 @@ services: - name: liip_imagine.cache.resolver resolver: format_extension - App\Command\BpnImportCommand: + App\Command\MyEpImportCommand: arguments: $connection: '@doctrine.dbal.default_connection' diff --git a/docs/operations.md b/docs/operations.md index 518ae54..f355311 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -131,7 +131,7 @@ Three failure modes worth knowing because they are quiet rather than loud: ## Scheduled tasks are a different mechanism `config/packages/zenstruck_schedule.yaml` holds the daily jobs (`app:cron`, -`app:bpn-import`, `app:teamer-status`, …), driven by a separate `schedule:run` cron entry. +`app:myep-import`, `app:teamer-status`, …), driven by a separate `schedule:run` cron entry. **Do not put `messenger:consume` in there.** The bundle runs tasks in-process and sequentially, so a consume task holds the entire schedule for its duration — every later diff --git a/src/Command/BpnImportCommand.php b/src/Command/BpnImportCommand.php deleted file mode 100644 index a0d8695..0000000 --- a/src/Command/BpnImportCommand.php +++ /dev/null @@ -1,245 +0,0 @@ -hotelDataProvider->getAll(); - $pickups = $this->pickupDataProvider->getAll(); - - if ([] === $hotels) { - $io->error('Hotel-Stammdaten konnten nicht von MyE&P geladen werden – Import abgebrochen.'); - $this->logger->error('Import aborted: hotel base data unavailable'); - - return Command::FAILURE; - } - - // Every destination this import touches gets its cost unit and its country written - // from the hotel, on update as well as on insert. If MyE&P does not report those two - // fields yet, a run would quietly blank them on rows that have them: the cost unit - // then drops off the invoices (App\Service\Pdf\InvoiceApprover) and a missing - // country code blanks the flag icons and takes the row out of the country filters. - // Refusing to run is the recoverable failure; overwriting good data is not. - if ([] !== $missingFields = $this->missingHotelFields($hotels)) { - $io->error(sprintf( - 'Die MyE&P-API liefert keine %s – Import abgebrochen, damit bestehende Werte nicht überschrieben werden.', - implode(' und ', $missingFields), - )); - $this->logger->error('Import aborted: the MyE&P hotel payload carries no '.implode(', ', $missingFields)); - - return Command::FAILURE; - } - - // Get all xml files - try { - $xmlFiles = $this - ->xmlExport - ->listContents('') - ->filter(fn (StorageAttributes $attributes) => $attributes->isFile() && str_starts_with($attributes->path(), 'Ziel_')) - ->toArray(); - } catch (FilesystemException $e) { - $io->error('Unable to list XML files'); - - return Command::FAILURE; - } - - $totalCount = count($xmlFiles); - - if (0 === $totalCount) { - $io->error('No XML files available or found'); - - return Command::FAILURE; - } - - $io->info('Found '.$totalCount.' XML files'); - $addedCount = 0; - $updatedCount = 0; - $warnings = []; - - if ([] === $pickups) { - $warnings[] = 'Zustiegs-Stammdaten konnten nicht von MyE&P geladen werden'; - } - - $progressBar = $io->createProgressBar($totalCount); - $progressBar->setFormat(" %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s%\n %message%"); - $progressBar->setMessage('Starting'); - $progressBar->start(); - - // Iterate over xml files - foreach ($xmlFiles as $xmlFile) { - $progressBar->setMessage($xmlFile->path()); - - // Load xml file into simplexml object - try { - $xml = simplexml_load_string($this->xmlExport->read($xmlFile->path())); - } catch (FilesystemException $e) { - $warnings[] = 'Unable to read XML file '.$xmlFile->path(); - $progressBar->advance(); - continue; - } - - // Iterate over all destinations - foreach ($xml->xpath('reise/termin') as $destinationXml) { - $product = (string) $destinationXml->xpath('text')[0]; - $busProId = (int) $destinationXml->attributes()['idbuspro']; - $code = (string) $destinationXml->attributes()['code']; - $dateFrom = \DateTimeImmutable::createFromFormat('d.m.Y', (string) $destinationXml->attributes()['termin']); - $dateTo = \DateTimeImmutable::createFromFormat('d.m.Y', (string) $destinationXml->attributes()['bis']); - - // Find pickups - $datePickups = []; - foreach ($destinationXml->xpath('zustiege/zustieg') as $pickupXml) { - $pickupBusProId = (int) $pickupXml->attributes()['idbuspro']; - if ($pickupBusProId) { - if (null === $pickup = $pickups[$pickupBusProId] ?? null) { - $warnings[] = 'Pickup with busProId '.$pickupBusProId.' not found'; - continue; - } - - $datePickups[] = [ - 'busProId' => $pickupBusProId, - 'city' => $pickup->getCity(), - 'street' => $pickup->getStreet(), - 'time' => (string) $pickupXml->attributes()['zeit'], - ]; - } - } - - // Iterate over all hotel entries - foreach ($destinationXml->xpath('hotel') as $hotelXml) { - $hotelBusProId = (int) $hotelXml->attributes()['idbuspro']; - if (null === $hotel = $hotels[$hotelBusProId] ?? null) { - $warnings[] = 'Hotel with busProId '.$hotelBusProId.' not found'; - continue; - } - - // Finally update or store collected data - if ($id = $this->connection->fetchOne('SELECT id FROM destination WHERE bus_pro_id=? AND hotel_bus_pro_id=?', [$busProId, $hotelBusProId])) { - $this - ->connection - ->update('destination', [ - 'code' => $code, - 'date_from' => $dateFrom->format('Y-m-d'), - 'date_to' => $dateTo->format('Y-m-d'), - 'product' => $product, - 'pickups' => json_encode($datePickups), - 'cost_unit' => $hotel->getCostUnit(), - 'country' => $hotel->getCountryCode(), - 'updated_at' => (new \DateTimeImmutable())->format('Y-m-d H:i:s'), - ], [ - 'id' => $id, - 'hotel_bus_pro_id' => $hotelBusProId, - ]) - ; - ++$updatedCount; - } else { - $this - ->connection - ->insert('destination', [ - 'code' => $code, - 'bus_pro_id' => $busProId, - 'date_from' => $dateFrom->format('Y-m-d'), - 'date_to' => $dateTo->format('Y-m-d'), - 'product' => $product, - 'hotel' => $hotel->getName(), - 'hotel_code' => $hotel->getCode(), - 'hotel_bus_pro_id' => $hotelBusProId, - 'pickups' => json_encode($datePickups), - 'cost_unit' => $hotel->getCostUnit(), - 'country' => $hotel->getCountryCode(), - 'created_at' => (new \DateTimeImmutable())->format('Y-m-d H:i:s'), - ]) - ; - ++$addedCount; - } - } - } - - $progressBar->advance(); - } - - $progressBar->setMessage('Done'); - $progressBar->finish(); - $io->newLine(2); - - if ($warnings) { - $io->warning(array_unique($warnings)); - } - - $logMessage = 'Added '.$addedCount.' and updated '.$updatedCount.' destinations'; - $io->info($logMessage); - $this->logger->info($logMessage); - - return Command::SUCCESS; - } - - /** - * Names the hotel fields MyE&P is not reporting, German labels for the operator. - * - * Asserted across the whole list rather than per hotel, because a single hotel - * legitimately has neither: only a minority of them carry a kostenstelle upstream, and - * some carry no country at all. A list where not one hotel has the field is a field - * missing from the response, not a gap in the data. - * - * @param array $hotels - * - * @return list - */ - private function missingHotelFields(array $hotels): array - { - $required = [ - 'Kostenstellen' => static fn (Hotel $hotel): bool => '' !== (string) $hotel->getCostUnit(), - 'Länderkennungen' => static fn (Hotel $hotel): bool => '' !== (string) $hotel->getCountryCode(), - ]; - - $missing = []; - - foreach ($required as $label => $isReported) { - foreach ($hotels as $hotel) { - if ($isReported($hotel)) { - continue 2; - } - } - - $missing[] = $label; - } - - return $missing; - } -} diff --git a/src/Command/MyEpCheckCommand.php b/src/Command/MyEpCheckCommand.php new file mode 100644 index 0000000..e538919 --- /dev/null +++ b/src/Command/MyEpCheckCommand.php @@ -0,0 +1,235 @@ +section('Base data'); + + $hotels = $this->hotelDataProvider->getAll(); + $pickups = $this->pickupDataProvider->getAll(); + + if ([] === $hotels) { + $io->error('No hotels returned - the import would refuse to run.'); + + return Command::FAILURE; + } + + $withCostUnit = count(array_filter($hotels, fn ($h) => null !== $h->getCostUnit())); + $withCountry = count(array_filter($hotels, fn ($h) => null !== $h->getCountryCode())); + + $io->definitionList( + ['hotels' => (string) count($hotels)], + ['with costUnit' => sprintf('%d%s', $withCostUnit, 0 === $withCostUnit ? ' <- import would refuse' : '')], + ['with countryCode' => sprintf('%d%s', $withCountry, 0 === $withCountry ? ' <- import would refuse' : '')], + ['pickups' => (string) count($pickups)], + ); + + $io->section('Travel export'); + + // A whole page rather than one row: the first row often has no boarding points, and an + // unread pickups array is exactly the failure this probe exists to catch. + $payload = $this->apiClient->get('api/travels/export', ['page' => 1, 'perPage' => 500]); + $meta = $payload['meta'] ?? []; + + $io->definitionList(...array_map( + fn (string $key) => [$key => var_export($meta[$key] ?? null, true)], + ['total', 'totalPages', 'perPage', 'skipped', 'pickupsWithoutMasterData', 'generatedAt'], + )); + + $rows = array_values(array_filter((array) ($payload['data'] ?? []), 'is_array')); + + if ([] === $rows) { + $io->error('The export returned no rows.'); + + return Command::FAILURE; + } + + $withPickups = array_values(array_filter($rows, fn (array $r) => [] !== ($r['pickups'] ?? []))); + + $io->definitionList( + ['rows on this page' => (string) count($rows)], + ['of them with pickups' => (string) count($withPickups)], + ); + + $row = $withPickups[0] ?? $rows[0]; + + // Does `datetime` actually resolve the overnight leg? A coach leaves in the evening and + // keeps boarding through the night, so a 00:30 stop belongs to the day after a 23:30 one. + // If every entry on such a row carries the same calendar date, the field is unresolved and + // ordering by it inverts the route. + $straddling = 0; + $unresolved = []; + + foreach ($withPickups as $candidate) { + $times = array_filter(array_column($candidate['pickups'], 'time')); + $dates = array_unique(array_map( + fn (?string $d) => null === $d ? null : substr($d, 0, 10), + array_column($candidate['pickups'], 'datetime'), + )); + + $late = array_filter($times, fn (string $t) => $t >= '18:00'); + $early = array_filter($times, fn (string $t) => $t <= '06:00'); + + if ([] === $late || [] === $early) { + continue; + } + + ++$straddling; + + if (1 === count($dates)) { + $unresolved[] = $candidate['busProId']; + } + } + + $io->definitionList( + ['rows straddling midnight' => (string) $straddling], + ['of them with one date only' => sprintf('%d%s', count($unresolved), [] === $unresolved ? '' : ' <- datetime unresolved')], + ); + + // The decisive question is not whether a date looks odd but whether adopting `datetime` + // would reorder a route. Compared against our own anchor-based ordering, which reads the + // bare clock times and is what runs today. + $disagree = []; + + foreach ($withPickups as $candidate) { + $pickups = $candidate['pickups']; + + if (2 > count($pickups)) { + continue; + } + + $legacy = array_column( + PickupDepartureOrder::sort(array_map( + function (array $p): array { + unset($p['datetime']); + + return $p; + }, + $pickups, + )), + 'busProId', + ); + + $dated = $pickups; + usort($dated, fn (array $a, array $b) => ($a['datetime'] ?? '') <=> ($b['datetime'] ?? '')); + + if ($legacy !== array_column($dated, 'busProId')) { + $disagree[] = $candidate['busProId']; + } + } + + $io->definitionList( + ['rows where datetime order differs' => sprintf( + '%d of %d%s', + count($disagree), + count($withPickups), + [] === $disagree ? '' : ' <- adopting datetime would reorder these', + )], + ); + + if ([] !== $unresolved) { + $io->warning(sprintf( + 'datetime does not roll past midnight on %d of %d straddling rows (e.g. busProId %s). ' + .'Ordering by it would put the early-morning stops before the evening departure.', + count($unresolved), + $straddling, + implode(', ', array_slice($unresolved, 0, 5)), + )); + } + + $resolvedStraddlers = array_values(array_filter( + $withPickups, + function (array $r) use ($unresolved): bool { + $times = array_filter(array_column($r['pickups'], 'time')); + $late = array_filter($times, fn (string $t) => $t >= '18:00'); + $early = array_filter($times, fn (string $t) => $t <= '06:00'); + + return [] !== $late && [] !== $early && false === in_array($r['busProId'], $unresolved, true); + }, + )); + + if ([] !== $resolvedStraddlers) { + $sample = $resolvedStraddlers[0]; + $io->writeln(sprintf('A straddling row where datetime DID roll (busProId %d, dateFrom %s)', $sample['busProId'], $sample['dateFrom'])); + foreach ($sample['pickups'] as $pickup) { + $io->writeln(sprintf(' %-14s %-6s %s', $pickup['city'] ?? '?', $pickup['time'] ?? '-', $pickup['datetime'] ?? '-')); + } + $io->newLine(); + } + + $io->writeln('Raw first row'); + $io->writeln(json_encode($row, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES)); + + $io->section('As the import reads it'); + + try { + $destinationRow = DestinationRow::fromArray($row); + } catch (InvalidRowException $e) { + $io->error(sprintf('The import would skip this row: %s', $e->getMessage())); + + return Command::FAILURE; + } + + $io->definitionList( + ['busProId' => (string) $destinationRow->busProId], + ['code' => var_export($destinationRow->code, true)], + ['product' => var_export($destinationRow->product, true)], + ['dateFrom' => $destinationRow->dateFrom->format('Y-m-d')], + ['dateTo' => $destinationRow->dateTo->format('Y-m-d')], + ['hotelBusProId' => (string) $destinationRow->hotelBusProId], + ['hotelCode' => var_export($destinationRow->hotelCode, true)], + ['hotelName' => var_export($destinationRow->hotelName, true)], + ['costUnit' => var_export($destinationRow->costUnit, true)], + ['country' => var_export($destinationRow->country, true)], + ['pickups' => json_encode($destinationRow->pickups, JSON_UNESCAPED_UNICODE)], + ); + + if ([] !== $row['pickups'] && [] === $destinationRow->pickups) { + $io->error('The export carries pickups but the import read none - check the id field name.'); + + return Command::FAILURE; + } + + $io->success('MyE&P answers and the import can read the row.'); + + return Command::SUCCESS; + } +} diff --git a/src/Command/MyEpImportCommand.php b/src/Command/MyEpImportCommand.php new file mode 100644 index 0000000..50c6bc2 --- /dev/null +++ b/src/Command/MyEpImportCommand.php @@ -0,0 +1,176 @@ +travelExportClient->fetchAll(); + } catch (ApiException $e) { + $io->error('Der Reise-Export konnte nicht von MyE&P geladen werden – Import abgebrochen.'); + $this->logger->error('Import aborted: the MyE&P travel export could not be read', [ + 'reason' => $e->getMessage(), + ]); + + return Command::FAILURE; + } + + // An empty export would otherwise be a run that touches nothing and reports success, + // which reads as "nothing changed upstream" rather than as a failure to read it. + if ([] === $export->rows) { + $io->error('Der Reise-Export von MyE&P enthält keine Reisen – Import abgebrochen.'); + $this->logger->error('Import aborted: the MyE&P travel export was empty'); + + return Command::FAILURE; + } + + // Every destination this import touches gets its cost unit and its country written from + // the export, on update as well as on insert. If MyE&P stops reporting those two fields, + // a run would quietly blank them on rows that have them: the cost unit then drops off + // the invoices (App\Service\Pdf\InvoiceApprover) and a missing country code blanks the + // flag icons and takes the row out of the country filters. Refusing to run is the + // recoverable failure; overwriting good data is not. + if ([] !== $missingFields = $this->missingHotelFields($export->rows)) { + $io->error(sprintf( + 'Die MyE&P-API liefert keine %s – Import abgebrochen, damit bestehende Werte nicht überschrieben werden.', + implode(' und ', $missingFields), + )); + $this->logger->error('Import aborted: the MyE&P travel export carries no '.implode(', ', $missingFields)); + + return Command::FAILURE; + } + + $io->info(sprintf('%d Reisen von MyE&P geladen', count($export->rows))); + + // Both are losses MyE&P made silently on its side: a skipped pair is a travel the + // disposition will never offer, and a boarding point without master data reaches the + // form with no town. Logged every run so they get chased instead of settling in. + $this->logger->info('MyE&P travel export read', [ + 'rows' => count($export->rows), + 'skipped' => $export->skipped, + 'pickups_without_master_data' => $export->pickupsWithoutMasterData, + 'generated_at' => $export->generatedAt, + ]); + + if (0 < $export->skipped || 0 < $export->pickupsWithoutMasterData) { + $io->warning(sprintf( + 'MyE&P hat %d Reise-Hotel-Kombinationen verworfen und %d Zustiege ohne Stammdaten gemeldet.', + $export->skipped, + $export->pickupsWithoutMasterData, + )); + } + + $now = new \DateTimeImmutable(); + $addedCount = 0; + $updatedCount = 0; + + $progressBar = $io->createProgressBar(count($export->rows)); + $progressBar->setFormat(" %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s%\n %message%"); + $progressBar->setMessage('Starting'); + $progressBar->start(); + + foreach ($export->rows as $row) { + $progressBar->setMessage(sprintf('%s %s', $row->dateFrom->format('d.m.Y'), $row->product)); + + $id = $this->connection->fetchOne( + 'SELECT id FROM destination WHERE bus_pro_id=? AND hotel_bus_pro_id=?', + [$row->busProId, $row->hotelBusProId], + ); + + if ($id) { + $this->connection->update('destination', $row->toUpdatePayload($now), ['id' => $id]); + ++$updatedCount; + } else { + $this->connection->insert('destination', $row->toInsertPayload($now)); + ++$addedCount; + } + + $progressBar->advance(); + } + + $progressBar->setMessage('Done'); + $progressBar->finish(); + $io->newLine(2); + + $io->success(sprintf('%d Destinationen angelegt, %d aktualisiert', $addedCount, $updatedCount)); + $this->logger->info('MyE&P destination import finished', [ + 'added' => $addedCount, + 'updated' => $updatedCount, + ]); + + return Command::SUCCESS; + } + + /** + * Names the hotel fields MyE&P is not reporting, German labels for the operator. + * + * Asserted across the whole export rather than per row, because a row legitimately has + * neither: only a minority of hotels carry a kostenstelle upstream, and some carry no + * country BusProNet can resolve. An export where not one row has the field is a field + * missing from the response, not a gap in the data. + * + * @param DestinationRow[] $rows + * + * @return list + */ + private function missingHotelFields(array $rows): array + { + $required = [ + 'Kostenstellen' => static fn (DestinationRow $row): bool => '' !== (string) $row->costUnit, + 'Länderkennungen' => static fn (DestinationRow $row): bool => '' !== (string) $row->country, + ]; + + $missing = []; + + foreach ($required as $label => $isReported) { + foreach ($rows as $row) { + if ($isReported($row)) { + continue 2; + } + } + + $missing[] = $label; + } + + return $missing; + } +} diff --git a/src/MyEp/Import/DestinationRow.php b/src/MyEp/Import/DestinationRow.php index decc44e..5380894 100644 --- a/src/MyEp/Import/DestinationRow.php +++ b/src/MyEp/Import/DestinationRow.php @@ -114,7 +114,12 @@ class DestinationRow /** * The shape here is what the destination table stores and what - * App\BusProNet\Model\Pickup::fromArray() reads back. + * App\BusProNet\Model\Pickup::fromArray() reads back, which ignores the extra `datetime`. + * + * `datetime` is not optional decoration. MyE&P sends `time` as a bare HH:MM, so without it + * App\Service\Assignment\PickupDepartureOrder never finds a dated entry, its anchor is null + * and it falls back to sorting on the clock alone - which puts a 00:30 boarding point ahead + * of the 22:00 departure on every route that runs through the night. */ private static function readPickups(mixed $pickups): array { @@ -140,6 +145,7 @@ class DestinationRow 'city' => self::readNullableString($pickup, 'city'), 'street' => self::readNullableString($pickup, 'street'), 'time' => self::readNullableString($pickup, 'time'), + 'datetime' => self::readNullableString($pickup, 'datetime'), ]; } diff --git a/src/MyEp/Import/TravelExport.php b/src/MyEp/Import/TravelExport.php new file mode 100644 index 0000000..9ea4a8b --- /dev/null +++ b/src/MyEp/Import/TravelExport.php @@ -0,0 +1,28 @@ +apiClient->get(self::PATH, [ @@ -63,6 +62,9 @@ class TravelExportClient } } + // The counters repeat unchanged on every page, so the first one answers for the run. + $meta = [] === $meta ? (array) ($payload['meta'] ?? []) : $meta; + $totalPages = max(1, (int) ($payload['meta']['totalPages'] ?? 1)); ++$page; } while ($page <= $totalPages && $page <= self::MAX_PAGES && [] !== $data); @@ -74,6 +76,11 @@ class TravelExportClient ]); } - return $rows; + return new TravelExport( + $rows, + (int) ($meta['skipped'] ?? 0), + (int) ($meta['pickupsWithoutMasterData'] ?? 0), + isset($meta['generatedAt']) ? (string) $meta['generatedAt'] : null, + ); } } diff --git a/tests/Command/BpnImportCommandTest.php b/tests/Command/BpnImportCommandTest.php deleted file mode 100644 index 3d36ab1..0000000 --- a/tests/Command/BpnImportCommandTest.php +++ /dev/null @@ -1,147 +0,0 @@ -runImport([$this->hotel(costUnit: null, countryCode: 'ch')]); - - $this->assertSame(Command::FAILURE, $tester->getStatusCode()); - $this->assertStringContainsString('Kostenstellen', $tester->getDisplay()); - $this->assertStringNotContainsString('Länderkennungen', $tester->getDisplay()); - } - - public function testItRefusesToRunWhenNoHotelReportsACountryCode(): void - { - $tester = $this->runImport([$this->hotel(costUnit: '4711', countryCode: null)]); - - $this->assertSame(Command::FAILURE, $tester->getStatusCode()); - $this->assertStringContainsString('Länderkennungen', $tester->getDisplay()); - } - - public function testItNamesBothFieldsWhenNeitherIsReported(): void - { - $tester = $this->runImport([$this->hotel(costUnit: null, countryCode: null)]); - - $this->assertSame(Command::FAILURE, $tester->getStatusCode()); - $this->assertStringContainsString('Kostenstellen und Länderkennungen', $tester->getDisplay()); - } - - public function testAnEmptyStringDoesNotCountAsAReportedField(): void - { - $tester = $this->runImport([$this->hotel(costUnit: '', countryCode: '')]); - - $this->assertSame(Command::FAILURE, $tester->getStatusCode()); - $this->assertStringContainsString('Kostenstellen und Länderkennungen', $tester->getDisplay()); - } - - public function testOneHotelCarryingAFieldIsEnoughToPassTheGuard(): void - { - // most hotels legitimately have no kostenstelle upstream, so the guard asks whether - // the field is reported at all, not whether every hotel fills it - $tester = $this->runImport([ - $this->hotel(costUnit: null, countryCode: null), - $this->hotel(costUnit: '4711', countryCode: 'ch'), - ]); - - $display = $tester->getDisplay(); - - $this->assertStringNotContainsString('Kostenstellen', $display); - $this->assertStringNotContainsString('Länderkennungen', $display); - } - - public function testItRefusesToRunWithoutAnyHotelBaseData(): void - { - $tester = $this->runImport([]); - - $this->assertSame(Command::FAILURE, $tester->getStatusCode()); - $this->assertStringContainsString('Hotel-Stammdaten', $tester->getDisplay()); - } - - /** - * @param list $hotels - */ - private function runImport(array $hotels): CommandTester - { - $keyed = []; - - foreach ($hotels as $index => $hotel) { - $keyed[$index + 1] = $hotel; - } - - $tester = new CommandTester(new BpnImportCommand( - $this->refusingConnection(), - $this->provider(HotelDataProvider::class, $keyed), - $this->provider(PickupDataProvider::class, []), - new NullLogger(), - // an auto stubbed listing, so it yields no files. The guards under test all run - // before it, and the one case that gets past them stops there without writing. - $this->createMock(FilesystemOperator::class), - )); - - $tester->execute([]); - - return $tester; - } - - /** - * @template T of HotelDataProvider|PickupDataProvider - * - * @param class-string $class - * - * @return T&MockObject - */ - private function provider(string $class, array $all): object - { - $provider = $this->createMock($class); - $provider->method('getAll')->willReturn($all); - - return $provider; - } - - /** - * Any query at all means the guard let a write through, which is the whole failure this - * covers, so the connection fails the test rather than the command. - */ - private function refusingConnection(): Connection&MockObject - { - $connection = $this->createMock(Connection::class); - $connection->expects($this->never())->method($this->anything()); - - return $connection; - } - - private function hotel(?string $costUnit, ?string $countryCode): Hotel - { - return (new Hotel()) - ->setBusProId(163959) - ->setCode('LPJGR') - ->setName('Sportclub Jenatsch') - ->setCostUnit($costUnit) - ->setCountryCode($countryCode) - ; - } -} diff --git a/tests/Command/MyEpImportCommandTest.php b/tests/Command/MyEpImportCommandTest.php new file mode 100644 index 0000000..85f0f23 --- /dev/null +++ b/tests/Command/MyEpImportCommandTest.php @@ -0,0 +1,239 @@ +runImport([$this->row(costUnit: null, country: 'ch')]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('Kostenstellen', $tester->getDisplay()); + $this->assertStringNotContainsString('Länderkennungen', $tester->getDisplay()); + } + + public function testItRefusesToRunWhenNoRowReportsACountryCode(): void + { + $tester = $this->runImport([$this->row(costUnit: '4711', country: null)]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('Länderkennungen', $tester->getDisplay()); + } + + public function testItNamesBothFieldsWhenNeitherIsReported(): void + { + $tester = $this->runImport([$this->row(costUnit: null, country: null)]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('Kostenstellen und Länderkennungen', $tester->getDisplay()); + } + + public function testAnEmptyStringDoesNotCountAsAReportedField(): void + { + $tester = $this->runImport([$this->row(costUnit: '', country: '')]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('Kostenstellen und Länderkennungen', $tester->getDisplay()); + } + + /** + * Most hotels legitimately have no kostenstelle upstream, so the guard asks whether the field + * is reported at all, not whether every row fills it. + */ + public function testOneRowCarryingAFieldIsEnoughToPassTheGuard(): void + { + $tester = $this->runImport( + [$this->row(costUnit: null, country: null), $this->row(costUnit: '4711', country: 'ch')], + $this->writingConnection(existingId: 7), + ); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringNotContainsString('Kostenstellen', $tester->getDisplay()); + } + + /** + * A run that touches nothing and reports success reads as "nothing changed upstream" rather + * than as a failure to read the export at all. + */ + public function testItRefusesToRunOnAnEmptyExport(): void + { + $tester = $this->runImport([]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('keine Reisen', $tester->getDisplay()); + } + + public function testItRefusesToRunWhenTheExportCannotBeRead(): void + { + $client = $this->createMock(TravelExportClient::class); + $client->method('fetchAll')->willThrowException(new ApiException('gateway timeout')); + + $tester = new CommandTester(new MyEpImportCommand( + $this->refusingConnection(), + $client, + new NullLogger(), + )); + $tester->execute([]); + + $this->assertSame(Command::FAILURE, $tester->getStatusCode()); + $this->assertStringContainsString('konnte nicht von MyE&P geladen werden', $tester->getDisplay()); + } + + public function testAKnownPairIsUpdatedRatherThanInserted(): void + { + $connection = $this->createMock(Connection::class); + $connection->method('fetchOne')->willReturn(42); + $connection->expects($this->once())->method('update')->with( + 'destination', + $this->callback(function (array $payload): bool { + // the hotel name and code are the point: leaving them out of the update was what + // let a renamed hotel keep a stale name forever and drop out of house matching + $this->assertArrayHasKey('hotel', $payload); + $this->assertArrayHasKey('hotel_code', $payload); + $this->assertArrayNotHasKey('bus_pro_id', $payload); + + return true; + }), + ['id' => 42], + ); + $connection->expects($this->never())->method('insert'); + + $tester = $this->runImport([$this->row(costUnit: '4711', country: 'ch')], $connection); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('0 Destinationen angelegt, 1 aktualisiert', $tester->getDisplay()); + } + + public function testAnUnknownPairIsInserted(): void + { + $connection = $this->createMock(Connection::class); + $connection->method('fetchOne')->willReturn(false); + $connection->expects($this->once())->method('insert')->with( + 'destination', + $this->callback(function (array $payload): bool { + $this->assertSame(12778, $payload['bus_pro_id']); + $this->assertSame(222928, $payload['hotel_bus_pro_id']); + $this->assertSame('ch', $payload['country']); + + return true; + }), + ); + $connection->expects($this->never())->method('update'); + + $tester = $this->runImport([$this->row(costUnit: '4711', country: 'ch')], $connection); + + $this->assertStringContainsString('1 Destinationen angelegt, 0 aktualisiert', $tester->getDisplay()); + } + + /** + * Both counters are losses MyE&P made on its side and would otherwise not surface anywhere. + */ + public function testTheOperatorIsToldWhatMyEpDropped(): void + { + $tester = $this->runImport( + [$this->row(costUnit: '4711', country: 'ch')], + $this->writingConnection(existingId: 7), + skipped: 3, + pickupsWithoutMasterData: 21, + ); + + $display = $tester->getDisplay(); + + $this->assertStringContainsString('3 Reise-Hotel-Kombinationen verworfen', $display); + $this->assertStringContainsString('21 Zustiege ohne Stammdaten', $display); + } + + public function testACleanExportSaysNothingAboutDroppedRows(): void + { + $tester = $this->runImport( + [$this->row(costUnit: '4711', country: 'ch')], + $this->writingConnection(existingId: 7), + ); + + $this->assertStringNotContainsString('verworfen', $tester->getDisplay()); + } + + /** + * @param DestinationRow[] $rows + */ + private function runImport( + array $rows, + ?Connection $connection = null, + int $skipped = 0, + int $pickupsWithoutMasterData = 0, + ): CommandTester { + $client = $this->createMock(TravelExportClient::class); + $client->method('fetchAll')->willReturn( + new TravelExport($rows, $skipped, $pickupsWithoutMasterData, '2026-09-29T03:20:00+02:00') + ); + + $tester = new CommandTester(new MyEpImportCommand( + $connection ?? $this->refusingConnection(), + $client, + new NullLogger(), + )); + + $tester->execute([]); + + return $tester; + } + + /** + * Any query at all means the guard let a write through, which is the whole failure the + * refusal tests cover, so the connection fails the test rather than the command. + */ + private function refusingConnection(): Connection&MockObject + { + $connection = $this->createMock(Connection::class); + $connection->expects($this->never())->method($this->anything()); + + return $connection; + } + + private function writingConnection(int $existingId): Connection&MockObject + { + $connection = $this->createMock(Connection::class); + $connection->method('fetchOne')->willReturn($existingId); + + return $connection; + } + + private function row(?string $costUnit, ?string $country): DestinationRow + { + return new DestinationRow( + 12778, + 'DWWSCH070227', + 'Davos - Sportclub Weissfluh', + new \DateTimeImmutable('2027-02-07'), + new \DateTimeImmutable('2027-02-12'), + 222928, + 'DWWGRU2', + 'Sportclub Weißfluh', + $costUnit, + $country, + [], + ); + } +} diff --git a/tests/MyEp/Import/DestinationRowTest.php b/tests/MyEp/Import/DestinationRowTest.php index 016ad30..1b4d8c5 100644 --- a/tests/MyEp/Import/DestinationRowTest.php +++ b/tests/MyEp/Import/DestinationRowTest.php @@ -60,9 +60,32 @@ class DestinationRowTest extends TestCase 'city' => 'Karlsruhe', 'street' => 'A5 - Autohof Bruchsal', 'time' => '23:59', + 'datetime' => '2026-04-04T23:59:00+02:00', ]], $row->pickups); } + /** + * MyE&P sends `time` as a bare HH:MM, so this is the only field that says which calendar day + * a boarding point belongs to. Dropping it left App\Service\Assignment\PickupDepartureOrder + * with no anchor and a naive clock sort, which puts a 00:30 stop before the 22:00 departure. + */ + public function testTheResolvedDepartureTimestampIsCarried(): void + { + $row = DestinationRow::fromArray($this->getRowData()); + + $this->assertSame('2026-04-04T23:59:00+02:00', $row->pickups[0]['datetime']); + } + + public function testAPickupWithoutAResolvedTimestampIsStillCarried(): void + { + $row = DestinationRow::fromArray($this->getRowData(['pickups' => [ + ['busProId' => 27, 'city' => 'Ulm', 'street' => 'Busparkplatz', 'time' => null], + ]])); + + $this->assertNull($row->pickups[0]['datetime']); + $this->assertCount(1, $row->pickups); + } + public function testStoredPickupsCanBeReadBackAsModels(): void { $row = DestinationRow::fromArray($this->getRowData()); @@ -129,7 +152,7 @@ class DestinationRowTest extends TestCase 'hotel' => 'Sportclub Jenatsch', 'hotel_code' => 'LPJGR', 'hotel_bus_pro_id' => 163959, - 'pickups' => '[{"busProId":4,"city":"Karlsruhe","street":"A5 - Autohof Bruchsal","time":"23:59"}]', + 'pickups' => '[{"busProId":4,"city":"Karlsruhe","street":"A5 - Autohof Bruchsal","time":"23:59","datetime":"2026-04-04T23:59:00+02:00"}]', 'cost_unit' => '4711', 'country' => 'ch', 'created_at' => '2026-08-13 03:00:00', diff --git a/tests/MyEp/Import/TravelExportClientTest.php b/tests/MyEp/Import/TravelExportClientTest.php index 8e86cad..c8ecd2c 100644 --- a/tests/MyEp/Import/TravelExportClientTest.php +++ b/tests/MyEp/Import/TravelExportClientTest.php @@ -21,7 +21,7 @@ class TravelExportClientTest extends TestCase $this->page([$this->row(3, 300)], 2, 2), ]); - $rows = $this->getClientInstance($httpClient)->fetchAll(); + $rows = $this->getClientInstance($httpClient)->fetchAll()->rows; $this->assertCount(3, $rows); $this->assertEquals([1, 2, 3], array_map(fn ($row) => $row->busProId, $rows)); @@ -62,7 +62,7 @@ class TravelExportClientTest extends TestCase ], 1, 1), ]); - $rows = $this->getClientInstance($httpClient)->fetchAll(); + $rows = $this->getClientInstance($httpClient)->fetchAll()->rows; $this->assertCount(1, $rows); $this->assertEquals(2, $rows[0]->busProId); @@ -72,7 +72,7 @@ class TravelExportClientTest extends TestCase { $httpClient = $this->mockClient([$this->page([], 1, 1)]); - $this->assertEquals([], $this->getClientInstance($httpClient)->fetchAll()); + $this->assertEquals([], $this->getClientInstance($httpClient)->fetchAll()->rows); } public function testAPayloadWithoutADataListFails(): void @@ -97,14 +97,71 @@ class TravelExportClientTest extends TestCase 'code' => 'AAA', 'name' => 'Haus', 'costUnit' => null, - 'country' => 'ch', + 'country' => 'CH', + 'countryCode' => 'ch', ], 'pickups' => [], ]; } - private function page(array $rows, int $page, int $totalPages): MockResponse + /** + * Both counters are losses MyE&P made on its side - a dropped travel, a boarding point with no + * town - and the import is the only place they surface, so the client has to carry them out. + */ + public function testTheCountersMyEpReportsAreCarriedOut(): void { + $httpClient = $this->mockClient([$this->page([$this->row(1, 100)], 1, 1, skipped: 3, pickupsWithoutMasterData: 21)]); + + $export = $this->getClientInstance($httpClient)->fetchAll(); + + $this->assertSame(3, $export->skipped); + $this->assertSame(21, $export->pickupsWithoutMasterData); + $this->assertSame('2026-09-29T03:20:00+02:00', $export->generatedAt); + } + + /** + * They describe the whole export rather than the page, so they repeat unchanged and the first + * page answers for the run. Reading the last one instead would be just as correct today and + * wrong the moment a page is served from a newer build. + */ + public function testTheCountersAreReadFromTheFirstPage(): void + { + $httpClient = $this->mockClient([ + $this->page([$this->row(1, 100)], 1, 2, skipped: 3, pickupsWithoutMasterData: 21), + $this->page([$this->row(2, 200)], 2, 2, skipped: 99, pickupsWithoutMasterData: 99), + ]); + + $export = $this->getClientInstance($httpClient)->fetchAll(); + + $this->assertSame(3, $export->skipped); + $this->assertSame(21, $export->pickupsWithoutMasterData); + } + + /** + * An export whose meta block says nothing must not read as one that dropped nothing by + * accident of a missing key, but zero is the only honest default available here. + */ + public function testAMetaBlockWithoutCountersReadsAsZero(): void + { + $httpClient = new MockHttpClient( + [new MockResponse(json_encode(['data' => [$this->row(1, 100)], 'meta' => ['totalPages' => 1]]))], + 'https://myep.example/', + ); + + $export = $this->getClientInstance($httpClient)->fetchAll(); + + $this->assertSame(0, $export->skipped); + $this->assertSame(0, $export->pickupsWithoutMasterData); + $this->assertNull($export->generatedAt); + } + + private function page( + array $rows, + int $page, + int $totalPages, + int $skipped = 0, + int $pickupsWithoutMasterData = 0, + ): MockResponse { return new MockResponse(json_encode([ 'data' => $rows, 'meta' => [ @@ -112,6 +169,9 @@ class TravelExportClientTest extends TestCase 'perPage' => 500, 'total' => count($rows), 'totalPages' => $totalPages, + 'skipped' => $skipped, + 'pickupsWithoutMasterData' => $pickupsWithoutMasterData, + 'generatedAt' => '2026-09-29T03:20:00+02:00', ], ])); }