Validate path params defined on the path item

[danielfone]

Since parameters can be defined on both the path item or the operation, we need to validate path params against both objects, like we do with the query params.

[ota42y]

Certainly, we need to check at that place as well.

But when Path Item and Operation have same name path parameters, we should overwrite by the latter.

If a parameter is already defined at the Path Item, the new definition will override it but can never remove it.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#operationObject

So if the path params is valid in Operation and invalid in Path Item, probably we'll get error 🤔

For example, GET /api/test/test_3 is valid request in this definition, but I think we'll get error.

openapi: 3.0.3
info:
  title: example
  version: '1.0'
paths:
  /api/test/{op}:
    parameters:
      - name: op
        in: path
        required: true
        schema:
          type: string
          enum: 
            - test1
            - test2
    get:
      parameters: 
      - name: op
        in: path
        required: true
        schema:
          type: string
          enum: 
            - test3
      responses:
        default:
          description: sample
          content:
            'application/json':
              schema:
                type: object

So we need merge these parameters and validate.
But this is a potential bug so I can take over and fix it 🙋

[danielfone]

@ota42y ahh good catch! I'm not likely to be able to look at this for a week or so. If I get to it before you I'm happy to have a go, otherwise all yours. Thanks! 😁

[ota42y]

There is no reason to rush to release a new version, so give it a try! 😄

[danielfone]

@ota42y I haven't added any specs for this yet, I wanted to see what you thought of the implementation. I've opted to merge the individual hashes explicitly in the OpenAPIParser::Schemas::Operation class since I think it makes it clearer what's going on.

Alternatively, we could add a public ParameterValidatable#merged_params method and keep the merging all inside the ParameterValidatable concern. Something like:

  # module OpenAPIParser::ParameterValidatable
  def merged_params
    return divided_parameter_hash unless parent&.respond_to?(:merged_params)

    @merged_params ||= Hash.new do |merged_params, location|
      parent_params = parent.merged_params[location] || {}
      merged_params[location] = parent_params.merge(divided_parameter_hash[location])
    end
  end

  def header_parameter_hash
    merged_params['header'] || {}
  end

  # etc…

I think this is a more abstract and less obvious approach though.

What do you think?

[ota42y]

Sorry for late reply... 🙇

I think this is a more abstract and less obvious approach though.
I think so too.

Your approach( merging in the OpenAPIParser::Schemas::Operation ) is very good 👍 👍 👍 👍 👍

• Performance is good because once merged it will not change after that.
• Components Object does not have Operation Object in OpenAPI 3 definition so Operation Object is always under the Path Item Object.

[danielfone]

@ota42y thanks for the feedback. I'll add some specs and ping you for a final review.