feat: expose hotel cost unit and iso country code via the api
The Teamer app groups hotels by cost unit and renders a flag per destination, and cannot derive either from what /api/hotels returns today. costUnit is the raw kostenstelle and stays a string: only 281 of 1931 hotels carry one and some hold a space-separated list such as "84 85". countryCode goes through CountryCodeMapper, whose table is part of the API contract: BusPro's land column mixes ISO-2, ISO-3 and European vehicle registration codes for the same country, and a wrong entry mislabels a destination's flag silently. An unmappable token is null and warns on the bpn channel, never a guess. country itself is untouched -- it remains the join key into /api/countries[].token. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
1 parent
65c1a7bd12
commit
f3745f918e
7 files changed
+322
-2
No files matched your search
@@ -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'
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -0,0 +1,120 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\BusProNet\Utility;
|
||||
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
/**
|
||||
* Maps BusPro's `hotel/land` token onto a lowercase ISO 3166-1 alpha-2 code.
|
||||
*
|
||||
* BusPro has no country vocabulary: the field is free text that has accumulated three
|
||||
* conventions side by side — ISO-2 (`CH`, `NL`), the European vehicle registration codes
|
||||
* (`D`, `A`, `F`, `S`, `FL`) and ISO-3 (`AUT`, `PRT`, `HRV`), with the same country often
|
||||
* under several tokens. The table below is therefore part of the API contract rather than
|
||||
* a derivation: consumers render a flag from it, and a wrong entry mislabels a destination
|
||||
* silently, which is worse than no flag at all. An unknown token maps to null — never to a
|
||||
* guess, never to an empty string.
|
||||
*/
|
||||
class CountryCodeMapper
|
||||
{
|
||||
/**
|
||||
* Keys are upper-cased tokens, values lowercase ISO-2.
|
||||
*
|
||||
* Entries worth their explanation:
|
||||
* - `IR` is Ireland, not Iran. Every row carrying it is Dublin or Carlow.
|
||||
* - `SLO` is Slovakia here, not Slovenia: its only row is a Slovak company, and
|
||||
* Slovenia already arrives under its own `SI`.
|
||||
* - `S`, `N`, `FL` are vehicle codes (Sweden, Norway, Liechtenstein), consistent with
|
||||
* the `D`/`A`/`F`/`I`/`E`/`B` in the same column.
|
||||
* - `UK` is the colloquial form of the ISO `GB`.
|
||||
*
|
||||
* @var array<string, string>
|
||||
*/
|
||||
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<string, true>
|
||||
*/
|
||||
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;
|
||||
}
|
||||
}
|
||||
@@ -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<int, Hotel> */
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\BusProNet\Utility;
|
||||
|
||||
use App\BusProNet\Utility\CountryCodeMapper;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Psr\Log\LoggerInterface;
|
||||
use Psr\Log\NullLogger;
|
||||
|
||||
class CountryCodeMapperTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* @return array<string, array{string, string}>
|
||||
*/
|
||||
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(' '));
|
||||
}
|
||||
}
|
||||
@@ -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(
|
||||
'<kostenstelle>83</kostenstelle><land>CH</land>'
|
||||
));
|
||||
|
||||
$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(
|
||||
'<kostenstelle>84 85</kostenstelle><land>D</land>'
|
||||
));
|
||||
|
||||
$this->assertSame('84 85', $hotel->costUnit);
|
||||
}
|
||||
|
||||
public function testParseXmlLeavesCostUnitNullWhenAbsent(): void
|
||||
{
|
||||
$hotel = $this->createLoader()->parseXml($this->hotelNode('<land>D</land>'));
|
||||
|
||||
$this->assertNull($hotel->costUnit);
|
||||
$this->assertSame('de', $hotel->countryCode);
|
||||
}
|
||||
|
||||
public function testParseXmlLeavesBothCountryFieldsNullWhenLandIsAbsent(): void
|
||||
{
|
||||
$hotel = $this->createLoader()->parseXml($this->hotelNode('<name>Ohne Land</name>'));
|
||||
|
||||
$this->assertNull($hotel->country);
|
||||
$this->assertNull($hotel->countryCode);
|
||||
}
|
||||
|
||||
public function testParseXmlLeavesCountryCodeNullForAnUnmappableToken(): void
|
||||
{
|
||||
$hotel = $this->createLoader()->parseXml($this->hotelNode('<land>XX</land>'));
|
||||
|
||||
$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('<hotels><hotel idbuspro="1" code="TEST">%s</hotel></hotels>', $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),
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user