Refactor "Domain" rule

The "Domain" rule duplicates a lot of its logic among the methods
"check()," "validate()," and "assert()." Such duplication makes it hard
to maintain and hard to understand.

Besides that:

- It triggers a PHP error when trying to validate a non-scalar value;

- It does not show the exceptions in the correct hierarchy;

- It allows to change the TLD validation after to object is created.

This commit will fix those issues and create better tests for the rule.

Signed-off-by: Henrique Moody <henriquemoody@gmail.com>
This commit is contained in:
Henrique Moody 2019-04-05 17:33:34 +02:00
parent 81ce5b090b
commit cd2390a9b0
No known key found for this signature in database
GPG key ID: 221E9281655813A6
4 changed files with 194 additions and 196 deletions

View file

@ -3,7 +3,7 @@
- `Domain()`
- `Domain(bool $tldCheck)`
Validates domain names.
Validates whether the input is a valid domain name or not.
```php
v::domain()->validate('google.com');

View file

@ -14,45 +14,146 @@ declare(strict_types=1);
namespace Respect\Validation\Rules;
use Respect\Validation\Exceptions\DomainException;
use Respect\Validation\Exceptions\NestedValidationException;
use Respect\Validation\Exceptions\ValidationException;
use Respect\Validation\Validatable;
use function array_merge;
use function array_pop;
use function count;
use function explode;
use function iterator_to_array;
use function mb_substr_count;
/**
* Validates whether the input is a valid domain name or not.
*
* @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
* @author Henrique Moody <henriquemoody@gmail.com>
* @author Mehmet Tolga Avcioglu <mehmet@activecom.net>
* @author Nick Lombard <github@jigsoft.co.za>
* @author Róbert Nagy <vrnagy@gmail.com>
*/
final class Domain extends AbstractComposite
final class Domain extends AbstractRule
{
/**
* @var Validatable
*/
private $tld;
private $genericRule;
/**
* @var Validatable[]
* @var Validatable
*/
private $checks = [];
private $tldRule;
/**
* @var AllOf
*
* @var Validatable
*/
private $otherParts;
private $partsRule;
public function __construct(bool $tldCheck = true)
{
$this->checks[] = new NoWhitespace();
$this->checks[] = new Contains('.');
$this->checks[] = new Length(3, null);
$this->tldCheck($tldCheck);
$this->otherParts = new AllOf(
$this->genericRule = $this->createGenericRule();
$this->tldRule = $this->createTldRule($tldCheck);
$this->partsRule = $this->createPartsRule();
}
/**
* {@inheritDoc}
*/
public function assert($input): void
{
$exceptions = [];
$this->collectAssertException($exceptions, $this->genericRule, $input);
$this->throwExceptions($exceptions, $input);
$parts = explode('.', (string) $input);
if (count($parts) >= 2) {
$this->collectAssertException($exceptions, $this->tldRule, array_pop($parts));
}
foreach ($parts as $part) {
$this->collectAssertException($exceptions, $this->partsRule, $part);
}
$this->throwExceptions($exceptions, $input);
}
/**
* {@inheritDoc}
*/
public function validate($input): bool
{
try {
$this->assert($input);
} catch (ValidationException $exception) {
return false;
}
return true;
}
/**
* @param ValidationException[] $exceptions
* @param mixed $input
*/
private function collectAssertException(array &$exceptions, Validatable $validator, $input): void
{
try {
$validator->assert($input);
} catch (NestedValidationException $nestedValidationException) {
$exceptions = array_merge(
$exceptions,
iterator_to_array($nestedValidationException)
);
} catch (ValidationException $validationException) {
$exceptions[] = $validationException;
}
}
/**
* {@inheritdoc}
*/
public function check($input): void
{
try {
$this->assert($input);
} catch (NestedValidationException $exception) {
/** @var ValidationException $childException */
foreach ($exception as $childException) {
throw $childException;
}
throw $exception;
}
}
private function createGenericRule(): Validatable
{
return new AllOf(
new StringType(),
new NoWhitespace(),
new Contains('.'),
new Length(3)
);
}
private function createTldRule(bool $realTldCheck): Validatable
{
if ($realTldCheck) {
return new Tld();
}
return new AllOf(
new Not(new StartsWith('-')),
new NoWhitespace(),
new Length(2)
);
}
private function createPartsRule(): Validatable
{
return new AllOf(
new Alnum('-'),
new Not(new StartsWith('-')),
new AnyOf(
@ -63,67 +164,14 @@ final class Domain extends AbstractComposite
),
new Not(new EndsWith('-'))
);
parent::__construct();
}
public function tldCheck(bool $do = true): void
{
$this->tld = $do ? new Tld() : new AllOf(
new Not(
new StartsWith('-')
),
new NoWhitespace(),
new Length(2, null)
);
}
/**
* {@inheritdoc}
* @param ValidationException[] $exceptions
* @param mixed $input
*/
public function validate($input): bool
private function throwExceptions(array $exceptions, $input): void
{
foreach ($this->checks as $chk) {
if (!$chk->validate($input)) {
return false;
}
}
$parts = explode('.', (string) $input);
if (count($parts) < 2
|| !$this->tld->validate(array_pop($parts))) {
return false;
}
foreach ($parts as $p) {
if (!$this->otherParts->validate($p)) {
return false;
}
}
return true;
}
/**
* {@inheritdoc}
*/
public function assert($input): void
{
$exceptions = [];
foreach ($this->checks as $chk) {
$this->collectAssertException($exceptions, $chk, $input);
}
$parts = explode('.', (string) $input);
if (count($parts) >= 2) {
$this->collectAssertException($exceptions, $this->tld, array_pop($parts));
}
foreach ($parts as $p) {
$this->collectAssertException($exceptions, $this->otherParts, $p);
}
if (count($exceptions)) {
/** @var DomainException $domainException */
$domainException = $this->reportError($input);
@ -132,36 +180,4 @@ final class Domain extends AbstractComposite
throw $domainException;
}
}
/**
* @param ValidationException[] $exceptions
* @param mixed $input
*/
protected function collectAssertException(array &$exceptions, Validatable $validator, $input): void
{
try {
$validator->assert($input);
} catch (ValidationException $e) {
$exceptions[] = $e;
}
}
/**
* {@inheritdoc}
*/
public function check($input): void
{
foreach ($this->checks as $chk) {
$chk->check($input);
}
$parts = explode('.', $input);
if (count($parts) >= 2) {
$this->tld->check(array_pop($parts));
}
foreach ($parts as $p) {
$this->otherParts->check($p);
}
}
}

View file

@ -0,0 +1,45 @@
--CREDITS--
Henrique Moody <henriquemoody@gmail.com>
--FILE--
<?php
declare(strict_types=1);
require 'vendor/autoload.php';
use Respect\Validation\Exceptions\NestedValidationException;
use Respect\Validation\Exceptions\ValidationException;
use Respect\Validation\Validator as v;
try {
v::domain()->check('batman');
} catch (ValidationException $exception) {
echo $exception->getMessage().PHP_EOL;
}
try {
v::not(v::domain())->check('r--w.com');
} catch (ValidationException $exception) {
echo $exception->getMessage().PHP_EOL;
}
try {
v::domain()->assert('p-éz-.kk');
} catch (NestedValidationException $exception) {
echo $exception->getFullMessage().PHP_EOL;
}
try {
v::not(v::domain())->assert('github.com');
} catch (NestedValidationException $exception) {
echo $exception->getFullMessage().PHP_EOL;
}
?>
--EXPECT--
"batman" must contain the value "."
"r--w.com" must not be a valid domain
- "p-éz-.kk" must be a valid domain
- "kk" must be a valid top-level domain name
- "p-éz-" must contain only letters (a-z), digits (0-9) and "-"
- "p-éz-" must not end with "-"
- "github.com" must not be a valid domain

View file

@ -13,14 +13,13 @@ declare(strict_types=1);
namespace Respect\Validation\Rules;
use Respect\Validation\Test\TestCase;
use Respect\Validation\Validator as v;
use function sprintf;
use function var_export;
use Respect\Validation\Test\RuleTestCase;
use Respect\Validation\Test\Stubs\ToStringStub;
use stdClass;
/**
* @group rule
* @covers \Respect\Validation\Exceptions\DomainException
* @group rule
*
* @covers \Respect\Validation\Rules\Domain
*
* @author Alexandre Gomes Gaigalas <alexandre@gaigalas.net>
@ -28,107 +27,45 @@ use function var_export;
* @author Henrique Moody <henriquemoody@gmail.com>
* @author Mehmet Tolga Avcioglu <mehmet@activecom.net>
*/
final class DomainTest extends TestCase
final class DomainTest extends RuleTestCase
{
/**
* @var Domain
* {@inheritDoc}
*/
protected $object;
protected function setUp(): void
{
$this->object = new Domain();
}
/**
* @dataProvider providerForDomain
*
* @test
*
* @param mixed $input
*/
public function validDomainsShouldReturnTrue($input, bool $tldcheck = true): void
{
$this->object->tldCheck($tldcheck);
self::assertTrue($this->object->__invoke($input));
$this->object->assert($input);
$this->object->check($input);
}
/**
* @dataProvider providerForNotDomain
* @expectedException \Respect\Validation\Exceptions\ValidationException
*
* @test
*
* @param mixed $input
*/
public function notDomain($input, bool $tldcheck = true): void
{
$this->object->tldCheck($tldcheck);
$this->object->check($input);
}
/**
* @dataProvider providerForNotDomain
* @expectedException \Respect\Validation\Exceptions\DomainException
*
* @test
*
* @param mixed $input
*/
public function notDomainCheck($input, bool $tldcheck = true): void
{
$this->object->tldCheck($tldcheck);
$this->object->assert($input);
}
/**
* @return mixed[][]
*/
public function providerForDomain(): array
public function providerForValidInput(): array
{
return [
['111111111111domain.local', false],
['111111111111.domain.local', false],
['example.com'],
['xn--bcher-kva.ch'],
['mail.xn--bcher-kva.ch'],
['example-hyphen.com'],
['example--valid.com'],
['std--a.com'],
['r--w.com'],
[new Domain(false), '111111111111domain.local'],
[new Domain(false), '111111111111.domain.local'],
[new Domain(), 'example.com'],
[new Domain(), 'xn--bcher-kva.ch'],
[new Domain(), 'mail.xn--bcher-kva.ch'],
[new Domain(), 'example-hyphen.com'],
[new Domain(), 'example--valid.com'],
[new Domain(), 'std--a.com'],
[new Domain(), 'r--w.com'],
];
}
/**
* @return mixed[][]
* {@inheritDoc}
*/
public function providerForNotDomain(): array
public function providerForInvalidInput(): array
{
return [
[null],
[''],
['2222222domain.local'],
['-example-invalid.com'],
['example.invalid.-com'],
['xn--bcher--kva.ch'],
['example.invalid-.com'],
['1.2.3.256'],
['1.2.3.4'],
[new Domain(), null],
[new Domain(), new stdClass()],
[new Domain(), []],
[new Domain(), new ToStringStub('google.com')],
[new Domain(), ''],
[new Domain(), 'no dots'],
[new Domain(), '2222222domain.local'],
[new Domain(), '-example-invalid.com'],
[new Domain(), 'example.invalid.-com'],
[new Domain(), 'xn--bcher--kva.ch'],
[new Domain(), 'example.invalid-.com'],
[new Domain(), '1.2.3.256'],
[new Domain(), '1.2.3.4'],
];
}
/**
* @dataProvider providerForDomain
*
* @test
*/
public function builder(string $validDomain, bool $checkTLD = true): void
{
self::assertTrue(
v::domain($checkTLD)->validate($validDomain),
sprintf('Domain "%s" should be valid. (Check TLD: %s)', $validDomain, var_export($checkTLD, true))
);
}
}