From 92e78af6d1e3e36b7baef6132cdaa7920c9f848f Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 22:21:23 +0600 Subject: [PATCH 1/3] refactor: replace abandoned rakit/validation with ValidationService Remove unmaintained rakit/validation and route customer/order/profile validation through a native pipe-rule adapter registered in DI. Closes #342 --- core/components/minishop3/composer.json | 1 - core/components/minishop3/composer.lock | 48 +- .../Api/Web/CustomerProfileController.php | 8 +- .../src/Controllers/Customer/Customer.php | 8 +- .../minishop3/src/ServiceRegistry.php | 4 + .../src/Services/Order/OrderFieldManager.php | 6 +- .../Services/Validation/PipeRuleValidator.php | 516 ++++++++++++++++++ .../Validation/ValidationErrorBag.php | 66 +++ .../Services/Validation/ValidationResult.php | 53 ++ .../Services/Validation/ValidationService.php | 34 ++ .../Validation/ValidationServiceLocator.php | 18 + .../Validation/ValidationServiceTest.php | 197 +++++++ .../src/components/ValidationRulesEditor.vue | 2 +- 13 files changed, 897 insertions(+), 64 deletions(-) create mode 100644 core/components/minishop3/src/Services/Validation/PipeRuleValidator.php create mode 100644 core/components/minishop3/src/Services/Validation/ValidationErrorBag.php create mode 100644 core/components/minishop3/src/Services/Validation/ValidationResult.php create mode 100644 core/components/minishop3/src/Services/Validation/ValidationService.php create mode 100644 core/components/minishop3/src/Services/Validation/ValidationServiceLocator.php create mode 100644 core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php diff --git a/core/components/minishop3/composer.json b/core/components/minishop3/composer.json index c490c96a..5037e9f4 100644 --- a/core/components/minishop3/composer.json +++ b/core/components/minishop3/composer.json @@ -11,7 +11,6 @@ ], "require": { "php": ">=8.2", - "rakit/validation": "^1.4", "robmorgan/phinx": "^0.16", "nikic/fast-route": "^1.3", "intervention/image": "^3.0", diff --git a/core/components/minishop3/composer.lock b/core/components/minishop3/composer.lock index d33bf786..727efe43 100644 --- a/core/components/minishop3/composer.lock +++ b/core/components/minishop3/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "abb3f629a7204e1e78da4c1c97536e90", + "content-hash": "541a2bb3a6d325164f8600968e830d0d", "packages": [ { "name": "brick/math", @@ -920,52 +920,6 @@ }, "time": "2021-10-29T13:26:27+00:00" }, - { - "name": "rakit/validation", - "version": "v1.4.0", - "source": { - "type": "git", - "url": "https://github.com/rakit/validation.git", - "reference": "ff003a35cdf5030a5f2482299f4c93f344a35b29" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/rakit/validation/zipball/ff003a35cdf5030a5f2482299f4c93f344a35b29", - "reference": "ff003a35cdf5030a5f2482299f4c93f344a35b29", - "shasum": "" - }, - "require": { - "ext-mbstring": "*", - "php": ">=7.0" - }, - "require-dev": { - "php-coveralls/php-coveralls": "^2.2", - "phpunit/phpunit": "^6.5", - "squizlabs/php_codesniffer": "^3" - }, - "type": "library", - "autoload": { - "psr-4": { - "Rakit\\Validation\\": "src" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Muhammad Syifa", - "email": "emsifa@gmail.com" - } - ], - "description": "PHP Laravel like standalone validation library", - "support": { - "issues": "https://github.com/rakit/validation/issues", - "source": "https://github.com/rakit/validation/tree/v1.4.0" - }, - "time": "2020-08-27T05:07:01+00:00" - }, { "name": "ramsey/collection", "version": "2.1.1", diff --git a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php index d8bf918c..c32dd67b 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php @@ -6,7 +6,7 @@ use MiniShop3\Model\msCustomer; use MiniShop3\Services\Customer\CustomerPublicDto; use MODX\Revolution\modX; -use Rakit\Validation\Validator; +use MiniShop3\Services\Validation\ValidationServiceLocator; /** * CustomerProfileController - Customer profile management API controller @@ -84,9 +84,7 @@ public function update(array $data): array } $customerId = (int)$customer->get('id'); - $validator = new Validator(); - $validation = $validator->make($data, $this->getProfileFieldRules()); - + $validation = ValidationServiceLocator::fromModx($this->modx)->make($data, $this->getProfileFieldRules()); $validation->validate(); if ($validation->fails()) { @@ -173,7 +171,7 @@ public function updateField(array $data): array $rules = $this->getProfileFieldRules(); if (isset($rules[$key])) { $value = trim((string) ($data['value'] ?? '')); - $validation = (new Validator())->make([$key => $value], [$key => $rules[$key]]); + $validation = ValidationServiceLocator::fromModx($this->modx)->make([$key => $value], [$key => $rules[$key]]); $validation->validate(); if ($validation->fails()) { diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index ebf23747..df0a284d 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -13,7 +13,7 @@ use MiniShop3\Utils\CookieHelper; use MODX\Revolution\modX; -use Rakit\Validation\Validator; +use MiniShop3\Services\Validation\ValidationServiceLocator; class Customer { @@ -251,16 +251,12 @@ public function validate(string $key, mixed $value): mixed // Standard validation if (!empty($this->validationRules[$key])) { - $validator = new Validator(); - - $validation = $validator->validate( + $validation = ValidationServiceLocator::fromModx($this->modx)->validate( [$key => $value], [$key => $this->validationRules[$key]], $this->validationMessages ); - $validation->validate(); - if ($validation->fails()) { $errors = $validation->errors(); diff --git a/core/components/minishop3/src/ServiceRegistry.php b/core/components/minishop3/src/ServiceRegistry.php index 04980615..6d671185 100644 --- a/core/components/minishop3/src/ServiceRegistry.php +++ b/core/components/minishop3/src/ServiceRegistry.php @@ -224,6 +224,10 @@ class ServiceRegistry 'class' => \MiniShop3\Services\CustomerFactory::class, 'interface' => null, ], + 'ms3_validation_service' => [ + 'class' => \MiniShop3\Services\Validation\ValidationService::class, + 'interface' => null, + ], ]; /** diff --git a/core/components/minishop3/src/Services/Order/OrderFieldManager.php b/core/components/minishop3/src/Services/Order/OrderFieldManager.php index 563bdc67..bfaf52fd 100644 --- a/core/components/minishop3/src/Services/Order/OrderFieldManager.php +++ b/core/components/minishop3/src/Services/Order/OrderFieldManager.php @@ -8,7 +8,7 @@ use MiniShop3\Model\msOrderLog; use MiniShop3\Services\Order\OrderLogService; use MODX\Revolution\modX; -use Rakit\Validation\Validator; +use MiniShop3\Services\Validation\ValidationServiceLocator; /** * Order Field Manager @@ -223,13 +223,11 @@ public function validate(array $orderData, string $key, mixed $value): array } // Run validation - $validator = new Validator(); - $validation = $validator->validate( + $validation = ValidationServiceLocator::fromModx($this->modx)->validate( [$key => $value], [$key => $this->validationRules[$key]], $this->validationMessages ); - $validation->validate(); if ($validation->fails()) { $errors = $validation->errors(); diff --git a/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php b/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php new file mode 100644 index 00000000..465ab462 --- /dev/null +++ b/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php @@ -0,0 +1,516 @@ + */ + private array $messages; + + /** @var array */ + private const DEFAULT_MESSAGES = [ + 'required' => 'The :attribute is required', + 'email' => 'The :attribute is not valid email', + 'numeric' => 'The :attribute must be numeric', + 'integer' => 'The :attribute must be integer', + 'min' => 'The :attribute minimum is :min', + 'max' => 'The :attribute maximum is :max', + 'between' => 'The :attribute must be between :min and :max', + 'url' => 'The :attribute must be valid url', + 'alpha' => 'The :attribute must be alphabetic', + 'alpha_num' => 'The :attribute must be alphanumeric', + 'alpha_dash' => 'The :attribute must be alpha dash', + 'alpha_spaces' => 'The :attribute must be alpha spaces', + 'boolean' => 'The :attribute must be boolean', + 'json' => 'The :attribute must be valid json', + 'array' => 'The :attribute must be array', + 'in' => 'The :attribute must be in list', + 'not_in' => 'The :attribute must not be in list', + 'regex' => 'The :attribute format is invalid', + 'digits' => 'The :attribute must be :digits digits', + 'same' => 'The :attribute must match :field', + 'different' => 'The :attribute must differ from :field', + 'date' => 'The :attribute must be valid date', + 'ip' => 'The :attribute must be valid ip', + 'ipv4' => 'The :attribute must be valid ipv4', + 'ipv6' => 'The :attribute must be valid ipv6', + 'accepted' => 'The :attribute must be accepted', + 'present' => 'The :attribute must be present', + 'uppercase' => 'The :attribute must be uppercase', + 'lowercase' => 'The :attribute must be lowercase', + 'extension' => 'The :attribute must have valid extension', + 'mimes' => 'The :attribute must have valid mime type', + 'unsupported' => 'The :attribute has unsupported validation rule', + ]; + + /** @var list */ + private const IMPLICIT_RULES = [ + 'required', + 'required_if', + 'required_unless', + 'required_with', + 'required_without', + 'required_with_all', + 'required_without_all', + 'accepted', + 'present', + ]; + + /** + * @param array $messages + */ + public function __construct(array $messages = []) + { + $this->messages = $messages; + } + + /** + * @return list}> + */ + public static function parseRules(string $ruleString): array + { + $parts = array_map('trim', explode('|', $ruleString)); + $parsed = []; + + foreach ($parts as $part) { + if ($part === '') { + continue; + } + + if (str_contains($part, ':')) { + [$name, $paramString] = explode(':', $part, 2); + $params = array_map('trim', explode(',', $paramString)); + $parsed[] = ['name' => $name, 'params' => $params]; + } else { + $parsed[] = ['name' => $part, 'params' => []]; + } + } + + return $parsed; + } + + /** + * @param list}> $rules + * @param array $allInputs + * @return array rule name => message + */ + public function validateField(string $field, mixed $value, array $rules, array $allInputs): array + { + $errors = []; + $ruleNames = array_column($rules, 'name'); + $isEmpty = $this->isEmpty($value); + + if (in_array('nullable', $ruleNames, true) && $isEmpty) { + return []; + } + + $hasNumericRule = $this->hasAnyRule($ruleNames, ['numeric', 'integer']); + + foreach ($rules as $rule) { + $name = $rule['name']; + $params = $rule['params']; + + if ($name === 'nullable') { + continue; + } + + if ($isEmpty && !$this->isImplicitRule($name) && $name !== 'required') { + continue; + } + + if (!$this->checkRule($name, $value, $params, $field, $allInputs, $hasNumericRule)) { + $messageRule = isset(self::DEFAULT_MESSAGES[$name]) || isset($this->messages[$name]) + ? $name + : 'unsupported'; + $errors[$messageRule] = $this->resolveMessage($messageRule, $field, $params); + if ($this->isImplicitRule($name)) { + break; + } + } + } + + return $errors; + } + + /** + * @param list $ruleNames + * @param list $needles + */ + private function hasAnyRule(array $ruleNames, array $needles): bool + { + foreach ($needles as $needle) { + if (in_array($needle, $ruleNames, true)) { + return true; + } + } + + return false; + } + + private function isImplicitRule(string $name): bool + { + return in_array($name, self::IMPLICIT_RULES, true) + || str_starts_with($name, 'required_'); + } + + private function isEmpty(mixed $value): bool + { + if ($value === null) { + return true; + } + + if (is_string($value)) { + return mb_strlen(trim($value), 'UTF-8') === 0; + } + + if (is_array($value)) { + return count($value) === 0; + } + + return false; + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRule( + string $name, + mixed $value, + array $params, + string $field, + array $allInputs, + bool $hasNumericRule, + ): bool { + return match ($name) { + 'required' => !$this->isEmpty($value), + 'present' => array_key_exists($field, $allInputs), + 'accepted' => in_array(strtolower((string) $value), ['yes', 'on', '1', 'true'], true), + 'nullable' => true, + 'email' => filter_var((string) $value, FILTER_VALIDATE_EMAIL) !== false, + 'url' => filter_var((string) $value, FILTER_VALIDATE_URL) !== false, + 'numeric' => is_numeric($value), + 'integer' => filter_var($value, FILTER_VALIDATE_INT) !== false, + 'boolean' => is_bool($value) || in_array(strtolower((string) $value), ['1', '0', 'true', 'false', 'yes', 'no', 'on', 'off'], true), + 'alpha' => (bool) preg_match('/^\p{L}+$/u', (string) $value), + 'alpha_num' => (bool) preg_match('/^[\p{L}\p{N}]+$/u', (string) $value), + 'alpha_dash' => (bool) preg_match('/^[\p{L}\p{N}_-]+$/u', (string) $value), + 'alpha_spaces' => (bool) preg_match('/^[\p{L}\s]+$/u', (string) $value), + 'uppercase' => (string) $value === mb_strtoupper((string) $value, 'UTF-8'), + 'lowercase' => (string) $value === mb_strtolower((string) $value, 'UTF-8'), + 'json' => $this->isValidJson($value), + 'array' => is_array($value), + 'ip' => filter_var((string) $value, FILTER_VALIDATE_IP) !== false, + 'ipv4' => filter_var((string) $value, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false, + 'ipv6' => filter_var((string) $value, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false, + 'min' => $this->checkMin($value, $params[0] ?? '0', $hasNumericRule), + 'max' => $this->checkMax($value, $params[0] ?? '0', $hasNumericRule), + 'between' => $this->checkBetween($value, $params, $hasNumericRule), + 'digits' => strlen(preg_replace('/\D/', '', (string) $value)) === (int) ($params[0] ?? -1), + 'digits_between' => $this->checkDigitsBetween($value, $params), + 'in' => in_array((string) $value, $params, true), + 'not_in' => !in_array((string) $value, $params, true), + 'same' => isset($allInputs[$params[0] ?? '']) && (string) $value === (string) $allInputs[$params[0]], + 'different' => !isset($allInputs[$params[0] ?? '']) || (string) $value !== (string) $allInputs[$params[0]], + 'regex' => $this->checkRegex($value, $params[0] ?? ''), + 'date' => $this->checkDate($value, $params[0] ?? null), + 'after' => $this->checkDateCompare($value, $params[0] ?? '', $allInputs, fn ($a, $b) => $a > $b), + 'before' => $this->checkDateCompare($value, $params[0] ?? '', $allInputs, fn ($a, $b) => $a < $b), + 'required_if' => $this->checkRequiredIf($value, $params, $allInputs), + 'required_unless' => $this->checkRequiredUnless($value, $params, $allInputs), + 'required_with' => $this->checkRequiredWith($value, $params, $allInputs), + 'required_without' => $this->checkRequiredWithout($value, $params, $allInputs), + 'required_with_all' => $this->checkRequiredWithAll($value, $params, $allInputs), + 'required_without_all' => $this->checkRequiredWithoutAll($value, $params, $allInputs), + 'extension' => $this->checkExtension($value, $params), + 'mimes' => $this->checkMimes($value, $params), + default => false, + }; + } + + private function checkMin(mixed $value, string $minParam, bool $hasNumericRule): bool + { + $min = (float) $minParam; + $size = $this->getValueSize($value, $hasNumericRule); + + return $size !== false && $size >= $min; + } + + private function checkMax(mixed $value, string $maxParam, bool $hasNumericRule): bool + { + $max = (float) $maxParam; + $size = $this->getValueSize($value, $hasNumericRule); + + return $size !== false && $size <= $max; + } + + /** + * @param list $params + */ + private function checkBetween(mixed $value, array $params, bool $hasNumericRule): bool + { + if (count($params) < 2) { + return false; + } + + $size = $this->getValueSize($value, $hasNumericRule); + + return $size !== false && $size >= (float) $params[0] && $size <= (float) $params[1]; + } + + private function getValueSize(mixed $value, bool $hasNumericRule): float|false + { + if ($hasNumericRule && is_numeric($value)) { + return (float) $value; + } + + if (is_int($value) || is_float($value)) { + return (float) $value; + } + + if (is_string($value)) { + return (float) mb_strlen($value, 'UTF-8'); + } + + if (is_array($value)) { + return (float) count($value); + } + + return false; + } + + private function checkDigitsBetween(mixed $value, array $params): bool + { + if (count($params) < 2) { + return false; + } + + $digits = strlen(preg_replace('/\D/', '', (string) $value)); + $min = (int) $params[0]; + $max = (int) $params[1]; + + return $digits >= $min && $digits <= $max; + } + + private function checkRegex(mixed $value, string $pattern): bool + { + if ($pattern === '') { + return false; + } + + $delimiter = $pattern[0]; + if (strlen($pattern) > 2 && ($pattern[strlen($pattern) - 1] ?? '') === $delimiter) { + return @preg_match($pattern, (string) $value) === 1; + } + + return @preg_match('/' . str_replace('/', '\/', $pattern) . '/u', (string) $value) === 1; + } + + private function isValidJson(mixed $value): bool + { + if (!is_string($value)) { + return false; + } + + json_decode($value); + + return json_last_error() === JSON_ERROR_NONE; + } + + private function checkDate(mixed $value, ?string $format): bool + { + if ($format === null || $format === '') { + return strtotime((string) $value) !== false; + } + + $date = \DateTimeImmutable::createFromFormat($format, (string) $value); + + return $date !== false && $date->format($format) === (string) $value; + } + + /** + * @param array $allInputs + * @param callable(int, int): bool $compare + */ + private function checkDateCompare(mixed $value, string $other, array $allInputs, callable $compare): bool + { + $valueTs = strtotime((string) $value); + if ($valueTs === false) { + return false; + } + + $otherValue = array_key_exists($other, $allInputs) ? (string) $allInputs[$other] : $other; + $otherTs = strtotime($otherValue); + if ($otherTs === false) { + return false; + } + + return $compare($valueTs, $otherTs); + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredIf(mixed $value, array $params, array $allInputs): bool + { + if ($params === []) { + return true; + } + + $otherField = array_shift($params); + $otherValue = $allInputs[$otherField] ?? null; + + if (in_array((string) $otherValue, $params, true)) { + return !$this->isEmpty($value); + } + + return true; + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredUnless(mixed $value, array $params, array $allInputs): bool + { + if ($params === []) { + return true; + } + + $otherField = array_shift($params); + $otherValue = $allInputs[$otherField] ?? null; + + if (!in_array((string) $otherValue, $params, true)) { + return !$this->isEmpty($value); + } + + return true; + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredWith(mixed $value, array $params, array $allInputs): bool + { + foreach ($params as $otherField) { + if (!$this->isEmpty($allInputs[$otherField] ?? null)) { + return !$this->isEmpty($value); + } + } + + return true; + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredWithout(mixed $value, array $params, array $allInputs): bool + { + foreach ($params as $otherField) { + if ($this->isEmpty($allInputs[$otherField] ?? null)) { + return !$this->isEmpty($value); + } + } + + return true; + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredWithAll(mixed $value, array $params, array $allInputs): bool + { + if ($params === []) { + return true; + } + + foreach ($params as $otherField) { + if ($this->isEmpty($allInputs[$otherField] ?? null)) { + return true; + } + } + + return !$this->isEmpty($value); + } + + /** + * @param list $params + * @param array $allInputs + */ + private function checkRequiredWithoutAll(mixed $value, array $params, array $allInputs): bool + { + if ($params === []) { + return true; + } + + foreach ($params as $otherField) { + if (!$this->isEmpty($allInputs[$otherField] ?? null)) { + return true; + } + } + + return !$this->isEmpty($value); + } + + /** + * @param list $params + */ + private function checkExtension(mixed $value, array $params): bool + { + if ($params === [] || $this->isEmpty($value)) { + return true; + } + + $extension = strtolower(pathinfo((string) $value, PATHINFO_EXTENSION)); + $allowed = array_map(static fn (string $item): string => strtolower(ltrim($item, '.')), $params); + + return $extension !== '' && in_array($extension, $allowed, true); + } + + /** + * @param list $params + */ + private function checkMimes(mixed $value, array $params): bool + { + if ($params === [] || $this->isEmpty($value)) { + return true; + } + + if (is_array($value) && isset($value['type'])) { + return in_array(strtolower((string) $value['type']), array_map('strtolower', $params), true); + } + + return $this->checkExtension($value, $params); + } + + /** + * @param list $params + */ + private function resolveMessage(string $rule, string $field, array $params): string + { + $template = $this->messages[$rule] + ?? self::DEFAULT_MESSAGES[$rule] + ?? 'The :attribute is invalid'; + + $replacements = [ + ':attribute' => $field, + ':field' => $params[0] ?? '', + ':min' => $params[0] ?? '', + ':max' => $params[1] ?? ($params[0] ?? ''), + ':digits' => $params[0] ?? '', + ]; + + foreach ($replacements as $placeholder => $replacement) { + $template = str_replace($placeholder, (string) $replacement, $template); + } + + return $template; + } +} diff --git a/core/components/minishop3/src/Services/Validation/ValidationErrorBag.php b/core/components/minishop3/src/Services/Validation/ValidationErrorBag.php new file mode 100644 index 00000000..a75f12c6 --- /dev/null +++ b/core/components/minishop3/src/Services/Validation/ValidationErrorBag.php @@ -0,0 +1,66 @@ +> */ + protected array $messages = []; + + public function add(string $field, string $rule, string $message): void + { + if (!isset($this->messages[$field])) { + $this->messages[$field] = []; + } + + $this->messages[$field][$rule] = $message; + } + + public function count(): int + { + $total = 0; + foreach ($this->messages as $fieldMessages) { + $total += count($fieldMessages); + } + + return $total; + } + + public function first(string $field): ?string + { + $fieldMessages = $this->messages[$field] ?? []; + + if ($fieldMessages === []) { + return null; + } + + return array_values($fieldMessages)[0]; + } + + /** + * @return array + */ + public function firstOfAll(): array + { + $results = []; + foreach ($this->messages as $field => $fieldMessages) { + if ($fieldMessages === []) { + continue; + } + $results[$field] = array_values($fieldMessages)[0]; + } + + return $results; + } + + /** + * @return array> + */ + public function toArray(): array + { + return $this->messages; + } +} diff --git a/core/components/minishop3/src/Services/Validation/ValidationResult.php b/core/components/minishop3/src/Services/Validation/ValidationResult.php new file mode 100644 index 00000000..cc90b004 --- /dev/null +++ b/core/components/minishop3/src/Services/Validation/ValidationResult.php @@ -0,0 +1,53 @@ +errors = new ValidationErrorBag(); + } + + public function validate(): void + { + $this->errors = new ValidationErrorBag(); + $validator = new PipeRuleValidator($this->messages); + + foreach ($this->rules as $field => $ruleString) { + if (!is_string($field) || $field === '') { + continue; + } + + $value = $this->inputs[$field] ?? null; + $parsedRules = PipeRuleValidator::parseRules((string) $ruleString); + + foreach ($validator->validateField($field, $value, $parsedRules, $this->inputs) as $rule => $message) { + $this->errors->add($field, $rule, $message); + } + } + } + + public function passes(): bool + { + return $this->errors->count() === 0; + } + + public function fails(): bool + { + return !$this->passes(); + } + + public function errors(): ValidationErrorBag + { + return $this->errors; + } +} diff --git a/core/components/minishop3/src/Services/Validation/ValidationService.php b/core/components/minishop3/src/Services/Validation/ValidationService.php new file mode 100644 index 00000000..7ec64241 --- /dev/null +++ b/core/components/minishop3/src/Services/Validation/ValidationService.php @@ -0,0 +1,34 @@ + $inputs + * @param array> $rules + * @param array $messages + */ + public function make(array $inputs, array $rules, array $messages = []): ValidationResult + { + return new ValidationResult($inputs, $rules, $messages); + } + + /** + * @param array $inputs + * @param array> $rules + * @param array $messages + */ + public function validate(array $inputs, array $rules, array $messages = []): ValidationResult + { + $result = $this->make($inputs, $rules, $messages); + $result->validate(); + + return $result; + } +} diff --git a/core/components/minishop3/src/Services/Validation/ValidationServiceLocator.php b/core/components/minishop3/src/Services/Validation/ValidationServiceLocator.php new file mode 100644 index 00000000..919f3ffd --- /dev/null +++ b/core/components/minishop3/src/Services/Validation/ValidationServiceLocator.php @@ -0,0 +1,18 @@ +services->get('ms3_validation_service'); + + return $service instanceof ValidationService ? $service : new ValidationService(); + } +} diff --git a/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php new file mode 100644 index 00000000..3fb44dab --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php @@ -0,0 +1,197 @@ +validator = new ValidationService(); + } + + public function testRequiredAndMinRulesMatchCustomerDefaults(): void + { + $messages = [ + 'required' => 'Required', + 'email' => 'Invalid email', + 'min' => 'Minimum :min characters', + ]; + + $fail = $this->validator->validate( + ['first_name' => 'a'], + ['first_name' => 'required|min:2'], + $messages + ); + + self::assertTrue($fail->fails()); + self::assertSame('Minimum 2 characters', $fail->errors()->first('first_name')); + + $pass = $this->validator->validate( + ['first_name' => 'Ann'], + ['first_name' => 'required|min:2'], + $messages + ); + + self::assertTrue($pass->passes()); + } + + public function testEmailRule(): void + { + $messages = ['email' => 'Invalid email']; + + $fail = $this->validator->validate( + ['email' => 'not-an-email'], + ['email' => 'required|email'], + $messages + ); + + self::assertSame('Invalid email', $fail->errors()->first('email')); + + $pass = $this->validator->validate( + ['email' => 'user@example.com'], + ['email' => 'required|email'], + $messages + ); + + self::assertTrue($pass->passes()); + } + + public function testOrderFieldNumericRules(): void + { + $messages = [ + 'required' => 'Required', + 'numeric' => 'Must be a number', + ]; + + $fail = $this->validator->validate( + ['delivery_id' => 'abc'], + ['delivery_id' => 'required|numeric'], + $messages + ); + + self::assertTrue($fail->fails()); + self::assertSame('Must be a number', $fail->errors()->first('delivery_id')); + + $pass = $this->validator->validate( + ['delivery_id' => '12'], + ['delivery_id' => 'required|numeric'], + $messages + ); + + self::assertTrue($pass->passes()); + } + + public function testProfileFieldRules(): void + { + $rules = [ + 'first_name' => 'required|min:2|max:100', + 'last_name' => 'required|min:2|max:100', + 'email' => 'required|email', + 'phone' => 'required|min:10|max:20', + ]; + + $data = [ + 'first_name' => 'Jo', + 'last_name' => 'Doe', + 'email' => 'john@example.com', + 'phone' => '1234567890', + ]; + + self::assertTrue($this->validator->validate($data, $rules)->passes()); + + $shortPhone = $data; + $shortPhone['phone'] = '123'; + + self::assertTrue($this->validator->validate($shortPhone, $rules)->fails()); + } + + public function testNullableSkipsOtherRulesWhenEmpty(): void + { + $result = $this->validator->validate( + ['nickname' => ''], + ['nickname' => 'nullable|email'] + ); + + self::assertTrue($result->passes()); + } + + public function testFirstOfAllReturnsFieldKeyedErrors(): void + { + $result = $this->validator->validate( + ['first_name' => '', 'email' => 'bad'], + [ + 'first_name' => 'required|min:2', + 'email' => 'required|email', + ], + [ + 'required' => 'Required', + 'email' => 'Invalid email', + ] + ); + + self::assertSame( + [ + 'first_name' => 'Required', + 'email' => 'Invalid email', + ], + $result->errors()->firstOfAll() + ); + } + + public function testMakeAllowsDeferredValidation(): void + { + $result = $this->validator->make( + ['phone' => '12345'], + ['phone' => 'required|min:10'] + ); + + self::assertSame(0, $result->errors()->count()); + + $result->validate(); + + self::assertTrue($result->fails()); + } + + public function testUnsupportedRuleFailsValidation(): void + { + $result = $this->validator->validate( + ['custom' => 'value'], + ['custom' => 'unknown_rule_name'] + ); + + self::assertTrue($result->fails()); + } + + #[DataProvider('deliverySeedRulesProvider')] + public function testDeliverySeedRules(string $field, mixed $value, bool $shouldPass): void + { + $rules = [ + 'first_name' => 'required', + 'last_name' => 'required', + 'email' => 'required|email', + ]; + + $result = $this->validator->validate([$field => $value], [$field => $rules[$field]]); + + self::assertSame($shouldPass, $result->passes(), "field {$field}"); + } + + /** + * @return iterable + */ + public static function deliverySeedRulesProvider(): iterable + { + yield 'first_name ok' => ['first_name', 'Ivan', true]; + yield 'first_name empty' => ['first_name', '', false]; + yield 'email ok' => ['email', 'a@b.co', true]; + yield 'email invalid' => ['email', 'nope', false]; + } +} diff --git a/vueManager/src/components/ValidationRulesEditor.vue b/vueManager/src/components/ValidationRulesEditor.vue index ca4c9312..cf7bb466 100644 --- a/vueManager/src/components/ValidationRulesEditor.vue +++ b/vueManager/src/components/ValidationRulesEditor.vue @@ -84,7 +84,7 @@ const fieldDefinitions = [ /** Extra definitions from model fields + Object Extension (filled on mount) */ const extensionFieldDefinitions = ref([]) -// Available validation rules from rakit/validation +// Pipe validation rules (MiniShop3 ValidationService, Rakit-compatible syntax) const ruleDefinitions = [ // Simple rules (no parameters) { name: 'required', hasParam: false }, From f0077e6e321ffe7ce310863fa0b8e0ee140c7f92 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 22:51:33 +0600 Subject: [PATCH 2/3] fix(validation): align digits and regex rules with Rakit parity digits/digits_between now reject non-digit characters, and regex params are no longer split on commas inside the pattern. --- .../Services/Validation/PipeRuleValidator.php | 26 ++++++++++++++---- .../Validation/ValidationServiceTest.php | 27 +++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php b/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php index 465ab462..f7bec449 100644 --- a/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php +++ b/core/components/minishop3/src/Services/Validation/PipeRuleValidator.php @@ -82,8 +82,12 @@ public static function parseRules(string $ruleString): array if (str_contains($part, ':')) { [$name, $paramString] = explode(':', $part, 2); - $params = array_map('trim', explode(',', $paramString)); - $parsed[] = ['name' => $name, 'params' => $params]; + if ($name === 'regex') { + $parsed[] = ['name' => $name, 'params' => [$paramString]]; + } else { + $params = array_map('trim', explode(',', $paramString)); + $parsed[] = ['name' => $name, 'params' => $params]; + } } else { $parsed[] = ['name' => $part, 'params' => []]; } @@ -209,7 +213,7 @@ private function checkRule( 'min' => $this->checkMin($value, $params[0] ?? '0', $hasNumericRule), 'max' => $this->checkMax($value, $params[0] ?? '0', $hasNumericRule), 'between' => $this->checkBetween($value, $params, $hasNumericRule), - 'digits' => strlen(preg_replace('/\D/', '', (string) $value)) === (int) ($params[0] ?? -1), + 'digits' => $this->checkDigits($value, (int) ($params[0] ?? -1)), 'digits_between' => $this->checkDigitsBetween($value, $params), 'in' => in_array((string) $value, $params, true), 'not_in' => !in_array((string) $value, $params, true), @@ -282,17 +286,29 @@ private function getValueSize(mixed $value, bool $hasNumericRule): float|false return false; } + private function checkDigits(mixed $value, int $length): bool + { + $stringValue = (string) $value; + + return !preg_match('/[^0-9]/', $stringValue) && strlen($stringValue) === $length; + } + private function checkDigitsBetween(mixed $value, array $params): bool { if (count($params) < 2) { return false; } - $digits = strlen(preg_replace('/\D/', '', (string) $value)); + $stringValue = (string) $value; + if (preg_match('/[^0-9]/', $stringValue)) { + return false; + } + + $length = strlen($stringValue); $min = (int) $params[0]; $max = (int) $params[1]; - return $digits >= $min && $digits <= $max; + return $length >= $min && $length <= $max; } private function checkRegex(mixed $value, string $pattern): bool diff --git a/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php index 3fb44dab..fc6ae9b0 100644 --- a/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php +++ b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php @@ -194,4 +194,31 @@ public static function deliverySeedRulesProvider(): iterable yield 'email ok' => ['email', 'a@b.co', true]; yield 'email invalid' => ['email', 'nope', false]; } + + public function testDigitsRuleRequiresNumericOnlyString(): void + { + $result = $this->validator->validate( + ['phone' => '12-345-67890'], + ['phone' => 'digits:10'] + ); + + self::assertTrue($result->fails()); + + $valid = $this->validator->validate( + ['phone' => '1234567890'], + ['phone' => 'digits:10'] + ); + + self::assertTrue($valid->passes()); + } + + public function testRegexRuleKeepsCommaInsidePattern(): void + { + $result = $this->validator->validate( + ['code' => 'foo,bar'], + ['code' => 'regex:/^[a-z,]+$/'] + ); + + self::assertTrue($result->passes()); + } } From e2f64ea1a4701b5ca81eff6c3460a621ecf8ef50 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 22:58:41 +0600 Subject: [PATCH 3/3] refactor(validation): use DI for ValidationService in call sites Replace ValidationServiceLocator static calls with ms3_validation_service DI resolution via protected getValidationService() helpers in Customer, CustomerProfileController and OrderFieldManager. Add parity unit tests for the digits_between rule. --- .../Api/Web/CustomerProfileController.php | 16 ++++-- .../src/Controllers/Customer/Customer.php | 15 ++++-- .../src/Services/Order/OrderFieldManager.php | 14 ++++- .../Validation/ValidationServiceTest.php | 51 +++++++++++++++++++ 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php index c32dd67b..419c58f0 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php @@ -5,8 +5,8 @@ use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; use MiniShop3\Services\Customer\CustomerPublicDto; +use MiniShop3\Services\Validation\ValidationService; use MODX\Revolution\modX; -use MiniShop3\Services\Validation\ValidationServiceLocator; /** * CustomerProfileController - Customer profile management API controller @@ -84,7 +84,7 @@ public function update(array $data): array } $customerId = (int)$customer->get('id'); - $validation = ValidationServiceLocator::fromModx($this->modx)->make($data, $this->getProfileFieldRules()); + $validation = $this->getValidationService()->make($data, $this->getProfileFieldRules()); $validation->validate(); if ($validation->fails()) { @@ -171,7 +171,7 @@ public function updateField(array $data): array $rules = $this->getProfileFieldRules(); if (isset($rules[$key])) { $value = trim((string) ($data['value'] ?? '')); - $validation = ValidationServiceLocator::fromModx($this->modx)->make([$key => $value], [$key => $rules[$key]]); + $validation = $this->getValidationService()->make([$key => $value], [$key => $rules[$key]]); $validation->validate(); if ($validation->fails()) { @@ -230,6 +230,16 @@ protected function getProfileFieldRules(): array ]; } + /** + * Resolve the canonical validation service from MODX DI. + */ + protected function getValidationService(): ValidationService + { + $service = $this->modx->services->get('ms3_validation_service'); + + return $service instanceof ValidationService ? $service : new ValidationService(); + } + /** * Coerce a single-field quick update value using xPDO field metadata (extra columns & OE fields). * diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index df0a284d..12f7a2de 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -10,11 +10,10 @@ use MiniShop3\Model\msCustomer; use MiniShop3\Model\msCustomerToken; use MiniShop3\Services\Customer\CustomerAddressManager; +use MiniShop3\Services\Validation\ValidationService; use MiniShop3\Utils\CookieHelper; use MODX\Revolution\modX; -use MiniShop3\Services\Validation\ValidationServiceLocator; - class Customer { /** @var modX $modx */ @@ -251,7 +250,7 @@ public function validate(string $key, mixed $value): mixed // Standard validation if (!empty($this->validationRules[$key])) { - $validation = ValidationServiceLocator::fromModx($this->modx)->validate( + $validation = $this->getValidationService()->validate( [$key => $value], [$key => $this->validationRules[$key]], $this->validationMessages @@ -384,6 +383,16 @@ protected function getAddressManager(): CustomerAddressManager return $service; } + /** + * Resolve the canonical validation service from MODX DI. + */ + protected function getValidationService(): ValidationService + { + $service = $this->modx->services->get('ms3_validation_service'); + + return $service instanceof ValidationService ? $service : new ValidationService(); + } + /** * Get or create customer for order * diff --git a/core/components/minishop3/src/Services/Order/OrderFieldManager.php b/core/components/minishop3/src/Services/Order/OrderFieldManager.php index bfaf52fd..8397afd5 100644 --- a/core/components/minishop3/src/Services/Order/OrderFieldManager.php +++ b/core/components/minishop3/src/Services/Order/OrderFieldManager.php @@ -7,8 +7,8 @@ use MiniShop3\Model\msOrder; use MiniShop3\Model\msOrderLog; use MiniShop3\Services\Order\OrderLogService; +use MiniShop3\Services\Validation\ValidationService; use MODX\Revolution\modX; -use MiniShop3\Services\Validation\ValidationServiceLocator; /** * Order Field Manager @@ -223,7 +223,7 @@ public function validate(array $orderData, string $key, mixed $value): array } // Run validation - $validation = ValidationServiceLocator::fromModx($this->modx)->validate( + $validation = $this->getValidationService()->validate( [$key => $value], [$key => $this->validationRules[$key]], $this->validationMessages @@ -318,6 +318,16 @@ public function setValidationMessages(array $messages): void $this->validationMessages = array_merge($this->validationMessages, $messages); } + /** + * Resolve the canonical validation service from MODX DI. + */ + protected function getValidationService(): ValidationService + { + $service = $this->modx->services->get('ms3_validation_service'); + + return $service instanceof ValidationService ? $service : new ValidationService(); + } + /** * Shorthand for success response */ diff --git a/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php index fc6ae9b0..690f62ba 100644 --- a/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php +++ b/core/components/minishop3/tests/Unit/Services/Validation/ValidationServiceTest.php @@ -212,6 +212,57 @@ public function testDigitsRuleRequiresNumericOnlyString(): void self::assertTrue($valid->passes()); } + public function testDigitsBetweenRuleAcceptsLengthInRange(): void + { + $tooShort = $this->validator->validate( + ['code' => '12'], + ['code' => 'digits_between:3,5'] + ); + + self::assertTrue($tooShort->fails()); + + $lowerBound = $this->validator->validate( + ['code' => '123'], + ['code' => 'digits_between:3,5'] + ); + + self::assertTrue($lowerBound->passes()); + + $upperBound = $this->validator->validate( + ['code' => '12345'], + ['code' => 'digits_between:3,5'] + ); + + self::assertTrue($upperBound->passes()); + + $tooLong = $this->validator->validate( + ['code' => '123456'], + ['code' => 'digits_between:3,5'] + ); + + self::assertTrue($tooLong->fails()); + } + + public function testDigitsBetweenRuleRejectsNonDigitCharacters(): void + { + $result = $this->validator->validate( + ['code' => '12a45'], + ['code' => 'digits_between:3,5'] + ); + + self::assertTrue($result->fails()); + } + + public function testDigitsBetweenRuleFailsWithMissingParams(): void + { + $result = $this->validator->validate( + ['code' => '123'], + ['code' => 'digits_between:3'] + ); + + self::assertTrue($result->fails()); + } + public function testRegexRuleKeepsCommaInsidePattern(): void { $result = $this->validator->validate(