Merge pull request #145 from davishmcclurg/discriminator

Limit anyOf/oneOf discriminator to listed refs

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## [2.1.0] - XXXX-XX-XX
 
+### Bug Fixes
+
+- Limit anyOf/oneOf discriminator to listed refs
+- Require discriminator `propertyName` property
+- Support `Schema#ref` in subschemas
+
 [2.1.0]: https://github.com/davishmcclurg/json_schemer/releases/tag/v2.1.0
 
 ## [2.0.0] - 2023-08-20

lib/json_schemer/openapi31/vocab/base.rb

@@ -34,51 +34,89 @@ def validate(*)
         end
 
         class Discriminator < Keyword
-          include Format::JSONPointer
+          # https://spec.openapis.org/oas/v3.1.0#components-object
+          FIXED_FIELD_REGEX = /\A[a-zA-Z0-9\.\-_]+$\z/
 
           attr_accessor :skip_ref_once
 
           def error(formatted_instance_location:, **)
             "value at #{formatted_instance_location} does not match `discriminator` schema"
           end
 
-          def validate(instance, instance_location, keyword_location, context)
-            property_name = value.fetch('propertyName')
-            mapping = value['mapping'] || {}
+          def mapping
+            @mapping ||= value['mapping'] || {}
+          end
+
+          def subschemas_by_property_value
+            @subschemas_by_property_value ||= if schema.parsed.key?('anyOf') || schema.parsed.key?('oneOf')
+              subschemas = schema.parsed['anyOf']&.parsed || []
+              subschemas += schema.parsed['oneOf']&.parsed || []
 
-            return result(instance, instance_location, keyword_location, true) unless instance.is_a?(Hash) && instance.key?(property_name)
+              subschemas_by_ref = {}
+              subschemas_by_schema_name = {}
 
-            property = instance.fetch(property_name)
-            ref = mapping.fetch(property, property)
+              subschemas.each do |subschema|
+                subschema_ref = subschema.parsed.fetch('$ref').parsed
+                subschemas_by_ref[subschema_ref] = subschema
 
-            ref_schema = nil
-            unless ref.start_with?('#') && valid_json_pointer?(ref.delete_prefix('#'))
-              ref_schema = begin
-                root.resolve_ref(URI.join(schema.base_uri, "#/components/schemas/#{ref}"))
-              rescue InvalidRefPointer
-                nil
+                if subschema_ref.start_with?('#/components/schemas/')
+                  schema_name = subschema_ref.delete_prefix('#/components/schemas/')
+                  subschemas_by_schema_name[schema_name] = subschema if FIXED_FIELD_REGEX.match?(schema_name)
+                end
               end
-            end
-            ref_schema ||= root.resolve_ref(URI.join(schema.base_uri, ref))
 
-            return if skip_ref_once == ref_schema.absolute_keyword_location
+              explicit_mapping = mapping.transform_values do |schema_name_or_ref|
+                subschemas_by_schema_name.fetch(schema_name_or_ref) { subschemas_by_ref.fetch(schema_name_or_ref) }
+              end
 
-            nested = []
+              implicit_mapping = subschemas_by_schema_name.reject do |_schema_name, subschema|
+                explicit_mapping.value?(subschema)
+              end
 
-            if schema.parsed.key?('anyOf') || schema.parsed.key?('oneOf')
-              subschemas = schema.parsed['anyOf']&.parsed || []
-              subschemas += schema.parsed['oneOf']&.parsed || []
-              subschemas.each do |subschema|
-                if subschema.parsed.fetch('$ref').ref_schema.absolute_keyword_location == ref_schema.absolute_keyword_location
-                  nested << subschema.validate_instance(instance, instance_location, keyword_location, context)
+              implicit_mapping.merge(explicit_mapping)
+            else
+              Hash.new do |hash, property_value|
+                schema_name_or_ref = mapping.fetch(property_value, property_value)
+
+                subschema = nil
+
+                if FIXED_FIELD_REGEX.match?(schema_name_or_ref)
+                  subschema = begin
+                    schema.ref("#/components/schemas/#{schema_name_or_ref}")
+                  rescue InvalidRefPointer
+                    nil
+                  end
+                end
+
+                subschema ||= begin
+                  schema.ref(schema_name_or_ref)
+                rescue InvalidRefResolution, UnknownRef
+                  nil
                 end
