From 889e4dcc1458092298d7d2c3b4f7d068081b1a34 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 2 May 2017 07:47:00 -0400 Subject: [PATCH] Don't require multi_json and multi_xml. --- .rubocop_todo.yml | 27 ++++++++++++------------ CHANGELOG.md | 1 + Gemfile | 2 ++ gemfiles/rack_1.5.2.gemfile | 2 ++ gemfiles/rack_edge.gemfile | 2 ++ gemfiles/rails_3.gemfile | 2 ++ gemfiles/rails_4.gemfile | 2 ++ gemfiles/rails_5.gemfile | 2 ++ gemfiles/rails_edge.gemfile | 2 ++ grape.gemspec | 2 -- lib/grape.rb | 4 ++-- lib/grape/error_formatter/json.rb | 2 +- lib/grape/error_formatter/txt.rb | 2 +- lib/grape/formatter/json.rb | 2 +- lib/grape/formatter/serializable_hash.rb | 4 ++-- lib/grape/parser/json.rb | 4 ++-- lib/grape/parser/xml.rb | 4 ++-- lib/grape/util/json.rb | 13 ++++++++++++ lib/grape/util/xml.rb | 13 ++++++++++++ spec/grape/api/invalid_format_spec.rb | 6 +++--- spec/grape/api_spec.rb | 18 ++++++++-------- spec/grape/endpoint_spec.rb | 12 +++++------ spec/grape/middleware/formatter_spec.rb | 2 +- 23 files changed, 85 insertions(+), 45 deletions(-) create mode 100644 lib/grape/util/json.rb create mode 100644 lib/grape/util/xml.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 605eb31184..4335fb96d7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,51 +1,52 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2017-02-19 15:40:46 -0500 using RuboCop version 0.47.1. +# on 2017-05-02 07:44:23 -0400 using RuboCop version 0.47.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 43 +# Offense count: 2 +Lint/HandleExceptions: + Exclude: + - 'lib/grape/util/json.rb' + - 'lib/grape/util/xml.rb' + +# Offense count: 45 Metrics/AbcSize: Max: 44 -# Offense count: 265 +# Offense count: 276 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: Max: 3104 -# Offense count: 1 -# Configuration parameters: CountBlocks. -Metrics/BlockNesting: - Max: 4 - # Offense count: 8 # Configuration parameters: CountComments. Metrics/ClassLength: Max: 283 -# Offense count: 26 +# Offense count: 28 Metrics/CyclomaticComplexity: Max: 14 -# Offense count: 993 +# Offense count: 1085 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: Max: 215 -# Offense count: 56 +# Offense count: 57 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 33 -# Offense count: 9 +# Offense count: 10 # Configuration parameters: CountComments. Metrics/ModuleLength: Max: 212 -# Offense count: 16 +# Offense count: 18 Metrics/PerceivedComplexity: Max: 14 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d5aae1e02..001b8b9ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#1622](https://github.com/ruby-grape/grape/pull/1622): Add `except_values` validator to replace `except` option of `values` validator - [@jlfaber](https://github.com/jlfaber). * [#1635](https://github.com/ruby-grape/grape/pull/1635): Instrument validators with ActiveSupport::Notifications - [@ktimothy](https://github.com/ktimothy). * [#1646](https://github.com/ruby-grape/grape/pull/1646): Add ability to include an array of modules as helpers - [@pablonahuelgomez](https://github.com/pablonahuelgomez). +* [#1623](https://github.com/ruby-grape/grape/pull/1623): Removed `multi_json` and `multi_xml` dependencies - [@dblock](https://github.com/dblock). * Your contribution here. #### Fixes diff --git a/Gemfile b/Gemfile index 406fe34eba..c6e893aee3 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rack_1.5.2.gemfile b/gemfiles/rack_1.5.2.gemfile index ae9e8174d4..529a73881c 100644 --- a/gemfiles/rack_1.5.2.gemfile +++ b/gemfiles/rack_1.5.2.gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rack_edge.gemfile b/gemfiles/rack_edge.gemfile index 589bf59c45..ed3d413fb0 100644 --- a/gemfiles/rack_edge.gemfile +++ b/gemfiles/rack_edge.gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rails_3.gemfile b/gemfiles/rails_3.gemfile index 29f753f494..b5cc6ac265 100644 --- a/gemfiles/rails_3.gemfile +++ b/gemfiles/rails_3.gemfile @@ -23,6 +23,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index e3cab94687..ca240d697e 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile index e00ea92243..87e96c64a8 100644 --- a/gemfiles/rails_5.gemfile +++ b/gemfiles/rails_5.gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/gemfiles/rails_edge.gemfile b/gemfiles/rails_edge.gemfile index 972f7fdee7..4b13b5be8f 100644 --- a/gemfiles/rails_edge.gemfile +++ b/gemfiles/rails_edge.gemfile @@ -22,6 +22,8 @@ group :development do end group :test do + gem 'multi_json' + gem 'multi_xml' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'rack-test' diff --git a/grape.gemspec b/grape.gemspec index 93e7777690..e2f43592a9 100644 --- a/grape.gemspec +++ b/grape.gemspec @@ -16,8 +16,6 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'mustermann-grape', '~> 1.0.0' s.add_runtime_dependency 'rack-accept' s.add_runtime_dependency 'activesupport' - s.add_runtime_dependency 'multi_json', '>= 1.3.2' - s.add_runtime_dependency 'multi_xml', '>= 0.5.2' s.add_runtime_dependency 'virtus', '>= 1.0.0' s.add_runtime_dependency 'builder' diff --git a/lib/grape.rb b/lib/grape.rb index 27a2e15280..d30ad3b530 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -16,8 +16,6 @@ require 'active_support/core_ext/hash/conversions' require 'active_support/dependencies/autoload' require 'active_support/notifications' -require 'multi_json' -require 'multi_xml' require 'i18n' require 'thread' @@ -44,6 +42,8 @@ module Grape autoload :Parser autoload :Request autoload :Env, 'grape/util/env' + autoload :Json, 'grape/util/json' + autoload :Xml, 'grape/util/xml' end module Http diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index 2885efd53d..e11210495c 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -10,7 +10,7 @@ def call(message, backtrace, options = {}, env = nil) if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end - MultiJson.dump(result) + ::Grape::Json.dump(result) end private diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index 6813a5a98d..db8762f954 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -7,7 +7,7 @@ class << self def call(message, backtrace, options = {}, env = nil) message = present(message, env) - result = message.is_a?(Hash) ? MultiJson.dump(message) : message + result = message.is_a?(Hash) ? ::Grape::Json.dump(message) : message if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result += "\r\n " result += backtrace.join("\r\n ") diff --git a/lib/grape/formatter/json.rb b/lib/grape/formatter/json.rb index 353ee32e0c..5f96918b28 100644 --- a/lib/grape/formatter/json.rb +++ b/lib/grape/formatter/json.rb @@ -4,7 +4,7 @@ module Json class << self def call(object, _env) return object.to_json if object.respond_to?(:to_json) - MultiJson.dump(object) + ::Grape::Json.dump(object) end end end diff --git a/lib/grape/formatter/serializable_hash.rb b/lib/grape/formatter/serializable_hash.rb index 99cfb7b30e..c31fcebb0e 100644 --- a/lib/grape/formatter/serializable_hash.rb +++ b/lib/grape/formatter/serializable_hash.rb @@ -4,9 +4,9 @@ module SerializableHash class << self def call(object, _env) return object if object.is_a?(String) - return MultiJson.dump(serialize(object)) if serializable?(object) + return ::Grape::Json.dump(serialize(object)) if serializable?(object) return object.to_json if object.respond_to?(:to_json) - MultiJson.dump(object) + ::Grape::Json.dump(object) end private diff --git a/lib/grape/parser/json.rb b/lib/grape/parser/json.rb index 99c5ba0e94..89b50d3f2b 100644 --- a/lib/grape/parser/json.rb +++ b/lib/grape/parser/json.rb @@ -3,8 +3,8 @@ module Parser module Json class << self def call(object, _env) - MultiJson.load(object) - rescue MultiJson::ParseError + ::Grape::Json.load(object) + rescue ::Grape::Json::ParseError # handle JSON parsing errors via the rescue handlers or provide error message raise Grape::Exceptions::InvalidMessageBody, 'application/json' end diff --git a/lib/grape/parser/xml.rb b/lib/grape/parser/xml.rb index 5fa06f57e3..9fadf1c64f 100644 --- a/lib/grape/parser/xml.rb +++ b/lib/grape/parser/xml.rb @@ -3,8 +3,8 @@ module Parser module Xml class << self def call(object, _env) - MultiXml.parse(object) - rescue MultiXml::ParseError + ::Grape::Xml.parse(object) + rescue ::Grape::Xml::ParseError # handle XML parsing errors via the rescue handlers or provide error message raise Grape::Exceptions::InvalidMessageBody, 'application/xml' end diff --git a/lib/grape/util/json.rb b/lib/grape/util/json.rb new file mode 100644 index 0000000000..a5686a342f --- /dev/null +++ b/lib/grape/util/json.rb @@ -0,0 +1,13 @@ +begin + require 'multi_json' +rescue LoadError +end + +module Grape + if Object.const_defined? :MultiJson + Json = ::MultiJson + else + Json = ::JSON + Json::ParseError = Json::ParserError + end +end diff --git a/lib/grape/util/xml.rb b/lib/grape/util/xml.rb new file mode 100644 index 0000000000..34b645a8e2 --- /dev/null +++ b/lib/grape/util/xml.rb @@ -0,0 +1,13 @@ +begin + require 'multi_xml' +rescue LoadError +end + +module Grape + if Object.const_defined? :MultiXml + Xml = ::MultiXml + else + Xml = ::ActiveSupport::XmlMini + Xml::ParseError = StandardError + end +end diff --git a/spec/grape/api/invalid_format_spec.rb b/spec/grape/api/invalid_format_spec.rb index 1d7ad8336b..5747694484 100644 --- a/spec/grape/api/invalid_format_spec.rb +++ b/spec/grape/api/invalid_format_spec.rb @@ -27,17 +27,17 @@ def app it 'no format' do get '/foo' expect(last_response.status).to eq 200 - expect(last_response.body).to eq(MultiJson.dump(id: 'foo', format: nil)) + expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: nil)) end it 'json format' do get '/foo.json' expect(last_response.status).to eq 200 - expect(last_response.body).to eq(MultiJson.dump(id: 'foo', format: 'json')) + expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: 'json')) end it 'invalid format' do get '/foo.invalid' expect(last_response.status).to eq 200 - expect(last_response.body).to eq(MultiJson.dump(id: 'foo', format: 'invalid')) + expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: 'invalid')) end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 8ed2ade68f..dc303e7ab9 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' require 'shared/versioning_examples' -require 'grape-entity' +# require 'grape-entity' describe Grape::API do subject { Class.new(Grape::API) } @@ -444,9 +444,9 @@ class DummyFormatClass subject.send(verb) do env['api.request.body'] end - send verb, '/', MultiJson.dump(object), 'CONTENT_TYPE' => 'application/json' + send verb, '/', ::Grape::Json.dump(object), 'CONTENT_TYPE' => 'application/json' expect(last_response.status).to eq(verb == :post ? 201 : 200) - expect(last_response.body).to eql MultiJson.dump(object) + expect(last_response.body).to eql ::Grape::Json.dump(object) expect(last_request.params).to eql({}) end it 'stores input in api.request.input' do @@ -454,9 +454,9 @@ class DummyFormatClass subject.send(verb) do env['api.request.input'] end - send verb, '/', MultiJson.dump(object), 'CONTENT_TYPE' => 'application/json' + send verb, '/', ::Grape::Json.dump(object), 'CONTENT_TYPE' => 'application/json' expect(last_response.status).to eq(verb == :post ? 201 : 200) - expect(last_response.body).to eql MultiJson.dump(object).to_json + expect(last_response.body).to eql ::Grape::Json.dump(object).to_json end context 'chunked transfer encoding' do it 'stores input in api.request.input' do @@ -464,9 +464,9 @@ class DummyFormatClass subject.send(verb) do env['api.request.input'] end - send verb, '/', MultiJson.dump(object), 'CONTENT_TYPE' => 'application/json', 'HTTP_TRANSFER_ENCODING' => 'chunked', 'CONTENT_LENGTH' => nil + send verb, '/', ::Grape::Json.dump(object), 'CONTENT_TYPE' => 'application/json', 'HTTP_TRANSFER_ENCODING' => 'chunked', 'CONTENT_LENGTH' => nil expect(last_response.status).to eq(verb == :post ? 201 : 200) - expect(last_response.body).to eql MultiJson.dump(object).to_json + expect(last_response.body).to eql ::Grape::Json.dump(object).to_json end end end @@ -2053,7 +2053,7 @@ def self.call(message, _backtrace, _option, _env) raise 'rain!' end get '/exception' - json = MultiJson.load(last_response.body) + json = ::Grape::Json.load(last_response.body) expect(json['error']).to eql 'rain!' expect(json['backtrace'].length).to be > 0 end @@ -3156,7 +3156,7 @@ def static end it 'path' do get '/endpoint/options' - options = MultiJson.load(last_response.body) + options = ::Grape::Json.load(last_response.body) expect(options['path']).to eq(['/endpoint/options']) expect(options['source_location'][0]).to include 'api_spec.rb' expect(options['source_location'][1].to_i).to be > 0 diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 4902a3efc8..25c71c1a53 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -429,7 +429,7 @@ def app '' end - post '/declared', MultiJson.dump(first: 'one', boolean: false), 'CONTENT_TYPE' => 'application/json' + post '/declared', ::Grape::Json.dump(first: 'one', boolean: false), 'CONTENT_TYPE' => 'application/json' expect(last_response.status).to eq(201) end @@ -444,7 +444,7 @@ def app '' end - post '/declared', MultiJson.dump(first: 'one', second: nil), 'CONTENT_TYPE' => 'application/json' + post '/declared', ::Grape::Json.dump(first: 'one', second: nil), 'CONTENT_TYPE' => 'application/json' expect(last_response.status).to eq(201) end @@ -832,7 +832,7 @@ def app end it 'converts JSON bodies to params' do - post '/request_body', MultiJson.dump(user: 'Bobby T.'), 'CONTENT_TYPE' => 'application/json' + post '/request_body', ::Grape::Json.dump(user: 'Bobby T.'), 'CONTENT_TYPE' => 'application/json' expect(last_response.body).to eq('Bobby T.') end @@ -856,7 +856,7 @@ def app error! 400, 'expected nil' if params[:version] params[:user] end - post '/omitted_params', MultiJson.dump(user: 'Bob'), 'CONTENT_TYPE' => 'application/json' + post '/omitted_params', ::Grape::Json.dump(user: 'Bob'), 'CONTENT_TYPE' => 'application/json' expect(last_response.status).to eq(201) expect(last_response.body).to eq('Bob') end @@ -879,7 +879,7 @@ def app subject.put '/request_body' do params[:user] end - put '/request_body', MultiJson.dump(user: 'Bob'), 'CONTENT_TYPE' => 'text/plain' + put '/request_body', ::Grape::Json.dump(user: 'Bob'), 'CONTENT_TYPE' => 'text/plain' expect(last_response.status).to eq(406) expect(last_response.body).to eq('{"error":"The requested content-type \'text/plain\' is not supported."}') @@ -893,7 +893,7 @@ def app subject.post do params[:data] end - post '/', MultiJson.dump(data: { some: 'payload' }), 'CONTENT_TYPE' => 'application/json' + post '/', ::Grape::Json.dump(data: { some: 'payload' }), 'CONTENT_TYPE' => 'application/json' end it 'should not response with 406 for same type without params' do diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index a094f5bf6a..937086cb9e 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -11,7 +11,7 @@ let(:body) { { 'abc' => 'def' } } it 'looks at the bodies for possibly serializable data' do _, _, bodies = *subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json') - bodies.each { |b| expect(b).to eq(MultiJson.dump(body)) } + bodies.each { |b| expect(b).to eq(::Grape::Json.dump(body)) } end context 'default format' do