Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update for Rails 5 #12

Merged
merged 7 commits into from Sep 10, 2015
Merged

Update for Rails 5 #12

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
af7a449
Bump dependencies for ActionPack
eileencodes Aug 26, 2015
7efe0eb
Replace `deep_munge` with `normalize_encode_params`
eileencodes Aug 26, 2015
ca0d9ce
Make AC::Parameters not inherited from Hash
eileencodes Aug 26, 2015
81839fd
Fix deprecation warning about passing strings to middleware
eileencodes Aug 26, 2015
526ffaf
Fix deprecation warnings about keyword args
eileencodes Aug 26, 2015
75fab06
Fix deprecation warnings by switching `text` to `plain`
eileencodes Aug 26, 2015
8ea038d
Bump required Ruby version and update travis.yml
eileencodes Aug 26, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
View file
Open in desktop
@@ -1,3 +1,5 @@
source 'https://rubygems.org'

gemspec
gem 'actionpack', github: 'rails/rails'
gem 'rack', github: 'rack/rack'
2 changes: 1 addition & 1 deletion actionpack-xml_parser.gemspec
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ Gem::Specification.new do |s|
s.extra_rdoc_files = %w( README.md )
s.rdoc_options.concat ['--main', 'README.md']

s.add_dependency('actionpack', '>= 4.0.0', '< 5')
s.add_dependency('actionpack', '~> 5.x')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about bumping the gem version to 2.0.x

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You commented on a wrong line.

But sure. @eileencodes do you mind changing the version in another commit as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i could not comment in line 4. 馃樃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the heck .. NVM then. I wish they allow you to comment on any line.


s.add_development_dependency('rake')
end
1 change: 1 addition & 0 deletions gemfiles/Gemfile-edge
View file
Open in desktop
Expand Up @@ -3,3 +3,4 @@ source 'https://rubygems.org'
gemspec path: '..'

gem 'actionpack', github: 'rails/rails', branch: 'master'
gem 'rack', github: 'rack/rack', branch: 'master'
5 changes: 3 additions & 2 deletions lib/action_dispatch/xml_params_parser.rb
View file
Open in desktop
Expand Up @@ -26,10 +26,11 @@ def parse_formatted_parameters(env)
request.content_mime_type

if mime_type == Mime::XML
# Rails 4.1 moved #deep_munge out of the request and into ActionDispatch::Request::Utils
# Rails 5 removed #deep_munge and replaced it with #normalize_encode_params
munger = defined?(Request::Utils) ? Request::Utils : request
params = Hash.from_xml(request.body.read) || {}

data = munger.deep_munge(Hash.from_xml(request.body.read) || {})
data = munger.normalize_encode_params(params)
request.body.rewind if request.body.respond_to?(:rewind)
data.with_indifferent_access
else
Expand Down
8 changes: 4 additions & 4 deletions test/helper.rb
View file
Open in desktop
Expand Up @@ -35,10 +35,10 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase

def self.build_app(routes = nil)
RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware|
middleware.use "ActionDispatch::ShowExceptions", ActionDispatch::PublicExceptions.new(FIXTURE_LOAD_PATH)
middleware.use "ActionDispatch::ParamsParser"
middleware.use "ActionDispatch::XmlParamsParser"
middleware.use "Rack::Head"
middleware.use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new(FIXTURE_LOAD_PATH)
middleware.use ActionDispatch::ParamsParser
middleware.use ActionDispatch::XmlParamsParser
middleware.use Rack::Head
yield(middleware) if block_given?
end
end
Expand Down
58 changes: 32 additions & 26 deletions test/webservice_test.rb
View file
Open in desktop
Expand Up @@ -4,16 +4,22 @@ class WebServiceTest < ActionDispatch::IntegrationTest
class TestController < ActionController::Base
def assign_parameters
if params[:full]
render :text => dump_params_keys
render plain: dump_params_keys
else
render :text => (params.keys - ['controller', 'action']).sort.join(", ")
render plain: (params.keys - ['controller', 'action']).sort.join(", ")
end
end

def dump_params_keys(hash = params)
hash.keys.sort.inject("") do |s, k|
value = hash[k]
value = Hash === value ? "(#{dump_params_keys(value)})" : ""

if value.is_a?(Hash) || value.is_a?(ActionController::Parameters)
value = "(#{dump_params_keys(value)})"
else
value = ""
end

