assert_schema_conform always fails with required query parameters for requests non-GET requests

[ydah]

Overview

The following PR tightens the validation of query parameters:

• Validate path, query and request parameters separately #349

In some cases, query parameters cannot be passed to committee. Test failed because the required query parameters were not provided. This is caused by the use of query parameters in non-GET requests. In previous versions, parameters and request bodies were not rigorously checked, so this was not a problem, but in newer versions of committee, rigorously checked.

  1) /groups PATCH /groups/:id returns ok
     Failure/Error: assert_schema_conform 200
     
     Committee::InvalidRequest:
       #/paths/~1groups~1{id}/patch missing required parameters: name
     # ./spec/requests/groups_spec.rb:53:in `block (3 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # OpenAPIParser::NotExistRequiredKey:
     #   #/paths/~1groups~1{id}/patch missing required parameters: name
     #   ./spec/requests/groups_spec.rb:53:in `block (3 levels) in <top (required)>'

Reproduction code

I create a reproduction code: https://github.com/ydah/committee-v5-issue
There are two simple endpoints for this project:
https://github.com/ydah/committee-v5-issue/blob/4e1271fbde675a254b25f8a7b8c2867d397f04d9/app/controllers/groups_controller.rb#L4-L18

  # GET /groups
  def index
    @groups = Group.all

    render json: @groups, status: :ok
  end

  # PATCH/PUT /groups/1
  def update
    if @group.update(group_params)
      render json: @group, status: :ok
    else
      render json: @group.errors, status: :unprocessable_entity
    end
  end

Both define required query parameters.
https://github.com/ydah/committee-v5-issue/blob/4e1271fbde675a254b25f8a7b8c2867d397f04d9/swagger/swagger.yml#L3-L49

paths:
  /groups:
    get:
      summary: Get all groups
      parameters:
        - in: query
          name: limit
          schema:
            type: integer
          required: true
          description: The number of items to return¥
# (snip)
  /groups/{id}:
    patch:
      summary: Update a group
      parameters:
        - in: path
          name: id
          schema:
            type: integer
          required: true
          description: The group id
        - in: query
          name: name
          schema:
            type: string
          required: true
          description: The group name¥

Both pass query parameters as well, but succeed for GET and fail for PATCH, or non-GET, requests.

https://github.com/ydah/committee-v5-issue/blob/4e1271fbde675a254b25f8a7b8c2867d397f04d9/spec/requests/groups_spec.rb#L29-L43

  describe "GET /groups" do
    it "returns ok" do
      Group.create!
      get groups_url, { limit: 1 }, as: :json
      assert_schema_conform 200
    end
  end

  describe "PATCH /groups/:id" do
    it "returns ok" do
      group = Group.create!
      patch group_url(group), { name: "something" }, as: :json
      assert_schema_conform 200
    end
  end

Survey

It is not possible to pass query parameters to committee for non-GET requests since before the update. Here is where query parameters are actually retrieved:

committee/lib/committee/request_unpacker.rb

Lines 55 to 57 in f0611dc

	def unpack_query_params(request)
	@allow_query_params ? self.class.indifferent_params(request.GET) : {}
	end

Retrieved with Rack:: Request:: Helpers#GET, where query parameters are empty for non-GET requests.

For example, adding the following monkey patch shows that query parameters are not available for non-GET requests:

module Committee
  class RequestUnpacker
    def unpack_query_params(request)
      pp request.GET
      pp request.POST
      @allow_query_params ? self.class.indifferent_params(request.GET) : {}
    end
  end
end

When the above test is performed, the results are as follows:

❯ bundle exec rspec spec/requests/groups_spec.rb
{"limit"=>"1"} # <--- index request.GET
{} #  <--- index request.POST
.{} # <--- patch request.GET
{"name"=>"something"} # <--- patch request.POST
F

That is, schema validation for non-GET requests and endpoints with required query parameters will always appear to fail.