Allow path param and request body to be passed at the same time

lib/committee/request_unpacker.rb

@@ -14,6 +14,48 @@ def initialize(request, options={})
     end
 
     def call
+      return call_hyperschema if hyperschema?
+
+      # if Content-Type is empty or JSON, and there was a request body, try to
+      # interpret it as JSON
+      params = {}
+
+      params['body'] = if [email protected]_type || @request.media_type =~ %r{application/.*json}
+        parse_json
+      elsif @optimistic_json
+        begin
+          parse_json
+        rescue JSON::ParserError
+          nil
+        end
+      else
+        {}
+      end
+
+      params['form_data'] = if @allow_form_params && %w[application/x-www-form-urlencoded multipart/form-data].include?(@request.media_type)
+        # Actually, POST means anything in the request body, could be from
+        # PUT or PATCH too. Silly Rack.
+        p = @request.POST
+
+        @schema_validator.coerce_form_params(p) if @coerce_form_params
+
+        p
+      else
+        {}
+      end
+
+      params['query'] = if @allow_query_params
+        indifferent_params(@request.GET)
+      else
+        {}
+      end
+
+      [params, headers]
+    end
+
+    private
+
+    def call_hyperschema
       # if Content-Type is empty or JSON, and there was a request body, try to
       # interpret it as JSON
       params = if [email protected]_type || @request.media_type =~ %r{application/.*json}
@@ -47,15 +89,17 @@ def call
       end
     end
 
-    private
-
     # Creates a Hash with indifferent access.
     #
     # (Copied from Sinatra)
     def indifferent_hash
       Hash.new { |hash,key| hash[key.to_s] if Symbol === key }
     end
 
+    def hyperschema?
+      @schema_validator.is_a?(Committee::SchemaValidator::HyperSchema)
+    end
+
     # Enable string or symbol key access to the nested params hash.
     #
     # (Copied from Sinatra)

lib/committee/schema_validator/open_api_3.rb

@@ -18,7 +18,8 @@ def request_validate(request)
 
         request_unpack(request)
 
-        request.env[validator_option.params_key]&.merge!(path_params) unless path_params.empty?
+        # committee.params
+        request.env[validator_option.params_key]&.merge!('path' => path_params) unless path_params.empty?
 
         request_schema_validation(request)
 
@@ -80,8 +81,11 @@ def request_unpack(request)
       def copy_coerced_data_to_query_hash(request)
         return if request.env["rack.request.query_hash"].nil? || request.env["rack.request.query_hash"].empty?
 
+        params = request.env[validator_option.params_key]
+        request_params = (params['path'] || {}).merge(params['query'] || {}).merge(params['form_data'] || {}).merge(params['body'] || {})
+
         request.env["rack.request.query_hash"].keys.each do |k|
-          request.env["rack.request.query_hash"][k] = request.env[validator_option.params_key][k]
+          request.env["rack.request.query_hash"][k] = request_params[k]
         end
       end
     end

lib/committee/schema_validator/open_api_3/operation_wrapper.rb

@@ -107,7 +107,14 @@ def build_openapi_parser_get_option(validator_option)
 
         def validate_get_request_params(params, headers, validator_option)
           # bad performance because when we coerce value, same check
