Support custom converter for request parameters

[supermomonga]

It would be helpful to have the ability to set custom converters for request parameters.

For example, consider the following cases:

• In a Rails application, convert a date-time string to ActiveSupport::TimeWithZone
• Wrap values with a custom ValueObject class

Personally, I would like to use this feature in the openapi_rails_typed_parameters gem I'm developing.

---

ref: #98 (comment)

[ahx]

Hi @supermomonga. Thanks for starting this discussion. When talking about request parameters, do you also mean the request body or just query parameters?

[supermomonga]

Hi @ahx

Yeah, I only meant for path and query parameters.
I don't have a confident opinion about custom converter for request body, at this moment.

[ahx]

One method to add a way to modify all validated properties could be via json_schemers :after_property_validation option (link). We could add an option to set this either globally or only for header, query, cookie, path, body. I have not thought about an interface, yet. Do you have an idea for an interface to do that?

[supermomonga]

Yeah that would be nice!

I imagined something like following:

Specify a pattern object for pattern matching feature as an argument, and if it matches, the value will be converted. Registered converters have a order, and if a converter matches, the converters rest of it will be ignored.

I'm not confident in whether to prioritize a global converter or a non-global converter.

# Rack middleware
use(
  OpenapiFirst::Middlewares::RequestValidation,
  spec: 'openapi.yaml',
  value_converters: [] # 'value_converters' optional argument
)

# or

# configurable Rack middleware
OpenapiFirst::Middlewares::RequestValidation.configure do |config|
  config.value_converters.append({ # append it. so enum, date-time, or other format converters will be match first.
    name: :my_string_converter,
    schema: { type: :string },
    converter: lambda { |value| value }}) # do nothing
  
  config.value_converters.prepend({
    name: :my_string_converter2,
    schema: { type: :string },
    converter: lambda { |value| 'hi, ' + value }}) # greeting
  
  config.value_converters.prepend({
    name: :my_enum_string_converter,
    schema: { type: :string, enum: Array },
    converter: lambda { |value| value.to_sym }}) # convert to symbol
  
  config.value_converters.prepend({
    name: :'my_date-time_string_converter',
    schema: { type: :string, 'date-time': Array },
    converter: lambda { |value| DateTime.parse(value) }}) # convert to datetime
  
  config.value_converters.reject! { |_| _ in { name: my_string_converter2 } } # remove converter by name using pattern match
  config.value_converters.reject! { |_| _ in { schema: { type: :string, enum: Array }} } # remove converter by schema using pattern match

  # for non-global converters.
  config.query_value_converters.append(...) # only for query params
  config.path_value_converters.append(...) # only for path params
end
p OpenapiFirst::Middlewares::RequestValidation.configuration.value_converters # list converters. Array[{ name: Symbol, schema: Pattern, converter: Lambda }]

# Manual use
definition = OpenapiFirst.load('petstore.yaml', value_converters: []) # 'value_converters' optional argument
definition.configuration.value_converters.append(...) # same usage as rack middleware config

[ahx]

This looks very smart. Thanks for taking the time to come up with that approach.

if a converter matches, the converters rest of it will be ignored

So if there are two converters that match only the first match will be applied? I like that. I guess this has to be clearly mentioned somewhere so people don't expect multiple converters to be run in sequence, but this makes sense to me!

I am a little hesitant to use pattern matching here since this will be run after validation on every property. I have read somewhere that there still is a performance impact as compared to an if statement, but I don't have real data to confirm that. Do you have any insights or opinions on that?

[supermomonga]

So if there are two converters that match only the first match will be applied? I like that. I guess this has to be clearly mentioned somewhere so people don't expect multiple converters to be run in sequence, but this makes sense to me!

Yes, since each converter cannot assume what other converters do, I thought that only a single converter should be applied.

I am a little hesitant to use pattern matching here since this will be run after validation on every property. I have read somewhere that there still is a performance impact as compared to an if statement, but I don't have real data to confirm that. Do you have any insights or opinions on that?

Indeed, as you say, I agree that we should prioritize performance over flexibility.
I don't have any insights on the performance of pattern matching, but intuitively it would be slower than O(1) access.

Maybe we might want to consider more simpler interface like following?

