request unpacker refactoring

lib/committee/drivers/open_api_2/driver.rb

@@ -156,7 +156,6 @@ def parse_routes!(data, schema, store)
 
             methods.each do |method, link_data|
               method = method.upcase
-
               link = Link.new
               link.enc_type = schema.consumes
               link.href = href

lib/committee/request_unpacker.rb

@@ -27,61 +27,66 @@ def indifferent_params(object)
       end
     end
 
-    def initialize(request, options={})
-      @request = request
-
+    def initialize(options={})
       @allow_form_params  = options[:allow_form_params]
       @allow_get_body     = options[:allow_get_body]
       @allow_query_params = options[:allow_query_params]
       @coerce_form_params = options[:coerce_form_params]
       @optimistic_json    = options[:optimistic_json]
-      @schema_validator   = options[:schema_validator]
+      @schema_validator   = options[:schema_validator] # TODO: remove
     end
 
-    def call
+    # reutrn params and is_form_params
+    def unpack_request_params(request)
       # if Content-Type is empty or JSON, and there was a request body, try to
       # interpret it as JSON
-      params = if !@request.media_type || @request.media_type =~ %r{application/(?:.*\+)?json}
-        parse_json
+      params = if !request.media_type || request.media_type =~ %r{application/(?:.*\+)?json}
+        parse_json(request)
       elsif @optimistic_json
         begin
-          parse_json
+          parse_json(request)
         rescue JSON::ParserError
           nil
         end
       end
 
-      params = if params
-        params
-      elsif @allow_form_params && %w[application/x-www-form-urlencoded multipart/form-data].include?(@request.media_type)
+      return [params, false] if params
+
+      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
+        return [request.POST, true] if request.POST
+      end
 
-        @schema_validator.coerce_form_params(p) if @coerce_form_params
+      [{}, false]
+    end
 
-        p
-      else
-        {}
-      end
+    def unpack_query_params(request)
+      @allow_query_params ? self.class.indifferent_params(request.GET) : {}
+    end
 
-      if @allow_query_params
-        [self.class.indifferent_params(@request.GET).merge(params), headers]
-      else
-        [params, headers]
+    def unpack_headers(request)
+      env = request.env
+      base = env.keys.grep(/HTTP_/).inject({}) do |headers, key|
+        headerized_key = key.gsub(/^HTTP_/, '').gsub(/_/, '-')
+        headers[headerized_key] = env[key]
+        headers
       end
+
+      base['Content-Type'] = env['CONTENT_TYPE'] if env['CONTENT_TYPE']
+      base
     end
 
     private
 
-    def parse_json
-      return nil if @request.request_method == "GET" && !@allow_get_body
+    def parse_json(request)
+      return nil if request.request_method == "GET" && !@allow_get_body
 
-      body = @request.body.read
+      body = request.body.read
       # if request body is empty, we just have empty params
       return nil if body.length == 0
 
-      @request.body.rewind
+      request.body.rewind
       hash = JSON.parse(body)
       # We want a hash specifically. '42', 42, and [42] will all be
       # decoded properly, but we can't use them here.
@@ -91,17 +96,5 @@ def parse_json
       end
       self.class.indifferent_params(hash)
     end
-
-    def headers
-      env = @request.env
-      base = env.keys.grep(/HTTP_/).inject({}) do |headers, key|
-        headerized_key = key.gsub(/^HTTP_/, '').gsub(/_/, '-')
-        headers[headerized_key] = env[key]
-        headers
-      end
-
-      base['Content-Type'] = env['CONTENT_TYPE'] if env['CONTENT_TYPE']
-      base
-    end
   end
 end

lib/committee/schema_validator/hyper_schema.rb

@@ -50,6 +50,7 @@ def link_exist?
         !link.nil?
       end
 
+      # TODO: move private
       def coerce_form_params(parameter)
         return unless link_exist?
         return unless link.schema
@@ -73,15 +74,22 @@ def coerce_query_params(request)
         end
 
         def request_unpack(request)
-          request.env[validator_option.params_key], request.env[validator_option.headers_key] = Committee::RequestUnpacker.new(
-              request,
-              allow_form_params:  validator_option.allow_form_params,
-              allow_get_body:     validator_option.allow_get_body,
-              allow_query_params: validator_option.allow_query_params,
-              coerce_form_params: validator_option.coerce_form_params,
-              optimistic_json:    validator_option.optimistic_json,
-              schema_validator:   self
-          ).call
+          unpacker = Committee::RequestUnpacker.new(
+            allow_form_params:  validator_option.allow_form_params,
+            allow_get_body:     validator_option.allow_get_body,
+            allow_query_params: validator_option.allow_query_params,
+            coerce_form_params: validator_option.coerce_form_params,
+            optimistic_json:    validator_option.optimistic_json,
+            schema_validator:   self
+          )
+
+          query_param = unpacker.unpack_query_params(request)
+          request_param, is_form_params = unpacker.unpack_request_params(request)
+
+          coerce_form_params(request_param) if validator_option.coerce_form_params && is_form_params
+          request.env[validator_option.params_key] = query_param.merge(request_param)
+
+          request.env[validator_option.headers_key] = unpacker.unpack_headers(request)
         end
 
         def request_schema_validation(request)

lib/committee/schema_validator/open_api_3.rb

@@ -74,15 +74,20 @@ def header(request)
       end
 
       def request_unpack(request)
