fix(symfony): do not use metadata when merging schema constraints in Collection constraint (api-platform#6057)

alanpoulain · web-flow · commit b1926f533f4d · 2023-12-19T19:13:43.000+01:00
diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php
@@ -82,6 +82,7 @@
         'no_superfluous_elseif' => true,
         'no_superfluous_phpdoc_tags' => [
             'allow_mixed' => false,
+            'allow_unused_params' => true,
         ],
         'no_unset_cast' => true,
         'no_unset_on_property' => true,
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaChoiceRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaChoiceRestriction.php
@@ -52,8 +52,7 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
 
         $restriction['type'] = 'array';
 
-        $builtInTypes = $propertyMetadata->getBuiltinTypes() ?? [];
-        $types = array_unique(array_map(fn (Type $type) => Type::BUILTIN_TYPE_STRING === $type->getBuiltinType() ? 'string' : 'number', $builtInTypes));
+        $types = array_values(array_unique(array_map(fn (mixed $choice) => \is_string($choice) ? 'string' : 'number', $choices)));
 
         if ($count = \count($types)) {
             if (1 === $count) {
@@ -80,6 +79,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_STRING];
+        }
 
         return $constraint instanceof Choice && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_STRING, Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaCollectionRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaCollectionRestriction.php
@@ -69,19 +69,19 @@ public function supports(Constraint $constraint, ApiProperty $propertyMetadata):
         return $constraint instanceof Collection;
     }
 
-    private function mergeConstraintRestrictions(Required|Optional $constraint, ApiProperty $propertyMetadata): array
+    private function mergeConstraintRestrictions(Required|Optional $constraint, ApiProperty $propertyMetadata): array|\ArrayObject
     {
         $propertyRestrictions = [];
         $nestedConstraints = $constraint->constraints;
 
         foreach ($nestedConstraints as $nestedConstraint) {
             foreach ($this->restrictionsMetadata as $restrictionMetadata) {
-                if ($restrictionMetadata->supports($nestedConstraint, $propertyMetadata) && !empty($nestedConstraintRestriction = $restrictionMetadata->create($nestedConstraint, $propertyMetadata))) {
+                if ($restrictionMetadata->supports($nestedConstraint, $propertyMetadata->withExtraProperties(($propertyMetadata->getExtraProperties() ?? []) + ['nested_schema' => true])) && !empty($nestedConstraintRestriction = $restrictionMetadata->create($nestedConstraint, $propertyMetadata))) {
                     $propertyRestrictions[] = $nestedConstraintRestriction;
                 }
             }
         }
 
-        return array_merge([], ...$propertyRestrictions);
+        return array_merge([], ...$propertyRestrictions) ?: new \ArrayObject();
     }
 }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaGreaterThanOrEqualRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaGreaterThanOrEqualRestriction.php
@@ -41,6 +41,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_INT];
+        }
 
         return $constraint instanceof GreaterThanOrEqual && is_numeric($constraint->value) && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaGreaterThanRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaGreaterThanRestriction.php
@@ -41,6 +41,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_INT];
+        }
 
         return $constraint instanceof GreaterThan && is_numeric($constraint->value) && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLengthRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLengthRestriction.php
@@ -51,6 +51,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_STRING];
+        }
 
         return $constraint instanceof Length && \count($types) && \in_array(Type::BUILTIN_TYPE_STRING, $types, true);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLessThanOrEqualRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLessThanOrEqualRestriction.php
@@ -41,6 +41,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_INT];
+        }
 
         return $constraint instanceof LessThanOrEqual && is_numeric($constraint->value) && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLessThanRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaLessThanRestriction.php
@@ -41,6 +41,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_INT];
+        }
 
         return $constraint instanceof LessThan && is_numeric($constraint->value) && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaRangeRestriction.php b/src/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaRangeRestriction.php
@@ -49,6 +49,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a
     public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool
     {
         $types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);
+        if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {
+            $types = [Type::BUILTIN_TYPE_INT];
+        }
 
         return $constraint instanceof Range && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);
     }
