From f5e6d2105d0c172d4f434ea938418f2da2ae79cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 29 Sep 2026 10:10:36 +0200 Subject: [PATCH] feat: make boarding point price levelling switchable --- .env | 9 +++++++ config/services.yaml | 4 +++ docs/buspro-database-reference.md | 11 ++++++++ src/BusProNet/XmlParser/ApiResponseParser.php | 7 ++++- src/BusProNet/XmlParser/TravelParser.php | 25 ++++++++++++++--- .../ApiResponseParserFailureTest.php | 19 +++++++++---- .../BusProNet/XmlParser/TravelParserTest.php | 27 ++++++++++++++++--- ...articipantFieldOptionsProviderBabyTest.php | 3 ++- 8 files changed, 92 insertions(+), 13 deletions(-) diff --git a/.env b/.env index 0f2ff8ed..683675a4 100644 --- a/.env +++ b/.env @@ -92,6 +92,15 @@ APP_TRAVEL_PREFER_REMOTE=false APP_TRAVEL_ENABLE_FALLBACK=true APP_TRAVEL_SNAPSHOT_RETENTION_BUFFER_DAYS=14 +# Compensates a BusPro export defect: a can omit its preis although the booking +# engine still bills the return-leg Abfahrtsortzuschlag, so we undercharge by exactly one leg. +# A boarding point costs the same in both directions, so the stated price is applied to both. +# Off = report the export verbatim. Provisional until the vendor fixes the export; see +# docs/buspro-database-reference.md section A7. +# Flipping this needs `app:bpn:xml-cache-invalidate` and `app:bpn:refresh-travel-snapshot`, +# because the cached travels and TravelSnapshot rows carry the parsed price. +APP_BPN_LEVEL_BOARDING_POINT_PRICES=true + # Emails APP_DEFAULT_EMAIL_FROM=info@ep-reisen.de APP_DEFAULT_EMAIL_TO=info@ep-reisen.de diff --git a/config/services.yaml b/config/services.yaml index 652f9dae..ad5eaca6 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -185,6 +185,10 @@ services: arguments: $retentionBufferDays: '%travel_snapshot_retention_buffer_days%' + App\BusProNet\XmlParser\TravelParser: + arguments: + $levelBoardingPointPrices: '%env(bool:APP_BPN_LEVEL_BOARDING_POINT_PRICES)%' + App\Service\TravelDataProvider: arguments: $logger: '@monolog.logger.core' diff --git a/docs/buspro-database-reference.md b/docs/buspro-database-reference.md index ae97816b..977db965 100644 --- a/docs/buspro-database-reference.md +++ b/docs/buspro-database-reference.md @@ -225,6 +225,17 @@ JOIN dbo.ZustiegPreis zp ON zp.IDArtZustiegPreis_FS=lb.IDArtZustiegPreis_FS WHERE l.IDLeistung IN (@OutboundServiceId,@ReturnServiceId); ``` +**V 2026-09-28 — the XML export can omit the return-leg price.** Since one price row is +surfaced twice, an asymmetric pair is an export defect: on products 701 and 2435 a +`` carries no `preis` although the engine still bills the return +`Abfahrtsortzuschlag`, so reading the export verbatim undercharges by one leg. Measured over +all exports, a stated return price never differs from its outbound counterpart (510 pairs, +zero counter-examples). `TravelParser` therefore levels a boarding point's price across both +directions, switchable through `APP_BPN_LEVEL_BOARDING_POINT_PRICES` while the defect is open +with the vendor. Flipping the flag needs `app:bpn:xml-cache-invalidate` and +`app:bpn:refresh-travel-snapshot`. A point appearing **only** in `zustiege_rueck` without a +price cannot be priced from our side — that is a catalog job. + **N — the departure surcharge is computed.** Booking responses carry a position `art="SON" unterart="TRA"` (*Abfahrtsortzuschlag*) with no matching row type: `ArtZuschlag` — the lookup behind `BuchungTNZuschlag` — holds only discounts and fees diff --git a/src/BusProNet/XmlParser/ApiResponseParser.php b/src/BusProNet/XmlParser/ApiResponseParser.php index df893b9a..9fab5779 100644 --- a/src/BusProNet/XmlParser/ApiResponseParser.php +++ b/src/BusProNet/XmlParser/ApiResponseParser.php @@ -8,6 +8,11 @@ use App\BusProNet\XmlCrawlerFactory; class ApiResponseParser extends AbstractParser { + public function __construct( + private readonly TravelParser $travelParser, + ) { + } + /** * @throws ResponseParserException */ @@ -81,7 +86,7 @@ class ApiResponseParser extends AbstractParser throw new ResponseParserException('No travel data node found in product data response'); } - return (new TravelParser())->parse($travelNode->first(), ...$additionalArgs); + return $this->travelParser->parse($travelNode->first(), ...$additionalArgs); case ApiClient::TYPE_AGENCIES: return (new AgencyParser())->parse($resultNode); case ApiClient::TYPE_PURCHASE_VOUCHER: diff --git a/src/BusProNet/XmlParser/TravelParser.php b/src/BusProNet/XmlParser/TravelParser.php index f5234181..15dd010f 100644 --- a/src/BusProNet/XmlParser/TravelParser.php +++ b/src/BusProNet/XmlParser/TravelParser.php @@ -29,8 +29,20 @@ class TravelParser extends AbstractParser Constants::PSEUDO_PRICE_ROOM_CODE, ]; + /** + * `$levelBoardingPointPrices` carries no default on purpose: a hand-built parser would + * otherwise silently ignore APP_BPN_LEVEL_BOARDING_POINT_PRICES and price one travel tier + * differently from the others. The registry gave up its own default along with it, so that + * there is one way to build this class — through the container, or explicitly in a test. + * + * @param bool $levelBoardingPointPrices Whether to compensate the BusPro export defect that + * omits a boarding point's return-leg price. The + * container wires it from + * APP_BPN_LEVEL_BOARDING_POINT_PRICES. + */ public function __construct( - private readonly AgeConstraintParserRegistry $ageConstraintRegistry = new AgeConstraintParserRegistry(), + private readonly AgeConstraintParserRegistry $ageConstraintRegistry, + private readonly bool $levelBoardingPointPrices, ) { } @@ -88,7 +100,9 @@ class TravelParser extends AbstractParser $travel->dropOffs = $this->getPickups($node->filterXPath('.//zustiege_rueck/zustieg_rueck'), $dateTo, false); // A boarding point costs the same in both directions - $this->levelBoardingPointPrices($travel->pickups, $travel->dropOffs); + if ($this->levelBoardingPointPrices) { + $this->applyBoardingPointPriceLevelling($travel->pickups, $travel->dropOffs); + } $travel->guide = $this->getGuide($node); @@ -465,10 +479,15 @@ class TravelParser extends AbstractParser * Where both directions state a price they agree in practice; the outbound one wins * to keep the outcome deterministic. Points without a counterpart are left as they are. * + * Compensating the defect locally is provisional, so it is switchable: with + * `$levelBoardingPointPrices` off the export is reported verbatim and the affected + * travels undercharge the return leg again. See + * `docs/buspro-database-reference.md` section A7. + * * @param array $pickups Pickups indexed by ID (modified in place) * @param array $dropOffs Drop-offs indexed by ID (modified in place) */ - private function levelBoardingPointPrices(array &$pickups, array $dropOffs): void + private function applyBoardingPointPriceLevelling(array $pickups, array $dropOffs): void { foreach ($dropOffs as $pickupId => $dropOff) { if (false === isset($pickups[$pickupId])) { diff --git a/tests/BusProNet/XmlParser/ApiResponseParserFailureTest.php b/tests/BusProNet/XmlParser/ApiResponseParserFailureTest.php index 4e4738f8..e635793f 100644 --- a/tests/BusProNet/XmlParser/ApiResponseParserFailureTest.php +++ b/tests/BusProNet/XmlParser/ApiResponseParserFailureTest.php @@ -7,6 +7,7 @@ namespace App\Tests\BusProNet\XmlParser; use App\BusProNet\ApiClient; use App\BusProNet\Exception\ResponseParserException; use App\BusProNet\XmlParser\ApiResponseParser; +use App\BusProNet\XmlParser\TravelParser; use PHPUnit\Framework\TestCase; /** @@ -22,7 +23,7 @@ class ApiResponseParserFailureTest extends TestCase $this->expectException(ResponseParserException::class); $this->expectExceptionMessage('empty response (0 bytes)'); - (new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, ''); + $this->createParser()->parseXmlString(ApiClient::TYPE_NOTIFICATION, ''); } public function testTruncatedResponseNamesTheLibxmlReason(): void @@ -30,7 +31,7 @@ class ApiResponseParserFailureTest extends TestCase $xml = 'parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml); + $this->createParser()->parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml); $this->fail('Expected a ResponseParserException'); } catch (ResponseParserException $e) { self::assertStringContainsString('('.strlen($xml).' bytes)', $e->getMessage()); @@ -45,7 +46,7 @@ class ApiResponseParserFailureTest extends TestCase $this->expectException(ResponseParserException::class); $this->expectExceptionMessage('Unrecognised BusProNet response type "IRGENDWAS"'); - (new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml); + $this->createParser()->parseXmlString(ApiClient::TYPE_NOTIFICATION, $xml); } public function testUnknownCustomerDataSubtype(): void @@ -55,7 +56,7 @@ class ApiResponseParserFailureTest extends TestCase $this->expectException(ResponseParserException::class); $this->expectExceptionMessage('Unrecognised customer data subtype "Irgendwas"'); - (new ApiResponseParser())->parseXmlString(ApiClient::TYPE_CUSTOMER_DATA, $xml); + $this->createParser()->parseXmlString(ApiClient::TYPE_CUSTOMER_DATA, $xml); } public function testDiagnosingDoesNotLeakLibxmlErrorState(): void @@ -64,7 +65,7 @@ class ApiResponseParserFailureTest extends TestCase try { try { - (new ApiResponseParser())->parseXmlString(ApiClient::TYPE_NOTIFICATION, 'createParser()->parseXmlString(ApiClient::TYPE_NOTIFICATION, 'createStub(TravelParser::class)); + } } diff --git a/tests/BusProNet/XmlParser/TravelParserTest.php b/tests/BusProNet/XmlParser/TravelParserTest.php index 8bc0cabb..ca836ef9 100644 --- a/tests/BusProNet/XmlParser/TravelParserTest.php +++ b/tests/BusProNet/XmlParser/TravelParserTest.php @@ -6,6 +6,7 @@ namespace App\Tests\BusProNet\XmlParser; use App\BusProNet\Model\Travel; use App\BusProNet\XmlCrawlerFactory; +use App\BusProNet\XmlParser\AgeConstraintParserRegistry; use App\BusProNet\XmlParser\TravelParser; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -16,7 +17,7 @@ class TravelParserTest extends TestCase protected function setUp(): void { - $this->parser = new TravelParser(); + $this->parser = $this->createParser(); } public function testParseServiceDescriptions(): void @@ -411,7 +412,27 @@ class TravelParserTest extends TestCase $this->assertSame(5.9, $travel->dropOffs[99]->price); } - private function parseTravelWithBoardingPoints(string $pickups, string $dropOffs): Travel + public function testBoardingPricesAreReportedVerbatimWhenLevellingIsDisabled(): void + { + // The levelling compensates a vendor export defect and is therefore switchable. + // With it off the product 701 shape undercharges the return leg again. + $travel = $this->parseTravelWithBoardingPoints( + '', + '', + $this->createParser(levelBoardingPointPrices: false) + ); + + $this->assertSame(17.9, $travel->pickups[18]->price); + $this->assertNull($travel->pickups[18]->priceInbound); + $this->assertNull($travel->dropOffs[18]->price); + } + + private function createParser(bool $levelBoardingPointPrices = true): TravelParser + { + return new TravelParser(new AgeConstraintParserRegistry(), $levelBoardingPointPrices); + } + + private function parseTravelWithBoardingPoints(string $pickups, string $dropOffs, ?TravelParser $parser = null): Travel { $xmlContent = ' @@ -426,7 +447,7 @@ class TravelParserTest extends TestCase $crawler = XmlCrawlerFactory::create($xmlContent); - return $this->parser->parse($crawler->filterXPath('//reise/termin')->first()); + return ($parser ?? $this->parser)->parse($crawler->filterXPath('//reise/termin')->first()); } public function testGetPickupsAdjustsOvernightTimes(): void diff --git a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php index dcb29bb7..7be900eb 100644 --- a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php +++ b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php @@ -10,6 +10,7 @@ use App\BusProNet\Model\Service; use App\BusProNet\Model\Travel; use App\BusProNet\Utility\DirectionMapper; use App\BusProNet\XmlCrawlerFactory; +use App\BusProNet\XmlParser\AgeConstraintParserRegistry; use App\BusProNet\XmlParser\TravelParser; use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; @@ -487,7 +488,7 @@ class ParticipantFieldOptionsProviderBabyTest extends TestCase */ private function createTravelWithExportedSkiPasses(array $serviceIds): Travel { - $parser = new TravelParser(); + $parser = new TravelParser(new AgeConstraintParserRegistry(), true); $crawler = XmlCrawlerFactory::create(file_get_contents('/var/www/html/var/xmlexport/Ziel_2456.xml')); $travelNode = $crawler->filterXPath('//reise/termin')->first(); $travel = $parser->parse($travelNode);