`$ref`s used in query parameters do not load

[moberegger]

Given an Openapi definition like

paths:
  /foo:
    get:
      parameters:
        - name: ids
           in: query
           style: form
           explode: true
           schema:
             type: array
             items:
               $ref: ../components/schemas/id.yml
             uniqueItems: true

json_schemer will raise an UnknownRef error when attempting to validate ids. This is because no ref_resolver is provided and thus its default proc { |uri| raise UnknownRef, uri.to_s } is used. This is happening because build_parameter_schema creates a JSONSchemer.schema inline instead of using the new RefResolver module recently introduced by openapi_first. I believe the same thing is also happening for response headers, because build_responses uses a similar approach for the headers schema. I haven't been able to prove this, though.

Not sure how easy it would be to get those using the same technique used for request bodies and response bodies. Still trying to understand how all of this magic is working 😆.

For now, I was able to workaround this by JSONSchemer.configureing a ref_resolver that would be used as a fallback when openapi_first doesn't provide its own.

        JSONSchemer.configure do |config|
          config.ref_resolver =
            JSONSchemer::CachedResolver.new do |uri|
              OpenapiFirst::FileLoader.load(
                File.join(File.dirname(schema_path), uri.path),
              )
            end
        end

Since that workaround solved my problem, at made me then wonder: instead of creating a new JSONSchemer::CachedResolver for each schema, can we just have a single one that is used by all of them? The current implementation is

      def schema(options = {})
        ref_resolver = JSONSchemer::CachedResolver.new do |uri|
          FileLoader.load(uri.path)
        end
        base_uri = URI::File.build({ path: "#{dir}/" })
        root = JSONSchemer::Schema.new(context, base_uri:, ref_resolver:, **options)
        JSONSchemer::Schema.new(value, nil, root, base_uri:, **options)
      end

The ref_resolver will be initialized for each schema, which means that each schema is getting its own file cache, and so the same sub schema can be loaded multiple times if it has multiple $refs to it from various parent schemas. I imagine the same ref_resolver could be used for all schemas since there is only a single implementation of it, and the uri.path should still be accurate because the schema has a base_uri provided to it. This might help with performance and memory, because it would be able to re-use any cached schemas across the entire Openapi definition.

[ahx]

Hey @moberegger. Oh noes, it looks like the change introduced with 2.2.0 came with a few issues 🤦🏼. I think you are right in all aspects.

[moberegger]

I'll see if I can get something working for $refs used in query parameters and response headers. Still need to wrap my head around a few things... the technique you're using is very clever 😆.

Perhaps later we can see about getting a single ref_resolver working across an entire Openapi definition. My assessment there may not be correct, though.

[ahx]

Hey @moberegger I have worked a little on this issue over here. Maybe you can take a look at that. I will leave that there for a few days I think.

the technique you're using is very clever

I usually don't like clever that much. :) Maybe we can do something about that. One reason why this is a bit different than the request body for example is, that the code currently is creating one Schema per parameter type. So one query parameter schema, one headers parameter schema etc. When unpacking the request we just have to throw all query parameters against a single schema instead of validating one parameter/value at a time. My thought here was that that should be simpler in the end, but maybe we should reconsider.

[ahx]

I can work on a fix for this, unless you want to take this on.

[moberegger]

I poked around to see what I was able to get working, but didn't get very far. To start, I tried to take a similar approach to #323 to see if I could get it working on a bundled schema and discovered an interesting problem.

Similar to the example above, let's say I have

paths:
  /foo:
    get:
      parameters:
        - name: ids
           in: query
           style: form
           explode: true
           schema:
             type: array
             items:
               $ref: '#/components/schemas/Ids'
             uniqueItems: true
components:
  schemas: 
    Ids: 
      type: integer

When ids makes it's way over to openapi_parameters, it doesn't know what to do with that $ref. I haven't explored the code too much yet, but it looks like that gem isn't aware of JSON schemas, nor does it have a mechanism built in to resolve references like openapi_first does.

What happens is if I make a request to /foos?ids=1&ids=2, openapi_parameters isn't able to coerce the ids into an integer, and so when those params make there way over to request validation, they are still what is in the raw Rack request - ids: ['1', '2']. openapi_first sees this and the json_schemer schema fails validation because the inputs are strings, not integers.

This one seems trickier to solve, because it would increase the scope of what openapi_parameters has to do - it would need know how to resolve references and/or know how to work with the json_schemer gem.

[ahx]

Yes. This is a problem. The initial approach was to dereference the parameter schemas before they get passed to openapi_parameters, but I think we can discard that approach now. For the coercion, openapi_parameters only takes a look at the types in the schema. It does not know how to validate this. I have created openapi_parameters as an extra gem so that other implementations like committee can use it (because all that style, explode, … stuff is hard to get right). So far it did not get picked up, but that's fine. If we get to the point where openapi_parameters being an external gem causes more pain than it benefits the project I am fine with just merging it in openapi_first. I can think of two approaches from the top of my head:

1. We don't pass an Array of Hashes to openapi_parameters, but something that quacks like that but takes care of derefencing. So something similar to RefResolver
2. We inline the code from openapi_parameters right now and make it work otherwise

[ahx]

FYI: I have two failing tests about this, parameter and reponse header validation, on this branch.

• https://github.com/ahx/openapi_first/blob/fix-parameter-schema-ref/spec/middlewares/request_validation/query_parameter_validation_spec.rb#L118
• https://github.com/ahx/openapi_first/blob/fix-parameter-schema-ref/spec/middlewares/response_validation/response_header_validation_spec.rb#L52

[ahx]

I have not looked at this again in detail, but my current thought about this is to stop merging all parameter schemas and change parameter validation to run each parameter schema individually. This would probably change quite a few things on the inside, but should not affect the public API (#validate_request). 🤔

[ahx]

This should be fixed on main via https://github.com/ahx/openapi_first/pull/342. It uses a little bit more memory now. I’ll do a new release in the coming days.

It would be great if you get to test main to confirm if this solved your case, @moberegger

[moberegger]

Seems to have worked like a charm! Was able to remove my JSONSchemer::CachedResolver workaround, too!