diff --git a/tests/Fixtures/DummyCollectionValidatedEntity.php b/tests/Fixtures/DummyCollectionValidatedEntity.php
@@ -37,6 +37,7 @@ class DummyCollectionValidatedEntity
             ]),
             'age' => new Assert\Optional([
                 new Assert\Type(type: 'int'),
+                new Assert\GreaterThan(0),
             ]),
             'social' => new Assert\Collection(
                 fields: [
diff --git a/tests/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaChoiceRestrictionTest.php b/tests/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaChoiceRestrictionTest.php
@@ -96,22 +96,22 @@ public static function createProvider(): \Generator
             new Type(Type::BUILTIN_TYPE_STRING),
             new Type(Type::BUILTIN_TYPE_INT),
             new Type(Type::BUILTIN_TYPE_FLOAT),
-        ]), ['type' => 'array', 'items' => ['type' => ['string', 'number'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2]]]];
+        ]), ['type' => 'array', 'items' => ['type' => ['number', 'string'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2]]]];
         yield 'multi string/int/float choice min with union types' => [new Choice(['choices' => [1, 2, 'a', 'b', 1.1, 2.2], 'multiple' => true, 'min' => 2]), (new ApiProperty())->withBuiltinTypes([
             new Type(Type::BUILTIN_TYPE_STRING),
             new Type(Type::BUILTIN_TYPE_INT),
             new Type(Type::BUILTIN_TYPE_FLOAT),
-        ]), ['type' => 'array', 'items' => ['type' => ['string', 'number'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2]], 'minItems' => 2]];
+        ]), ['type' => 'array', 'items' => ['type' => ['number', 'string'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2]], 'minItems' => 2]];
         yield 'multi string/int/float choice max with union types' => [new Choice(['choices' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4], 'multiple' => true, 'max' => 4]), (new ApiProperty())->withBuiltinTypes([
             new Type(Type::BUILTIN_TYPE_STRING),
             new Type(Type::BUILTIN_TYPE_INT),
             new Type(Type::BUILTIN_TYPE_FLOAT),
-        ]), ['type' => 'array', 'items' => ['type' => ['string', 'number'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4]], 'maxItems' => 4]];
+        ]), ['type' => 'array', 'items' => ['type' => ['number', 'string'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4]], 'maxItems' => 4]];
         yield 'multi string/int/float choice min/max with union types' => [new Choice(['choices' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4], 'multiple' => true, 'min' => 2, 'max' => 4]), (new ApiProperty())->withBuiltinTypes([
             new Type(Type::BUILTIN_TYPE_STRING),
             new Type(Type::BUILTIN_TYPE_INT),
             new Type(Type::BUILTIN_TYPE_FLOAT),
-        ]), ['type' => 'array', 'items' => ['type' => ['string', 'number'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4]], 'minItems' => 2, 'maxItems' => 4]];
+        ]), ['type' => 'array', 'items' => ['type' => ['number', 'string'], 'enum' => [1, 2, 'a', 'b', 1.1, 2.2, 3.3, 4.4]], 'minItems' => 2, 'maxItems' => 4]];
 
         yield 'single choice callback' => [new Choice(['callback' => ChoiceCallback::getChoices(...)]), (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)]), ['enum' => ['a', 'b', 'c', 'd']]];
         yield 'multi choice callback' => [new Choice(['callback' => ChoiceCallback::getChoices(...), 'multiple' => true]), (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)]), ['type' => 'array', 'items' => ['type' => 'string', 'enum' => ['a', 'b', 'c', 'd']]]];
diff --git a/tests/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaCollectionRestrictionTest.php b/tests/Symfony/Validator/Metadata/Property/Restriction/PropertySchemaCollectionRestrictionTest.php
@@ -16,13 +16,15 @@
 use ApiPlatform\Metadata\ApiProperty;
 use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaCollectionRestriction;
 use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaFormat;
