From 916aba9c24e80304ec703749762e8764e99ba025 Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 3 Jul 2024 15:10:04 +0100 Subject: [PATCH] Refactor `Iban` validator - Remove getters and setters - Accepts only an options array to the constructor - Improve docs - Improve coverage - Add types Signed-off-by: George Steel --- docs/book/v3/validators/iban.md | 25 ++++++--- psalm-baseline.xml | 13 ----- src/Iban.php | 97 +++++++-------------------------- test/IbanTest.php | 61 ++++++--------------- 4 files changed, 55 insertions(+), 141 deletions(-) diff --git a/docs/book/v3/validators/iban.md b/docs/book/v3/validators/iban.md index 18b2d9855..89dd413b1 100644 --- a/docs/book/v3/validators/iban.md +++ b/docs/book/v3/validators/iban.md @@ -9,6 +9,7 @@ The following options are supported for `Laminas\Validator\Iban`: - `country_code`: Sets the country code which is used to get the IBAN format for validation. +- `allow_non_sepa`: A boolean that limits allowable account numbers to SEPA countries when `false` ## IBAN validation @@ -19,13 +20,12 @@ to use `Laminas\Validator\Iban`. ### Ungreedy IBAN validation -Sometime it is useful just to validate if the given value is a IBAN number or +Sometimes it is useful just to validate if the given value is a IBAN number or not. This means that you don't want to validate it against a defined country. -This can be done by using `false` as locale. +This can be done by omitting the `country_code` option. ```php -$validator = new Laminas\Validator\Iban(['country_code' => false]); -// Note: you can also provide FALSE as the sole parameter +$validator = new Laminas\Validator\Iban(); if ($validator->isValid('AT611904300234573201')) { // IBAN appears to be valid @@ -40,9 +40,8 @@ country! ### Region aware IBAN validation -To validate against a defined country, you just provide a country code. You can -do this during instaniation via the option `country_code`, or afterwards by -using `setCountryCode()`. +To validate against a defined country, you must provide a country code. You can +do this during instantiation via the option `country_code`. ```php $validator = new Laminas\Validator\Iban(['country_code' => 'AT']); @@ -53,3 +52,15 @@ if ($validator->isValid('AT611904300234573201')) { // IBAN is not valid } ``` + +### Restrict to SEPA Countries + +To only accept bank accounts from within the Single Euro Payments Area (SEPA), you can set the option `allow_non_sepa` to `false`: + +```php +$validator = new Laminas\Validator\Iban(['allow_non_sepa' => false]); + +$validator->isValid('AT611904300234573201'); // true +$validator->isValid('BA391290079401028494'); // false + +``` diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c4a62fd07..df2c75993 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -989,19 +989,6 @@ - - - - - - - - - - - - - maxDepth]]> diff --git a/src/Iban.php b/src/Iban.php index 9688e0250..290778f50 100644 --- a/src/Iban.php +++ b/src/Iban.php @@ -4,8 +4,7 @@ namespace Laminas\Validator; -use Laminas\Stdlib\ArrayUtils; -use Traversable; +use Laminas\Validator\Exception\InvalidArgumentException; use function array_key_exists; use function in_array; @@ -19,6 +18,11 @@ /** * Validates IBAN Numbers (International Bank Account Numbers) + * + * @psalm-type OptionsArgument = array{ + * country_code?: string, + * allow_non_sepa?: bool, + * } */ final class Iban extends AbstractValidator { @@ -41,17 +45,13 @@ final class Iban extends AbstractValidator /** * Optional country code by ISO 3166-1 - * - * @var string|null */ - protected $countryCode; + private readonly ?string $countryCode; /** * Optionally allow IBAN codes from non-SEPA countries. Defaults to true - * - * @var bool */ - protected $allowNonSepa = true; + private readonly bool $allowNonSepa; /** * SEPA ISO 3166-1country codes @@ -168,83 +168,24 @@ final class Iban extends AbstractValidator 'VG' => 'VG[0-9]{2}[A-Z]{4}[0-9]{16}', ]; - /** - * Sets validator options - * - * @param array|Traversable $options OPTIONAL - */ - public function __construct($options = []) + /** @param OptionsArgument $options */ + public function __construct(array $options = []) { - if ($options instanceof Traversable) { - $options = ArrayUtils::iteratorToArray($options); + $countryCode = $options['country_code'] ?? null; + if ($countryCode !== null && ! isset(self::IBAN_REGEX[$countryCode])) { + throw new InvalidArgumentException( + "Country code '{$countryCode}' invalid by ISO 3166-1 or not supported" + ); } - if (array_key_exists('country_code', $options)) { - $this->setCountryCode($options['country_code']); - } + $this->countryCode = $countryCode; + $this->allowNonSepa = $options['allow_non_sepa'] ?? true; - if (array_key_exists('allow_non_sepa', $options)) { - $this->setAllowNonSepa($options['allow_non_sepa']); - } + unset($options['country_code'], $options['allow_non_sepa']); parent::__construct($options); } - /** - * Returns the optional country code by ISO 3166-1 - * - * @return string|null - */ - public function getCountryCode() - { - return $this->countryCode; - } - - /** - * Sets an optional country code by ISO 3166-1 - * - * @param string|null $countryCode - * @return $this provides a fluent interface - * @throws Exception\InvalidArgumentException - */ - public function setCountryCode($countryCode = null) - { - if ($countryCode !== null) { - $countryCode = (string) $countryCode; - - if (! isset(self::IBAN_REGEX[$countryCode])) { - throw new Exception\InvalidArgumentException( - "Country code '{$countryCode}' invalid by ISO 3166-1 or not supported" - ); - } - } - - $this->countryCode = $countryCode; - return $this; - } - - /** - * Returns the optional allow non-sepa countries setting - * - * @return bool - */ - public function allowNonSepa() - { - return $this->allowNonSepa; - } - - /** - * Sets the optional allow non-sepa countries setting - * - * @param bool $allowNonSepa - * @return $this provides a fluent interface - */ - public function setAllowNonSepa($allowNonSepa) - { - $this->allowNonSepa = (bool) $allowNonSepa; - return $this; - } - /** * Returns true if $value is a valid IBAN */ @@ -258,7 +199,7 @@ public function isValid(mixed $value): bool $value = str_replace(' ', '', strtoupper($value)); $this->setValue($value); - $countryCode = $this->getCountryCode(); + $countryCode = $this->countryCode; if ($countryCode === null) { $countryCode = substr($value, 0, 2); } diff --git a/test/IbanTest.php b/test/IbanTest.php index dee6ffe19..65b149702 100644 --- a/test/IbanTest.php +++ b/test/IbanTest.php @@ -110,29 +110,12 @@ public function testBasic(string $iban, bool $expected): void ); } - public function testSettingAndGettingCountryCode(): void + public function testInvalidCountryCodeIsExceptional(): void { - $validator = new IbanValidator(); - $validator->setCountryCode('DE'); - - self::assertSame('DE', $validator->getCountryCode()); - - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('ISO 3166-1'); - - $validator->setCountryCode('foo'); - } - - public function testInstanceWithCountryCode(): void - { - $validator = new IbanValidator(['country_code' => 'AT']); - - self::assertSame('AT', $validator->getCountryCode()); - $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('ISO 3166-1'); - new IbanValidator(['country_code' => 'BAR']); + new IbanValidator(['country_code' => 'foo']); } public function testSepaNotSupportedCountryCode(): void @@ -141,13 +124,11 @@ public function testSepaNotSupportedCountryCode(): void self::assertTrue($validator->isValid('DO17552081023122561803924090')); - $validator->setAllowNonSepa(false); + $validator = new IbanValidator([ + 'allow_non_sepa' => false, + ]); self::assertFalse($validator->isValid('DO17552081023122561803924090')); - - $validator->setAllowNonSepa(true); - - self::assertTrue($validator->isValid('DO17552081023122561803924090')); } public function testIbanNotSupportedCountryCode(): void @@ -181,37 +162,31 @@ public function testEqualsMessageTemplates(): void self::assertSame($validator->getOption('messageTemplates'), $validator->getMessageTemplates()); } - public function testConstructorAllowsSettingOptionsViaOptionsArray(): void - { - $validator = new IbanValidator(['country_code' => 'AT', 'allow_non_sepa' => false]); - - self::assertSame('AT', $validator->getCountryCode()); - self::assertFalse($validator->allowNonSepa()); - } - /** - * @psalm-return array + * @psalm-return array */ public static function invalidValues(): array { return [ - 'null' => [null], - 'true' => [true], - 'false' => [false], - 'zero' => [0], - 'int' => [1], - 'zero-float' => [0.0], - 'float' => [1.1], - 'array' => [['foo']], - 'object' => [(object) []], + 'null' => [null, IbanValidator::FALSEFORMAT], + 'true' => [true, IbanValidator::FALSEFORMAT], + 'false' => [false, IbanValidator::FALSEFORMAT], + 'zero' => [0, IbanValidator::FALSEFORMAT], + 'int' => [1, IbanValidator::FALSEFORMAT], + 'zero-float' => [0.0, IbanValidator::FALSEFORMAT], + 'float' => [1.1, IbanValidator::FALSEFORMAT], + 'array' => [['foo'], IbanValidator::FALSEFORMAT], + 'object' => [(object) [], IbanValidator::FALSEFORMAT], + 'Not match regex' => ['GB123', IbanValidator::FALSEFORMAT], ]; } #[DataProvider('invalidValues')] - public function testIsValidReturnsFalseForNonStringValue(mixed $value): void + public function testIsValidReturnsFalseForNonStringValue(mixed $value, string $errorKey): void { $validator = new IbanValidator(); self::assertFalse($validator->isValid($value)); + self::assertArrayHasKey($errorKey, $validator->getMessages()); } }