config.value_converters[:string] = lambda { # ... }
config.value_converters[:enum_string] = lambda { # ... }
config.value_converters[:'date-time_string'] = lambda { # ... }
config.value_converters[[:string, :integer]] = lambda { # ... } # mixed type https://swagger.io/docs/specification/data-models/data-types/

# or

config.value_converters[{ type: :string }] = lambda { # ... }
config.value_converters[{ type: :string, format: :enum}] = lambda { # ... }
config.value_converters[{ type: :string, format: :'date-time'}] = lambda { # ... }
config.value_converters[{ type: [:string, :integer] }] = lambda { # ... } # mixed type https://swagger.io/docs/specification/data-models/data-types/

[ahx]

[…] config.value_converters[[:string, :integer]]

I think I understand the approach where the part inside the brackets almost looks like a pattern, but is one item of a predefined list. My concern is that maintaining that list would be cumbersome.

Here is another one:

# Only call the first converter that matches for a given property 
module MyConverter
  def self.match?(data, property, property_schema, _parent)
    property_schema['format'] == 'date-time'
  end

  def self.call(data, property, property_schema, _parent)
    data[property] = DateTime.parse(data[property])
  end
end

# use this in every Schema validation
config.converters.append MyConverter
# use this only in query parameters (can be query, path, header, body, cookie)
config.converters.query.prepend MyConverter

I think it's a bit bulky, but also easy to understand. What do you think?

[supermomonga]

I think I understand the approach where the part inside the brackets almost looks like a pattern, but is one item of a predefined list. My concern is that maintaining that list would be cumbersome.

I think that approach have to iterate converters until find matched one. but if the cost of finding matched converter per property can be O(n) instead of O(1), I think MyConverter.match? approach is the best. That interface is more easier to understand than my first suggestion.

[ahx]

I have another question about the feature in general:

Since this feature, at least as it looks in my head right now, would modify the data after validation, it would change the return value of request.query for instance. Is that good or do we need a dedicated interface for that?

[supermomonga]

I basically don't like implicit side effects. I prefer the interface like following:

definition = OpenapiFirst.load('petstore.yaml').request(...)

# validation only. not affect side effect
definition.validate

# coerce values using converters, and returns new value. no side effect for `definition`
coerced_params = definition.coerce_params
coerced_path_params = definition.coerce_path_parameters

# coerce values using converters, modify `definition`
definition.coerce_params!
definition.coerce_path_parameters!

But, I think JSONSchemer's after_property_validation feature will automatically applied when validation.
so this interface may can not be implemented straightforwardly?

[ahx]

Here are three variants that should summarize the above:

Variant A – adding request.validated (No breaking change)

Here request.validate sets a validated object on the request that returns converted values

request.query # => unconverted query params
request.validated # => nil
request.validate # ⚡️ Validate request and collect converters
request.validated.query # => converted query params
request.validated # => nil if validation failed

# Access validated, converted fields:
request.validated.body
request.validated.cookies
request.validated.headers
request.validated.path_parameters
request.validated.query

Variant B – request.validate returns a new object with copied & converted values (Breaking change)

Here validate no longer returns nil if validation passes, but returns a new object that can return converted values

request.query # => unconverted query params
result = request.validate # ⚡️ Validate request and call converters

if result.valid?
  # Access validated, converted fields:
  result.query # => {...} unconverted query params
  result.body
  result.cookies
  result.headers
  result.path_parameters
  result.failure # nil
end

if result.invalid?
  result.body # => nil or unconverted body?
  result.failure # Failure instance
end

Variant C – Convert the values in-place (No breaking change)

While this might not be ideal, it's probably the easiest to implement and probably the most performant variant

request.query # => unconverted query params
request.validate
request.query # => converted query params

[supermomonga]

@ahx Thanks! I like variant B, or A.

In variant A, validate method has side effect to validated property, so maybe validate! is better signature?

[ahx]

validate! is already taken to validate the request and raise an exception is validation fails. We could change that in 2.0 … I am bit lost here what the best option would be. 🤔

[ahx]

Here is another suggested API that touches this topic: #221

[supermomonga]

@ahx Thanks! I'll take look it.

[ahx]

Hey @supermomonga. I have implemented an interface where you could pass procs to json_schemers after_property_validation on the 1.4.0 branch. The after_request_parameter_property_validation should work for you. See README for details. Maybe you can try that out and give me some feedback out this? That would be much appreciated.

[supermomonga]

@ahx Thank you! I'll try it this weekend!

[ahx]

FYI I have added these hooks to main. These will be available in version 2.0, because adding them requires a breaking change.

[supermomonga]

@ahx sorry for delay. I've been swamped with work for a while. I am planning to try this ASAP.

[ahx]

No problem at all, @supermomonga. Take care of yourself.

[ahx]

There is an API to build custom converters in the "dev" branch. See [after_request_parameter_property_validation](https://github.com/ahx/openapi_first/tree/dev?tab=readme-ov-file#hooks). This hook will be part of 2.0. There is no release date, yet.

I am closing this for now, but think this topic will pop up again in the future in a new discussion building on top of the hooks API.