Skip to content

Commit

Permalink
fix: improve the flarum validator (#4133)
Browse files Browse the repository at this point in the history
  • Loading branch information
SychO9 authored Dec 6, 2024
1 parent e43449c commit 570580d
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 40 deletions.
31 changes: 0 additions & 31 deletions extensions/package-manager/src/AllValidatorRules.php

This file was deleted.

2 changes: 1 addition & 1 deletion extensions/package-manager/src/ConfigureAuthValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

class ConfigureAuthValidator extends AbstractValidator
{
use AllValidatorRules;
protected bool $validateMissingKeys = true;

protected array $rules = [
'github-oauth' => ['sometimes', 'array'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

class ConfigureComposerValidator extends AbstractValidator
{
use AllValidatorRules;
protected bool $validateMissingKeys = true;

protected array $rules = [
'minimum-stability' => ['sometimes', 'in:stable,RC,beta,alpha,dev'],
Expand Down
41 changes: 36 additions & 5 deletions framework/core/src/Foundation/AbstractValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
namespace Flarum\Foundation;

use Flarum\Locale\TranslatorInterface;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\Str;
use Illuminate\Validation\Factory;
use Illuminate\Validation\ValidationException;
use Illuminate\Validation\Validator;
Expand All @@ -22,13 +23,12 @@ abstract class AbstractValidator
*/
protected array $configuration = [];

/**
* @var array
*/
protected array $rules = [];

protected ?Validator $laravelValidator = null;

protected bool $validateMissingKeys = false;

public function __construct(
protected Factory $validator,
protected TranslatorInterface $translator
Expand All @@ -54,6 +54,16 @@ public function assertValid(array $attributes): void
}
}

/**
* Whether to validate missing keys or to only validate provided data keys.
*/
public function validateMissingKeys(bool $validateMissingKeys = true): static
{
$this->validateMissingKeys = $validateMissingKeys;

return $this;
}

public function prepare(array $attributes): static
{
$this->laravelValidator ??= $this->makeValidator($attributes);
Expand All @@ -71,14 +81,35 @@ protected function getRules(): array
return $this->rules;
}

protected function getActiveRules(array $attributes): array
{
$rules = $this->getRules();

if ($this->validateMissingKeys) {
return $rules;
}

return Collection::make($rules)
->filter(function (mixed $rule, string $key) use ($attributes) {
foreach ($attributes as $attributeKey => $attributeValue) {
if ($attributeKey === $key || Str::startsWith($key, $attributeKey.'.')) {
return true;
}
}

return false;
})
->all();
}

protected function getMessages(): array
{
return [];
}

protected function makeValidator(array $attributes): Validator
{
$rules = Arr::only($this->getRules(), array_keys($attributes));
$rules = $this->getActiveRules($attributes);

$validator = $this->validator->make($attributes, $rules, $this->getMessages());

Expand Down
59 changes: 57 additions & 2 deletions framework/core/tests/integration/extenders/ValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

class ValidatorTest extends TestCase
{
private function extendToRequireLongPassword()
private function extendToRequireLongPassword(): void
{
$this->extend((new Extend\Validator(CustomUserValidator::class))->configure(function ($flarumValidator, $validator) {
$validator->setRules([
Expand All @@ -30,7 +30,7 @@ private function extendToRequireLongPassword()
}));
}

private function extendToRequireLongPasswordViaInvokableClass()
private function extendToRequireLongPasswordViaInvokableClass(): void
{
$this->extend((new Extend\Validator(CustomUserValidator::class))->configure(CustomValidatorClass::class));
}
Expand Down Expand Up @@ -74,6 +74,51 @@ public function custom_validation_rule_doesnt_affect_other_validators()
// If we have gotten this far, no validation exception has been thrown, so the test is successful.
$this->assertTrue(true);
}

#[Test]
public function validator_only_validates_provided_data_by_default()
{
/** @var SecondCustomValidator $validator */
$validator = $this->app()->getContainer()->make(SecondCustomValidator::class);

$validator->assertValid([
'my_key' => 'value',
]);

// If we have gotten this far, no validation exception has been thrown, so the test is successful.
$this->assertTrue(true);
}

#[Test]
public function validator_includes_path_based_rules()
{
/** @var SecondCustomValidator $validator */
$validator = $this->app()->getContainer()->make(SecondCustomValidator::class);

$this->expectException(ValidationException::class);

$validator->assertValid([
'my_key' => 'value',
'my_third_key' => [null],
]);
}

#[Test]
public function validator_can_validate_missing_keys()
{
/** @var SecondCustomValidator $validator */
$validator = $this->app()->getContainer()->make(SecondCustomValidator::class)->validateMissingKeys();

$this->expectException(ValidationException::class);

$validator->validateMissingKeys()->assertValid([
'my_key' => 'value',
'my_third_key' => [
'2021-01-01 00:00:00',
'2021-01-02 00:00:00'
]
]);
}
}

class CustomValidatorClass
Expand Down Expand Up @@ -142,3 +187,13 @@ class CustomValidator extends AbstractValidator
'name_plural' => ['required']
];
}

class SecondCustomValidator extends AbstractValidator
{
protected array $rules = [
'my_key' => ['required'],
'my_other_key' => ['required'],
'my_third_key' => ['required', 'array'],
'my_third_key.*' => ['required', 'date']
];
}

0 comments on commit 570580d

Please sign in to comment.