Skip to content

Commit d8dd33d

Browse files
moiikanaAbban
moiikana
authored and
Abban
committed
Adjust validation for higher accuracy
The static data might need to go somewhere else, also it is not clear to me yet (because the tests say otherwise) what value for the field "country" can actually be expected here. https://phabricator.wikimedia.org/T247227
1 parent 50cf29b commit d8dd33d

File tree

2 files changed

+57
-20
lines changed

2 files changed

+57
-20
lines changed

‎src/Validators/AddressValidator.php‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,16 @@ class AddressValidator {
3636
self::SOURCE_STREET_ADDRESS => 100,
3737
self::SOURCE_CITY => 100,
3838
self::SOURCE_COUNTRY => 8,
39+
self::SOURCE_POSTAL_CODE => 16,
3940
];
4041

41-
public function validatePostalAddress( string $streetAddress, string $postalCode, string $city, string $country ): ValidationResult {
42+
private $countriesPostcodePatterns;
43+
44+
public function __construct( array $countriesPostcodePatterns ) {
45+
$this->countriesPostcodePatterns = $countriesPostcodePatterns;
46+
}
47+
48+
public function validatePostalAddress( string $streetAddress, string $postalCode, string $city, string $countryCode ): ValidationResult {
4249
$violations = [];
4350

4451
if ( $streetAddress === '' ) {
@@ -51,12 +58,15 @@ public function validatePostalAddress( string $streetAddress, string $postalCode
5158
$violations[] = $this->validateFieldLength( $streetAddress, self::SOURCE_STREET_ADDRESS );
5259
}
5360

54-
if ( !preg_match( '/^\\d{4,5}$/', $postalCode ) ) {
55-
$violations[] = new ConstraintViolation(
56-
$postalCode,
57-
self::VIOLATION_NOT_POSTCODE,
58-
self::SOURCE_POSTAL_CODE
59-
);
61+
if ( isset( $this->countriesPostcodePatterns[$countryCode] ) ) {
62+
$violations[] = $this->validatePostalCode( $this->countriesPostcodePatterns[$countryCode], $postalCode );
63+
} else {
64+
$postalCodeLengthViolation = $this->validateFieldLength( $postalCode, self::SOURCE_POSTAL_CODE );
65+
if ( $postalCodeLengthViolation === null ) {
66+
$violations[] = $this->validatePostalCode( '/^.+$/', $postalCode );
67+
} else {
68+
$violations[] = $postalCodeLengthViolation;
69+
}
6070
}
6171

6272
if ( $city === '' ) {
@@ -69,19 +79,30 @@ public function validatePostalAddress( string $streetAddress, string $postalCode
6979
$violations[] = $this->validateFieldLength( $city, self::SOURCE_CITY );
7080
}
7181

72-
if ( $country === '' ) {
82+
if ( $countryCode === '' ) {
7383
$violations[] = new ConstraintViolation(
74-
$country,
84+
$countryCode,
7585
self::VIOLATION_MISSING,
7686
self::SOURCE_COUNTRY
7787
);
7888
} else {
79-
$violations[] = $this->validateFieldLength( $country, self::SOURCE_COUNTRY );
89+
$violations[] = $this->validateFieldLength( $countryCode, self::SOURCE_COUNTRY );
8090
}
8191

8292
return new ValidationResult( ...array_filter( $violations ) );
8393
}
8494

95+
private function validatePostalCode( string $pattern, string $postalCode ): ?ConstraintViolation {
96+
if ( !preg_match( $pattern, $postalCode ) ) {
97+
return new ConstraintViolation(
98+
$postalCode,
99+
self::VIOLATION_NOT_POSTCODE,
100+
self::SOURCE_POSTAL_CODE
101+
);
102+
}
103+
return null;
104+
}
105+
85106
public function validatePersonName( string $salutation, string $title, string $firstname, string $lastname ): ValidationResult {
86107
$violations = [];
87108

‎tests/Unit/Validators/AddressValidatorTest.php‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,39 @@
1111
*/
1212
class AddressValidatorTest extends \PHPUnit\Framework\TestCase {
1313

14+
private const COUNTRY_POSTCODE_PATTERNS = [
15+
'DE' => '/^[0-9]{5}$/',
16+
'AT' => '/^[0-9]{4}$/',
17+
'CH' => '/^[0-9]{4}$/',
18+
'BE' => '/^[0-9]{4}$/',
19+
'IT' => '/^[0-9]{5}$/',
20+
'LI' => '/^[0-9]{4}$/',
21+
'LU' => '/^[0-9]{4}$/',
22+
];
23+
1424
public function testGivenValidPostalAddress_noViolationsAreReturned(): void {
15-
$validator = new AddressValidator();
25+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
1626
$validationResult = $validator->validatePostalAddress( 'Test 1234', '12345', 'Test City', 'Germany' );
1727
$this->assertTrue( $validationResult->isSuccessful() );
1828
}
1929

2030
public function testGivenValidPersonName_noViolationsAreReturned(): void {
21-
$validator = new AddressValidator();
31+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
2232
$validationResult = $validator->validatePersonName( 'Herr', 'Prof. Dr.', 'Tester', 'Testing' );
2333
$this->assertTrue( $validationResult->isSuccessful() );
2434
}
2535

2636
public function testGivenValidCompany_noViolationsAreReturned(): void {
27-
$validator = new AddressValidator();
37+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
2838
$validationResult = $validator->validateCompanyName( 'Test Company GmbH & Co. KG' );
2939
$this->assertTrue( $validationResult->isSuccessful() );
3040
}
3141

3242
public function testGivenTooLongPostalValues_correctViolationsAreReturned(): void {
33-
$validator = new AddressValidator();
43+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
3444
$validationResult = $validator->validatePostalAddress(
3545
str_repeat( 'a', 101 ),
36-
str_repeat( '1', 6 ),
46+
str_repeat( '1', 17 ),
3747
str_repeat( 'a', 101 ),
3848
str_repeat( 'a', 9 )
3949
);
@@ -46,7 +56,7 @@ public function testGivenTooLongPostalValues_correctViolationsAreReturned(): voi
4656
}
4757

4858
public function testGivenTooLongNameValues_correctViolationsAreReturned(): void {
49-
$validator = new AddressValidator();
59+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
5060
$validationResult = $validator->validatePersonName(
5161
str_repeat( 'a', 17 ),
5262
str_repeat( 'a', 17 ),
@@ -62,7 +72,7 @@ public function testGivenTooLongNameValues_correctViolationsAreReturned(): void
6272
}
6373

6474
public function testGivenTooLongCompanyValues_correctViolationsAreReturned(): void {
65-
$validator = new AddressValidator();
75+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
6676
$validationResult = $validator->validateCompanyName(
6777
str_repeat( 'a', 101 )
6878
);
@@ -72,7 +82,7 @@ public function testGivenTooLongCompanyValues_correctViolationsAreReturned(): vo
7282
}
7383

7484
public function testGivenEmptyPostalValues_correctViolationsAreReturned(): void {
75-
$validator = new AddressValidator();
85+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
7686
$validationResult = $validator->validatePostalAddress(
7787
'',
7888
'',
@@ -89,7 +99,7 @@ public function testGivenEmptyPostalValues_correctViolationsAreReturned(): void
8999
}
90100

91101
public function testGivenEmptyNameValues_correctViolationsAreReturned(): void {
92-
$validator = new AddressValidator();
102+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
93103
$validationResult = $validator->validatePersonName(
94104
'',
95105
'',
@@ -105,10 +115,16 @@ public function testGivenEmptyNameValues_correctViolationsAreReturned(): void {
105115
}
106116

107117
public function testGivenEmptyCompanyValues_correctViolationsAreReturned(): void {
108-
$validator = new AddressValidator();
118+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
109119
$validationResult = $validator->validateCompanyName( '' );
110120
$this->assertFalse( $validationResult->isSuccessful() );
111121
$this->assertCount( 1, $validationResult->getViolations() );
112122
$this->assertSame( 'companyName', $validationResult->getViolations()[0]->getSource() );
113123
}
124+
125+
public function testGivenBadPostcodeForCountry_correctViolationsAreReturned(): void {
126+
$validator = new AddressValidator( self::COUNTRY_POSTCODE_PATTERNS );
127+
$validationResult = $validator->validatePostalAddress( 'Test 1234', '123', 'Test City', 'DE' );
128+
$this->assertSame( 'postcode', $validationResult->getViolations()[0]->getSource() );
129+
}
114130
}

0 commit comments

Comments
 (0)