+use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaGreaterThanRestriction;
 use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaLengthRestriction;
 use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaRegexRestriction;
 use PHPUnit\Framework\TestCase;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\Constraints\Collection;
 use Symfony\Component\Validator\Constraints\Email;
+use Symfony\Component\Validator\Constraints\GreaterThan;
 use Symfony\Component\Validator\Constraints\Length;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\NotNull;
@@ -44,6 +46,7 @@ final class PropertySchemaCollectionRestrictionTest extends TestCase
     protected function setUp(): void
     {
         $this->propertySchemaCollectionRestriction = new PropertySchemaCollectionRestriction([
+            new PropertySchemaGreaterThanRestriction(),
             new PropertySchemaLengthRestriction(),
             new PropertySchemaRegexRestriction(),
             new PropertySchemaFormat(),
@@ -71,7 +74,7 @@ public static function supportsProvider(): \Generator
      */
     public function testCreate(Constraint $constraint, ApiProperty $propertyMetadata, array $expectedResult): void
     {
-        self::assertSame($expectedResult, $this->propertySchemaCollectionRestriction->create($constraint, $propertyMetadata));
+        self::assertEquals($expectedResult, $this->propertySchemaCollectionRestriction->create($constraint, $propertyMetadata));
     }
 
     public static function createProvider(): \Generator
@@ -93,6 +96,7 @@ public static function createProvider(): \Generator
             ]),
             'age' => new Optional([
                 new Type(['type' => 'int']),
+                new GreaterThan(0),
             ]),
             'social' => new Collection([
                 'fields' => [
@@ -101,11 +105,11 @@ public static function createProvider(): \Generator
             ]),
         ];
         $properties = [
-            'name' => [],
-            'email' => ['format' => 'email'],
+            'name' => new \ArrayObject(),
+            'email' => ['minLength' => 2, 'maxLength' => 255, 'format' => 'email'],
             'phone' => ['pattern' => '^([+]*[(]{0,1}[0-9]{1,4}[)]{0,1}[-\s\./0-9]*)$'],
-            'age' => [],
-            'social' => ['type' => 'object', 'properties' => ['githubUsername' => []], 'additionalProperties' => false, 'required' => ['githubUsername']],
+            'age' => ['exclusiveMinimum' => 0],
+            'social' => ['type' => 'object', 'properties' => ['githubUsername' => new \ArrayObject()], 'additionalProperties' => false, 'required' => ['githubUsername']],
         ];
         $required = ['name', 'email', 'social'];
 
diff --git a/tests/Symfony/Validator/Metadata/Property/ValidatorPropertyMetadataFactoryTest.php b/tests/Symfony/Validator/Metadata/Property/ValidatorPropertyMetadataFactoryTest.php
@@ -589,18 +589,22 @@ public function testCreateWithPropertyCollectionRestriction(): void
             (new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_ARRAY)])
         )->shouldBeCalled();
 