+
+                hash[property_value] = subschema
               end
-            else
-              ref_schema.parsed['allOf']&.skip_ref_once = schema.absolute_keyword_location
-              nested << ref_schema.validate_instance(instance, instance_location, keyword_location, context)
             end
+          end
+
+          def validate(instance, instance_location, keyword_location, context)
+            return result(instance, instance_location, keyword_location, true) unless instance.is_a?(Hash)
+
+            property_name = value.fetch('propertyName')
+
+            return result(instance, instance_location, keyword_location, false) unless instance.key?(property_name)
+
+            property_value = instance.fetch(property_name)
+            subschema = subschemas_by_property_value[property_value]
+
+            return result(instance, instance_location, keyword_location, false) unless subschema
+
+            return if skip_ref_once == subschema.absolute_keyword_location
+            subschema.parsed['allOf']&.skip_ref_once = schema.absolute_keyword_location
+
+            subschema_result = subschema.validate_instance(instance, instance_location, keyword_location, context)
 
-            result(instance, instance_location, keyword_location, (nested.any? && nested.all?(&:valid)), nested)
+            result(instance, instance_location, keyword_location, subschema_result.valid, subschema_result.nested)
           ensure
             self.skip_ref_once = nil
           end

lib/json_schemer/schema.rb

@@ -113,7 +113,7 @@ def validate_schema
     end
 
     def ref(value)
-      resolve_ref(URI.join(base_uri, value))
+      root.resolve_ref(URI.join(base_uri, value))
     end
 
     def validate_instance(instance, instance_location, keyword_location, context)

test/json_schemer_test.rb

