Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValidatorChain should only validate #26

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/BetweenValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,6 @@ public function setInclusive($inclusive = true): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if ($this->getInclusive()) {
if ($this->getMin() > $value || $value > $this->getMax()) {
$this->addMessage(sprintf(
Expand Down
3 changes: 0 additions & 3 deletions src/CallbackValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ public function __construct(callable $callback)

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

return call_user_func($this->callback, $value, $this);
}
}
2 changes: 0 additions & 2 deletions src/CidrValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ class CidrValidator extends BaseValidator

public function isValid($value): bool
{
$this->clearMessages();

$pieces = Str::trimSplit($value, '/');
if (count($pieces) !== 2) {
$this->addMessage(sprintf(
Expand Down
3 changes: 0 additions & 3 deletions src/DateTimeValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ public function __construct($local = true)
*/
public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if (! $value instanceof DateTime && ! is_string($value)) {
$this->addMessage($this->translate('Invalid date/time given.'));

Expand Down
2 changes: 0 additions & 2 deletions src/EmailAddressValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,6 @@ private function validateIp(string $value): bool
*/
public function isValid($value): bool
{
$this->clearMessages();

$matches = [];
$length = true;

Expand Down
3 changes: 0 additions & 3 deletions src/FileValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ public function setMaxFileNameLength(?int $maxFileNameLength): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if (is_array($value)) {
foreach ($value as $file) {
if (! $this->validateFile($file)) {
Expand Down
3 changes: 0 additions & 3 deletions src/GreaterThanValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ public function setMin($min): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if ($this->getMin() >= $value) {
$this->addMessage(sprintf(
$this->translate("'%s' is not greater than '%s'"),
Expand Down
3 changes: 0 additions & 3 deletions src/HexColorValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ class HexColorValidator extends BaseValidator
*/
public function isValid($value): bool
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if (! preg_match('/\A#[0-9a-f]{6}\z/i', $value)) {
$this->addMessage(sprintf(
$this->translate('Color string not in the expected format %s'),
Expand Down
2 changes: 0 additions & 2 deletions src/HostnameValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ class HostnameValidator extends BaseValidator
*/
public function isValid($value)
{
$this->clearMessages();

$asciiHostname = idn_to_ascii($value, 0, INTL_IDNA_VARIANT_UTS46);
if (filter_var($asciiHostname, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) === false) {
$this->addMessage(sprintf(
Expand Down
3 changes: 0 additions & 3 deletions src/InArrayValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ public function setStrict(bool $strict = true): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

$notInArray = $this->findInvalid((array) $value);

if (empty($notInArray)) {
Expand Down
3 changes: 0 additions & 3 deletions src/LessThanValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ public function setMax($max): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if ($this->getMax() <= $value) {
$this->addMessage(sprintf(
$this->translate("'%s' is not less than '%s'"),
Expand Down
3 changes: 0 additions & 3 deletions src/PrivateKeyValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ class PrivateKeyValidator extends BaseValidator

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if (preg_match('/\A\s*\w+:/', $value)) {
$this->addMessage($this->translate('URLs are not allowed'));

Expand Down
3 changes: 0 additions & 3 deletions src/StringLengthValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ public function setEncoding(?string $encoding): self

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if ($encoding = $this->getEncoding()) { // because encoding is only nullable in php >= 8.0
$length = mb_strlen($value, $encoding);
} else {
Expand Down
37 changes: 37 additions & 0 deletions src/ValidatedResult.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace ipl\Validator;

use ipl\Stdlib\Messages;

class ValidatedResult
{
use Messages;

/** @var bool */
protected $isValid = true;

/**
* Get whether the validation was successful
*
* @return bool
*/
public function isValid(): bool
{
return $this->isValid;
}

/**
* Set whether the validation was successful
*
* @param bool $valid
*
* @return $this
*/
public function setIsValid(bool $valid): self
{
$this->isValid = $valid;

return $this;
}
}
72 changes: 64 additions & 8 deletions src/ValidatorChain.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,16 @@

use function ipl\Stdlib\get_php_type;

class ValidatorChain implements Countable, IteratorAggregate, Validator
class ValidatorChain implements Countable, IteratorAggregate
{
use Messages;
use Messages {
hasMessages as private baseHasMessages;
getMessages as private baseGetMessages;
setMessages as private baseSetMessages;
addMessage as private baseAddMessage;
addMessages as private baseAddMessages;
clearMessages as private baseClearMessages;
}
use Plugins;

/** Default priority at which validators are added */
Expand Down Expand Up @@ -269,26 +276,75 @@ public function getIterator(): Traversable
return clone $this->validators;
}

/** @deprecated Use {@see self::validate()} instead */
public function isValid($value)
{
$this->clearMessages();
$result = $this->validate($value);
$this->addMessages($result->getMessages());

return $result->isValid();
}

$valid = true;
public function validate($value): ValidatedResult
{
$result = new ValidatedResult();

foreach ($this as $validator) {
if ((empty($value) && ! $validator->validateEmpty()) || $validator->isValid($value)) {
if (empty($value) && ! $validator->validateEmpty()) {
continue;
}

if ($validator->isValid($value)) {
continue;
}

$valid = false;
// TODO: $validator->validate($result->setValue($value)), in the future?

$this->addMessages($validator->getMessages());
$result->setIsValid(false);
$result->addMessages($validator->getMessages());
$validator->clearMessages(); // Validators aren't supposed to retain their messages forever, now

if ($this->validatorsThatBreakTheChain->contains($validator)) {
break;
}
}

return $valid;
return $result;
}

/** @deprecated Use {@see self::validate()}->hasMessages() instead */
public function hasMessages()
{
return $this->baseHasMessages();
}

/** @deprecated Use {@see self::validate()}->getMessages() instead */
public function getMessages()
{
return $this->baseGetMessages();
}

/** @deprecated Use {@see self::validate()}->setMessages() instead */
public function setMessages(array $messages)
{
return $this->baseSetMessages($messages);
}

/** @deprecated Use {@see self::validate()}->addMessage() instead */
public function addMessage($message, ...$args)
{
return $this->baseAddMessage($message, ...$args);
}

/** @deprecated Use {@see self::validate()}->addMessages() instead */
public function addMessages(array $messages)
{
return $this->baseAddMessages($messages);
}

/** @deprecated Use {@see self::validate()}->clearMessages() instead */
public function clearMessages()
{
return $this->baseClearMessages();
}
}
3 changes: 0 additions & 3 deletions src/X509CertValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ class X509CertValidator extends BaseValidator

public function isValid($value)
{
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

if (preg_match('/\A\s*\w+:/', $value)) {
$this->addMessage($this->translate('URLs are not allowed'));

Expand Down
22 changes: 0 additions & 22 deletions tests/PrivateKeyValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,6 @@ class PrivateKeyValidatorTest extends TestCase
-----END PRIVATE KEY-----
EOF;

public function testIsValidClearsPreviousMessagesIfInvalid()
{
StaticTranslator::$instance = new NoopTranslator();

$validator = new PrivateKeyValidator();
$validator->addMessage('will disappear');
$validator->isValid(preg_replace('/[A-Z]/', '%', self::KEY));

$this->assertSame(['Not a valid PEM-encoded private key'], $validator->getMessages());
}

public function testIsValidClearsPreviousMessagesIfValid()
{
StaticTranslator::$instance = new NoopTranslator();

$validator = new PrivateKeyValidator();
$validator->addMessage('will disappear');
$validator->isValid(self::KEY);

$this->assertSame([], $validator->getMessages());
}

public function testIsValidDisallowsUrls()
{
StaticTranslator::$instance = new NoopTranslator();
Expand Down
10 changes: 5 additions & 5 deletions tests/ValidatorChainTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,11 @@ public function testIsValidClearsMessages()
})
]);

// Call isValid() more than once
$this->assertFalse($validators->isValid('value'));
$this->assertFalse($validators->isValid('value'));
// Assert that we only have the messages from the last isValid() run
$this->assertSame(['Validation failed', 'Validation failed again'], $validators->getMessages());
$validators->validate('value');
$result = $validators->validate('value');

// Assert that we only have the messages from the last validate() run
$this->assertSame(['Validation failed', 'Validation failed again'], $result->getMessages());
}

public function testMerge()
Expand Down
22 changes: 0 additions & 22 deletions tests/X509CertValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,6 @@ class X509CertValidatorTest extends TestCase
-----END CERTIFICATE-----
EOF;

public function testIsValidClearsPreviousMessagesIfInvalid()
{
StaticTranslator::$instance = new NoopTranslator();

$validator = new X509CertValidator();
$validator->addMessage('will disappear');
$validator->isValid(preg_replace('/[A-Z]/', '%', self::CERT));

$this->assertSame(['Not a valid PEM-encoded X.509 certificate'], $validator->getMessages());
}

public function testIsValidClearsPreviousMessagesIfValid()
{
StaticTranslator::$instance = new NoopTranslator();

$validator = new X509CertValidator();
$validator->addMessage('will disappear');
$validator->isValid(self::CERT);

$this->assertSame([], $validator->getMessages());
}

public function testIsValidDisallowsUrls()
{
StaticTranslator::$instance = new NoopTranslator();
Expand Down