s << ", " unless s.empty?
s << "#{k}#{value}"
end
Expand All @@ -34,8 +40,8 @@ def test_check_parameters

def test_post_xml
with_test_route_set do
post "/", '<entry attributed="true"><summary>content...</summary></entry>',
{'CONTENT_TYPE' => 'application/xml'}
post "/", params: '<entry attributed="true"><summary>content...</summary></entry>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'entry', @controller.response.body
assert @controller.params.has_key?(:entry)
Expand All @@ -46,8 +52,8 @@ def test_post_xml

def test_put_xml
with_test_route_set do
put "/", '<entry attributed="true"><summary>content...</summary></entry>',
{'CONTENT_TYPE' => 'application/xml'}
put "/", params: '<entry attributed="true"><summary>content...</summary></entry>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'entry', @controller.response.body
assert @controller.params.has_key?(:entry)
Expand All @@ -58,8 +64,8 @@ def test_put_xml

def test_put_xml_using_a_type_node
with_test_route_set do
put "/", '<type attributed="true"><summary>content...</summary></type>',
{'CONTENT_TYPE' => 'application/xml'}
put "/", params: '<type attributed="true"><summary>content...</summary></type>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'type', @controller.response.body
assert @controller.params.has_key?(:type)
Expand All @@ -70,8 +76,8 @@ def test_put_xml_using_a_type_node

def test_put_xml_using_a_type_node_and_attribute
with_test_route_set do
put "/", '<type attributed="true"><summary type="boolean">false</summary></type>',
{'CONTENT_TYPE' => 'application/xml'}
put "/", params: '<type attributed="true"><summary type="boolean">false</summary></type>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'type', @controller.response.body
assert @controller.params.has_key?(:type)
Expand All @@ -82,8 +88,8 @@ def test_put_xml_using_a_type_node_and_attribute

def test_post_xml_using_a_type_node
with_test_route_set do
post "/", '<font attributed="true"><type>arial</type></font>',
{'CONTENT_TYPE' => 'application/xml'}
post "/", params: '<font attributed="true"><type>arial</type></font>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'font', @controller.response.body
assert @controller.params.has_key?(:font)
Expand All @@ -94,8 +100,8 @@ def test_post_xml_using_a_type_node

def test_post_xml_using_a_root_node_named_type
with_test_route_set do
post "/", '<type type="integer">33</type>',
{'CONTENT_TYPE' => 'application/xml'}
post "/", params: '<type type="integer">33</type>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert @controller.params.has_key?(:type)
assert_equal 33, @controller.params['type']
Expand All @@ -105,8 +111,8 @@ def test_post_xml_using_a_root_node_named_type
def test_post_xml_using_an_attributted_node_named_type
with_test_route_set do
with_params_parsers Mime::XML => Proc.new { |data| Hash.from_xml(data)['request'].with_indifferent_access } do
post "/", '<request><type type="string">Arial,12</type><z>3</z></request>',
{'CONTENT_TYPE' => 'application/xml'}
post "/", params: '<request><type type="string">Arial,12</type><z>3</z></request>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'type, z', @controller.response.body
assert @controller.params.has_key?(:type)
Expand All @@ -119,10 +125,10 @@ def test_post_xml_using_an_attributted_node_named_type
def test_post_xml_using_a_disallowed_type_attribute
$stderr = StringIO.new
with_test_route_set do
post '/', '<foo type="symbol">value</foo>', 'CONTENT_TYPE' => 'application/xml'
post '/', params: '<foo type="symbol">value</foo>', headers: {'CONTENT_TYPE' => 'application/xml'}
assert_response 400

post '/', '<foo type="yaml">value</foo>', 'CONTENT_TYPE' => 'application/xml'
post '/', params: '<foo type="yaml">value</foo>', headers: {'CONTENT_TYPE' => 'application/xml'}
assert_response 400
end
ensure
Expand All @@ -132,8 +138,8 @@ def test_post_xml_using_a_disallowed_type_attribute
def test_register_and_use_xml_simple
with_test_route_set do
with_params_parsers Mime::XML => Proc.new { |data| Hash.from_xml(data)['request'].with_indifferent_access } do
post "/", '<request><summary>content...</summary><title>SimpleXml</title></request>',
{'CONTENT_TYPE' => 'application/xml'}
post "/", params: '<request><summary>content...</summary><title>SimpleXml</title></request>',
headers: {'CONTENT_TYPE' => 'application/xml'}

