Fix Symfony Constraints Count On Maps With MinProperties

Hey everyone! Today, we're diving into a tricky bug I encountered while working with Symfony constraints and maps (specifically, array<X, Y>) in my NelmioApiDocBundle setup. It's all about ensuring our OpenApi definitions accurately reflect the constraints we've set on our data structures. So, grab your favorite beverage, and let's get started!

The Problem: minItems vs. minProperties

The core issue revolves around how Symfony constraints, particularly the Count constraint, interact with array properties that are defined as maps (i.e., associative arrays with string keys and mixed values). Consider the following example:

    /**
     * @var array<string, mixed>
     */
    #[Assert\NotNull, Assert\Count(min: 1)]
    public array $something;

Here, we have a property $something which is an array where the keys are strings and the values can be anything (mixed). We've also applied the Count constraint with a min value of 1, meaning we expect this array to have at least one element. The problem arises when NelmioApiDocBundle generates the OpenApi schema for this property. Instead of using minProperties it was using minItems.

The incorrect OpenApi generated looks like this:

{
    "something": {
        "type": "object",
        "minItems:": 1
    }
}

Notice the minItems property within the something object definition. This is where things go wrong. In the OpenApi specification, the minItems property is applicable to arrays, not objects. For objects, which represent maps in this case, the correct property to use is minProperties. The same applies for the max constraint, where maxItems was incorrectly used instead of maxProperties.

Why does this matter, you ask? Well, if your OpenApi definition includes minItems for an object, it's simply invalid according to the specification. This can lead to issues with validation, code generation, and other tools that rely on accurate OpenApi definitions. It can also cause confusion for anyone trying to understand your API's expected data structures.

The (Temporary) Solution: A Custom Describer Hack

To address this issue, I implemented a temporary workaround in our custom describer. This hack essentially intercepts the minItems and maxItems values and remaps them to minProperties and maxProperties respectively. It's not pretty, but it gets the job done until a proper fix is implemented in the NelmioApiDocBundle itself.

        if (Generator::UNDEFINED !== $property->minItems) {
            $property->minProperties = $property->minItems;
            $property->minItems = Generator::UNDEFINED /** @phpstan-ignore assign.propertyType */;
        }
        if (Generator::UNDEFINED !== $property->maxItems) {
            $property->maxProperties = $property->maxItems;
            $property->maxItems = Generator::UNDEFINED /** @phpstan-ignore assign.propertyType */;
        }

Let's break down the code:

1. Check for minItems and maxItems: The code first checks if the minItems and maxItems properties are defined on the $property object. Generator::UNDEFINED is used to represent an undefined value.
2. Remap to minProperties and maxProperties: If minItems is defined, its value is assigned to minProperties, and minItems is then set to Generator::UNDEFINED. The same logic is applied to maxItems and maxProperties.
3. @phpstan-ignore assign.propertyType: This annotation is used to suppress a potential type error in PHPStan, as the types of minItems/maxItems and minProperties/maxProperties might not perfectly align.

This hack ensures that the generated OpenApi now includes the correct minProperties and maxProperties for map-like properties, resulting in a valid and accurate schema.

Diving Deeper: Type Hinting and Generator::UNDEFINED

While investigating this issue, I also noticed something interesting about the typing of minItems and maxItems. According to the code, they are defined as @var int. However, they are initialized with Generator::UNDEFINED. This suggests a potential inconsistency in the type hinting. Ideally, these properties should either be nullable integers (?int) or use a more appropriate type that can represent an undefined state.

This observation further highlights the need for a proper fix within the SymfonyConstraintAnnotationReader.

The Root Cause: SymfonyConstraintAnnotationReader

The most likely location for the proper fix is within the SymfonyConstraintAnnotationReader class, specifically around lines 90-96 (as the original bug reporter pointed out). This class is responsible for reading Symfony constraint annotations and translating them into OpenApi schema properties. It appears that the logic for handling the Count constraint on array properties needs to be updated to correctly use minProperties and maxProperties when dealing with maps.

Here's what needs to happen:

1. Detect Map Types: The SymfonyConstraintAnnotationReader needs to be able to detect when a property is a map (i.e., an array with string keys and mixed values). This might involve inspecting the property's type hints or using reflection.
2. Apply minProperties and maxProperties: When a map type is detected, the reader should use the min and max values from the Count constraint to set the minProperties and maxProperties properties in the OpenApi schema, instead of minItems and maxItems.
3. Update Type Hints: The type hints for minItems and maxItems should be reviewed and updated to accurately reflect their possible values (e.g., ?int or a custom type that can represent an undefined state).

The Solution

To resolve this, modify the SymfonyConstraintAnnotationReader to correctly handle the Count constraint when applied to map-like arrays. Instead of setting minItems and maxItems, it should set minProperties and maxProperties on the generated OpenAPI schema. This ensures that the schema accurately reflects the constraints on the data structure, preventing validation errors and improving the overall API definition.

Conclusion: Ensuring Accurate OpenApi Definitions

In conclusion, this bug highlights the importance of carefully ensuring that our OpenApi definitions accurately reflect the constraints we've defined on our data structures. While the custom describer hack provides a temporary workaround, a proper fix within the SymfonyConstraintAnnotationReader is essential for a long-term solution. By correctly handling the Count constraint on map-like properties, we can ensure that our OpenApi schemas are valid, accurate, and provide a clear representation of our API's expected data formats.

I hope this deep dive into the issue, the workaround, and the potential fix has been helpful! Let me know if you have any questions or insights to share. Happy coding, guys!