@@ -380,6 +380,12 @@ def test_schema_ref
       'type' => 'integer',
       '$defs' => {
         'foo' => {
+          '$id' => 'subschemer',
+          '$defs' => {
+            'bar' => {
+              'required' => ['z']
+            }
+          },
           'type' => 'object',
           'required' => ['x', 'y'],
           'properties' => {
@@ -401,10 +407,20 @@ def test_schema_ref
 
     refute(subschemer.valid?(1))
     assert_equal(
-      [["/x", "/$defs/foo/properties/x", "string"], ["", "/$defs/foo", "required"]],
+      [['/x', '/$defs/foo/properties/x', 'string'], ['', '/$defs/foo', 'required']],
       subschemer.validate({ 'x' => 1 }).map { |error| error.values_at('data_pointer', 'schema_pointer', 'type') }
     )
     assert(subschemer.valid?({ 'x' => '1', 'y' => 1 }))
+
+    subsubschemer = subschemer.ref('#/$defs/bar')
+    refute(subsubschemer.valid?({ 'x' => 1 }))
+    assert_equal(
+      [['', '/$defs/foo/$defs/bar', 'required']],
+      subsubschemer.validate({ 'x' => 1 }).map { |error| error.values_at('data_pointer', 'schema_pointer', 'type') }
+    )
+
+    assert_equal(subschemer, subschemer.ref('#'))
+    assert_equal(subschemer, subsubschemer.ref('#'))
   end
 
   def test_published_meta_schemas

test/open_api_test.rb

@@ -205,9 +205,9 @@ def test_discriminator_specification_example
     assert_equal([['enum', '/components/schemas/Cat/allOf/1/properties/huntingSkill']], schemer.validate(invalid_hunting_skill).map { |error| error.values_at('type', 'schema_pointer') })
     assert_equal([['required', '/components/schemas/Dog/allOf/1']], schemer.validate(missing_pack_size).map { |error| error.values_at('type', 'schema_pointer') })
     assert_equal([['format', '/components/schemas/Dog/allOf/1/properties/packSize']], schemer.validate(invalid_pack_size).map { |error| error.values_at('type', 'schema_pointer') })
-    assert_equal([['required', '/components/schemas/Pet']], schemer.validate(missing_pet_type).map { |error| error.values_at('type', 'schema_pointer') })
+    assert_equal([['required', '/components/schemas/Pet'], ['discriminator', '/components/schemas/Pet']], schemer.validate(missing_pet_type).map { |error| error.values_at('type', 'schema_pointer') })
     assert_equal([['required', '/components/schemas/Pet'], ['required', '/components/schemas/Cat/allOf/1']], schemer.validate(missing_name).map { |error| error.values_at('type', 'schema_pointer') })
-    assert_raises(JSONSchemer::UnknownRef) { schemer.validate(invalid_pet_type) }
+    assert_equal([['discriminator', '/components/schemas/Pet']], schemer.validate(invalid_pet_type).map { |error| error.values_at('type', 'schema_pointer') })
   end
 
   def test_all_of_discriminator
@@ -404,7 +404,7 @@ def test_all_of_discriminator_subclass_schemas_work_on_their_own
     assert(schemer.valid?(CAT))
     assert(schemer.valid?(MISTY))
     assert_equal(['/components/schemas/Cat/allOf/1/properties/name'], schemer.validate(INVALID_CAT).map { |error| error.fetch('schema_pointer') })
-    assert_equal([['required', '/components/schemas/Pet']], schemer.validate({}).map { |error| error.values_at('type', 'schema_pointer') })
+    assert_equal([['required', '/components/schemas/Pet'], ['discriminator', '/components/schemas/Pet']], schemer.validate({}).map { |error| error.values_at('type', 'schema_pointer') })
   end
 
   def test_all_of_discriminator_with_non_discriminator_ref
@@ -444,7 +444,52 @@ def test_all_of_discriminator_with_non_discriminator_ref
     refute(schemer.valid?(CAT))
     assert(schemer.valid?(CAT.merge('other' => 'y')))
     assert_equal(['/components/schemas/Other', '/components/schemas/Cat/allOf/2/properties/name'], schemer.validate(INVALID_CAT).map { |error| error.fetch('schema_pointer') })
-    assert_equal([['required', '/components/schemas/Pet'], ['required', '/components/schemas/Other']], schemer.validate({}).map { |error| error.values_at('type', 'schema_pointer') })
+    assert_equal([['required', '/components/schemas/Pet'], ['discriminator', '/components/schemas/Pet'], ['required', '/components/schemas/Other']], schemer.validate({}).map { |error| error.values_at('type', 'schema_pointer') })
+  end
+
+  def test_all_of_discriminator_with_remote_ref
+    schema = {
+      '$id' => 'http://example.com/schema',
+      'discriminator' => {
+        'propertyName' => 'petType',
+        'mapping' => {
+          'Dog' => 'http://example.com/dog'
+        }
+      }
+    }
+    schemer = JSONSchemer.schema(
+      schema,
+      :meta_schema => JSONSchemer.openapi31,
+      :ref_resolver => {
+        URI('http://example.com/schema') => schema,
+        URI('http://example.com/cat') => {
+          'allOf' => [
+            { '$ref' => 'http://example.com/schema' },
+            CAT_SCHEMA
+          ]
+        },
+        URI('http://example.com/dog') => {
+          'allOf' => [
+            { '$ref' => 'http://example.com/schema' },
+            DOG_SCHEMA
+          ]
+        }
+      }.to_proc
+    )
+
+    assert(schemer.valid_schema?)
+    refute(schemer.valid?(CAT))
+    assert(schemer.valid?(CAT.merge('petType' => 'http://example.com/cat')))
+    assert(schemer.valid?(DOG))
+
+    invalid_cat = INVALID_CAT.merge('petType' => 'http://example.com/cat')
+    invalid_cat_result = schemer.validate(invalid_cat, output_format: 'basic', resolve_enumerators: true)
+    assert_equal('/discriminator/allOf/1/properties/name/type', invalid_cat_result.dig('errors', 0, 'keywordLocation'))
+    assert_equal('http://example.com/cat#/allOf/1/properties/name/type', invalid_cat_result.dig('errors', 0, 'absoluteKeywordLocation'))
+
+    invalid_dog_result = schemer.validate(INVALID_DOG, output_format: 'basic', resolve_enumerators: true)
+    assert_equal('/discriminator/allOf/1/properties/bark/type', invalid_dog_result.dig('errors', 0, 'keywordLocation'))
+    assert_equal('http://example.com/dog#/allOf/1/properties/bark/type', invalid_dog_result.dig('errors', 0, 'absoluteKeywordLocation'))
   end
 
   def test_any_of_discriminator_without_matching_schema
@@ -513,6 +558,60 @@ def test_one_of_discriminator_without_matching_schema
     assert_equal([['discriminator', '/components/schemas/MyResponseType']], schemer.validate(INVALID_LIZARD).map { |error| error.values_at('type', 'schema_pointer') })
   end
 
+  def test_any_of_discriminator_ignores_nested_schemas
+    openapi = {
+      'openapi' => '3.1.0',
+      'components' => {
+        'schemas' => {
+          'MyResponseType' => {
+            'anyOf' => [
+              { '$ref' => '#/components/schemas/Cat' },
+              { '$ref' => '#/components/schemas/Cat/$defs/nah' }
+            ],
+            'discriminator' => {
+              'propertyName' => 'petType'
+            }
+          },
+          'Cat' => CAT_SCHEMA.merge('$defs' => { 'nah' => {} })
+        }
+      }
+    }
+
+    schemer = JSONSchemer.openapi(openapi).schema('MyResponseType')
+
+    assert(schemer.valid_schema?)
+    assert(schemer.valid?(CAT))
+    refute(schemer.valid?(CAT.merge('petType' => 'nah')))
+    refute(schemer.valid?(CAT.merge('petType' => 'Cat/$defs/nah')))
+  end
+
+  def test_one_of_discriminator_ignores_nested_schemas
+    openapi = {
+      'openapi' => '3.1.0',
+      'components' => {
+        'schemas' => {
+          'MyResponseType' => {
+            'oneOf' => [
+              { '$ref' => '#/components/schemas/Cat' },
+              { '$ref' => '#/components/schemas/Cat/$defs/nah' }
+            ],
+            'discriminator' => {
+              'propertyName' => 'petType'
+            }
+          },
+          'Cat' => CAT_SCHEMA.merge('$defs' => { 'nah' => {} })
+        }
+      }
+    }
+
+    schemer = JSONSchemer.openapi(openapi).schema('MyResponseType')
+
+    assert(schemer.valid_schema?)
+    assert(schemer.valid?(CAT))
+    refute(schemer.valid?(CAT.merge('petType' => 'nah')))
+    refute(schemer.valid?(CAT.merge('petType' => 'Cat/$defs/nah')))
+  end
+
   def test_discrimator_mapping
     openapi = {
       'openapi' => '3.1.0',
@@ -542,7 +641,7 @@ def test_discrimator_mapping
 
     assert(schemer.valid_schema?)
     assert(schemer.valid?(CAT.merge('petType' => 'c')))
-    assert(schemer.valid?(MISTY.merge('petType' => 'Cat')))
+    refute(schemer.valid?(MISTY.merge('petType' => 'Cat')))
     assert_equal(['/components/schemas/Cat/properties/name'], schemer.validate(INVALID_CAT.merge('petType' => 'c')).map { |error| error.fetch('schema_pointer') })
     assert(schemer.valid?(DOG.merge('petType' => 'd')))
     assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG.merge('petType' => 'dog')).map { |error| error.fetch('schema_pointer') })
@@ -585,8 +684,9 @@ def test_non_json_pointer_discriminator
     assert(schemer.valid?(CAT))
     assert(schemer.valid?(MISTY))
     assert_equal(['/components/schemas/Cat/properties/name'], schemer.validate(INVALID_CAT).map { |error| error.fetch('schema_pointer') })
-    assert(schemer.valid?(DOG))
-    assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG).map { |error| error.fetch('schema_pointer') })
+    refute(schemer.valid?(DOG))
+    assert_equal(['/components/schemas/MyResponseType'], schemer.validate(INVALID_DOG).map { |error| error.fetch('schema_pointer') })
+    assert_equal(['/components/schemas/Dog/properties/bark'], schemer.validate(INVALID_DOG.merge('petType' => 'dog')).map { |error| error.fetch('schema_pointer') })
     assert(schemer.valid?(LIZARD))
     assert_equal(['/components/schemas/Lizard/properties/lovesRocks'], schemer.validate(INVALID_LIZARD).map { |error| error.fetch('schema_pointer') })
     assert(schemer.valid?(MONSTER))
@@ -597,7 +697,7 @@ def test_non_json_pointer_discriminator
   def test_discriminator_non_object_and_missing_property_name
     schemer = JSONSchemer.schema({ 'discriminator' => { 'propertyName' => 'x' } }, :meta_schema => JSONSchemer.openapi31)
     assert(schemer.valid?(1))
-    assert(schemer.valid?({ 'y' => 'z' }))
+    refute(schemer.valid?({ 'y' => 'z' }))
   end
 
   def test_openapi31_formats