-          request_operation.validate_request_parameter(params, headers, build_openapi_parser_get_option(validator_option))
+          request_params = (params['path'] || {}).merge(params['query'] || {}).merge(params['body'] || {}).merge(params['form_data'] || {})
+          result = request_operation.validate_request_parameter(request_params, headers, build_openapi_parser_get_option(validator_option))
+          # Copy coerced params
+          params['path'] = (params['path'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['query'] = (params['query'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['body'] = (params['body'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['form_data'] = (params['form_data'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          result
         rescue OpenAPIParser::OpenAPIError => e
           raise Committee::InvalidRequest.new(e.message)
         end
@@ -117,8 +124,15 @@ def validate_post_request_params(params, headers, validator_option)
 
           # bad performance because when we coerce value, same check
           schema_validator_options = build_openapi_parser_post_option(validator_option)
-          request_operation.validate_request_parameter(params, headers, schema_validator_options)
-          request_operation.validate_request_body(content_type, params, schema_validator_options)
+          request_params = (params['path'] || {}).merge(params['query'] || {}).merge(params['body'] || {}).merge(params['form_data'] || {})
+          request_operation.validate_request_parameter(request_params, headers, schema_validator_options)
+          result = request_operation.validate_request_body(content_type, params['body'], schema_validator_options)
+          # Copy coerced params
+          params['path'] = (params['path'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['query'] = (params['query'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['body'] = (params['body'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          params['form_data'] = (params['form_data'] || {}).keys.map { |k| [k, request_params[k]] }.to_h
+          result
         rescue => e
           raise Committee::InvalidRequest.new(e.message)
         end

test/data/openapi3/normal.yaml

@@ -544,6 +544,31 @@ paths:
         '204':
           description: no content
 
+  /request_body_and_path_params/{id}:
+    post:
+      description: request body
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              type: object
+              additionalProperties: false
+              required:
+              - data
+              properties:
+                data:
+                  type: string
+      parameters:
+      - name: id
+        in: path
+        required: true
+        schema:
+          type: string
+      responses:
+        '204':
+          description: no content
+
 components:
   schemas:
     nested_array:

test/middleware/request_validation_open_api_3_test.rb

@@ -44,6 +44,15 @@ def app
     assert_equal 200, last_response.status
   end
 
+  it "passes given a valid request body and path param" do
+    @app = new_rack_app(schema: open_api_3_schema)
+    params = { "data" => "abc" }
+    header "Content-Type", "application/json"
+    post "/request_body_and_path_params/abc", JSON.generate(params)
+
+    assert_equal 200, last_response.status
+  end
+
   it "passes given a valid parameter on GET endpoint with request body and allow_get_body=true" do
     params = { "data" => "abc" }
 
@@ -66,7 +75,7 @@ def app
     params = { "datetime_string" => "2016-04-01T16:00:00.000+09:00" }
 
     check_parameter = lambda { |env|
-      assert_equal DateTime, env['committee.params']["datetime_string"].class
+      assert_equal DateTime, env['committee.params']['body']["datetime_string"].class
       [200, {}, []]
     }
 
@@ -108,7 +117,7 @@ def app
     }
 
     check_parameter = lambda { |env|
-      nested_array = env['committee.params']["nested_array"]
+      nested_array = env['committee.params']['body']["nested_array"]
       first_data = nested_array[0]
       assert_kind_of DateTime, first_data["update_time"]
 
@@ -220,7 +229,7 @@ def app
     }
 
     check_parameter = lambda { |env|
-      hash = env['committee.params']
+      hash = env['committee.params']['body']
       array = hash['nested_array']
 
       assert_equal DateTime, array.first['update_time'].class
@@ -388,7 +397,7 @@ def app
 
   it "coerce string to integer" do
     check_parameter_string = lambda { |env|
-      assert env['committee.params']['integer'].is_a?(Integer)
+      assert env['committee.params']['path']['integer'].is_a?(Integer)
       [200, {}, []]
     }
 

test/request_unpacker_test.rb

@@ -10,7 +10,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request).call
-    assert_equal({ "x" => "y" }, params)
+    assert_equal({ "x" => "y" }, params["body"])
   end
 
   it "unpacks JSON on no Content-Type" do
@@ -19,7 +19,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request).call
-    assert_equal({ "x" => "y" }, params)
+    assert_equal({ "x" => "y" }, params["body"])
   end
 
   it "doesn't unpack JSON under other Content-Types" do
@@ -30,7 +30,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request).call
-      assert_equal({}, params)
+      assert_equal({}, params["body"])
     end
   end
 
@@ -42,7 +42,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request, optimistic_json: true).call
-      assert_equal({ "x" => "y" }, params)
+      assert_equal({ "x" => "y" }, params["body"])
     end
   end
 
@@ -54,7 +54,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request, optimistic_json: true).call
-      assert_equal({}, params)
+      assert_equal(nil, params["body"])
     end
   end
 
@@ -65,7 +65,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request).call
-    assert_equal({}, params)
+    assert_nil params["body"]
   end
 
   it "doesn't unpack form params" do
@@ -76,7 +76,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request).call
-      assert_equal({}, params)
+      assert_equal({}, params["body"])
     end
   end
 
@@ -88,7 +88,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request, allow_form_params: true).call
-      assert_equal({ "x" => "y" }, params)
+      assert_equal({ "x" => "y" }, params["form_data"])
     end
   end
 
@@ -143,7 +143,7 @@
           schema_validator: validator,
           ).call
       # openapi3 not support coerce in request unpacker
-      assert_equal({ "limit" => '20' }, params)
+      assert_equal({ "limit" => '20' }, params["form_data"])
     end
   end
 
@@ -167,7 +167,7 @@
           coerce_form_params: true,
           schema_validator: validator,
           ).call
-      assert_equal({ "limit" => "twenty" }, params)
+      assert_equal({ "limit" => "twenty" }, params["form_data"])
     end
   end
 
@@ -180,7 +180,7 @@
       }
       request = Rack::Request.new(env)
       params, _ = Committee::RequestUnpacker.new(request, allow_form_params: true, allow_query_params: true).call
-      assert_equal({ "x" => "y", "a" => "b" }, params)
+      assert_equal({"body" => {}, "form_data" => {"x" => "y"}, "query" => {"a"=>"b"}}, params)
     end
   end
 
@@ -191,7 +191,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request, allow_query_params: true).call
-    assert_equal({ "a" => "b" }, params)
+    assert_equal({ "a" => "b" }, params["query"])
   end
 
   it "errors if JSON is not an object" do
@@ -212,7 +212,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request).call
-    assert_equal({}, params)
+    assert_equal({"body" => {}, "form_data" => {}, "query" => {}}, params)
   end
 
   # this is mostly here for line coverage
@@ -222,7 +222,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request).call
-    assert_equal({ "x" => [] }, params)
+    assert_equal({ "x" => [] }, params["body"])
   end
 
   it "unpacks http header" do
@@ -243,7 +243,7 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request, { allow_query_params: true, allow_get_body: true }).call
-    assert_equal({ 'data' => 'value', 'x' => 1, 'y' => 2 }, params)
+    assert_equal({"body" => {"x" => 1, "y" => 2}, "form_data" => {}, "query" => {"data" => "value", "x" => "aaa"}}, params)
   end
 
   it "doesn't include request body when `use_get_body` is false" do
@@ -254,6 +254,6 @@
     }
     request = Rack::Request.new(env)
     params, _ = Committee::RequestUnpacker.new(request, { allow_query_params: true, use_get_body: false }).call
-    assert_equal({ 'data' => 'value', 'x' => 'aaa' }, params)
+    assert_equal({ 'data' => 'value', 'x' => 'aaa' }, params["query"])
   end
 end