From 57d890fcaefa15f788368df71d8e51e24fab83c2 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 18:30:20 +0200 Subject: [PATCH 1/6] Remove file method and specs A --- lib/grape/dsl/inside_route.rb | 14 ---------- spec/grape/dsl/inside_route_spec.rb | 43 ----------------------------- 2 files changed, 57 deletions(-) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index ba1715de24..72b365bc44 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -305,20 +305,6 @@ def return_no_content body false end - # Deprecated method to send files to the client. Use `sendfile` or `stream` - def file(value = nil) - if value.is_a?(String) - Grape.deprecator.warn('Use sendfile or stream to send files.') - sendfile(value) - elsif !value.is_a?(NilClass) - Grape.deprecator.warn('Use stream to use a Stream object.') - stream(value) - else - Grape.deprecator.warn('Use sendfile or stream to send files.') - sendfile - end - end - # Allows you to send a file to the client via sendfile. # # @example diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 8963a73a27..d684f25973 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -202,38 +202,6 @@ def initialize end end - describe '#file' do - 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(Grape.deprecator).to receive(:warn).with(/Use sendfile or stream/) - expect(subject).to receive(:sendfile).with(file_path) - subject.file file_path - end - end - - context 'as object (backward compatibility)' do - let(:file_object) { double('StreamerObject', each: nil) } - - it 'emits a warning that this method is deprecated' do - expect(Grape.deprecator).to receive(:warn).with(/Use stream to use a Stream object/) - expect(subject).to receive(:stream).with(file_object) - subject.file file_object - end - end - end - - describe 'get' do - it 'emits a warning that this method is deprecated' do - expect(Grape.deprecator).to receive(:warn).with(/Use sendfile or stream/) - expect(subject).to receive(:sendfile) - subject.file - end - end - end - describe '#sendfile' do describe 'set' do context 'as file path' do @@ -248,29 +216,18 @@ def initialize subject.header Rack::CACHE_CONTROL, 'cache' subject.header Rack::CONTENT_LENGTH, 123 subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' - end - - it 'sends no deprecation warnings' do - expect(Grape.deprecator).not_to receive(:warn) - subject.sendfile file_path end it 'returns value wrapped in StreamResponse' do - subject.sendfile file_path - expect(subject.sendfile).to eq file_response end it 'does not change the Cache-Control header' do - subject.sendfile file_path - expect(subject.header[Rack::CACHE_CONTROL]).to eq 'cache' end it 'does not change the Content-Length header' do - subject.sendfile file_path - expect(subject.header[Rack::CONTENT_LENGTH]).to eq 123 end From 4a2b6b5d684805aa2b559079393d9233132f9ec0 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 18:34:31 +0200 Subject: [PATCH 2/6] Add CHANGELOG.md and UPGRADING.md --- CHANGELOG.md | 1 + UPGRADING.md | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa56c6c119..09b7d2e88e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Features * [#2497](https://github.com/ruby-grape/grape/pull/2497): Update RuboCop to 1.66.1 - [@ericproulx](https://github.com/ericproulx). +* [#2500](https://github.com/ruby-grape/grape/pull/2500): Remove deprecated `file` method - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index cba4774bbe..acc74fbac8 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,14 @@ Upgrading Grape =============== +### Upgrading to >= 2.3.0 + +#### Remove deprecated methods + +Deprecated `file` method has been removed. Use `send_file` or `stream`. + +See [#2500](https://github.com/ruby-grape/grape/pull/2500) + ### Upgrading to >= 2.2.0 ### `Length` validator From b7c284e3f5b4a597d4c57a3558e68baf686096fb Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 18:58:26 +0200 Subject: [PATCH 3/6] Remove leaky dummy class in inside_route_spec.rb --- spec/grape/dsl/inside_route_spec.rb | 30 +++++++++++++---------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index d684f25973..ea33468daa 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -1,25 +1,21 @@ # frozen_string_literal: true -module Grape - module DSL - module InsideRouteSpec - class Dummy - include Grape::DSL::InsideRoute - - attr_reader :env, :request, :new_settings - - def initialize - @env = {} - @header = {} - @new_settings = { namespace_inheritable: {}, namespace_stackable: {} } - end +describe Grape::Endpoint do + subject { dummy_class.new } + + let(:dummy_class) do + Class.new do + include Grape::DSL::InsideRoute + + attr_reader :env, :request, :new_settings + + def initialize + @env = {} + @header = {} + @new_settings = { namespace_inheritable: {}, namespace_stackable: {} } end end end -end - -describe Grape::Endpoint do - subject { Grape::DSL::InsideRouteSpec::Dummy.new } describe '#version' do it 'defaults to nil' do From b3697e6d004432af51d6a7816880fccdc71ad7c1 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 19:22:40 +0200 Subject: [PATCH 4/6] Use match operator instead of multiple examples --- spec/grape/dsl/inside_route_spec.rb | 56 +++-------------------------- 1 file changed, 4 insertions(+), 52 deletions(-) diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index ea33468daa..435dcbe4b9 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -262,42 +262,15 @@ def initialize subject.header Rack::CACHE_CONTROL, 'cache' subject.header Rack::CONTENT_LENGTH, 123 subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' - end - - it 'emits no deprecation warnings' do - expect(Grape.deprecator).not_to receive(:warn) - subject.stream file_path end it 'returns file body wrapped in StreamResponse' do - subject.stream file_path - expect(subject.stream).to eq file_response end - it 'sets Cache-Control header to no-cache' do - subject.stream file_path - - expect(subject.header[Rack::CACHE_CONTROL]).to eq 'no-cache' - end - - it 'does not change Cache-Control header' do - subject.stream - - expect(subject.header[Rack::CACHE_CONTROL]).to eq 'cache' - end - - it 'sets Content-Length header to nil' do - subject.stream file_path - - expect(subject.header[Rack::CONTENT_LENGTH]).to be_nil - end - - it 'sets Transfer-Encoding header to nil' do - subject.stream file_path - - expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil + it 'sets only the cache-control header' do + expect(subject.header).to match(Rack::CACHE_CONTROL => 'no-cache') end end @@ -312,36 +285,15 @@ def initialize subject.header Rack::CACHE_CONTROL, 'cache' subject.header Rack::CONTENT_LENGTH, 123 subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' - end - - it 'emits no deprecation warnings' do - expect(Grape.deprecator).not_to receive(:warn) - subject.stream stream_object end it 'returns value wrapped in StreamResponse' do - subject.stream stream_object - expect(subject.stream).to eq stream_response end - it 'sets Cache-Control header to no-cache' do - subject.stream stream_object - - expect(subject.header[Rack::CACHE_CONTROL]).to eq 'no-cache' - end - - it 'sets Content-Length header to nil' do - subject.stream stream_object - - expect(subject.header[Rack::CONTENT_LENGTH]).to be_nil - end - - it 'sets Transfer-Encoding header to nil' do - subject.stream stream_object - - expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil + it 'set only the cache-control header' do + expect(subject.header).to match(Rack::CACHE_CONTROL => 'no-cache') end end From 1e041495715aa7ad760f6fbb483c0584d14cc413 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 19:28:35 +0200 Subject: [PATCH 5/6] Use match operator instead of multiple examples --- spec/grape/dsl/inside_route_spec.rb | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 435dcbe4b9..3e09e23d70 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -219,19 +219,14 @@ def initialize expect(subject.sendfile).to eq file_response end - it 'does not change the Cache-Control header' do - expect(subject.header[Rack::CACHE_CONTROL]).to eq 'cache' + it 'set the correct headers' do + expect(subject.header).to match( + Rack::CACHE_CONTROL => 'cache', + Rack::CONTENT_LENGTH => 123, + Grape::Http::Headers::TRANSFER_ENCODING => 'base64' + ) end - it 'does not change the Content-Length header' do - expect(subject.header[Rack::CONTENT_LENGTH]).to eq 123 - end - - it 'does not change the Transfer-Encoding header' do - subject.sendfile file_path - - expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to eq 'base64' - end end context 'as object' do @@ -308,7 +303,7 @@ def initialize it 'returns default' do expect(subject.stream).to be_nil - expect(subject.header[Rack::CACHE_CONTROL]).to be_nil + expect(subject.header).to be_empty end end From 9cf01a91bdfb7e72f2d32021fb33649e75f7025a Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 25 Sep 2024 19:29:27 +0200 Subject: [PATCH 6/6] Fix rubocop --- spec/grape/dsl/inside_route_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 3e09e23d70..038e203140 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -221,12 +221,11 @@ def initialize it 'set the correct headers' do expect(subject.header).to match( - Rack::CACHE_CONTROL => 'cache', - Rack::CONTENT_LENGTH => 123, - Grape::Http::Headers::TRANSFER_ENCODING => 'base64' - ) + Rack::CACHE_CONTROL => 'cache', + Rack::CONTENT_LENGTH => 123, + Grape::Http::Headers::TRANSFER_ENCODING => 'base64' + ) end - end context 'as object' do