feat: make boarding point price levelling switchable

This commit is contained in:
fromme committed 2026-09-29 10:11:03 +02:00
1 parent 0c6126010f
commit f5e6d2105d
8 files changed
+92 -13

No files matched your search

+9
View File
@@ -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 <zustieg_rueck> 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=[email protected]
APP_DEFAULT_EMAIL_TO=[email protected]
+4
View File
@@ -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'
+11
View File
@@ -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
`<zustieg_rueck>` 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
@@ -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:
+22 -3
View File
@@ -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<int, Pickup> $pickups Pickups indexed by ID (modified in place)
* @param array<int, Pickup> $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])) {
@@ -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 = '<?xml version="1.0" encoding="utf-8"?><ergebnis><satz typ="HINWEIS"></satz><hinweis';
try {
(new ApiResponseParser())->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, '<ergebnis');
$this->createParser()->parseXmlString(ApiClient::TYPE_NOTIFICATION, '<ergebnis');
} catch (ResponseParserException) {
}
@@ -74,4 +75,12 @@ class ApiResponseParserFailureTest extends TestCase
libxml_use_internal_errors($previous);
}
}
/**
* None of these responses reach the travel parser; a stub keeps the dependency out of view.
*/
private function createParser(): ApiResponseParser
{
return new ApiResponseParser($this->createStub(TravelParser::class));
}
}
+24 -3
View File
@@ -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(
'<zustieg idbuspro="18" zeit="12.12.2026 21:45:00" preis="17,90" />',
'<zustieg_rueck idbuspro="18" />',
$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 = '<?xml version="1.0" encoding="utf-8"?>
<reisen>
@@ -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
@@ -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);