`before_property_validation` and `after_property_validation` hooks run twice when `insert_property_defaults: true`

[moberegger]

Given a schema like the following, the before_property_validation and after_property_validation hooks run a single time. So

obj_schema = {
  'type' => 'object',
  'properties' => {
    'minimumDate' => {
      'type' => 'string',
      'format' => 'date-time',
    },
    'maximumDate' => {
      'type' => 'string',
      'format' => 'date-time',
    },
    'limit' => {
      'type' => 'integer',
      'format' => 'int32',
      'minimum' => 0,
      'maximum' => 100,
      'default' => 10,
    },
  },
  'required' => [],
}

schemer =
  JSONSchemer.schema(
    obj_schema,
    meta_schema: 'https://spec.openapis.org/oas/3.1/dialect/base',
    before_property_validation: ->(_params, property, schema, _parent) do
      puts "before validation > #{property}: #{schema}"
    end,
    after_property_validation: ->(_params, property, schema, _parent) do
      puts "after validation > #{property}: #{schema}"
    end,
  )

input = { 'minimumDate' => DateTime.now.rfc3339, 'maximumDate' => DateTime.now.rfc3339 }

schemer.validate(input)

will result in an output of

before validation > minimumDate: {"type" => "string", "format" => "date-time"}
before validation > maximumDate: {"type" => "string", "format" => "date-time"}
before validation > limit: {"type" => "integer", "format" => "int32", "minimum" => 0, "maximum" => 100, "default" => 10}
after validation > minimumDate: {"type" => "string", "format" => "date-time"}
after validation > maximumDate: {"type" => "string", "format" => "date-time"}

If you configure the JSONSchemer to have insert_property_defaults: true

schemer =
  JSONSchemer.schema(
    obj_schema,
    meta_schema: 'https://spec.openapis.org/oas/3.1/dialect/base',
    insert_property_defaults: true,
    before_property_validation: ->(_params, property, schema, _parent) do
      puts "before validation > #{property}: #{schema}"
    end,
    after_property_validation: ->(_params, property, schema, _parent) do
      puts "after validation > #{property}: #{schema}"
    end,
  )

the hooks run twice

before validation > minimumDate: {"type" => "string", "format" => "date-time"}
before validation > maximumDate: {"type" => "string", "format" => "date-time"}
before validation > limit: {"type" => "integer", "format" => "int32", "minimum" => 0, "maximum" => 100, "default" => 10}
after validation > minimumDate: {"type" => "string", "format" => "date-time"}
after validation > maximumDate: {"type" => "string", "format" => "date-time"}
after validation > limit: {"type" => "integer", "format" => "int32", "minimum" => 0, "maximum" => 100, "default" => 10}
before validation > minimumDate: {"type" => "string", "format" => "date-time"}
before validation > maximumDate: {"type" => "string", "format" => "date-time"}
before validation > limit: {"type" => "integer", "format" => "int32", "minimum" => 0, "maximum" => 100, "default" => 10}
after validation > minimumDate: {"type" => "string", "format" => "date-time"}
after validation > maximumDate: {"type" => "string", "format" => "date-time"}
after validation > limit: {"type" => "integer", "format" => "int32", "minimum" => 0, "maximum" => 100, "default" => 10}

This can be a problem if you attempt to coerce values in the after_property_validation block. For example, the following

schemer =
  JSONSchemer.schema(
    obj_schema,
    meta_schema: 'https://spec.openapis.org/oas/3.1/dialect/base',
    insert_property_defaults: true,
    after_property_validation:
      proc do |data, property, property_schema, _parent|
        if property_schema['format'] == 'date-time' && data[property].is_a?(String)
          data[property] = DateTime.parse(data[property])
        end
      end,
  )

input = { 'minimumDate' => DateTime.now.rfc3339, 'maximumDate' => DateTime.now.rfc3339 }

puts(schemer.validate(input).to_a.map { it['error'] })

Will actually result in a schema validation error

value at `/minimumDate` is not a string
value at `/maximumDate` is not a string

This is because when insert_property_defaults: true, the input gets validated twice - once before the defaults are applied, and once after, so the second time the validations runs minimumDate and maximumDate will have already been coerced to Dates, which fails the schema validation as they are no longer strings.

I poked around a bit in the source and you can see where this is happening in Schema#validate:

    def validate(instance, output_format: @configuration.output_format, resolve_enumerators: @configuration.resolve_enumerators, access_mode: @configuration.access_mode)
      instance_location = Location.root
      context = Context.new(instance, [], nil, (!insert_property_defaults && output_format == 'flag'), access_mode)
      result = validate_instance(deep_stringify_keys(instance), instance_location, root_keyword_location, context)
      if insert_property_defaults && result.insert_property_defaults(context, &property_default_resolver)
        result = validate_instance(deep_stringify_keys(instance), instance_location, root_keyword_location, context)
      end
      output = result.output(output_format)
      resolve_enumerators!(output) if resolve_enumerators
      output
    end

The validate_instance runs twice when insert_property_defaults is true, which causes the hooks to run an additional time. I tried to adjust this condition so that validate_instance only runs a single time, but it appears there is some load bearing logic in that method that is required to run in order for insert_property_defaults to function correctly.

Root cause may be similar to what is being discussed in #196.

[ekzobrain]

@davishmcclurg the intention was to validate again after default values insertion to ensure that data remains valid?
Generally that is correct, cause schema may have conditional dependencies on this value. But hooks also should not run twice or should be considered idempotent. Second case is the worst, so we should think of some algo that would not require this.

[davishmcclurg]

@davishmcclurg the intention was to validate again after default values insertion to ensure that data remains valid?

Yeah, that's correct—the intention is to validate the data after defaults are inserted. Inserting defaults requires validating the initial data to determine which defaults to insert (eg, only from a valid subtree).