assert_equal 'summary, title', @controller.response.body
assert @controller.params.has_key?(:summary)
Expand All @@ -146,15 +152,15 @@ def test_register_and_use_xml_simple

def test_use_xml_ximple_with_empty_request
with_test_route_set do
assert_nothing_raised { post "/", "", {'CONTENT_TYPE' => 'application/xml'} }
assert_nothing_raised { post "/", params: "", headers: {'CONTENT_TYPE' => 'application/xml'} }
assert_equal '', @controller.response.body
end
end

def test_dasherized_keys_as_xml
with_test_route_set do
post "/?full=1", "<first-key>\n<sub-key>...</sub-key>\n</first-key>",
{'CONTENT_TYPE' => 'application/xml'}
post "/?full=1", params: "<first-key>\n<sub-key>...</sub-key>\n</first-key>",
headers: {'CONTENT_TYPE' => 'application/xml'}
assert_equal 'action, controller, first_key(sub_key), full', @controller.response.body
assert_equal "...", @controller.params[:first_key][:sub_key]
end
Expand All @@ -175,7 +181,7 @@ def test_typecast_as_xml
<g type="date">1974-07-25</g>
</data>
XML
post "/", xml, {'CONTENT_TYPE' => 'application/xml'}
post "/", params: xml, headers: {'CONTENT_TYPE' => 'application/xml'}

params = @controller.params
assert_equal 15, params[:data][:a]
Expand All @@ -193,7 +199,7 @@ def test_entities_unescaped_as_xml_simple
xml = <<-XML
<data>&lt;foo &quot;bar&apos;s&quot; &amp; friends&gt;</data>
XML
post "/", xml, {'CONTENT_TYPE' => 'application/xml'}
post "/", params: xml, headers: {'CONTENT_TYPE' => 'application/xml'}
assert_equal %(<foo "bar's" & friends>), @controller.params[:data]
end
end
Expand Down
14 changes: 7 additions & 7 deletions test/xml_params_parsing_test.rb
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@ def call(env)

def assert_parses(expected, xml)
with_test_routing do
post "/parse", xml, default_headers
post "/parse", params: xml, headers: default_headers
assert_response :ok
assert_equal(expected, TestController.last_request_parameters)
end
Expand All @@ -62,7 +62,7 @@ def assert_parses(expected, xml)
test "parses hash params" do
with_test_routing do
xml = "<person><name>David</name></person>"
post "/parse", xml, default_headers
post "/parse", params: xml, headers: default_headers
assert_response :ok
assert_equal({"person" => {"name" => "David"}}, TestController.last_request_parameters)
end
Expand All @@ -71,7 +71,7 @@ def assert_parses(expected, xml)
test "parses single file" do
with_test_routing do
xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{::Base64.encode64('ABC')}</avatar></person>"
post "/parse", xml, default_headers
post "/parse", params: xml, headers: default_headers
assert_response :ok

person = TestController.last_request_parameters
Expand All @@ -85,7 +85,7 @@ def assert_parses(expected, xml)
with_test_routing do
output = StringIO.new
xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{::Base64.encode64('ABC')}</avatar></pineapple>"
post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => ActiveSupport::Logger.new(output))
post "/parse", params: xml, headers: default_headers.merge('action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => ActiveSupport::Logger.new(output))
assert_response :bad_request
output.rewind && err = output.read
assert err =~ /Error occurred while parsing request parameters/
Expand Down Expand Up @@ -118,7 +118,7 @@ def assert_parses(expected, xml)
end_body

with_test_routing do
post "/parse", xml, default_headers
post "/parse", params: xml, headers: default_headers
assert_response :ok
end

Expand All @@ -137,7 +137,7 @@ def assert_parses(expected, xml)
xml = "<person><name>Marie</name></person>"

with_test_routing do
post "/parse", xml, default_headers
post "/parse", params: xml, headers: default_headers
assert_equal TestController.last_request.body.read, xml
end
end
Expand Down Expand Up @@ -185,7 +185,7 @@ def teardown
test "parses hash params" do
with_test_routing do
xml = "<name>David</name>"
post "/parse", xml, {'CONTENT_TYPE' => 'application/xml'}
post "/parse", params: xml, headers: {'CONTENT_TYPE' => 'application/xml'}
assert_response :ok
assert_equal({"name" => "David", "person" => {"name" => "David"}}, TestController.last_request_parameters)
end
Expand Down