+        $greaterThanRestriction = new PropertySchemaGreaterThanRestriction();
         $lengthRestriction = new PropertySchemaLengthRestriction();
         $regexRestriction = new PropertySchemaRegexRestriction();
         $formatRestriction = new PropertySchemaFormat();
         $restrictionsMetadata = [
+            $greaterThanRestriction,
             $lengthRestriction,
             $regexRestriction,
             $formatRestriction,
             new PropertySchemaCollectionRestriction([
+                $greaterThanRestriction,
                 $lengthRestriction,
                 $regexRestriction,
                 $formatRestriction,
                 new PropertySchemaCollectionRestriction([
+                    $greaterThanRestriction,
                     $lengthRestriction,
                     $regexRestriction,
                     $formatRestriction,
@@ -619,14 +623,16 @@ public function testCreateWithPropertyCollectionRestriction(): void
         $this->assertEquals([
             'type' => 'object',
             'properties' => [
-                'name' => [],
-                'email' => ['format' => 'email'],
+                'name' => new \ArrayObject(),
+                'email' => ['format' => 'email', 'minLength' => 2, 'maxLength' => 255],
                 'phone' => ['pattern' => '^([+]*[(]{0,1}[0-9]{1,4}[)]{0,1}[-\s\./0-9]*)$'],
-                'age' => [],
+                'age' => [
+                    'exclusiveMinimum' => 0,
+                ],
                 'social' => [
                     'type' => 'object',
                     'properties' => [
-                        'githubUsername' => [],
+                        'githubUsername' => new \ArrayObject(),
                     ],
                     'additionalProperties' => false,
                     'required' => ['githubUsername'],

Original file line number	Diff line number	Diff line change
`@@ -69,19 +69,19 @@ public function supports(Constraint $constraint, ApiProperty $propertyMetadata):`
`69`	`69`	`return $constraint instanceof Collection;`
`70`	`70`	`}`
`71`	`71`
`72`		`- private function mergeConstraintRestrictions(Required\|Optional $constraint, ApiProperty $propertyMetadata): array`
	`72`	`+ private function mergeConstraintRestrictions(Required\|Optional $constraint, ApiProperty $propertyMetadata): array\|\ArrayObject`
`73`	`73`	`{`
`74`	`74`	`$propertyRestrictions = [];`
`75`	`75`	`$nestedConstraints = $constraint->constraints;`
`76`	`76`
`77`	`77`	`foreach ($nestedConstraints as $nestedConstraint) {`
`78`	`78`	`foreach ($this->restrictionsMetadata as $restrictionMetadata) {`
`79`		`- if ($restrictionMetadata->supports($nestedConstraint, $propertyMetadata) && !empty($nestedConstraintRestriction = $restrictionMetadata->create($nestedConstraint, $propertyMetadata))) {`
	`79`	`+ if ($restrictionMetadata->supports($nestedConstraint, $propertyMetadata->withExtraProperties(($propertyMetadata->getExtraProperties() ?? []) + ['nested_schema' => true])) && !empty($nestedConstraintRestriction = $restrictionMetadata->create($nestedConstraint, $propertyMetadata))) {`
`80`	`80`	`$propertyRestrictions[] = $nestedConstraintRestriction;`
`81`	`81`	`}`
`82`	`82`	`}`
`83`	`83`	`}`
`84`	`84`
`85`		`- return array_merge([], ...$propertyRestrictions);`
	`85`	`+ return array_merge([], ...$propertyRestrictions) ?: new \ArrayObject();`
`86`	`86`	`}`
`87`	`87`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a`
`41`	`41`	`public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool`
`42`	`42`	`{`
`43`	`43`	`$types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);`
	`44`	`+ if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {`
	`45`	`+ $types = [Type::BUILTIN_TYPE_INT];`
	`46`	`+ }`
`44`	`47`
`45`	`48`	`return $constraint instanceof GreaterThanOrEqual && is_numeric($constraint->value) && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);`
`46`	`49`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a`
`51`	`51`	`public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool`
`52`	`52`	`{`
`53`	`53`	`$types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);`
	`54`	`+ if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {`
	`55`	`+ $types = [Type::BUILTIN_TYPE_STRING];`
	`56`	`+ }`
`54`	`57`
`55`	`58`	`return $constraint instanceof Length && \count($types) && \in_array(Type::BUILTIN_TYPE_STRING, $types, true);`
`56`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,9 @@ public function create(Constraint $constraint, ApiProperty $propertyMetadata): a`
`49`	`49`	`public function supports(Constraint $constraint, ApiProperty $propertyMetadata): bool`
`50`	`50`	`{`
`51`	`51`	`$types = array_map(fn (Type $type) => $type->getBuiltinType(), $propertyMetadata->getBuiltinTypes() ?? []);`
	`52`	`+ if ($propertyMetadata->getExtraProperties()['nested_schema'] ?? false) {`
	`53`	`+ $types = [Type::BUILTIN_TYPE_INT];`
	`54`	`+ }`
`52`	`55`
`53`	`56`	`return $constraint instanceof Range && \count($types) && array_intersect($types, [Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT]);`
`54`	`57`	`}`