BAP-22860: Small refactoring of constraint converters for JS validati…

…on (#39958)

src/Oro/Bundle/FormBundle/Form/Extension/JsValidation/ConstraintConverterInterface.php

@@ -7,8 +7,6 @@
 
 /**
  * Represents a service to convert a validation constraint to a form suitable for JS validation.
- *
- * Interface that all ConstraintConverters (tag oro_form.extension.js_validation.constraint_converter) should implement
  */
 interface ConstraintConverterInterface
 {

src/Oro/Bundle/FormBundle/Form/Extension/JsValidation/GenericConstraintConverter.php

@@ -14,16 +14,10 @@
  */
 class GenericConstraintConverter implements ConstraintConverterInterface
 {
-    private ConstraintFactory $constraintFactory;
-
-    private ConstraintConverterInterface $constraintConverter;
-
     public function __construct(
-        ConstraintFactory $constraintFactory,
-        ConstraintConverterInterface $constraintConverter,
+        private readonly ConstraintFactory $constraintFactory,
+        private readonly ConstraintConverterInterface $constraintConverter,
     ) {
-        $this->constraintFactory = $constraintFactory;
-        $this->constraintConverter = $constraintConverter;
     }
 
     #[\Override]

src/Oro/Bundle/FormBundle/Form/Extension/JsValidation/PercentRangeConstraintConverter.php

@@ -18,13 +18,11 @@ public function supports(Constraint $constraint, ?FormInterface $form = null): b
         return $constraint instanceof PercentRange;
     }
 
-    /**
-     *
-     * @param PercentRange $constraint
-     */
     #[\Override]
     public function convertConstraint(Constraint $constraint, ?FormInterface $form = null): ?Constraint
     {
+        /** @var PercentRange $constraint */
+
         $options = [
             'invalidMessage' => $constraint->invalidMessage,
         ];

src/Oro/Bundle/FormBundle/Form/Extension/JsValidation/RangeConstraintConverter.php

@@ -3,7 +3,6 @@
 namespace Oro\Bundle\FormBundle\Form\Extension\JsValidation;
 
 use Symfony\Component\Form\FormInterface;
-use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\Constraints\Range;
@@ -13,12 +12,9 @@
  */
 class RangeConstraintConverter implements ConstraintConverterInterface
 {
-    private ?PropertyAccessorInterface $propertyAccessor = null;
-
     public function __construct(
-        ?PropertyAccessorInterface $propertyAccessor = null
+        private readonly PropertyAccessorInterface $propertyAccessor
     ) {
-        $this->propertyAccessor = $propertyAccessor;
     }
 
     #[\Override]
@@ -27,18 +23,16 @@ public function supports(Constraint $constraint, ?FormInterface $form = null): b
         return $constraint instanceof Range && !isset($constraint->payload['jsValidation']);
     }
 
-    /**
-     *
-     * @param Range $constraint
-     */
     #[\Override]
     public function convertConstraint(Constraint $constraint, ?FormInterface $form = null): ?Constraint
     {
+        /** @var Range $constraint */
+
         $convertedConstraint = clone $constraint;
-        if ($convertedConstraint->maxPropertyPath !== null || $convertedConstraint->minPropertyPath !== null) {
+        if (null !== $convertedConstraint->maxPropertyPath || null !== $convertedConstraint->minPropertyPath) {
             // Get the parent, because the current data is the numeric value
             $formData = $form?->getParent()?->getData();
-            if (is_object($formData)) {
+            if (\is_object($formData)) {
                 $this->setMaxValue($convertedConstraint, $formData);
                 $this->setMinValue($convertedConstraint, $formData);
             }
@@ -47,38 +41,25 @@ public function convertConstraint(Constraint $constraint, ?FormInterface $form =
         return $convertedConstraint;
     }
 
-    private function setMaxValue($convertedConstraint, $formData): void
+    private function setMaxValue(Range $constraint, object $formData): void
     {
-        if ($convertedConstraint->maxPropertyPath !== null && $convertedConstraint->max === null) {
-            if ($this->getPropertyAccessor()->isReadable($formData, $convertedConstraint->maxPropertyPath)) {
-                $convertedConstraint->max = $this->getPropertyAccessor()->getValue(
-                    $formData,
-                    $convertedConstraint->maxPropertyPath
-                );
-                $convertedConstraint->maxPropertyPath = null;
-            }
+        if (null !== $constraint->maxPropertyPath
+            && null === $constraint->max
+            && $this->propertyAccessor->isReadable($formData, $constraint->maxPropertyPath)
+        ) {
+            $constraint->max = $this->propertyAccessor->getValue($formData, $constraint->maxPropertyPath);
+            $constraint->maxPropertyPath = null;
         }
     }
 
-    private function setMinValue($convertedConstraint, $formData): void
+    private function setMinValue(Range $constraint, object $formData): void
     {
-        if ($convertedConstraint->minPropertyPath !== null && $convertedConstraint->min === null) {
-            if ($this->getPropertyAccessor()->isReadable($formData, $convertedConstraint->minPropertyPath)) {
-                $convertedConstraint->min = $this->getPropertyAccessor()->getValue(
-                    $formData,
-                    $convertedConstraint->minPropertyPath
-                );
-                $convertedConstraint->minPropertyPath = null;
-            }
+        if (null !== $constraint->minPropertyPath
+            && null === $constraint->min
+            && $this->propertyAccessor->isReadable($formData, $constraint->minPropertyPath)
+        ) {
+            $constraint->min = $this->propertyAccessor->getValue($formData, $constraint->minPropertyPath);
+            $constraint->minPropertyPath = null;
         }
     }
-
-    private function getPropertyAccessor(): PropertyAccessorInterface
-    {
-        if (null === $this->propertyAccessor) {
-            $this->propertyAccessor = PropertyAccess::createPropertyAccessor();
-        }
-
-        return $this->propertyAccessor;
-    }
 }

src/Oro/Bundle/FormBundle/Resources/config/form_type.yml

@@ -211,12 +211,6 @@ services:
         arguments:
             - !tagged_iterator oro_form.extension.js_validation.constraint_converter
 
-    Oro\Bundle\FormBundle\Form\Extension\JsValidation\ConstraintConverter:
-        alias: oro_form.extension.js_validation.constraint_converter
-
-    Oro\Bundle\FormBundle\Form\Extension\JsValidation\ConstraintConverterInterface:
-        alias: oro_form.extension.js_validation.constraint_converter
-
     oro_form.extension.js_validation.constraint_converter.generic:
         class: Oro\Bundle\FormBundle\Form\Extension\JsValidation\GenericConstraintConverter
         arguments:

src/Oro/Bundle/FormBundle/Tests/Unit/Form/Extension/JsValidation/GenericConstraintConverterTest.php

@@ -15,9 +15,7 @@
 class GenericConstraintConverterTest extends TestCase
 {
     private ConstraintFactory|MockObject $constraintFactory;
-
     private ConstraintConverterInterface|MockObject $constraintConverter;
-
     private GenericConstraintConverter $converter;
 
     #[\Override]

src/Oro/Bundle/FormBundle/Tests/Unit/Form/Extension/JsValidation/PercentRangeConstraintConverterTest.php

@@ -5,7 +5,6 @@
 use Oro\Bundle\FormBundle\Form\Extension\JsValidation\PercentRangeConstraintConverter;
 use Oro\Bundle\FormBundle\Validator\Constraints\PercentRange;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\Range;
 
@@ -19,43 +18,28 @@ protected function setUp(): void
         $this->converter = new PercentRangeConstraintConverter();
     }
 
-    /**
-     * @dataProvider supportsDataProvider
-     */
-    public function testSupports(bool $expected, Constraint $constraint): void
+    public function testSupportsForPercentRange(): void
     {
-        self::assertSame($expected, $this->converter->supports($constraint));
+        self::assertTrue($this->converter->supports(new PercentRange(['min' => -100])));
     }
 
-    public function supportsDataProvider(): \Generator
+    public function testSupportsForNotPercentRange(): void
     {
-        yield [
-            'expected' => true,
-            'constraint' => new PercentRange([
-                'min' => -100,
-                'minMessage' => 'min msg',
-                'invalidMessage' => 'invalid msg',
-            ]),
-        ];
-
-        yield [
-            'expected' => false,
-            'constraint' => new NotBlank(),
-        ];
+        self::assertFalse($this->converter->supports(new NotBlank()));
     }
 
     public function testPercentRangeConstraintWithMinLimit(): void
     {
         $constraint = new PercentRange([
             'min' => -100,
             'minMessage' => 'min msg',
-            'invalidMessage' => 'invalid msg',
+            'invalidMessage' => 'invalid msg'
         ]);
 
         $expectedConstraint = new Range([
             'min' => $constraint->min,
             'minMessage' => $constraint->minMessage,
-            'invalidMessage' => $constraint->invalidMessage,
+            'invalidMessage' => $constraint->invalidMessage
         ]);
         self::assertEquals($expectedConstraint, $this->converter->convertConstraint($constraint));
     }
@@ -65,13 +49,13 @@ public function testPercentRangeConstraintWithMaxLimit(): void
         $constraint = new PercentRange([
             'max' => 100,
             'maxMessage' => 'max msg',
-            'invalidMessage' => 'invalid msg',
+            'invalidMessage' => 'invalid msg'
         ]);
 
         $expectedConstraint = new Range([
             'max' => $constraint->max,
             'maxMessage' => $constraint->maxMessage,
-            'invalidMessage' => $constraint->invalidMessage,
+            'invalidMessage' => $constraint->invalidMessage
         ]);
         self::assertEquals($expectedConstraint, $this->converter->convertConstraint($constraint));
     }
@@ -82,14 +66,14 @@ public function testPercentRangeConstraintWithRange(): void
             'min' => -100,
             'max' => 100,
             'notInRangeMessage' => 'not in range msg',
-            'invalidMessage' => 'invalid msg',
+            'invalidMessage' => 'invalid msg'
         ]);
 
         $expectedConstraint = new Range([
             'min' => $constraint->min,
             'max' => $constraint->max,
             'notInRangeMessage' => $constraint->notInRangeMessage,
-            'invalidMessage' => $constraint->invalidMessage,
+            'invalidMessage' => $constraint->invalidMessage
         ]);
         self::assertEquals($expectedConstraint, $this->converter->convertConstraint($constraint));
     }