From e92bd1cb9e31f38608a736f0804cc44256a8ba7d Mon Sep 17 00:00:00 2001 From: pgaertig Date: Mon, 11 Aug 2014 23:12:24 +0200 Subject: [PATCH 1/5] Respect Content-Type instead of Accept header in consume! --- lib/roar/rails/controller_additions.rb | 23 +++++++++- lib/roar/rails/railtie.rb | 3 ++ test/consume_test.rb | 61 +++++++++++++++++++++++++- test/responder_test.rb | 4 ++ 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/lib/roar/rails/controller_additions.rb b/lib/roar/rails/controller_additions.rb index 4dc146d..c2fba7d 100644 --- a/lib/roar/rails/controller_additions.rb +++ b/lib/roar/rails/controller_additions.rb @@ -23,11 +23,30 @@ def represents(format, options) end + class UnsupportedMediaType < StandardError #:nodoc: + end + + def consume!(model, options={}) - format = formats.first # FIXME: i expected request.content_mime_type to do the job. copied from responder.rb. this will return the wrong format when the controller responds to :json and :xml and the Content-type is :xml (?) + content_type = request.content_type + if content_type.nil? + raise UnsupportedMediaType.new("Cannot consume input without content type") + end + + format = Mime::Type.lookup(content_type).try(:symbol) + + unless format + raise UnsupportedMediaType.new("Cannot consume unregistered media type '#{content_type}'") + end + + parsing_method = compute_parsing_method(format) representer = prepare_model_for(format, model, options) - representer.send(compute_parsing_method(format), incoming_string, options) # e.g. from_json("...") + if parsing_method && !representer.respond_to?(parsing_method) + raise UnsupportedMediaType.new("Cannot consume unsupported media type '#{content_type}'") + end + + representer.send(parsing_method, incoming_string, options) # e.g. from_json("...") model end diff --git a/lib/roar/rails/railtie.rb b/lib/roar/rails/railtie.rb index 43652a4..1553a86 100644 --- a/lib/roar/rails/railtie.rb +++ b/lib/roar/rails/railtie.rb @@ -5,6 +5,9 @@ module Roar module Rails class Railtie < ::Rails::Railtie config.representer = ActiveSupport::OrderedOptions.new + config.action_dispatch.rescue_responses.merge!( + 'Roar::Rails::ControllerAdditions::UnsupportedMediaType' => :unsupported_media_type + ) initializer "roar.set_configs" do |app| ::Roar::Representer.module_eval do diff --git a/test/consume_test.rb b/test/consume_test.rb index 585718f..be07952 100644 --- a/test/consume_test.rb +++ b/test/consume_test.rb @@ -16,11 +16,25 @@ class ConsumeTest < ActionController::TestCase tests UnnamespaceSingersController test "#consume parses incoming document and updates the model" do - post :consume_json, "{\"name\": \"Bumi\"}", :format => 'json' + @request.headers['Content-Type'] = 'application/json' + post :consume_json, "{\"name\": \"Bumi\"}" assert_equal %{#}, @response.body end end +class ConsumeHalWithNoHalRespondTest < ActionController::TestCase + include Roar::Rails::TestCase + + tests UnnamespaceSingersController + + test "#consume parses hal document and updates the model" do + @request.headers['Content-Type'] = 'application/json+hal' + assert_raises Roar::Rails::ControllerAdditions::UnsupportedMediaType do + post :consume_json, "{\"name\": \"Bumi\"}" + end + end +end + class ConsumeWithConfigurationTest < ActionController::TestCase include Roar::Rails::TestCase @@ -44,9 +58,52 @@ def consume_json tests SingersController test "#consume uses ConsumeWithConfigurationTest::MusicianRepresenter to parse incoming document" do + @request.headers['Content-Type'] = 'application/json' post :consume_json, %{{"called":"Bumi"}}, :format => :json assert_equal %{#}, @response.body end + + test "#do not consume missing content type" do + assert_raises Roar::Rails::ControllerAdditions::UnsupportedMediaType do + post :consume_json, "{\"name\": \"Bumi\"}" + end + end + + + test "#do not consume parses unknown content type" do + @request.headers['Content-Type'] = 'application/custom+json' + assert_raises Roar::Rails::ControllerAdditions::UnsupportedMediaType do + post :consume_json, "{\"name\": \"Bumi\"}" + end + end +end + +class ConsumeHalTest < ActionController::TestCase + include Roar::Rails::TestCase + + module MusicianRepresenter + include Roar::Representer::JSON::HAL + property :name + end + + + class SingersController < ActionController::Base + include Roar::Rails::ControllerAdditions + represents :hal, :entity => MusicianRepresenter + + def consume_hal + singer = consume!(Singer.new) + render :text => singer.inspect + end + end + + tests SingersController + + test "#consume parses HAL document and updates the model" do + @request.headers['Content-Type'] = 'application/json+hal' + post :consume_hal, "{\"name\": \"Bumi\"}" + assert_equal %{#}, @response.body + end end class ConsumeWithOptionsOverridingConfigurationTest < ActionController::TestCase @@ -66,6 +123,7 @@ def consume_json tests SingersController test "#consume uses #represents config to parse incoming document" do + @request.headers['Content-Type'] = 'application/json' post :consume_json, %{{"called":"Bumi"}}, :format => :json assert_equal %{#}, @response.body end @@ -73,6 +131,7 @@ def consume_json class RequestBodyStringTest < ConsumeTest test "#read rewinds before reading" do + @request.headers['Content-Type'] = 'application/json' @request.instance_eval do def body incoming = super diff --git a/test/responder_test.rb b/test/responder_test.rb index 079e6fa..e5f30ec 100644 --- a/test/responder_test.rb +++ b/test/responder_test.rb @@ -282,6 +282,8 @@ class MusicianController < BaseController test "parsing uses decorating representer" do # FIXME: move to controller_test. created_singer = nil + @request.headers['Content-Type'] = 'application/json' + put singer.to_json do created_singer = consume!(Singer.new) respond_with created_singer @@ -325,6 +327,8 @@ class MusicianController < BaseController test "passes options in #consume!" do created_singer = nil + @request.headers['Content-Type'] = 'application/json' + put singer.to_json do created_singer = consume!(Singer.new, :title => "Mr.") respond_with created_singer From 3a2b6a74c5a33b08847944fcf7978697f214a5cf Mon Sep 17 00:00:00 2001 From: pgaertig Date: Fri, 29 Aug 2014 00:18:55 +0200 Subject: [PATCH 2/5] Fixed tests and updated docs. --- README.markdown | 14 ++++++++++---- lib/roar/rails/controller_additions.rb | 2 +- test/consume_test.rb | 14 +++++++------- test/responder_test.rb | 4 ++-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/README.markdown b/README.markdown index da2f4d7..95dafd7 100644 --- a/README.markdown +++ b/README.markdown @@ -129,6 +129,7 @@ end ## Parsing incoming documents In `#create` and `#update` actions it is often necessary to parse the incoming representation and map it to a model instance. Use the `#consume!` method for this. +The client must provide `Content-Type` header with proper MIME type to let `#consume!` know what kind of ```ruby class SingersController < ApplicationController @@ -143,17 +144,22 @@ class SingersController < ApplicationController end ``` -The `consume!` call will roughly do the following. +If content type is set to `application/xml` the `consume!` call will roughly do the following. ```ruby singer. extend(SingerRepresenter) - from_json(request.body) + from_xml(request.body) ``` -So, `#consume!` helps you figuring out the representer module and reading the incoming document. +So, `#consume!` helps you figuring out the representer module and reading the incoming document. It also chooses +parser according to content type header provided in request. -Note that it respects settings from `#represents`. It uses the same mechanics known from `#respond_with` to choose a representer. +It is important to provide known content type in request. If content type is missing or not supported by the responder then +`#consume!` will raise an exception `Roar::Rails::ControllerAdditions::UnsupportedMediaType`. Unless you rescue the exception +the action will stop and respond with HTTP status `406 Unsupported Media Type`. + +Note that `#consume!` respects settings from `#represents`. It uses the same mechanics known from `#respond_with` to choose a representer. ```ruby consume!(singer, :represent_with => MusicianRepresenter) diff --git a/lib/roar/rails/controller_additions.rb b/lib/roar/rails/controller_additions.rb index c2fba7d..123ee7c 100644 --- a/lib/roar/rails/controller_additions.rb +++ b/lib/roar/rails/controller_additions.rb @@ -30,7 +30,7 @@ class UnsupportedMediaType < StandardError #:nodoc: def consume!(model, options={}) content_type = request.content_type if content_type.nil? - raise UnsupportedMediaType.new("Cannot consume input without content type") + raise UnsupportedMediaType.new("Cannot consume input without content type.#{request.inspect}") end format = Mime::Type.lookup(content_type).try(:symbol) diff --git a/test/consume_test.rb b/test/consume_test.rb index be07952..31345fb 100644 --- a/test/consume_test.rb +++ b/test/consume_test.rb @@ -16,7 +16,7 @@ class ConsumeTest < ActionController::TestCase tests UnnamespaceSingersController test "#consume parses incoming document and updates the model" do - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' post :consume_json, "{\"name\": \"Bumi\"}" assert_equal %{#}, @response.body end @@ -28,7 +28,7 @@ class ConsumeHalWithNoHalRespondTest < ActionController::TestCase tests UnnamespaceSingersController test "#consume parses hal document and updates the model" do - @request.headers['Content-Type'] = 'application/json+hal' + @request.env['CONTENT_TYPE'] = 'application/json+hal' assert_raises Roar::Rails::ControllerAdditions::UnsupportedMediaType do post :consume_json, "{\"name\": \"Bumi\"}" end @@ -58,7 +58,7 @@ def consume_json tests SingersController test "#consume uses ConsumeWithConfigurationTest::MusicianRepresenter to parse incoming document" do - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' post :consume_json, %{{"called":"Bumi"}}, :format => :json assert_equal %{#}, @response.body end @@ -71,7 +71,7 @@ def consume_json test "#do not consume parses unknown content type" do - @request.headers['Content-Type'] = 'application/custom+json' + @request.env['CONTENT_TYPE'] = 'application/custom+json' assert_raises Roar::Rails::ControllerAdditions::UnsupportedMediaType do post :consume_json, "{\"name\": \"Bumi\"}" end @@ -100,7 +100,7 @@ def consume_hal tests SingersController test "#consume parses HAL document and updates the model" do - @request.headers['Content-Type'] = 'application/json+hal' + @request.env['CONTENT_TYPE'] = 'application/json+hal' post :consume_hal, "{\"name\": \"Bumi\"}" assert_equal %{#}, @response.body end @@ -123,7 +123,7 @@ def consume_json tests SingersController test "#consume uses #represents config to parse incoming document" do - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' post :consume_json, %{{"called":"Bumi"}}, :format => :json assert_equal %{#}, @response.body end @@ -131,7 +131,7 @@ def consume_json class RequestBodyStringTest < ConsumeTest test "#read rewinds before reading" do - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' @request.instance_eval do def body incoming = super diff --git a/test/responder_test.rb b/test/responder_test.rb index e5f30ec..c5a4505 100644 --- a/test/responder_test.rb +++ b/test/responder_test.rb @@ -282,7 +282,7 @@ class MusicianController < BaseController test "parsing uses decorating representer" do # FIXME: move to controller_test. created_singer = nil - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' put singer.to_json do created_singer = consume!(Singer.new) @@ -327,7 +327,7 @@ class MusicianController < BaseController test "passes options in #consume!" do created_singer = nil - @request.headers['Content-Type'] = 'application/json' + @request.env['CONTENT_TYPE'] = 'application/json' put singer.to_json do created_singer = consume!(Singer.new, :title => "Mr.") From c9a12fa18753b8eee5bf3118902134447d6bc25e Mon Sep 17 00:00:00 2001 From: pgaertig Date: Fri, 29 Aug 2014 00:36:39 +0200 Subject: [PATCH 3/5] Rails 3.0 rescue response compatibility fix --- lib/roar/rails/railtie.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/roar/rails/railtie.rb b/lib/roar/rails/railtie.rb index 1553a86..b3cf116 100644 --- a/lib/roar/rails/railtie.rb +++ b/lib/roar/rails/railtie.rb @@ -5,7 +5,9 @@ module Roar module Rails class Railtie < ::Rails::Railtie config.representer = ActiveSupport::OrderedOptions.new - config.action_dispatch.rescue_responses.merge!( + + rescue_responses = config.action_dispatch.rescue_responses || ActionDispatch::ShowExceptions.rescue_responses #newer or fallback to 3.0 + rescue_responses.merge!( 'Roar::Rails::ControllerAdditions::UnsupportedMediaType' => :unsupported_media_type ) From cbb213201d79f7b297b03870ad2060733b376105 Mon Sep 17 00:00:00 2001 From: pgaertig Date: Fri, 29 Aug 2014 00:39:58 +0200 Subject: [PATCH 4/5] Docs typo --- README.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.markdown b/README.markdown index 95dafd7..a67bab5 100644 --- a/README.markdown +++ b/README.markdown @@ -129,7 +129,7 @@ end ## Parsing incoming documents In `#create` and `#update` actions it is often necessary to parse the incoming representation and map it to a model instance. Use the `#consume!` method for this. -The client must provide `Content-Type` header with proper MIME type to let `#consume!` know what kind of +The client must provide `Content-Type` request header with proper MIME type to let `#consume!` know what kind of parser it should use. ```ruby class SingersController < ApplicationController From 81b0de4f92d9b8d3428534486ba9bc9852a01d8d Mon Sep 17 00:00:00 2001 From: pgaertig Date: Fri, 29 Aug 2014 00:45:52 +0200 Subject: [PATCH 5/5] Removed debug stuff. --- lib/roar/rails/controller_additions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/roar/rails/controller_additions.rb b/lib/roar/rails/controller_additions.rb index 123ee7c..bf4e30d 100644 --- a/lib/roar/rails/controller_additions.rb +++ b/lib/roar/rails/controller_additions.rb @@ -30,7 +30,7 @@ class UnsupportedMediaType < StandardError #:nodoc: def consume!(model, options={}) content_type = request.content_type if content_type.nil? - raise UnsupportedMediaType.new("Cannot consume input without content type.#{request.inspect}") + raise UnsupportedMediaType.new("Cannot consume input without content type.") end format = Mime::Type.lookup(content_type).try(:symbol)