From 1808581f2cb08b7fa79f79760668f3e17abe9162 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Thu, 27 May 2021 16:53:35 -0400 Subject: [PATCH 1/3] Introduce Conditional trait on validation rules --- src/Illuminate/Support/Traits/Conditional.php | 45 +++++++++++++++++++ .../Validation/Rules/Dimensions.php | 4 ++ src/Illuminate/Validation/Rules/Exists.php | 3 ++ src/Illuminate/Validation/Rules/Password.php | 3 ++ src/Illuminate/Validation/Rules/Unique.php | 2 + .../ValidationDimensionsRuleTest.php | 9 ++++ tests/Validation/ValidationExistsRuleTest.php | 29 ++++++++++++ .../Validation/ValidationPasswordRuleTest.php | 19 ++++++++ tests/Validation/ValidationUniqueRuleTest.php | 10 +++++ 9 files changed, 124 insertions(+) create mode 100644 src/Illuminate/Support/Traits/Conditional.php diff --git a/src/Illuminate/Support/Traits/Conditional.php b/src/Illuminate/Support/Traits/Conditional.php new file mode 100644 index 000000000000..f620809206f7 --- /dev/null +++ b/src/Illuminate/Support/Traits/Conditional.php @@ -0,0 +1,45 @@ +when(! $value, $callback, $default); + } +} diff --git a/src/Illuminate/Validation/Rules/Dimensions.php b/src/Illuminate/Validation/Rules/Dimensions.php index e2326c7732b1..d390b8c8823b 100644 --- a/src/Illuminate/Validation/Rules/Dimensions.php +++ b/src/Illuminate/Validation/Rules/Dimensions.php @@ -2,8 +2,12 @@ namespace Illuminate\Validation\Rules; +use Illuminate\Support\Traits\Conditional; + class Dimensions { + use Conditional; + /** * The constraints for the dimensions rule. * diff --git a/src/Illuminate/Validation/Rules/Exists.php b/src/Illuminate/Validation/Rules/Exists.php index 72c378600964..59df5f3da0ee 100644 --- a/src/Illuminate/Validation/Rules/Exists.php +++ b/src/Illuminate/Validation/Rules/Exists.php @@ -2,9 +2,12 @@ namespace Illuminate\Validation\Rules; +use Illuminate\Support\Traits\Conditional; + class Exists { use DatabaseRule; + use Conditional; /** * Convert the rule to a validation string. diff --git a/src/Illuminate/Validation/Rules/Password.php b/src/Illuminate/Validation/Rules/Password.php index eddb897f43c4..a6d4e6b8c18e 100644 --- a/src/Illuminate/Validation/Rules/Password.php +++ b/src/Illuminate/Validation/Rules/Password.php @@ -8,10 +8,13 @@ use Illuminate\Contracts\Validation\UncompromisedVerifier; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Validator; +use Illuminate\Support\Traits\Conditional; use InvalidArgumentException; class Password implements Rule, DataAwareRule { + use Conditional; + /** * The data under validation. * diff --git a/src/Illuminate/Validation/Rules/Unique.php b/src/Illuminate/Validation/Rules/Unique.php index 64e910240382..426c313142f6 100644 --- a/src/Illuminate/Validation/Rules/Unique.php +++ b/src/Illuminate/Validation/Rules/Unique.php @@ -3,10 +3,12 @@ namespace Illuminate\Validation\Rules; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Traits\Conditional; class Unique { use DatabaseRule; + use Conditional; /** * The ID that should be ignored. diff --git a/tests/Validation/ValidationDimensionsRuleTest.php b/tests/Validation/ValidationDimensionsRuleTest.php index 469d060e9bd8..dd9b021a11d1 100644 --- a/tests/Validation/ValidationDimensionsRuleTest.php +++ b/tests/Validation/ValidationDimensionsRuleTest.php @@ -29,5 +29,14 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule() $rule = Rule::dimensions()->minWidth(300)->minHeight(400); $this->assertSame('dimensions:min_width=300,min_height=400', (string) $rule); + + $rule = Rule::dimensions() + ->when(true, function($rule) { + $rule->height('100'); + }) + ->unless(true, function($rule) { + $rule->width('200'); + }); + $this->assertSame('dimensions:height=100', (string) $rule); } } diff --git a/tests/Validation/ValidationExistsRuleTest.php b/tests/Validation/ValidationExistsRuleTest.php index e0039d6c6cfd..4f80816f54c8 100644 --- a/tests/Validation/ValidationExistsRuleTest.php +++ b/tests/Validation/ValidationExistsRuleTest.php @@ -116,6 +116,35 @@ public function testItChoosesValidRecordsUsingWhereNotInRule() $this->assertTrue($v->passes()); } + public function testItChoosesValidRecordsUsingConditionalModifiers() + { + $rule = new Exists('users', 'id'); + $rule->when(true, function($rule) { + $rule->whereNotIn('type', [ 'foo', 'bar' ]); + }); + $rule->unless(true, function($rule) { + $rule->whereNotIn('type', [ 'baz', 'other' ]); + }); + + User::create([ 'id' => '1', 'type' => 'foo' ]); + User::create([ 'id' => '2', 'type' => 'bar' ]); + User::create([ 'id' => '3', 'type' => 'baz' ]); + User::create([ 'id' => '4', 'type' => 'other' ]); + + $trans = $this->getIlluminateArrayTranslator(); + $v = new Validator($trans, [], [ 'id' => $rule ]); + $v->setPresenceVerifier(new DatabasePresenceVerifier(Eloquent::getConnectionResolver())); + + $v->setData([ 'id' => 1 ]); + $this->assertFalse($v->passes()); + $v->setData([ 'id' => 2 ]); + $this->assertFalse($v->passes()); + $v->setData([ 'id' => 3 ]); + $this->assertTrue($v->passes()); + $v->setData([ 'id' => 4 ]); + $this->assertTrue($v->passes()); + } + public function testItChoosesValidRecordsUsingWhereNotInAndWhereNotInRulesTogether() { $rule = new Exists('users', 'id'); diff --git a/tests/Validation/ValidationPasswordRuleTest.php b/tests/Validation/ValidationPasswordRuleTest.php index 5d16de98f224..524413777ad0 100644 --- a/tests/Validation/ValidationPasswordRuleTest.php +++ b/tests/Validation/ValidationPasswordRuleTest.php @@ -41,6 +41,25 @@ public function testMin() $this->passes(new Password(8), ['88888888']); } + public function testConditional() + { + $is_privileged_user = true; + $rule = (new Password(8))->when($is_privileged_user, function($rule) { + $rule->symbols(); + }); + + $this->fails($rule, [ 'aaaaaaaa', '11111111' ], [ + 'The my password must contain at least one symbol.', + ]); + + $is_privileged_user = false; + $rule = (new Password(8))->when($is_privileged_user, function($rule) { + $rule->symbols(); + }); + + $this->passes($rule, [ 'aaaaaaaa', '11111111' ]); + } + public function testMixedCase() { $this->fails(Password::min(2)->mixedCase(), ['nn', 'MM'], [ diff --git a/tests/Validation/ValidationUniqueRuleTest.php b/tests/Validation/ValidationUniqueRuleTest.php index c967ab7c077c..98e105fb3ac4 100644 --- a/tests/Validation/ValidationUniqueRuleTest.php +++ b/tests/Validation/ValidationUniqueRuleTest.php @@ -40,6 +40,16 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule() $rule->where('foo', 'bar'); $this->assertSame('unique:table,column,"Taylor, Otwell",id_column,foo,"bar"', (string) $rule); + $rule = new Unique(EloquentModelStub::class, 'column'); + $rule->where('foo', 'bar'); + $rule->when(true, function($rule) { + $rule->ignore('Taylor, Otwell', 'id_column'); + }); + $rule->unless(true, function($rule) { + $rule->ignore('Chris', 'id_column'); + }); + $this->assertSame('unique:table,column,"Taylor, Otwell",id_column,foo,"bar"', (string) $rule); + $rule = new Unique('table', 'column'); $rule->ignore('Taylor, Otwell"\'..-"', 'id_column'); $rule->where('foo', 'bar'); From f917417e12586ab9c36e43c88502dfc3b5ff7c1b Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Thu, 27 May 2021 17:08:25 -0400 Subject: [PATCH 2/3] StyleCI --- src/Illuminate/Support/Traits/Conditional.php | 5 ---- .../ValidationDimensionsRuleTest.php | 4 +-- tests/Validation/ValidationExistsRuleTest.php | 26 +++++++++---------- .../Validation/ValidationPasswordRuleTest.php | 8 +++--- tests/Validation/ValidationUniqueRuleTest.php | 4 +-- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Illuminate/Support/Traits/Conditional.php b/src/Illuminate/Support/Traits/Conditional.php index f620809206f7..9e83464e9ce0 100644 --- a/src/Illuminate/Support/Traits/Conditional.php +++ b/src/Illuminate/Support/Traits/Conditional.php @@ -2,11 +2,6 @@ namespace Illuminate\Support\Traits; -use BadMethodCallException; -use Closure; -use ReflectionClass; -use ReflectionMethod; - trait Conditional { /** diff --git a/tests/Validation/ValidationDimensionsRuleTest.php b/tests/Validation/ValidationDimensionsRuleTest.php index dd9b021a11d1..29d518a16081 100644 --- a/tests/Validation/ValidationDimensionsRuleTest.php +++ b/tests/Validation/ValidationDimensionsRuleTest.php @@ -31,10 +31,10 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule() $this->assertSame('dimensions:min_width=300,min_height=400', (string) $rule); $rule = Rule::dimensions() - ->when(true, function($rule) { + ->when(true, function ($rule) { $rule->height('100'); }) - ->unless(true, function($rule) { + ->unless(true, function ($rule) { $rule->width('200'); }); $this->assertSame('dimensions:height=100', (string) $rule); diff --git a/tests/Validation/ValidationExistsRuleTest.php b/tests/Validation/ValidationExistsRuleTest.php index 4f80816f54c8..77918ce3992a 100644 --- a/tests/Validation/ValidationExistsRuleTest.php +++ b/tests/Validation/ValidationExistsRuleTest.php @@ -119,29 +119,29 @@ public function testItChoosesValidRecordsUsingWhereNotInRule() public function testItChoosesValidRecordsUsingConditionalModifiers() { $rule = new Exists('users', 'id'); - $rule->when(true, function($rule) { - $rule->whereNotIn('type', [ 'foo', 'bar' ]); + $rule->when(true, function ($rule) { + $rule->whereNotIn('type', ['foo', 'bar']); }); - $rule->unless(true, function($rule) { - $rule->whereNotIn('type', [ 'baz', 'other' ]); + $rule->unless(true, function ($rule) { + $rule->whereNotIn('type', ['baz', 'other']); }); - User::create([ 'id' => '1', 'type' => 'foo' ]); - User::create([ 'id' => '2', 'type' => 'bar' ]); - User::create([ 'id' => '3', 'type' => 'baz' ]); - User::create([ 'id' => '4', 'type' => 'other' ]); + User::create(['id' => '1', 'type' => 'foo']); + User::create(['id' => '2', 'type' => 'bar']); + User::create(['id' => '3', 'type' => 'baz']); + User::create(['id' => '4', 'type' => 'other']); $trans = $this->getIlluminateArrayTranslator(); - $v = new Validator($trans, [], [ 'id' => $rule ]); + $v = new Validator($trans, [], ['id' => $rule]); $v->setPresenceVerifier(new DatabasePresenceVerifier(Eloquent::getConnectionResolver())); - $v->setData([ 'id' => 1 ]); + $v->setData(['id' => 1]); $this->assertFalse($v->passes()); - $v->setData([ 'id' => 2 ]); + $v->setData(['id' => 2]); $this->assertFalse($v->passes()); - $v->setData([ 'id' => 3 ]); + $v->setData(['id' => 3]); $this->assertTrue($v->passes()); - $v->setData([ 'id' => 4 ]); + $v->setData(['id' => 4]); $this->assertTrue($v->passes()); } diff --git a/tests/Validation/ValidationPasswordRuleTest.php b/tests/Validation/ValidationPasswordRuleTest.php index 524413777ad0..d484226e2d57 100644 --- a/tests/Validation/ValidationPasswordRuleTest.php +++ b/tests/Validation/ValidationPasswordRuleTest.php @@ -44,20 +44,20 @@ public function testMin() public function testConditional() { $is_privileged_user = true; - $rule = (new Password(8))->when($is_privileged_user, function($rule) { + $rule = (new Password(8))->when($is_privileged_user, function ($rule) { $rule->symbols(); }); - $this->fails($rule, [ 'aaaaaaaa', '11111111' ], [ + $this->fails($rule, ['aaaaaaaa', '11111111'], [ 'The my password must contain at least one symbol.', ]); $is_privileged_user = false; - $rule = (new Password(8))->when($is_privileged_user, function($rule) { + $rule = (new Password(8))->when($is_privileged_user, function ($rule) { $rule->symbols(); }); - $this->passes($rule, [ 'aaaaaaaa', '11111111' ]); + $this->passes($rule, ['aaaaaaaa', '11111111']); } public function testMixedCase() diff --git a/tests/Validation/ValidationUniqueRuleTest.php b/tests/Validation/ValidationUniqueRuleTest.php index 98e105fb3ac4..c1545887db02 100644 --- a/tests/Validation/ValidationUniqueRuleTest.php +++ b/tests/Validation/ValidationUniqueRuleTest.php @@ -42,10 +42,10 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule() $rule = new Unique(EloquentModelStub::class, 'column'); $rule->where('foo', 'bar'); - $rule->when(true, function($rule) { + $rule->when(true, function ($rule) { $rule->ignore('Taylor, Otwell', 'id_column'); }); - $rule->unless(true, function($rule) { + $rule->unless(true, function ($rule) { $rule->ignore('Chris', 'id_column'); }); $this->assertSame('unique:table,column,"Taylor, Otwell",id_column,foo,"bar"', (string) $rule); From 7a5d4620bdb008505166173047f0f7c330cc3669 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Fri, 28 May 2021 15:28:04 -0400 Subject: [PATCH 3/3] Fix docblock --- src/Illuminate/Support/Traits/Conditional.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Illuminate/Support/Traits/Conditional.php b/src/Illuminate/Support/Traits/Conditional.php index 9e83464e9ce0..0b76e65e48c1 100644 --- a/src/Illuminate/Support/Traits/Conditional.php +++ b/src/Illuminate/Support/Traits/Conditional.php @@ -7,11 +7,11 @@ trait Conditional /** * Apply the callback if the given "value" is true. * - * @param mixed $value - * @param callable $callback - * @param callable|null $default + * @param mixed $value + * @param callable $callback + * @param callable|null $default * - * @return mixed|$this + * @return mixed */ public function when($value, $callback, $default = null) { @@ -27,11 +27,11 @@ public function when($value, $callback, $default = null) /** * Apply the callback if the given "value" is false. * - * @param mixed $value - * @param callable $callback - * @param callable|null $default + * @param mixed $value + * @param callable $callback + * @param callable|null $default * - * @return mixed|$this + * @return mixed */ public function unless($value, $callback, $default = null) {