Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fix loop validation input context is mutable #24

Merged
merged 6 commits into from
Sep 1, 2015
Merged

Fix loop validation input context is mutable #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c35a20
[Input] Add hasValue()/resetValue() methods
Maks3w Aug 21, 2015
de0e3c9
Delegate to InputInterface::isValid when data not exists
Maks3w Aug 27, 2015
152ee5b
Remove legacy setFallbackValue tests
Maks3w Sep 1, 2015
44cd418
Input fallback feature should work when required and optional
Maks3w Sep 1, 2015
aec915e
Fix loop validation input context is mutable.
Maks3w Aug 27, 2015
d743878
Remove old tests
Maks3w Aug 27, 2015
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
28 changes: 26 additions & 2 deletions src/ArrayInput.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ public function setValue($value)
return parent::setValue($value);
}

/**
* {@inheritdoc}
*/
public function resetValue()
{
$this->value = [];
$this->hasValue = false;
return $this;
}

/**
* @return array
*/
Expand All @@ -50,6 +60,20 @@ public function getValue()
*/
public function isValid($context = null)
{
$hasValue = $this->hasValue();
$required = $this->isRequired();
$hasFallback = $this->hasFallback();

if (! $hasValue && $hasFallback) {
$this->setValue($this->getFallbackValue());
return true;
}

if (! $hasValue && $required) {
$this->setErrorMessage('Value is required');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this resolve the issue with the NotEmpty messages that have occurred since 2.4? It seems like we should set it in the same format, using the same key, in order to retain the previous behavior?

return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good per our discussion last week on IRC.

For those reading:

  • If the value was not present but has a fallback, we set it to the default value and consider it valid.
  • If the value was not present, but required, we mark it as invalid.


if (!$this->continueIfEmpty() && !$this->allowEmpty()) {
$this->injectNotEmptyValidator();
}
Expand All @@ -64,9 +88,9 @@ public function isValid($context = null)
}
$result = $validator->isValid($value, $context);
if (!$result) {
if ($this->hasFallback()) {
if ($hasFallback) {
$this->setValue($this->getFallbackValue());
$result = true;
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the third part of the fallback value series:

  • If the value is invalid, but has a fallback value, we reset the value to the fallback, and mark the value valid.

}
break;
}
Expand Down
46 changes: 8 additions & 38 deletions src/BaseInputFilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ public function isValid($context = null)
*/
protected function validateInputs(array $inputs, $data = [], $context = null)
{
$inputContext = $context ?: (array_merge($this->getRawValues(), (array) $data));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May this should use the filtered version instead the raw value

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that the original from two lines above was using the raw values, we should keep it that way for now. We can address again with the next major version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note before the context was always null for optional values. getRawValues already change the behavior if user call setValue before


$this->validInputs = [];
$this->invalidInputs = [];
$valid = true;
Expand All @@ -254,51 +256,14 @@ protected function validateInputs(array $inputs, $data = [], $context = null)
continue;
}

$hasFallback = ($input instanceof Input && $input->hasFallback());

// If input is optional (not required), and value is not set, then ignore.
if (!array_key_exists($name, $data)
&& !$input->isRequired()
) {
continue;
}

// If the value is required, not present in the data set, and
// has no fallback, validation fails.
if (!array_key_exists($name, $data)
&& $input->isRequired()
&& !$hasFallback
) {
$input->setErrorMessage('Value is required');
$this->invalidInputs[$name] = $input;

if ($input->breakOnFailure()) {
return false;
}

$valid = false;
continue;
}

// If the value is required, not present in the data set, and
// has a fallback, validation passes, and we set the input
// value to the fallback.
if (!array_key_exists($name, $data)
&& $input->isRequired()
&& $hasFallback
) {
$input->setValue($input->getFallbackValue());
continue;
}

// Make sure we have a value (empty) for validation of context
if (!array_key_exists($name, $data)) {
$data[$name] = null;
}

// Validate an input
$inputContext = $context ?: $data;

if (!$input->isValid($inputContext)) {
// Validation failure
$this->invalidInputs[$name] = $input;
Expand Down Expand Up @@ -545,7 +510,7 @@ protected function populate()
$input->clearRawValues();
}

if (!isset($this->data[$name])) {
if (!array_key_exists($name, $this->data)) {
// No value; clear value in this input
if ($input instanceof InputFilterInterface) {
$input->setData([]);
Expand All @@ -557,6 +522,11 @@ protected function populate()
continue;
}

if ($input instanceof Input) {
$input->resetValue();
continue;
}

$input->setValue(null);
continue;
}
Expand Down
6 changes: 6 additions & 0 deletions src/FileInput.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,17 @@ public function isEmptyFile($rawValue)
public function isValid($context = null)
{
$rawValue = $this->getRawValue();
$hasValue = $this->hasValue();
$empty = $this->isEmptyFile($rawValue);
$required = $this->isRequired();
$allowEmpty = $this->allowEmpty();
$continueIfEmpty = $this->continueIfEmpty();

if (! $hasValue && $required && ! $this->hasFallback()) {
$this->setErrorMessage('Value is required');
return false;
}

if ($empty && ! $required && ! $continueIfEmpty) {
return true;
}
Expand Down
61 changes: 61 additions & 0 deletions src/Input.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ class Input implements
*/
protected $value;

/**
* Flag for distinguish when $value contains a real `null` or the PHP default property value.
*
* PHP gives to all properties a default value of `null` unless other default value is set.
*
* @var bool
*/
protected $hasValue = false;

/**
* @var mixed
*/
Expand Down Expand Up @@ -163,12 +172,36 @@ public function setValidatorChain(ValidatorChain $validatorChain)
}

/**
* Set the input value.
*
* If you want to remove/unset the current value use {@link Input::resetValue()}.
*
* @see Input::getValue() For retrieve the input value.
* @see Input::hasValue() For to know if input value was set.
* @see Input::resetValue() For reset the input value to the default state.
*
* @param mixed $value
* @return Input
*/
public function setValue($value)
{
$this->value = $value;
$this->hasValue = true;
return $this;
}

/**
* Reset input value to the default state.
*
* @see Input::hasValue() For to know if input value was set.
* @see Input::setValue() For set a new value.
*
* @return Input
*/
public function resetValue()
{
$this->value = null;
$this->hasValue = false;
return $this;
}

Expand Down Expand Up @@ -270,6 +303,23 @@ public function getValue()
return $filter->filter($this->value);
}

/**
* Flag for inform if input value was set.
*
* This flag used for distinguish when {@link Input::getValue()} will return a real `null` value or the PHP default
* value.
*
* @see Input::getValue() For retrieve the input value.
* @see Input::setValue() For set a new value.
* @see Input::resetValue() For reset the input value to the default state.
*
* @return bool
*/
public function hasValue()
{
return $this->hasValue;
}

/**
* @return mixed
*/
Expand Down Expand Up @@ -321,11 +371,22 @@ public function merge(InputInterface $input)
public function isValid($context = null)
{
$value = $this->getValue();
$hasValue = $this->hasValue();
$empty = ($value === null || $value === '' || $value === []);
$required = $this->isRequired();
$allowEmpty = $this->allowEmpty();
$continueIfEmpty = $this->continueIfEmpty();

if (! $hasValue && $this->hasFallback()) {
$this->setValue($this->getFallbackValue());
return true;
}

if (! $hasValue && $required) {
$this->setErrorMessage('Value is required');
return false;
}

if ($empty && ! $required && ! $continueIfEmpty) {
return true;
}
Expand Down
45 changes: 12 additions & 33 deletions test/ArrayInputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,39 +187,6 @@ public function testDoNotInjectNotEmptyValidatorIfAnywhereInChain()
$this->assertEquals($notEmptyMock, $validators[1]['instance']);
}

public function dataFallbackValue()
{
return [
[
'fallbackValue' => []
],
[
'fallbackValue' => [''],
],
[
'fallbackValue' => [null],
],
[
'fallbackValue' => ['some value'],
],
];
}

/**
* @dataProvider dataFallbackValue
*/
public function testFallbackValue($fallbackValue)
{
$this->input->setFallbackValue($fallbackValue);
$validator = new Validator\Date();
$this->input->getValidatorChain()->attach($validator);
$this->input->setValue(['123']); // not a date

$this->assertTrue($this->input->isValid());
$this->assertEmpty($this->input->getMessages());
$this->assertSame($fallbackValue, $this->input->getValue());
}

public function emptyValuesProvider()
{
return [
Expand All @@ -246,4 +213,16 @@ public function testNotAllowEmptyWithFilterConvertsEmptyToNonEmptyIsValid()
}));
$this->assertTrue($this->input->isValid());
}

public function fallbackValueVsIsValidProvider()
{
$dataSets = parent::fallbackValueVsIsValidProvider();
array_walk($dataSets, function (&$set) {
$set[1] = [$set[1]]; // Wrap fallback value into an array.
$set[2] = [$set[2]]; // Wrap value into an array.
$set[4] = [$set[4]]; // Wrap expected value into an array.
});

return $dataSets;
}
}
Loading