Refactor "AbstractComposite" class

The class had too much complexity and some duplication on its children. I started doing this because I'm refactoring the tests to upgrade PHPUnit later, and then I made some improvements along the way. Signed-off-by: Henrique Moody <[email protected]>
Respect · Jan 28, 2024 · ae1620b · ae1620b
1 parent e983e52
commit ae1620b
Show file tree

Hide file tree

Showing 11 changed files with 223 additions and 184 deletions.
diff --git a/library/Exceptions/AllOfException.php b/library/Exceptions/AllOfException.php
@@ -24,4 +24,9 @@ class AllOfException extends GroupedValidationException
             self::SOME => 'These rules must not pass for {{name}}',
         ],
     ];
+
+    protected function chooseTemplate(): string
+    {
+        return self::SOME;
+    }
 }
diff --git a/library/Exceptions/GroupedValidationException.php b/library/Exceptions/GroupedValidationException.php
@@ -9,8 +9,6 @@
 
 namespace Respect\Validation\Exceptions;
 
-use function count;
-
 class GroupedValidationException extends NestedValidationException
 {
     public const NONE = 'none';
@@ -29,12 +27,4 @@ class GroupedValidationException extends NestedValidationException
             self::SOME => 'These rules must not pass for {{name}}',
         ],
     ];
-
-    protected function chooseTemplate(): string
-    {
-        $numRules = $this->getParam('passed');
-        $numFailed = count($this->getChildren());
-
-        return $numRules === $numFailed ? self::NONE : self::SOME;
-    }
 }
diff --git a/library/Rules/AbstractComposite.php b/library/Rules/AbstractComposite.php
@@ -13,9 +13,6 @@
 use Respect\Validation\Exceptions\ValidationException;
 use Respect\Validation\Validatable;
 
