From 5f20eaadd99f755916f065e4c1026aa58314ee31 Mon Sep 17 00:00:00 2001 From: David Harsha Date: Sun, 19 Jan 2025 11:59:02 -0800 Subject: [PATCH] Require discriminator instance to be an object Previously, all non-hash instances were valid when using `discriminator`, which I think is incorrect and confusing. There are a couple options: 1. Require instances to be objects when using `discriminator` 2. Fall back to regular `allOf`/`anyOf`/`oneOf` processing for non-object instances I went with option 1, because it seems safer and I don't know if it makes sense to ever allow non-object instances. `discriminator` depends on `propertyName`, which itself requires hash instances. Allowing `null` is still possible by wrapping the `discriminator` schema in another `oneOf` (like the test example: `nullable_union_schema`). The [OpenAPI specification][0] doesn't say anything about non-object instances directly, but it does say: > The expectation now is that a property with name petType MUST be present in the response payload, and the value will correspond to the name of a schema defined in the OAS document. Which I think can be interpreted as requiring the response payload to be an object. Fixes: https://github.com/davishmcclurg/json_schemer/issues/204 Related: https://github.com/davishmcclurg/json_schemer/commit/be45f04e515df7da9eb0d9694bf3b1db3a7fbfdd [0]: https://github.com/oai/openapi-specification/blob/main/versions/3.1.0.md#discriminator-object --- lib/json_schemer/openapi31/vocab/base.rb | 2 +- test/open_api_test.rb | 102 ++++++++++++++++++++++- 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/lib/json_schemer/openapi31/vocab/base.rb b/lib/json_schemer/openapi31/vocab/base.rb index 7a59c5b3..6da58e65 100644 --- a/lib/json_schemer/openapi31/vocab/base.rb +++ b/lib/json_schemer/openapi31/vocab/base.rb @@ -100,7 +100,7 @@ def subschemas_by_property_value end def validate(instance, instance_location, keyword_location, context) - return result(instance, instance_location, keyword_location, true) unless instance.is_a?(Hash) + return result(instance, instance_location, keyword_location, false) unless instance.is_a?(Hash) property_name = value.fetch('propertyName') diff --git a/test/open_api_test.rb b/test/open_api_test.rb index fcdc36cc..9209e19d 100644 --- a/test/open_api_test.rb +++ b/test/open_api_test.rb @@ -695,9 +695,107 @@ def test_non_json_pointer_discriminator end def test_discriminator_non_object_and_missing_property_name - schemer = JSONSchemer.schema({ 'discriminator' => { 'propertyName' => 'x' } }, :meta_schema => JSONSchemer.openapi31) - assert(schemer.valid?(1)) + schemer = JSONSchemer.schema( + { + 'anyOf' => [{ '$ref' => '#/components/schemas/z' }], + 'discriminator' => { 'propertyName' => 'x' }, + 'components' => { 'schemas' => { 'z' => true } }, + }, + :meta_schema => JSONSchemer.openapi31 + ) + refute(schemer.valid?(1)) refute(schemer.valid?({ 'y' => 'z' })) + assert(schemer.valid?({ 'x' => 'z' })) + end + + def test_discriminator_nullable + cat = { id: 1, type: 'cat', meow: 'Meow' } + dog = { id: 1, type: 'dog', bark: 'Woof' } + junk = { id: 1, type: 'cat', junk: 'junk' } + + refs = { + URI('json-schemer://schema/cat') => { + type: 'object', + properties: { + id: { + type: 'integer', + }, + type: { + type: 'string', + const: 'cat', + }, + meow: { + type: 'string', + }, + }, + required: %w[id type meow], + }, + URI('json-schemer://schema/dog') => { + type: 'object', + properties: { + id: { + type: 'integer', + }, + type: { + type: 'string', + const: 'dog', + }, + bark: { + type: 'string', + }, + }, + required: %w[id type bark], + }, + } + + nullable_union_schema = { + oneOf: [ + { type: 'null' }, + { + oneOf: [{ '$ref': 'cat' }, { '$ref': 'dog' }], + discriminator: { + propertyName: 'type', + mapping: { + cat: 'cat', + dog: 'dog', + }, + }, + }, + ], + } + + nullable_union_schemer = JSONSchemer.schema( + nullable_union_schema, + meta_schema: 'https://spec.openapis.org/oas/3.1/dialect/base', + ref_resolver: refs.to_proc, + ) + + assert(nullable_union_schemer.valid?(cat)) + assert(nullable_union_schemer.valid?(dog)) + refute(nullable_union_schemer.valid?(junk)) + assert(nullable_union_schemer.valid?(nil)) + + non_nullable_union_schema = { + oneOf: [{ '$ref': 'cat' }, { '$ref': 'dog' }], + discriminator: { + propertyName: 'type', + mapping: { + cat: 'cat', + dog: 'dog', + }, + }, + } + + non_nullable_union_schemer = JSONSchemer.schema( + non_nullable_union_schema, + meta_schema: 'https://spec.openapis.org/oas/3.1/dialect/base', + ref_resolver: refs.to_proc, + ) + + assert(non_nullable_union_schemer.valid?(cat)) + assert(non_nullable_union_schemer.valid?(dog)) + refute(non_nullable_union_schemer.valid?(junk)) + refute(non_nullable_union_schemer.valid?(nil)) end def test_openapi31_formats