Conversation
|
Hello, indeed the issue is valid, thank you for working on it. Overall the fix looks good to me, but one more higher-level test would be good to have, which reads up a real openapi yaml (which somewhere uses a multi-type definition). You can use |
|
@erosb I think that validation need an update of json-sKema. I tried without, I replace the type with array by equivalent with oneOf. The vaidation works but the violation message not I don’t understand why. I will have some difficuties to work on json-sKema... |
|
Hello @erosb, Hope you're doing well. Just a quick update on this PR: the required change in json-sKema has been merged (PR #202 – Improve error message for multi-type validation). This resolves the dependency that was blocking the work here. From our side, the implementation should now be ready for merge if everything looks good to you. Thanks again for your time reviewing this, and please let us know if you'd like any further adjustments! |
|
Hi @erosb, Any updates on this PR please ? |
erosb
left a comment
erosb
left a comment
There was a problem hiding this comment.
One comment added, otherwise this looks very good overall.
| JsonNode rawJson = TreeUtil.json.convertValue(this, JsonNode.class); | ||
|
|
||
| // Transform array type to oneOf for proper validation | ||
| if (hasMultipleTypes() && rawJson instanceof ObjectNode) { |
There was a problem hiding this comment.
Are you sure this transformation is necessary? Does anything break without it?
There was a problem hiding this comment.
I believe the transformation is necessary because the underlying validator (and its discriminator check) often expects the type property to be a single string.
As you know In OAS 3.1, types are frequently defined as arrays (e.g., ["string", "null"]). By rewriting these as an anyOf or oneOf block, we kinda normalize the schema into a structure that the validation engine and discriminator logic can process without triggering wrongType syntax errors.
This ensures full compatibility with OAS 3.1 nullability patterns while keeping the core validator stable.
There was a problem hiding this comment.
i'd prefer to take precaution and handle any potential errors
There was a problem hiding this comment.
Hello again, I checked out your branch, commented out line 388-397 from Schema.java and all tests are passing. I'd rather delete these, since this transformation looks unnecessray, and it would be better not to have such transformation. This added "anyOf" would be present in the returned validation errors, which would be confusing for the user since they didn't write this "anyOf" in their schema. The underlying json-sKema already handles "type" arrays properly, so I'm not worried about potential errors.
Please delete line 388-397 then we can go ahead with merging and publishing a new kappa release :)
There was a problem hiding this comment.
Sure! will do and get back to you
feat: type must accept an array of string
|
Hello @araffass , thank you for your work. I published version |
3 participants
No description provided.