Skip to content

Commit

Permalink
Merge pull request #12783 from chancancode/eliminate_direct_json_gem_use
Browse files Browse the repository at this point in the history
Eliminate `JSON.{parse,load,generate,dump}` and `def to_json`
  • Loading branch information
jeremy committed Nov 6, 2013
2 parents aeaf3a9 + ff1192f commit 0f5a36e
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ def move(t, a)
move_string(t, a).concat(move_regexp(t, a))
end

def to_json
require 'json'

def as_json(options = nil)
simple_regexp = Hash.new { |h,k| h[k] = {} }

@regexp_states.each do |from, hash|
Expand All @@ -54,11 +52,11 @@ def to_json
end
end

JSON.dump({
{
regexp_states: simple_regexp,
string_states: @string_states,
accepting: @accepting
})
}
end

def to_svg
Expand Down
29 changes: 15 additions & 14 deletions actionpack/test/controller/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'abstract_unit'
require 'controller/fake_controllers'
require 'active_support/core_ext/object/with_options'
require 'active_support/core_ext/object/json'

class MilestonesController < ActionController::Base
def index() head :ok end
Expand Down Expand Up @@ -86,61 +87,61 @@ def setup
def test_symbols_with_dashes
rs.draw do
get '/:artist/:song-omg', :to => lambda { |env|
resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
[200, {}, [resp]]
}
end

hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/faithfully-omg'))
assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
end

def test_id_with_dash
rs.draw do
get '/journey/:id', :to => lambda { |env|
resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
[200, {}, [resp]]
}
end

hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/faithfully-omg'))
assert_equal({"id"=>"faithfully-omg"}, hash)
end

def test_dash_with_custom_regexp
rs.draw do
get '/:artist/:song-omg', :constraints => { :song => /\d+/ }, :to => lambda { |env|
resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
[200, {}, [resp]]
}
end

hash = JSON.load get(URI('http://example.org/journey/123-omg'))
hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/123-omg'))
assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
assert_equal 'Not Found', get(URI('http://example.org/journey/faithfully-omg'))
end

def test_pre_dash
rs.draw do
get '/:artist/omg-:song', :to => lambda { |env|
resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
[200, {}, [resp]]
}
end

hash = JSON.load get(URI('http://example.org/journey/omg-faithfully'))
hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/omg-faithfully'))
assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
end

def test_pre_dash_with_custom_regexp
rs.draw do
get '/:artist/omg-:song', :constraints => { :song => /\d+/ }, :to => lambda { |env|
resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
resp = ActiveSupport::JSON.encode env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
[200, {}, [resp]]
}
end

hash = JSON.load get(URI('http://example.org/journey/omg-123'))
hash = ActiveSupport::JSON.decode get(URI('http://example.org/journey/omg-123'))
assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
assert_equal 'Not Found', get(URI('http://example.org/journey/omg-faithfully'))
end
Expand All @@ -160,14 +161,14 @@ def test_star_paths_are_greedy
def test_star_paths_are_greedy_but_not_too_much
rs.draw do
get "/*path", :to => lambda { |env|
x = JSON.dump env["action_dispatch.request.path_parameters"]
x = ActiveSupport::JSON.encode env["action_dispatch.request.path_parameters"]
[200, {}, [x]]
}
end

expected = { "path" => "foo/bar", "format" => "html" }
u = URI('http://example.org/foo/bar.html')
assert_equal expected, JSON.parse(get(u))
assert_equal expected, ActiveSupport::JSON.decode(get(u))
end

def test_optional_star_paths_are_greedy
Expand All @@ -185,15 +186,15 @@ def test_optional_star_paths_are_greedy
def test_optional_star_paths_are_greedy_but_not_too_much
rs.draw do
get "/(*filters)", :to => lambda { |env|
x = JSON.dump env["action_dispatch.request.path_parameters"]
x = ActiveSupport::JSON.encode env["action_dispatch.request.path_parameters"]
[200, {}, [x]]
}
end

expected = { "filters" => "ne_27.065938,-80.6092/sw_25.489856,-82",
"format" => "542794" }
u = URI('http://example.org/ne_27.065938,-80.6092/sw_25.489856,-82.542794')
assert_equal expected, JSON.parse(get(u))
assert_equal expected, ActiveSupport::JSON.decode(get(u))
end

def test_regexp_precidence
Expand Down
5 changes: 3 additions & 2 deletions actionpack/test/controller/test_case_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'abstract_unit'
require 'controller/fake_controllers'
require 'active_support/json/decoding'

