diff --git a/CHANGELOG.md b/CHANGELOG.md index b862edf6a3..e3e890a33b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Features * [#2326](https://github.com/ruby-grape/grape/pull/2326): Use ActiveSupport extensions - [@ericproulx](https://github.com/ericproulx). +* [#2327](https://github.com/ruby-grape/grape/pull/2327): Use ActiveSupport deprecation - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape.rb b/lib/grape.rb index ed309e16f5..db1e1d06d2 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -26,6 +26,7 @@ require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/duplicable' require 'active_support/dependencies/autoload' +require 'active_support/deprecation' require 'active_support/notifications' require 'i18n' diff --git a/lib/grape/dsl/desc.rb b/lib/grape/dsl/desc.rb index 4da4786905..64d5eed910 100644 --- a/lib/grape/dsl/desc.rb +++ b/lib/grape/dsl/desc.rb @@ -68,7 +68,7 @@ def desc(description, options = {}, &config_block) end config_class.configure(&config_block) - warn '[DEPRECATION] Passing a options hash and a block to `desc` is deprecated. Move all hash options to block.' unless options.empty? + ActiveSupport::Deprecation.warn('Passing a options hash and a block to `desc` is deprecated. Move all hash options to block.') if options.any? options = config_class.settings else options = options.merge(description: description) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index e2ee006c78..4299888722 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -280,13 +280,13 @@ def return_no_content # Deprecated method to send files to the client. Use `sendfile` or `stream` def file(value = nil) if value.is_a?(String) - warn '[DEPRECATION] Use sendfile or stream to send files.' + ActiveSupport::Deprecation.warn('Use sendfile or stream to send files.') sendfile(value) elsif !value.is_a?(NilClass) - warn '[DEPRECATION] Use stream to use a Stream object.' + ActiveSupport::Deprecation.warn('Use stream to use a Stream object.') stream(value) else - warn '[DEPRECATION] Use sendfile or stream to send files.' + ActiveSupport::Deprecation.warn('Use sendfile or stream to send files.') sendfile end end diff --git a/lib/grape/exceptions/missing_group_type.rb b/lib/grape/exceptions/missing_group_type.rb index 48ef996abc..1f104321f5 100644 --- a/lib/grape/exceptions/missing_group_type.rb +++ b/lib/grape/exceptions/missing_group_type.rb @@ -10,9 +10,4 @@ def initialize end end -Grape::Exceptions::MissingGroupTypeError = Class.new(Grape::Exceptions::MissingGroupType) do - def initialize(*) - super - warn '[DEPRECATION] `Grape::Exceptions::MissingGroupTypeError` is deprecated. Use `Grape::Exceptions::MissingGroupType` instead.' - end -end +Grape::Exceptions::MissingGroupTypeError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('Grape::Exceptions::MissingGroupTypeError', 'Grape::Exceptions::MissingGroupType') diff --git a/lib/grape/exceptions/unsupported_group_type.rb b/lib/grape/exceptions/unsupported_group_type.rb index 5d845aad64..da45abafdd 100644 --- a/lib/grape/exceptions/unsupported_group_type.rb +++ b/lib/grape/exceptions/unsupported_group_type.rb @@ -10,9 +10,4 @@ def initialize end end -Grape::Exceptions::UnsupportedGroupTypeError = Class.new(Grape::Exceptions::UnsupportedGroupType) do - def initialize(*) - super - warn '[DEPRECATION] `Grape::Exceptions::UnsupportedGroupTypeError` is deprecated. Use `Grape::Exceptions::UnsupportedGroupType` instead.' - end -end +Grape::Exceptions::UnsupportedGroupTypeError = ActiveSupport::Deprecation::DeprecatedConstantProxy.new('Grape::Exceptions::UnsupportedGroupTypeError', 'Grape::Exceptions::UnsupportedGroupType') diff --git a/lib/grape/router/route.rb b/lib/grape/router/route.rb index 3ced9ca2ac..7bbfe6a0c5 100644 --- a/lib/grape/router/route.rb +++ b/lib/grape/router/route.rb @@ -84,9 +84,7 @@ def warn_route_methods(name, location, expected = nil) path, line = *location.scan(SOURCE_LOCATION_REGEXP).first path = File.realpath(path) if Pathname.new(path).relative? expected ||= name - warn <<~WARNING - #{path}:#{line}: The route_xxx methods such as route_#{name} have been deprecated, please use #{expected}. - WARNING + ActiveSupport::Deprecation.warn("#{path}:#{line}: The route_xxx methods such as route_#{name} have been deprecated, please use #{expected}.") end end end diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 86813bf9be..b3bcfc50c7 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -95,8 +95,8 @@ def fail_fast? end Grape::Validations::Base = Class.new(Grape::Validations::Validators::Base) do - def initialize(*) + def self.inherited(*) + ActiveSupport::Deprecation.warn 'Grape::Validations::Base is deprecated! Use Grape::Validations::Validators::Base instead.' super - warn '[DEPRECATION] `Grape::Validations::Base` is deprecated. Use `Grape::Validations::Validators::Base` instead.' end end diff --git a/lib/grape/validations/validators/values_validator.rb b/lib/grape/validations/validators/values_validator.rb index b8b05fb6b3..0fba1a91c3 100644 --- a/lib/grape/validations/validators/values_validator.rb +++ b/lib/grape/validations/validators/values_validator.rb @@ -10,13 +10,11 @@ def initialize(attrs, options, required, scope, **opts) @values = options[:value] @proc = options[:proc] - warn '[DEPRECATION] The values validator except option is deprecated. ' \ - 'Use the except validator instead.' if @excepts + ActiveSupport::Deprecation.warn('The values validator except option is deprecated. Use the except validator instead.') if @excepts raise ArgumentError, 'proc must be a Proc' if @proc && !@proc.is_a?(Proc) - warn '[DEPRECATION] The values validator proc option is deprecated. ' \ - 'The lambda expression can now be assigned directly to values.' if @proc + ActiveSupport::Deprecation.warn('The values validator proc option is deprecated. The lambda expression can now be assigned directly to values.') if @proc else @excepts = nil @values = nil diff --git a/spec/grape/api/custom_validations_spec.rb b/spec/grape/api/custom_validations_spec.rb index 25879fc57f..bfb821de86 100644 --- a/spec/grape/api/custom_validations_spec.rb +++ b/spec/grape/api/custom_validations_spec.rb @@ -1,48 +1,17 @@ # frozen_string_literal: true -describe Grape::Validations do - context 'deprecated Grape::Validations::Base' do - subject do - Class.new(Grape::API) do - params do - requires :text, validator_with_old_base: true - end - get do - end - end - end - - let(:validator_with_old_base) do - Class.new(Grape::Validations::Base) do - def validate_param!(_attr_name, _params) - true - end - end - end - - before do - described_class.register_validator('validator_with_old_base', validator_with_old_base) - allow(Warning).to receive(:warn) - end +require 'shared/deprecated_class_examples' - after do - described_class.deregister_validator('validator_with_old_base') - end - - def app - subject +describe Grape::Validations do + describe 'Grape::Validations::Base' do + let(:deprecated_class) do + Class.new(Grape::Validations::Base) end - it 'puts a deprecation warning' do - expect(Warning).to receive(:warn) do |message| - expect(message).to include('`Grape::Validations::Base` is deprecated') - end - - get '/' - end + it_behaves_like 'deprecated class' end - context 'using a custom length validator' do + describe 'using a custom length validator' do subject do Class.new(Grape::API) do params do @@ -64,6 +33,7 @@ def validate_param!(attr_name, params) end end end + let(:app) { Rack::Builder.new(subject) } before do described_class.register_validator('default_length', default_length_validator) @@ -73,10 +43,6 @@ def validate_param!(attr_name, params) described_class.deregister_validator('default_length') end - def app - subject - end - it 'under 140 characters' do get '/', text: 'abc' expect(last_response.status).to eq 200 @@ -96,7 +62,7 @@ def app end end - context 'using a custom body-only validator' do + describe 'using a custom body-only validator' do subject do Class.new(Grape::API) do params do @@ -115,6 +81,7 @@ def validate(request) end end end + let(:app) { Rack::Builder.new(subject) } before do described_class.register_validator('in_body', in_body_validator) @@ -124,10 +91,6 @@ def validate(request) described_class.deregister_validator('in_body') end - def app - subject - end - it 'allows field in body' do get '/', text: 'abc' expect(last_response.status).to eq 200 @@ -141,7 +104,7 @@ def app end end - context 'using a custom validator with message_key' do + describe 'using a custom validator with message_key' do subject do Class.new(Grape::API) do params do @@ -160,6 +123,7 @@ def validate_param!(attr_name, _params) end end end + let(:app) { Rack::Builder.new(subject) } before do described_class.register_validator('with_message_key', message_key_validator) @@ -169,10 +133,6 @@ def validate_param!(attr_name, _params) described_class.deregister_validator('with_message_key') end - def app - subject - end - it 'fails with message' do get '/', text: 'foobar' expect(last_response.status).to eq 400 @@ -180,7 +140,7 @@ def app end end - context 'using a custom request/param validator' do + describe 'using a custom request/param validator' do subject do Class.new(Grape::API) do params do @@ -208,6 +168,7 @@ def validate(request) end end end + let(:app) { Rack::Builder.new(subject) } before do described_class.register_validator('admin', admin_validator) @@ -217,10 +178,6 @@ def validate(request) described_class.deregister_validator('admin') end - def app - subject - end - it 'fail when non-admin user sets an admin field' do get '/', admin_field: 'tester', non_admin_field: 'toaster' expect(last_response.status).to eq 400 diff --git a/spec/grape/dsl/desc_spec.rb b/spec/grape/dsl/desc_spec.rb index 718e3a9fb0..c2acde9105 100644 --- a/spec/grape/dsl/desc_spec.rb +++ b/spec/grape/dsl/desc_spec.rb @@ -1,101 +1,98 @@ # frozen_string_literal: true -module Grape - module DSL - module DescSpec - class Dummy - extend Grape::DSL::Desc - end +describe Grape::DSL::Desc do + subject { dummy_class } + + let(:dummy_class) do + Class.new do + extend Grape::DSL::Desc end - describe Desc do - subject { Class.new(DescSpec::Dummy) } + end - describe '.desc' do - it 'sets a description' do - desc_text = 'The description' - options = { message: 'none' } - subject.desc desc_text, options - expect(subject.namespace_setting(:description)).to eq(options.merge(description: desc_text)) - expect(subject.route_setting(:description)).to eq(options.merge(description: desc_text)) - end + describe '.desc' do + it 'sets a description' do + desc_text = 'The description' + options = { message: 'none' } + subject.desc desc_text, options + expect(subject.namespace_setting(:description)).to eq(options.merge(description: desc_text)) + expect(subject.route_setting(:description)).to eq(options.merge(description: desc_text)) + end - it 'can be set with a block' do - expected_options = { - summary: 'summary', - description: 'The description', - detail: 'more details', - params: { first: :param }, - entity: Object, - default: { code: 400, message: 'Invalid' }, - http_codes: [[401, 'Unauthorized', 'Entities::Error']], - named: 'My named route', - body_name: 'My body name', - headers: [ - XAuthToken: { - description: 'Valdates your identity', - required: true - }, - XOptionalHeader: { - description: 'Not really needed', - required: false - } - ], - hidden: false, - deprecated: false, - is_array: true, - nickname: 'nickname', - produces: %w[array of mime_types], - consumes: %w[array of mime_types], - tags: %w[tag1 tag2], - security: %w[array of security schemes] + it 'can be set with a block' do + expected_options = { + summary: 'summary', + description: 'The description', + detail: 'more details', + params: { first: :param }, + entity: Object, + default: { code: 400, message: 'Invalid' }, + http_codes: [[401, 'Unauthorized', 'Entities::Error']], + named: 'My named route', + body_name: 'My body name', + headers: [ + XAuthToken: { + description: 'Valdates your identity', + required: true + }, + XOptionalHeader: { + description: 'Not really needed', + required: false } + ], + hidden: false, + deprecated: false, + is_array: true, + nickname: 'nickname', + produces: %w[array of mime_types], + consumes: %w[array of mime_types], + tags: %w[tag1 tag2], + security: %w[array of security schemes] + } - subject.desc 'The description' do - summary 'summary' - detail 'more details' - params(first: :param) - success Object - default code: 400, message: 'Invalid' - failure [[401, 'Unauthorized', 'Entities::Error']] - named 'My named route' - body_name 'My body name' - headers [ - XAuthToken: { - description: 'Valdates your identity', - required: true - }, - XOptionalHeader: { - description: 'Not really needed', - required: false - } - ] - hidden false - deprecated false - is_array true - nickname 'nickname' - produces %w[array of mime_types] - consumes %w[array of mime_types] - tags %w[tag1 tag2] - security %w[array of security schemes] - end + subject.desc 'The description' do + summary 'summary' + detail 'more details' + params(first: :param) + success Object + default code: 400, message: 'Invalid' + failure [[401, 'Unauthorized', 'Entities::Error']] + named 'My named route' + body_name 'My body name' + headers [ + XAuthToken: { + description: 'Valdates your identity', + required: true + }, + XOptionalHeader: { + description: 'Not really needed', + required: false + } + ] + hidden false + deprecated false + is_array true + nickname 'nickname' + produces %w[array of mime_types] + consumes %w[array of mime_types] + tags %w[tag1 tag2] + security %w[array of security schemes] + end - expect(subject.namespace_setting(:description)).to eq(expected_options) - expect(subject.route_setting(:description)).to eq(expected_options) - end + expect(subject.namespace_setting(:description)).to eq(expected_options) + expect(subject.route_setting(:description)).to eq(expected_options) + end - it 'can be set with options and a block' do - expect(subject).to receive(:warn).with('[DEPRECATION] Passing a options hash and a block to `desc` is deprecated. Move all hash options to block.') + it 'can be set with options and a block' do + expect(ActiveSupport::Deprecation).to receive(:warn).with('Passing a options hash and a block to `desc` is deprecated. Move all hash options to block.') - desc_text = 'The description' - detail_text = 'more details' - options = { message: 'none' } - subject.desc desc_text, options do - detail detail_text - end - expect(subject.namespace_setting(:description)).to eq(description: desc_text, detail: detail_text) - expect(subject.route_setting(:description)).to eq(description: desc_text, detail: detail_text) - end + desc_text = 'The description' + detail_text = 'more details' + options = { message: 'none' } + subject.desc desc_text, options do + detail detail_text end + expect(subject.namespace_setting(:description)).to eq(description: desc_text, detail: detail_text) + expect(subject.route_setting(:description)).to eq(description: desc_text, detail: detail_text) end end end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index b6f4aba878..e9a529b513 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -203,16 +203,12 @@ def initialize end describe '#file' do - before do - allow(subject).to receive(:warn) - end - describe 'set' do context 'as file path' do let(:file_path) { '/some/file/path' } it 'emits a warning that this method is deprecated' do - expect(subject).to receive(:warn).with(/Use sendfile or stream/) + expect(ActiveSupport::Deprecation).to receive(:warn).with(/Use sendfile or stream/) subject.file file_path end @@ -228,7 +224,7 @@ def initialize let(:file_object) { double('StreamerObject', each: nil) } it 'emits a warning that this method is deprecated' do - expect(subject).to receive(:warn).with(/Use stream to use a Stream object/) + expect(ActiveSupport::Deprecation).to receive(:warn).with(/Use stream to use a Stream object/) subject.file file_object end @@ -243,7 +239,7 @@ def initialize describe 'get' do it 'emits a warning that this method is deprecated' do - expect(subject).to receive(:warn).with(/Use sendfile or stream/) + expect(ActiveSupport::Deprecation).to receive(:warn).with(/Use sendfile or stream/) subject.file end @@ -273,7 +269,7 @@ def initialize end it 'sends no deprecation warnings' do - expect(subject).not_to receive(:warn) + expect(ActiveSupport::Deprecation).not_to receive(:warn) subject.sendfile file_path end @@ -334,7 +330,7 @@ def initialize end it 'emits no deprecation warnings' do - expect(subject).not_to receive(:warn) + expect(ActiveSupport::Deprecation).not_to receive(:warn) subject.stream file_path end @@ -384,7 +380,7 @@ def initialize end it 'emits no deprecation warnings' do - expect(subject).not_to receive(:warn) + expect(ActiveSupport::Deprecation).not_to receive(:warn) subject.stream stream_object end diff --git a/spec/grape/exceptions/missing_group_type_spec.rb b/spec/grape/exceptions/missing_group_type_spec.rb index b6612882fa..7a6ca52695 100644 --- a/spec/grape/exceptions/missing_group_type_spec.rb +++ b/spec/grape/exceptions/missing_group_type_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'shared/deprecated_class_examples' + RSpec.describe Grape::Exceptions::MissingGroupType do describe '#message' do subject { described_class.new.message } @@ -7,15 +9,9 @@ it { is_expected.to include 'group type is required' } end - describe 'deprecated Grape::Exceptions::MissingGroupTypeError' do - subject { Grape::Exceptions::MissingGroupTypeError.new } - - it 'puts a deprecation warning' do - expect(Warning).to receive(:warn) do |message| - expect(message).to include('`Grape::Exceptions::MissingGroupTypeError` is deprecated') - end + describe 'Grape::Exceptions::MissingGroupTypeError' do + let(:deprecated_class) { Grape::Exceptions::MissingGroupTypeError } - subject - end + it_behaves_like 'deprecated class' end end diff --git a/spec/grape/exceptions/unsupported_group_type_spec.rb b/spec/grape/exceptions/unsupported_group_type_spec.rb index ee1451aaa0..b6282ab813 100644 --- a/spec/grape/exceptions/unsupported_group_type_spec.rb +++ b/spec/grape/exceptions/unsupported_group_type_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'shared/deprecated_class_examples' + RSpec.describe Grape::Exceptions::UnsupportedGroupType do subject { described_class.new } @@ -9,15 +11,9 @@ it { is_expected.to include 'group type must be Array, Hash, JSON or Array[JSON]' } end - describe 'deprecated Grape::Exceptions::UnsupportedGroupTypeError' do - subject { Grape::Exceptions::UnsupportedGroupTypeError.new } - - it 'puts a deprecation warning' do - expect(Warning).to receive(:warn) do |message| - expect(message).to include('`Grape::Exceptions::UnsupportedGroupTypeError` is deprecated') - end + describe 'Grape::Exceptions::UnsupportedGroupTypeError' do + let(:deprecated_class) { Grape::Exceptions::UnsupportedGroupTypeError } - subject - end + it_behaves_like 'deprecated class' end end diff --git a/spec/shared/deprecated_class_examples.rb b/spec/shared/deprecated_class_examples.rb new file mode 100644 index 0000000000..93a4f120cf --- /dev/null +++ b/spec/shared/deprecated_class_examples.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'deprecated class' do + subject { deprecated_class.new } + + around do |example| + old_deprec_behavior = ActiveSupport::Deprecation.behavior + ActiveSupport::Deprecation.behavior = :raise + example.run + ActiveSupport::Deprecation.behavior = old_deprec_behavior + end + + it 'raises an ActiveSupport::DeprecationException' do + expect { subject }.to raise_error(ActiveSupport::DeprecationException) + end +end