-use function array_filter;
-use function array_map;
-
 abstract class AbstractComposite extends AbstractRule
 {
     /**
@@ -62,27 +59,37 @@ public function getRules(): array
         return $this->rules;
     }
 
+    public function assert(mixed $input): void
+    {
+        $exceptions = $this->getAllThrownExceptions($input);
+        if (empty($exceptions)) {
+            return;
+        }
+
+        $exception = $this->reportError($input);
+        if ($exception instanceof NestedValidationException) {
+            $exception->addChildren($exceptions);
+        }
+
+        throw $exception;
+    }
+
     /**
      * @return ValidationException[]
      */
-    protected function getAllThrownExceptions(mixed $input): array
+    private function getAllThrownExceptions(mixed $input): array
     {
-        return array_filter(
-            array_map(
-                function (Validatable $rule) use ($input): ?ValidationException {
-                    try {
-                        $rule->assert($input);
-                    } catch (ValidationException $exception) {
-                        $this->updateExceptionTemplate($exception);
-
-                        return $exception;
-                    }
-
-                    return null;
-                },
-                $this->getRules()
-            )
-        );
+        $exceptions = [];
+        foreach ($this->getRules() as $rule) {
+            try {
+                $rule->assert($input);
+            } catch (ValidationException $exception) {
+                $this->updateExceptionTemplate($exception);
+                $exceptions[] = $exception;
+            }
+        }
+
+        return $exceptions;
     }
 
     private function shouldHaveNameOverwritten(Validatable $rule): bool

diff --git a/library/Rules/AllOf.php b/library/Rules/AllOf.php
@@ -17,20 +17,14 @@ class AllOf extends AbstractComposite
 {
     public function assert(mixed $input): void
     {
-        $exceptions = $this->getAllThrownExceptions($input);
-        $numRules = count($this->getRules());
-        $numExceptions = count($exceptions);
-        $summary = [
-            'total' => $numRules,
-            'failed' => $numExceptions,
-            'passed' => $numRules - $numExceptions,
-        ];
-        if (!empty($exceptions)) {
-            /** @var AllOfException $allOfException */
-            $allOfException = $this->reportError($input, $summary);
-            $allOfException->addChildren($exceptions);
-
-            throw $allOfException;
+        try {
+            parent::assert($input);
+        } catch (AllOfException $exception) {
+            if (count($exception->getChildren()) === count($this->getRules()) && !$exception->hasCustomTemplate()) {
+                $exception->updateTemplate(AllOfException::NONE);
+            }
+
+            throw $exception;
         }
     }
 

diff --git a/library/Rules/AnyOf.php b/library/Rules/AnyOf.php
@@ -18,16 +18,12 @@ final class AnyOf extends AbstractComposite
 {
     public function assert(mixed $input): void
     {
-        $validators = $this->getRules();
-        $exceptions = $this->getAllThrownExceptions($input);
-        $numRules = count($validators);
-        $numExceptions = count($exceptions);
-        if ($numExceptions === $numRules) {
-            /** @var AnyOfException $anyOfException */
-            $anyOfException = $this->reportError($input);
-            $anyOfException->addChildren($exceptions);
-
-            throw $anyOfException;
+        try {
+            parent::assert($input);
+        } catch (AnyOfException $exception) {
+            if (count($exception->getChildren()) === count($this->getRules())) {
+                throw $exception;
+            }
         }
     }
 

diff --git a/library/Rules/NoneOf.php b/library/Rules/NoneOf.php
@@ -17,15 +17,12 @@ final class NoneOf extends AbstractComposite
 {
     public function assert(mixed $input): void
     {
-        $exceptions = $this->getAllThrownExceptions($input);
-        $numRules = count($this->getRules());
-        $numExceptions = count($exceptions);
-        if ($numRules !== $numExceptions) {
-            /** @var NoneOfException $noneOfException */
-            $noneOfException = $this->reportError($input);
-            $noneOfException->addChildren($exceptions);
-
-            throw $noneOfException;
+        try {
+            parent::assert($input);
+        } catch (NoneOfException $exception) {
+            if (count($exception->getChildren()) !== count($this->getRules())) {
+                throw $exception;
+            }
         }
     }
 

diff --git a/library/Rules/OneOf.php b/library/Rules/OneOf.php
@@ -19,16 +19,12 @@ final class OneOf extends AbstractComposite
 {
     public function assert(mixed $input): void
     {
-        $validators = $this->getRules();
-        $exceptions = $this->getAllThrownExceptions($input);
-        $numRules = count($validators);
-        $numExceptions = count($exceptions);
-        if ($numExceptions !== $numRules - 1) {
-            /** @var OneOfException $oneOfException */
-            $oneOfException = $this->reportError($input);
-            $oneOfException->addChildren($exceptions);
-
-            throw $oneOfException;
+        try {
+            parent::assert($input);
+        } catch (OneOfException $exception) {
+            if (count($exception->getChildren()) !== count($this->getRules()) - 1) {
+                throw $exception;
+            }
         }
     }
 

diff --git a/tests/integration/translator-assert.phpt b/tests/integration/translator-assert.phpt
@@ -20,7 +20,7 @@ Factory::setDefaultInstance(
                     => '{{name}} deve ser do tipo string',
                 '{{name}} must have a length between {{minValue}} and {{maxValue}}'
                     => '{{name}} deve possuir de {{minValue}} a {{maxValue}} caracteres',
-            ][$message];
+            ][$message] ?? $message;
         })
 );
 

diff --git a/tests/library/Exceptions/CompositeStubException.php b/tests/library/Exceptions/CompositeStubException.php
@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * Copyright (c) Alexandre Gomes Gaigalas <[email protected]>
+ * SPDX-License-Identifier: MIT
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Exceptions;
+
+use Respect\Validation\Exceptions\NestedValidationException;
+
+final class CompositeStubException extends NestedValidationException
+{
+}
diff --git a/tests/library/Stubs/CompositeSub.php b/tests/library/Stubs/CompositeSub.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * Copyright (c) Alexandre Gomes Gaigalas <[email protected]>
+ * SPDX-License-Identifier: MIT
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+use Respect\Validation\Message\Formatter;
+use Respect\Validation\Message\Stringifier\KeepOriginalStringName;
+use Respect\Validation\Rules\AbstractComposite;
+use Respect\Validation\Test\Exceptions\CompositeStubException;
+
+final class CompositeSub extends AbstractComposite
+{
+    public function validate(mixed $input): bool
+    {
+        return true;
+    }
+
+    /**
+     * @param array<string, mixed> $extraParameters
+     */
+    public function reportError(mixed $input, array $extraParameters = []): CompositeStubException
+    {
+        return new CompositeStubException(
+            input: $input,
+            id: 'CompositeStub',
+            params: $extraParameters,
+            formatter: new Formatter(static fn ($value) => $value, new KeepOriginalStringName())
+        );
+    }
+}