-        request.env[validator_option.params_key], request.env[validator_option.headers_key] = Committee::RequestUnpacker.new(
-            request,
-            allow_form_params:  validator_option.allow_form_params,
-            allow_get_body:     validator_option.allow_get_body,
-            allow_query_params: validator_option.allow_query_params,
-            coerce_form_params: validator_option.coerce_form_params,
-            optimistic_json:    validator_option.optimistic_json,
-            schema_validator:   self
-        ).call
+        unpacker = Committee::RequestUnpacker.new(
+          allow_form_params:  validator_option.allow_form_params,
+          allow_get_body:     validator_option.allow_get_body,
+          allow_query_params: validator_option.allow_query_params,
+          coerce_form_params: validator_option.coerce_form_params,
+          optimistic_json:    validator_option.optimistic_json,
+          schema_validator:   self
+        )
+
+        query_param = unpacker.unpack_query_params(request)
+        request_param, is_form_params = unpacker.unpack_request_params(request)
+        request.env[validator_option.params_key] = query_param.merge(request_param)
+
+        request.env[validator_option.headers_key] = unpacker.unpack_headers(request)
       end
 
       def copy_coerced_data_to_query_hash(request)

test/data/openapi2/petstore-expanded-form.json

@@ -0,0 +1,116 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "version": "1.0.0",
+    "title": "Swagger Petstore",
+    "description": "A sample API that uses a petstore as an example to demonstrate features in the swagger-2.0 specification",
+    "termsOfService": "http://swagger.io/terms/",
+    "contact": {
+      "name": "Swagger API Team",
+      "email": "[email protected]",
+      "url": "http://madskristensen.net"
+    },
+    "license": {
+      "name": "MIT",
+      "url": "http://github.com/gruntjs/grunt/blob/master/LICENSE-MIT"
+    }
+  },
+  "host": "petstore.swagger.io",
+  "basePath": "/api",
+  "schemes": [
+    "http"
+  ],
+  "consumes": [
+    "application/x-www-form-urlencoded"
+  ],
+  "produces": [
+    "application/x-www-form-urlencoded"
+  ],
+  "paths": {
+    "/pets": {
+      "post": {
+        "description": "Creates a new pet in the store.  Duplicates are allowed",
+        "operationId": "addPet",
+        "parameters": [
+          {
+            "name": "pet",
+            "in": "body",
+            "description": "Pet to add to the store",
+            "required": true,
+            "schema": {
+              "$ref": "#/definitions/NewPet"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "pet response",
+            "schema": {
+              "$ref": "#/definitions/Pet"
+            }
+          },
+          "default": {
+            "description": "unexpected error",
+            "schema": {
+              "$ref": "#/definitions/Error"
+            }
+          }
+        }
+      }
+    }
+  },
+  "definitions": {
+    "Pet": {
+      "type": "object",
+      "allOf": [
+        {
+          "$ref": "#/definitions/NewPet"
+        },
+        {
+          "required": [
+            "id"
+          ],
+          "properties": {
+            "id": {
+              "type": "integer",
+              "format": "int64"
+            }
+          }
+        }
+      ]
+    },
+    "NewPet": {
+      "type": "object",
+      "required": [
+        "name"
+      ],
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "tag": {
+          "type": "string"
+        },
+        "age": {
+          "type": "integer"
+        }
+      }
+    },
+    "Error": {
+      "type": "object",
+      "required": [
+        "code",
+        "message"
+      ],
+      "properties": {
+        "code": {
+          "type": "integer",
+          "format": "int32"
+        },
+        "message": {
+          "type": "string"
+        }
+      }
+    }
+  }
+}

test/data/openapi3/normal.yaml

@@ -313,7 +313,12 @@ paths:
                   enum:
                   - 1.0
                   - 2.1
-
+          application/x-www-form-urlencoded:
+            schema:
+              type: object
+              properties:
+                integer:
+                  type: integer
       responses:
         '200':
           description: success

test/middleware/request_validation_open_api_3_test.rb

@@ -426,6 +426,21 @@ def app
     assert_equal 204, last_response.status
   end
 
+  it "unpacker test" do
+    @app = new_rack_app_with_lambda(lambda do |env|
+      assert_equal env['committee.params']['integer'], 42
+      assert_equal env['committee.params'][:integer], 42
+      # overwrite by request body...
+      assert_equal env['rack.request.query_hash']['integer'], 42
+      # assert_equal env['rack.request.query_hash'][:integer], 42
+      [204, {}, []]
+    end, schema: open_api_3_schema, raise: true)
+
+    header "Content-Type", "application/x-www-form-urlencoded"
+    post '/validate?integer=21', "integer=42"
+    assert_equal 204, last_response.status
+  end
+
   it "OpenAPI3 raise not support method" do
     @app = new_rack_app(schema: open_api_3_schema)
 

test/middleware/request_validation_test.rb

@@ -435,6 +435,18 @@ def app
     assert_equal 200, last_response.status
   end
 
+  it "aacorce form params" do
+    check_parameter = lambda { |env|
+      assert_equal 3, env['committee.params']['age']
+      [200, {}, []]
+    }
+
+    @app = new_rack_app_with_lambda(check_parameter, schema: open_api_2_form_schema, raise: true, allow_form_params: true, coerce_form_params: true)
+    header "Content-Type", "application/x-www-form-urlencoded"
+    post "/api/pets", "age=3&name=ab"
+    assert_equal 200, last_response.status
+  end
+
   it "detects an invalid request for OpenAPI" do
     @app = new_rack_app(schema: open_api_2_schema)
     get "/api/pets?limit=foo", nil, { "HTTP_AUTH_TOKEN" => "xxx" }