From 1958fadb5133e7b8f9dd1487ad375352e3d9400d Mon Sep 17 00:00:00 2001 From: Ted Kulp Date: Sun, 5 Jun 2011 11:54:50 -0400 Subject: [PATCH 1/3] Ability to handle incoming JSON in the body This allows Grape to work with ActiveResource in JSON mode. If the body contains JSON, it's contents will be added to the params hash just like if they came over the GET query string or as regular params. --- lib/grape/middleware/formatter.rb | 40 +++++++++++++++++++++++-- spec/grape/middleware/formatter_spec.rb | 12 ++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 014072434..90ab6a72e 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -15,12 +15,16 @@ class Formatter < Base :json => :encode_json, :txt => :encode_txt, } + PARSERS = { + :json => :decode_json + } def default_options { :default_format => :txt, :formatters => {}, - :content_types => {} + :content_types => {}, + :parsers => {} } end @@ -31,6 +35,10 @@ def content_types def formatters FORMATTERS.merge(options[:formatters]) end + + def parsers + PARSERS.merge(options[:parsers]) + end def mime_types content_types.invert @@ -44,7 +52,19 @@ def before fmt = format_from_extension || format_from_header || options[:default_format] if content_types.key?(fmt) - env['api.format'] = fmt + if !env['rack.input'].nil? and (body = env['rack.input'].read).length != 0 + parser = parser_for fmt + unless parser.nil? + begin + env['rack.request.form_hash'] = !env['rack.request.form_hash'].nil? ? env['rack.request.form_hash'].merge(parser.call(body)) : parser.call(body) + env['rack.request.form_input'] = env['rack.input'] + rescue + throw :error, :status => 400, :message => 'Body content could not be parsed.' + end + end + env['rack.input'].rewind + end + env['api.format'] = fmt else throw :error, :status => 406, :message => 'The requested format is not supported.' end @@ -103,6 +123,22 @@ def formatter_for(api_format) spec end end + + def parser_for(api_format) + spec = parsers[api_format] + case spec + when nil + nil + when Symbol + method(spec) + else + spec + end + end + + def decode_json(object) + MultiJson.decode(object) + end def encode_json(object) if object.respond_to? :serializable_hash diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 55c25f0e6..c5ca66271 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -123,4 +123,16 @@ def serializable_hash body.body.should == ['CUSTOM JSON FORMAT'] end end + + context 'Input' do + it 'should parse the body from a POST/PUT and put the contents into rack.request.form_hash' do + subject.call({'PATH_INFO' => '/info', 'Accept' => 'application/json', 'rack.input' => StringIO.new('{"is_boolean":true,"string":"thing"}')}) + subject.env['rack.request.form_hash']['is_boolean'].should be_true + subject.env['rack.request.form_hash']['string'].should == 'thing' + end + it 'should be able to fail gracefully if the body is invalid' do + err = catch(:error){ subject.call({'PATH_INFO' => '/info', 'Accept' => 'application/json', 'rack.input' => StringIO.new('{"is_boolean":true,string:"thing"}')}) } + err.should == {:status => 400, :message => "Body content could not be parsed."} + end + end end From 11f71639c541094d8c9cfd96f7605642075b8e8e Mon Sep 17 00:00:00 2001 From: Ted Kulp Date: Sun, 5 Jun 2011 13:25:42 -0400 Subject: [PATCH 2/3] Now it doesn't throw a 400 error Didn't account for regular POST content being in the body on some requests, even if JSON is expected as the result. It now fails gracefully. --- lib/grape/middleware/formatter.rb | 7 ++++--- spec/grape/middleware/formatter_spec.rb | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 90ab6a72e..96ab52e10 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -52,14 +52,15 @@ def before fmt = format_from_extension || format_from_header || options[:default_format] if content_types.key?(fmt) - if !env['rack.input'].nil? and (body = env['rack.input'].read).length != 0 + if !env['rack.input'].nil? and (body = env['rack.input'].read).strip.length != 0 parser = parser_for fmt unless parser.nil? begin - env['rack.request.form_hash'] = !env['rack.request.form_hash'].nil? ? env['rack.request.form_hash'].merge(parser.call(body)) : parser.call(body) + body = parser.call(body) + env['rack.request.form_hash'] = !env['rack.request.form_hash'].nil? ? env['rack.request.form_hash'].merge(body) : body env['rack.request.form_input'] = env['rack.input'] rescue - throw :error, :status => 400, :message => 'Body content could not be parsed.' + # It's possible that it's just regular POST content -- just back off end end env['rack.input'].rewind diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index c5ca66271..d8c676283 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -130,9 +130,9 @@ def serializable_hash subject.env['rack.request.form_hash']['is_boolean'].should be_true subject.env['rack.request.form_hash']['string'].should == 'thing' end - it 'should be able to fail gracefully if the body is invalid' do - err = catch(:error){ subject.call({'PATH_INFO' => '/info', 'Accept' => 'application/json', 'rack.input' => StringIO.new('{"is_boolean":true,string:"thing"}')}) } - err.should == {:status => 400, :message => "Body content could not be parsed."} + it 'should be able to fail gracefully if the body is regular POST content' do + subject.call({'PATH_INFO' => '/info', 'Accept' => 'application/json', 'rack.input' => StringIO.new('name=Other+Test+Thing')}) + subject.env['rack.request.form_hash'].should be_nil end end end From 3def84a40f30ef9cca8b2c3c68ca5fcf6ea6c2bf Mon Sep 17 00:00:00 2001 From: Ted Kulp Date: Thu, 14 Jul 2011 23:51:55 -0400 Subject: [PATCH 3/3] Updated patch to test cleanly against master --- lib/grape/middleware/base.rb | 38 +++++++++++++++++++++++++++++++ lib/grape/middleware/formatter.rb | 15 ------------ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 60fbbcbf6..089878d27 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -1,3 +1,5 @@ +require 'multi_json' + module Grape module Middleware class Base @@ -53,11 +55,18 @@ module Formats :json => :encode_json, :txt => :encode_txt, } + PARSERS = { + :json => :decode_json + } def formatters FORMATTERS.merge(options[:formatters] || {}) end + def parsers + PARSERS.merge(options[:parsers] || {}) + end + def content_types CONTENT_TYPES.merge(options[:content_types] || {}) end @@ -82,6 +91,35 @@ def formatter_for(api_format) end end + def parser_for(api_format) + spec = parsers[api_format] + case spec + when nil + nil + when Symbol + method(spec) + else + spec + end + end + + def decode_json(object) + MultiJson.decode(object) + end + + def encode_json(object) + if object.respond_to? :serializable_hash + MultiJson.encode(object.serializable_hash) + elsif object.respond_to? :to_json + object.to_json + else + MultiJson.encode(object) + end + end + + def encode_txt(object) + object.respond_to?(:to_txt) ? object.to_txt : object.to_s + end end end diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 938f987e5..1c0c48470 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -1,5 +1,4 @@ require 'grape/middleware/base' -require 'multi_json' module Grape module Middleware @@ -83,20 +82,6 @@ def after headers['Content-Type'] = content_types[env['api.format']] Rack::Response.new(bodymap, status, headers).to_a end - - def encode_json(object) - if object.respond_to? :serializable_hash - MultiJson.encode(object.serializable_hash) - elsif object.respond_to? :to_json - object.to_json - else - MultiJson.encode(object) - end - end - - def encode_txt(object) - object.respond_to?(:to_txt) ? object.to_txt : object.to_s - end end end end