From 1845441bc29a02ccec3e0a1d7c2f1c2572429ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 29 Sep 2026 12:07:13 +0200 Subject: [PATCH] feat: expose hotel cost unit and iso country code via the api --- config/services.yaml | 4 + src/BusProNet/Model/Hotel.php | 11 ++ src/BusProNet/Utility/CountryCodeMapper.php | 120 ++++++++++++++++++ src/BusProNet/XmlLoader/HotelLoader.php | 15 +++ .../DataLoader/HotelDataLoaderTest.php | 6 +- .../Utility/CountryCodeMapperTest.php | 102 +++++++++++++++ tests/BusProNet/XmlLoader/HotelLoaderTest.php | 66 +++++++++- 7 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 src/BusProNet/Utility/CountryCodeMapper.php create mode 100644 tests/BusProNet/Utility/CountryCodeMapperTest.php diff --git a/config/services.yaml b/config/services.yaml index 4d9c21fb..520396d9 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -207,6 +207,10 @@ services: arguments: $cache: '@bpn.cache' + App\BusProNet\Utility\CountryCodeMapper: + arguments: + $logger: '@monolog.logger.bpn' + App\BusProNet\XmlLoader\HotelLoader: arguments: $cache: '@bpn.cache' diff --git a/src/BusProNet/Model/Hotel.php b/src/BusProNet/Model/Hotel.php index 755605b7..de9abee2 100644 --- a/src/BusProNet/Model/Hotel.php +++ b/src/BusProNet/Model/Hotel.php @@ -35,4 +35,15 @@ class Hotel #[Groups(['api:list', 'api:single', 'snapshot'])] public ?string $country = null; + + /** + * Raw `kostenstelle`, deliberately a string: coverage is thin and some hotels carry a + * space-separated list such as "84 85", so it is an opaque label, not a number. + */ + #[Groups(['api:list', 'api:single', 'snapshot'])] + public ?string $costUnit = null; + + /** {@see \App\BusProNet\Utility\CountryCodeMapper} — lowercase ISO-2, null when unmappable. */ + #[Groups(['api:list', 'api:single', 'snapshot'])] + public ?string $countryCode = null; } diff --git a/src/BusProNet/Utility/CountryCodeMapper.php b/src/BusProNet/Utility/CountryCodeMapper.php new file mode 100644 index 00000000..404306e7 --- /dev/null +++ b/src/BusProNet/Utility/CountryCodeMapper.php @@ -0,0 +1,120 @@ + + */ + private const MAP = [ + 'A' => 'at', + 'AT' => 'at', + 'AUT' => 'at', + 'B' => 'be', + 'CA' => 'ca', + 'CH' => 'ch', + 'CZ' => 'cz', + 'D' => 'de', + 'DEU' => 'de', + 'DK' => 'dk', + 'E' => 'es', + 'ES' => 'es', + 'F' => 'fr', + 'FI' => 'fi', + 'FL' => 'li', + 'FR' => 'fr', + 'GR' => 'gr', + 'GRE' => 'gr', + 'HR' => 'hr', + 'HRV' => 'hr', + 'HU' => 'hu', + 'I' => 'it', + 'IL' => 'il', + 'IR' => 'ie', + 'IS' => 'is', + 'IT' => 'it', + 'LT' => 'lt', + 'LV' => 'lv', + 'MT' => 'mt', + 'N' => 'no', + 'NL' => 'nl', + 'PL' => 'pl', + 'PRT' => 'pt', + 'PT' => 'pt', + 'RUS' => 'ru', + 'S' => 'se', + 'SI' => 'si', + 'SLO' => 'sk', + 'UK' => 'gb', + 'USA' => 'us', + 'ZA' => 'za', + ]; + + /** + * Tokens already reported in this process, so a master data file full of the same + * unknown token produces one log line rather than one per hotel. + * + * @var array + */ + private array $reported = []; + + public function __construct( + private readonly LoggerInterface $logger, + ) { + } + + /** + * @param string|null $token the raw `hotel/land` value + * + * @return string|null lowercase ISO-2, or null when absent, empty or unmappable + */ + public function map(?string $token): ?string + { + if (null === $token) { + return null; + } + + $normalized = strtoupper(trim($token)); + + if ('' === $normalized) { + return null; + } + + if (isset(self::MAP[$normalized])) { + return self::MAP[$normalized]; + } + + if (false === isset($this->reported[$normalized])) { + $this->reported[$normalized] = true; + $this->logger->warning('Unmapped BusPro country token', ['token' => $token]); + } + + return null; + } +} diff --git a/src/BusProNet/XmlLoader/HotelLoader.php b/src/BusProNet/XmlLoader/HotelLoader.php index e8244d0a..102d16d7 100644 --- a/src/BusProNet/XmlLoader/HotelLoader.php +++ b/src/BusProNet/XmlLoader/HotelLoader.php @@ -4,14 +4,25 @@ namespace App\BusProNet\XmlLoader; use App\BusProNet\Model\Hotel; use App\BusProNet\Model\Travel; +use App\BusProNet\Utility\CountryCodeMapper; use App\BusProNet\XmlCrawlerFactory; use App\Exception\HotelNotFoundException; +use League\Flysystem\FilesystemOperator; use Psr\Cache\InvalidArgumentException; use Symfony\Component\DomCrawler\Crawler; +use Symfony\Contracts\Cache\CacheInterface; use Symfony\Contracts\Cache\ItemInterface; class HotelLoader extends AbstractLoader { + public function __construct( + private readonly CountryCodeMapper $countryCodeMapper, + CacheInterface $cache, + FilesystemOperator $xmlExport, + ) { + parent::__construct($cache, $xmlExport); + } + /** @return array */ public function loadAll(?string $filename = 'hotel.xml'): array { @@ -87,6 +98,10 @@ class HotelLoader extends AbstractLoader $hotel->country = $this->getStringOrNullValue($node->filterXPath('//land')); $hotel->street = $this->getStringOrNullValue($node->filterXPath('//strasse')); $hotel->phone = $this->getStringOrNullValue($node->filterXPath('//telefon')); + // String, never int: see Hotel::$costUnit. + $hotel->costUnit = $this->getStringOrNullValue($node->filterXPath('//kostenstelle')); + // `country` itself stays the raw token — it is the join key into /api/countries[].token. + $hotel->countryCode = $this->countryCodeMapper->map($hotel->country); return $hotel; } diff --git a/tests/BusProNet/DataLoader/HotelDataLoaderTest.php b/tests/BusProNet/DataLoader/HotelDataLoaderTest.php index 1a4b4737..d41f0661 100644 --- a/tests/BusProNet/DataLoader/HotelDataLoaderTest.php +++ b/tests/BusProNet/DataLoader/HotelDataLoaderTest.php @@ -3,9 +3,11 @@ namespace App\Tests\BusProNet\DataLoader; use App\BusProNet\Model\Hotel; +use App\BusProNet\Utility\CountryCodeMapper; use App\BusProNet\XmlLoader\HotelLoader; use League\Flysystem\FilesystemOperator; use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\TagAwareAdapter; @@ -34,7 +36,7 @@ class HotelDataLoaderTest extends TestCase ->willReturn($xmlContent); $cache = new TagAwareAdapter(new ArrayAdapter()); - $loader = new HotelLoader($cache, $filesystem); + $loader = new HotelLoader(new CountryCodeMapper(new NullLogger()), $cache, $filesystem); $hotels = $loader->loadAll(); @@ -50,5 +52,7 @@ class HotelDataLoaderTest extends TestCase $this->assertEquals('F', $hotel->country); $this->assertEquals('Rue des Lacs', $hotel->street); $this->assertEquals('', $hotel->phone); + $this->assertEquals('fr', $hotel->countryCode); + $this->assertNull($hotel->costUnit); } } diff --git a/tests/BusProNet/Utility/CountryCodeMapperTest.php b/tests/BusProNet/Utility/CountryCodeMapperTest.php new file mode 100644 index 00000000..d2deebb8 --- /dev/null +++ b/tests/BusProNet/Utility/CountryCodeMapperTest.php @@ -0,0 +1,102 @@ + + */ + public static function tokenProvider(): array + { + return [ + 'iso-2 passes through' => ['CH', 'ch'], + 'vehicle code D' => ['D', 'de'], + 'iso-3 DEU' => ['DEU', 'de'], + 'vehicle code A' => ['A', 'at'], + 'iso-2 AT' => ['AT', 'at'], + 'iso-3 AUT' => ['AUT', 'at'], + 'vehicle code F' => ['F', 'fr'], + 'vehicle code I' => ['I', 'it'], + 'iso-2 IT' => ['IT', 'it'], + 'vehicle code E' => ['E', 'es'], + 'vehicle code B' => ['B', 'be'], + 'vehicle code S is Sweden' => ['S', 'se'], + 'vehicle code N is Norway' => ['N', 'no'], + 'vehicle code FL is Liechtenstein' => ['FL', 'li'], + 'IR is Ireland, not Iran' => ['IR', 'ie'], + 'UK maps to the iso GB' => ['UK', 'gb'], + 'iso-3 PRT' => ['PRT', 'pt'], + 'iso-3 GRE' => ['GRE', 'gr'], + 'iso-3 HRV' => ['HRV', 'hr'], + 'iso-3 RUS' => ['RUS', 'ru'], + 'iso-3 USA' => ['USA', 'us'], + 'SI is Slovenia' => ['SI', 'si'], + 'SLO is Slovakia' => ['SLO', 'sk'], + 'ZA is South Africa' => ['ZA', 'za'], + ]; + } + + #[DataProvider('tokenProvider')] + public function testMapsKnownToken(string $token, string $expected): void + { + $this->assertSame($expected, (new CountryCodeMapper(new NullLogger()))->map($token)); + } + + public function testEveryMappedCodeIsLowercaseIsoTwo(): void + { + $mapper = new CountryCodeMapper(new NullLogger()); + + foreach (array_keys(self::tokenProvider()) as $name) { + [$token] = self::tokenProvider()[$name]; + $code = $mapper->map($token); + + $this->assertNotNull($code); + $this->assertMatchesRegularExpression('/^[a-z]{2}$/', $code, $token.' must map to lowercase ISO-2'); + } + } + + public function testIsCaseAndWhitespaceInsensitive(): void + { + $mapper = new CountryCodeMapper(new NullLogger()); + + $this->assertSame('ch', $mapper->map('ch')); + $this->assertSame('at', $mapper->map(' aut ')); + } + + public function testUnknownTokenMapsToNullAndWarnsOnce(): void + { + $logger = $this->createMock(LoggerInterface::class); + $logger + ->expects($this->once()) + ->method('warning') + ->with('Unmapped BusPro country token', ['token' => 'XX']); + + $mapper = new CountryCodeMapper($logger); + + $this->assertNull($mapper->map('XX')); + $this->assertNull($mapper->map('XX')); + } + + public function testAbsentTokenMapsToNullWithoutWarning(): void + { + $logger = $this->createMock(LoggerInterface::class); + $logger + ->expects($this->never()) + ->method('warning'); + + $mapper = new CountryCodeMapper($logger); + + $this->assertNull($mapper->map(null)); + $this->assertNull($mapper->map('')); + $this->assertNull($mapper->map(' ')); + } +} diff --git a/tests/BusProNet/XmlLoader/HotelLoaderTest.php b/tests/BusProNet/XmlLoader/HotelLoaderTest.php index aa71c2cb..257eb3f7 100644 --- a/tests/BusProNet/XmlLoader/HotelLoaderTest.php +++ b/tests/BusProNet/XmlLoader/HotelLoaderTest.php @@ -5,10 +5,14 @@ declare(strict_types=1); namespace App\Tests\BusProNet\XmlLoader; use App\BusProNet\Model\Travel; +use App\BusProNet\Utility\CountryCodeMapper; +use App\BusProNet\XmlCrawlerFactory; use App\BusProNet\XmlLoader\HotelLoader; use League\Flysystem\FilesystemOperator; use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\DomCrawler\Crawler; class HotelLoaderTest extends TestCase { @@ -19,7 +23,7 @@ class HotelLoaderTest extends TestCase ->expects($this->never()) ->method('read'); - $loader = new HotelLoader(new ArrayAdapter(), $filesystem); + $loader = $this->createLoader($filesystem); $travel = new Travel(); $travel->hotelId = null; $travel->hotel = null; @@ -28,4 +32,64 @@ class HotelLoaderTest extends TestCase $this->assertNull($travel->hotel); } + + public function testParseXmlMapsCostUnitAndCountryCode(): void + { + $hotel = $this->createLoader()->parseXml($this->hotelNode( + '83CH' + )); + + $this->assertSame('83', $hotel->costUnit); + $this->assertSame('CH', $hotel->country, 'the raw token is the join key and must not be rewritten'); + $this->assertSame('ch', $hotel->countryCode); + } + + public function testParseXmlKeepsAListOfCostUnitsAsAString(): void + { + $hotel = $this->createLoader()->parseXml($this->hotelNode( + '84 85D' + )); + + $this->assertSame('84 85', $hotel->costUnit); + } + + public function testParseXmlLeavesCostUnitNullWhenAbsent(): void + { + $hotel = $this->createLoader()->parseXml($this->hotelNode('D')); + + $this->assertNull($hotel->costUnit); + $this->assertSame('de', $hotel->countryCode); + } + + public function testParseXmlLeavesBothCountryFieldsNullWhenLandIsAbsent(): void + { + $hotel = $this->createLoader()->parseXml($this->hotelNode('Ohne Land')); + + $this->assertNull($hotel->country); + $this->assertNull($hotel->countryCode); + } + + public function testParseXmlLeavesCountryCodeNullForAnUnmappableToken(): void + { + $hotel = $this->createLoader()->parseXml($this->hotelNode('XX')); + + $this->assertSame('XX', $hotel->country); + $this->assertNull($hotel->countryCode, 'an unknown token must never be guessed into a code'); + } + + private function hotelNode(string $children): Crawler + { + $xml = sprintf('%s', $children); + + return XmlCrawlerFactory::create($xml)->filterXPath('//hotels/hotel'); + } + + private function createLoader(?FilesystemOperator $filesystem = null): HotelLoader + { + return new HotelLoader( + new CountryCodeMapper(new NullLogger()), + new ArrayAdapter(), + $filesystem ?? $this->createStub(FilesystemOperator::class), + ); + } }