Configure `additionalProperties` validation strategy

[thunderer]

Hi, I would like to provide stricter validation by forcing additionalProperties to false in all schemas without requiring its presence. My current project has a large specification (hundreds of endpoints and even more schemas) and I need to thoroughly validate API contract and gradually adjust OpenAPI specification.

I can submit a PR, can you advise on what would be a good approach here? I see that Properties class has $additionalProperties constructor parameter which is then used exactly the way I need when its false. From what I see Properties::validate() is called only once in SchemaValidator line 136 and the value comes directly from the schema. Unfortunately, BodyValidator and other validators are created as ValidatorChain directly in RequestValidator and ResponseValidator which rules out customization through injecting custom options. SchemaValidator is created directly in BodyValidator as well.

What would be a good way to inject custom behaviour into these classes?

[lezhnev74]

Hey @thunderer!

I would like to provide stricter validation by forcing additionalProperties to false in all schemas without requiring its presence.

This is possible, but it is against OpenApi spec:

additionalProperties - Value can be boolean or object. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema. Consistent with JSON Schema, additionalProperties defaults to true.

If I had to do that I would fork this repo and changed this file src/Schema/Keywords/Properties.php on line 91.

[thunderer]

Hi @lezhnev74! Thanks for your awesome work that led to this library!

I am fully aware that this can be against the specification, that's why I'm asking for a configurable option that defaults to what's compliant. Sorry if that was not clear from my first post. I see three strategies that could be applied here:

• respect specification value (default, compliant with OpenAPI),
• apply additionalProperties only when not specified (stricter, IMO still "compliant" as it just changes default),
• force additionalProperties everywhere (strictest, not compliant as it overrides explicit values).

Currently my code is roughly equivalent to:

$openapi = (new ValidatorBuilder())->fromJsonFile($path);
$openApiRequest = $openapi->getRequestValidator();
$openApiResponse = $openapi->getResponseValidator();
// later
$openApiRequest->validate($request);
$address = new OperationAddress($request->getPathInfo(), strtolower($request->getMethod()));
$openApiResponse->validate($address, $response);

I would like to be able to access the SchemaValidator in the ValidatorChain -> BodyValidator -> UnipartValidator chain and add the ability to configure its additionalProperties strategy. It could be done better (proper strategy implementations with interface), but just to illustrate a simplest approach:

$strategy = fn($value) => $value ?: null; // default compliant
$strategy = fn($value) => $value ?: false; // stricter
$strategy = fn() => false; // strictest

$openApiRequest->getBodyValidator()->replaceAdditionalPropertiesStrategy($strategy);
$openApiResponse->getBodyValidator()->replaceAdditionalPropertiesStrategy($strategy);

[scaytrase]

My first impression is you need some kind of pre-processor for you schema. You can build schema from file and then process it to configure all additionalProperties to be false if not set explicitly to match schema you want to check instead of schema you have

[thunderer]

@scaytrase I don't think such a tool exists, and building one from scratch is not feasible in my current situation. What do you think about the possible solutions I described above? I would be glad to discuss the preferred approach and provide a PR.

[scaytrase]

I think different interpretations of schema should be done by changing schema itself. If we step on the way of variating validation for custom rules interpretation or overrides then will be at some moment in a situation with a dozens of custom logic overrides in code.

I can imaging a lot of customization which could be useful on adoption stage, like make all fields non-required or normalize all object field names keeping originals, etc. Each of this, including your one, could be achieved by properly processing input schema and without touching the logic (multiplying the complexity).

I don't think building schema processor is too complicated to consume much time. In a nutshell it should be just a recursive tree traverse which apply modifications for each object param

[thunderer]

@scaytrase @lezhnev74 This discussion seems to revolve more around our design principles than my initial issue, so let me rephrase the question a bit.

From what I see your goal is to make this library the reference implementation of OpenAPI standard. I support that goal, that's why all of us are here. My initial suggestion was to make certain internal behaviour configurable with compliant defaults, but I can understand if that's not what you want as maintainers.

There are other ways to achieve my goal that I think would benefit the library as well. As noted above, most of the dependencies are created directly rather than being injected. Making these dependencies explicit would benefit the overall design and allow users to provide their own instances without any effort from your side or need for support. Then I could replace only the parts I need on my side. We could, for example, inject the validators separately in the constructor, but since sometimes there are so many of them, we could also introduce a registry which other classes would receive in the constructor and fetch necessary instances. If you have better ideas, I'm all ears. What do you think?

[lezhnev74]

...most of the dependencies are created directly rather than being injected. Making these dependencies explicit would benefit the overall design and allow users to provide their own instances without any effort from your side or need for support

In general, this makes sense and indeed makes the package much more extensible. Any part of it can be extended with custom logic, while we can provide a default implementation.

From what I see your goal is to make this library the reference implementation of OpenAPI standard

Yes, the goal of this library is to validate PSR-7 messages against OpenAPI specs. We do our best to support all OpenAPI current features (still not nearly there). This package's interfaces have limited external dependencies because all this library needs to achieve the above purpose can be inferred from OpenAPI specification. Customization is only supposed to be within the OpenAPI specification language.

We followed UNIX philosophy - one tool should do one job only, but do it well. If we need to provide some complex behaviour we combine tools, rather than making a single one. In your case, "preprocessing step | this tool" ;

I'd love to continue this conversation, so please feel free to provide references to a similar design so we can review and understand deeper your suggestions and how it benefits other packages in PHP ecosystem.

[scaytrase]

Making these dependencies explicit would benefit the overall design and allow users to provide their own instances without any effort from your side or need for support.

Yea, this is possible, but this requires pretty big refactoring to the way how the validators are being instantiated now. But since we have the factories for it this may be hidden for end user. This idea sounds better then implementing strategies in each place we want logic override, so I'll be glad to see your ideas in PR