New feature: validate examples based on discriminator feature
Closes #155
From this PR, we are able to validate examples that contain "discriminator". However to can validate a specification, inheritance have to be in the following form :
---
openapi: 3.0.2
info:
title: petstore Inheritance
version: 1.0.7
paths:
"/pet":
post:
tags:
- pet
summary: Add a new pet to the store
description: Add a new pet to the store
operationId: addPet
requestBody:
description: Create a new pet in the store
content:
application/json:
schema:
$ref: "#/components/schemas/Pet"
examples:
dog:
value:
name: "12"
catName: "123"
type: "CAT"
required: true
responses:
'200':
description: Successful operation
content:
application/json:
schema:
"$ref": "#/components/schemas/Pet"
'405':
description: Invalid input
components:
schemas:
Dog:
type: object
properties:
dogName:
type: string
type:
type: string
enum:
- DOG
Cat:
type: object
properties:
catName:
type: string
type:
type: string
enum:
- CAT
Pet:
required:
- name
- type
type: object
properties:
id:
type: integer
format: int64
example: 10
name:
type: string
example: doggie
type:
type: string
discriminator:
propertyName: type
oneOf:
- $ref: "#/components/schemas/Cat"
- $ref: "#/components/schemas/Dog"
In other terms :
-
mappingkeyword hasn't be present - main schema has reference sub-schemas by using
oneOfkeyword, and not by usinganyOffrom sub-schemas - each sub-schema has to specify the discriminator property with its related value
Just for reference.. this is the continuation of #156
Hi @codekie ,
Can you tell me when you think that this PR could be merged? I'm waiting for your next release on my project ;)
Regards,
Hi @gigaga . I will need some time to thoroughly review and test this before I merge it into the main-branch, as this feature contains breaking changes. Due to my current workload, it's unlikely to happen this month.
But you gotta be aware that even when I have merged this, there won't be a new major release very soon, as there are other features that I'd like to include (in the next major release) which take some time to develop.
But what I can tell at a quick glance (I still have to do a thorough review yet, though) is that the included tests for the new feature are rather rudimentary. I'd like to see additional test-cases that cover (for example):
- Unsupported inheritance-type (eg.
MOUSE) - With
noAdditionalPropertiesset (to also make sure that the child-classes are not merged)
I also suggest to make dogName and catName required fields (that's the only field that distinguishes the types Cat and `Dog from each other and right now, it's optional and thus could be omitted, or simply disabled by typos in the property-name).
I know, the code-change for this feature may appear minor on the first glance, but it opens a whole new set of potential issues. And the more we can cover with automated tests, the better. The tests are not only there to check the current implementation, but also make it more robust to future changes (like refactoring, dependency upgrades, ...). That's why I mentioned cases like before. So, the more test-cases you can come up with, that are related to inheritance, the better.
Hi @codekie Can you tell me when this feature could be merged ? ;)
Regards,
Hi @gigaga . I can't make any promises, but maybe I have some time in the upcoming two weeks to look into this.
Great news :) Thanks for all :D
Hi @codekie ,
Can you tell me when this PR could be merged ? ;)
Regards,
Sorry, no. I still have to address the before-mentioned issues.
Hey @codekie, do you have any updates on this? We are working on a customer project and this is the only issue that stops us from having this library go to production code. Do you have any estimate on when you could have a look and merge this? Would be really helpful for us. Thank you in advance!
@crosa7 I had a look into it again. I have fixed the failed tests after the update-merge, but I have identified a couple of breaking changes that I need to have a closer look at first.
I don't want to make any estimates at this point as I have made them before and I couldn't hold them, but I'm on it and I'd like to have this feature there. I just want to make sure that everything else still works properly afterwards.
Thanks a lot for your effort @codekie. We will be looking forward to it