Add deep discriminator support #1106
Conversation
|
@cdimascio any thoughts on this ? :) |
|
Thanks for the PR. Will have a look within the next week. Cheers! |
|
@cdimascio just kind reminder :) |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances discriminator validation support in OpenAPI by introducing a new discriminator: true parameter. The implementation focuses on enabling proper discriminator validation for nested array structures and deep schema hierarchies.
- Adds a comprehensive Operation discriminator example with nested array validation
- Introduces the
discriminatorparameter to request validation options - Provides extensive test coverage for discriminator validation scenarios
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/resources/discriminator.yaml | Defines Operation schema with discriminator mapping in array context for testing |
| test/discriminator.spec.ts | Comprehensive test suite covering valid/invalid discriminator operations |
| test/ajv.options.spec.ts | Tests for new discriminator parameter handling in AjvOptions |
| src/framework/types.ts | Adds discriminator boolean property to ValidateRequestOpts |
| src/framework/ajv/options.ts | Implements discriminator parameter in request validation options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| key: string; | ||
| titleTranslationKey: string; | ||
| type: 'normal' | 'intermediate'; | ||
| step?: string; |
There was a problem hiding this comment.
The step property is marked as optional in the TypeScript type definition but is required in the OpenAPI schema (line 179 in discriminator.yaml). This inconsistency could lead to confusion and validation errors.
| step?: string; | |
| step: string; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
fixed, check one more time
src/framework/ajv/options.ts
Outdated
| allowUnknownQueryParameters, | ||
| coerceTypes, | ||
| removeAdditional, | ||
| discriminator |
There was a problem hiding this comment.
Missing comma after the discriminator property. This will cause a syntax error.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
fixed, check one more time
|
comments/suggestions by Copilot were fixed, thanks @cdimascio |
|
Thank you @nt0tsky 👍 PR looks good. I will get this merged and rolled into a new version. One last thing, could you please update the doc here, https://github.com/cdimascio/express-openapi-validator-documentation, with this new option. Best, thanks again |
|
@cdimascio sure, I will do it today/tomorrow. Thank you |
|
Thanks appreciate it 🚀 |
2 participants
This PR adds a new Operation discriminator example from openapi.yaml to illustrate and validate discriminator usage in nested array structures.
While top-level discriminators are already supported, additional work was needed for deep structures: this PR introduces the new parameter discriminator: true to enable proper validation when discriminators are used inside arrays or other nested schemas.
Changes
Purpose