class TestCaseTest < ActionController::TestCase
class TestController < ActionController::Base
Expand Down Expand Up @@ -622,7 +623,7 @@ def test_params_passing_doesnt_modify_in_place
@request.headers['Referer'] = "http://nohost.com/home"
@request.headers['Content-Type'] = "application/rss+xml"
get :test_headers
parsed_env = JSON.parse(@response.body)
parsed_env = ActiveSupport::JSON.decode(@response.body)
assert_equal "http://nohost.com/home", parsed_env["HTTP_REFERER"]
assert_equal "application/rss+xml", parsed_env["CONTENT_TYPE"]
end
Expand All @@ -631,7 +632,7 @@ def test_params_passing_doesnt_modify_in_place
@request.headers['HTTP_REFERER'] = "http://example.com/about"
@request.headers['CONTENT_TYPE'] = "application/json"
get :test_headers
parsed_env = JSON.parse(@response.body)
parsed_env = ActiveSupport::JSON.decode(@response.body)
assert_equal "http://example.com/about", parsed_env["HTTP_REFERER"]
assert_equal "application/json", parsed_env["CONTENT_TYPE"]
end
Expand Down
3 changes: 2 additions & 1 deletion actionpack/test/controller/webservice_test.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'abstract_unit'
require 'active_support/json/decoding'

class WebServiceTest < ActionDispatch::IntegrationTest
class TestController < ActionController::Base
Expand Down Expand Up @@ -54,7 +55,7 @@ def test_put_json

def test_register_and_use_json_simple
with_test_route_set do
with_params_parsers Mime::JSON => Proc.new { |data| JSON.parse(data)['request'].with_indifferent_access } do
with_params_parsers Mime::JSON => Proc.new { |data| ActiveSupport::JSON.decode(data)['request'].with_indifferent_access } do
post "/", '{"request":{"summary":"content...","title":"JSON"}}',
'CONTENT_TYPE' => 'application/json'

Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/journey/gtg/transition_table_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require 'abstract_unit'
require 'json'
require 'active_support/json/decoding'

module ActionDispatch
module Journey
Expand All @@ -13,7 +13,7 @@ def test_to_json
/articles/:id(.:format)
}

json = JSON.load table.to_json
json = ActiveSupport::JSON.decode table.to_json
assert json['regexp_states']
assert json['string_states']
assert json['accepting']
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/lib/controller/fake_models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def persisted?
end

class RenderJsonTestException < Exception
def to_json(options = nil)
return { :error => self.class.name, :message => self.to_s }.to_json
def as_json(options = nil)
{ :error => self.class.name, :message => self.to_s }
end
end
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/core_ext/object/json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def encode_json(encoder) #:nodoc:

class Float
# Encoding Infinity or NaN to JSON should return "null". The default returns
# "Infinity" or "NaN" which breaks parsing the JSON. E.g. JSON.parse('[NaN]').
# "Infinity" or "NaN" which are not valid JSON.
def as_json(options = nil) #:nodoc:
finite? ? self : nil
end
Expand Down
8 changes: 4 additions & 4 deletions activesupport/test/json/encoding_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def test_to_json_should_not_keep_options_around

hash = {"foo" => f, "other_hash" => {"foo" => "other_foo", "test" => "other_test"}}
assert_equal({"foo"=>{"foo"=>"hello","bar"=>"world"},
"other_hash" => {"foo"=>"other_foo","test"=>"other_test"}}, JSON.parse(hash.to_json))
"other_hash" => {"foo"=>"other_foo","test"=>"other_test"}}, ActiveSupport::JSON.decode(hash.to_json))
end

def test_struct_encoding
Expand All @@ -335,13 +335,13 @@ def test_struct_encoding
assert_equal({"name" => "David",
"sub" => {
"name" => "David",
"date" => "2010-01-01" }}, JSON.parse(json_custom))
"date" => "2010-01-01" }}, ActiveSupport::JSON.decode(json_custom))

assert_equal({"name" => "David", "email" => "sample@example.com"},
JSON.parse(json_strings))
ActiveSupport::JSON.decode(json_strings))

assert_equal({"name" => "David", "date" => "2010-01-01"},
JSON.parse(json_string_and_date))
ActiveSupport::JSON.decode(json_string_and_date))
end

def test_opt_out_big_decimal_string_serialization
Expand Down

0 comments on commit 0f5a36e

Please sign in to comment.