From c4bf2aae7d0d3418780c017f18d9228d0561840b Mon Sep 17 00:00:00 2001 From: Michael Thomas Date: Sat, 9 Mar 2024 12:57:34 -0500 Subject: [PATCH] fix: migrate action expression validation to separate rule class --- .../Models/Rule/ExpressionController.php | 17 ++----- .../V1/Requests/Models/Rule/StoreRequest.php | 3 +- .../V1/Requests/Models/Rule/UpdateRequest.php | 3 +- .../Models/Rule/ValidateExpressionRequest.php | 33 ++++++++++--- app/Http/Requests/RuleFormRequest.php | 3 +- app/Rules/IsValidActionExpression.php | 48 +++++++++++++++++++ app/Validation/FireflyValidator.php | 21 -------- 7 files changed, 83 insertions(+), 45 deletions(-) create mode 100644 app/Rules/IsValidActionExpression.php diff --git a/app/Api/V1/Controllers/Models/Rule/ExpressionController.php b/app/Api/V1/Controllers/Models/Rule/ExpressionController.php index fd1d51ab2a..1912c9f09f 100644 --- a/app/Api/V1/Controllers/Models/Rule/ExpressionController.php +++ b/app/Api/V1/Controllers/Models/Rule/ExpressionController.php @@ -25,7 +25,6 @@ namespace FireflyIII\Api\V1\Controllers\Models\Rule; use FireflyIII\Api\V1\Controllers\Controller; use FireflyIII\Api\V1\Requests\Models\Rule\ValidateExpressionRequest; -use FireflyIII\TransactionRules\Expressions\ActionExpression; use Illuminate\Http\JsonResponse; /** @@ -43,18 +42,8 @@ class ExpressionController extends Controller */ public function validateExpression(ValidateExpressionRequest $request): JsonResponse { - $value = $request->getExpression(); - $expr = new ActionExpression($value); - - if ($expr->isValid()) { - return response()->json([ - "valid" => true, - ]); - } else { - return response()->json([ - "valid" => false, - "error" => $expr->getValidationError()->getMessage() - ]); - } + return response()->json([ + "valid" => true, + ]); } } diff --git a/app/Api/V1/Requests/Models/Rule/StoreRequest.php b/app/Api/V1/Requests/Models/Rule/StoreRequest.php index 03532a2465..669604a99a 100644 --- a/app/Api/V1/Requests/Models/Rule/StoreRequest.php +++ b/app/Api/V1/Requests/Models/Rule/StoreRequest.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests\Models\Rule; use FireflyIII\Rules\IsBoolean; +use FireflyIII\Rules\IsValidActionExpression; use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\GetRuleConfiguration; @@ -123,7 +124,7 @@ class StoreRequest extends FormRequest 'triggers.*.stop_processing' => [new IsBoolean()], 'triggers.*.active' => [new IsBoolean()], 'actions.*.type' => 'required|in:'.implode(',', $validActions), - 'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|ruleActionValue', + 'actions.*.value' => ['required_if:actions.*.type,'.$contextActions, new IsValidActionExpression(), 'ruleActionValue'], 'actions.*.stop_processing' => [new IsBoolean()], 'actions.*.active' => [new IsBoolean()], 'strict' => [new IsBoolean()], diff --git a/app/Api/V1/Requests/Models/Rule/UpdateRequest.php b/app/Api/V1/Requests/Models/Rule/UpdateRequest.php index 64fc66455b..2d7488b505 100644 --- a/app/Api/V1/Requests/Models/Rule/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Rule/UpdateRequest.php @@ -25,6 +25,7 @@ namespace FireflyIII\Api\V1\Requests\Models\Rule; use FireflyIII\Models\Rule; use FireflyIII\Rules\IsBoolean; +use FireflyIII\Rules\IsValidActionExpression; use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\GetRuleConfiguration; @@ -140,7 +141,7 @@ class UpdateRequest extends FormRequest 'triggers.*.stop_processing' => [new IsBoolean()], 'triggers.*.active' => [new IsBoolean()], 'actions.*.type' => 'required|in:'.implode(',', $validActions), - 'actions.*.value' => 'required_if:actions.*.type,'.$contextActions.'|ruleActionExpression|ruleActionValue', + 'actions.*.value' => ['required_if:actions.*.type,'.$contextActions, new IsValidActionExpression(), 'ruleActionValue'], 'actions.*.stop_processing' => [new IsBoolean()], 'actions.*.active' => [new IsBoolean()], 'strict' => [new IsBoolean()], diff --git a/app/Api/V1/Requests/Models/Rule/ValidateExpressionRequest.php b/app/Api/V1/Requests/Models/Rule/ValidateExpressionRequest.php index dbd9faa1d7..b0ef5a3998 100644 --- a/app/Api/V1/Requests/Models/Rule/ValidateExpressionRequest.php +++ b/app/Api/V1/Requests/Models/Rule/ValidateExpressionRequest.php @@ -24,23 +24,42 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests\Models\Rule; +use FireflyIII\Rules\IsValidActionExpression; use FireflyIII\Support\Request\ChecksLogin; -use FireflyIII\Support\Request\ConvertsDataTypes; +use Illuminate\Contracts\Validation\Validator; use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Validation\ValidationException; /** * Class ValidateExpressionRequest */ class ValidateExpressionRequest extends FormRequest { - use ConvertsDataTypes; use ChecksLogin; - /** - * @return string - */ - public function getExpression(): string + public function rules(): array { - return $this->convertString("expression"); + return ['expression' => ['required', new IsValidActionExpression()]]; + } + + /** + * Handle a failed validation attempt. + * + * @param \Illuminate\Contracts\Validation\Validator $validator + * @return void + * + * @throws \Illuminate\Validation\ValidationException + */ + protected function failedValidation(Validator $validator): void + { + $error = $validator->errors()->first('expression'); + + throw new ValidationException( + $validator, + response()->json([ + 'valid' => false, + 'error' => $error + ], 200) + ); } } diff --git a/app/Http/Requests/RuleFormRequest.php b/app/Http/Requests/RuleFormRequest.php index 2bc74cc52c..29c2352a56 100644 --- a/app/Http/Requests/RuleFormRequest.php +++ b/app/Http/Requests/RuleFormRequest.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace FireflyIII\Http\Requests; use FireflyIII\Models\Rule; +use FireflyIII\Rules\IsValidActionExpression; use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ConvertsDataTypes; use FireflyIII\Support\Request\GetRuleConfiguration; @@ -147,7 +148,7 @@ class RuleFormRequest extends FormRequest 'triggers.*.type' => 'required|in:'.implode(',', $validTriggers), 'triggers.*.value' => sprintf('required_if:triggers.*.type,%s|max:1024|min:1|ruleTriggerValue', $contextTriggers), 'actions.*.type' => 'required|in:'.implode(',', $validActions), - 'actions.*.value' => sprintf('required_if:actions.*.type,%s|min:0|max:1024|ruleActionExpression|ruleActionValue', $contextActions), + 'actions.*.value' => [sprintf('required_if:actions.*.type,%s|min:0|max:1024', $contextActions), new IsValidActionExpression(), 'ruleActionValue'], 'strict' => 'in:0,1', ]; diff --git a/app/Rules/IsValidActionExpression.php b/app/Rules/IsValidActionExpression.php new file mode 100644 index 0000000000..f1c488331d --- /dev/null +++ b/app/Rules/IsValidActionExpression.php @@ -0,0 +1,48 @@ +. + */ + +namespace FireflyIII\Rules; + +use Closure; +use Illuminate\Contracts\Validation\ValidationRule; +use FireflyIII\TransactionRules\Expressions\ActionExpression; + +class IsValidActionExpression implements ValidationRule +{ + /** + * Check that the given action expression is syntactically valid. + * + * @param \Closure(string): \Illuminate\Translation\PotentiallyTranslatedString $fail + */ + public function validate(string $attribute, mixed $value, Closure $fail): void + { + $value ??= ''; + $expr = new ActionExpression($value); + + if (!$expr->isValid()) { + $fail('validation.rule_action_expression')->translate([ + 'error' => $expr->getValidationError()->getMessage() + ]); + } + } +} diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index 667e3c1411..9a554d79b4 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -254,27 +254,6 @@ class FireflyValidator extends Validator return 1 === $count; } - public function validateRuleActionExpression(string $attribute, string $value = null): bool - { - $value ??= ''; - $expr = new ActionExpression($value); - - return $expr->isValid(); - } - - public function replaceRuleActionExpression(string $message, string $attribute): string - { - $value = $this->getValue($attribute); - $expr = new ActionExpression($value); - $err = $expr->getValidationError(); - - if ($err == null) { - return $message; - } - - return str_replace(":error", $err->getMessage(), $message); - } - public function validateRuleActionValue(string $attribute, string $value = null): bool { // first, get the index from this string: