From e2b23dee8ae7ee9ce946308916f82c974d954a0c Mon Sep 17 00:00:00 2001 From: Dhruv Paranjape Date: Sat, 6 Apr 2024 20:30:32 +0200 Subject: [PATCH] Replace Hash with Rack::Header/Rack::Utils::HeaderHash --- CHANGELOG.md | 1 + lib/grape/dsl/headers.rb | 2 +- lib/grape/endpoint.rb | 2 +- lib/grape/request.rb | 12 +++--------- lib/grape/util/header.rb | 13 +++++++++++++ spec/grape/api/custom_validations_spec.rb | 4 ++-- spec/grape/dsl/headers_spec.rb | 20 ++++++++++---------- spec/grape/endpoint_spec.rb | 11 +++++++---- spec/grape/request_spec.rb | 4 ++-- 9 files changed, 40 insertions(+), 29 deletions(-) create mode 100644 lib/grape/util/header.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 96c236ec38..06581d5972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ * [#2426](https://github.com/ruby-grape/grape/pull/2426): Drop support for rack 1.x series - [@ericproulx](https://github.com/ericproulx). * [#2427](https://github.com/ruby-grape/grape/pull/2427): Use `rack-contrib` jsonp instead of rack-jsonp - [@ericproulx](https://github.com/ericproulx). * [#2363](https://github.com/ruby-grape/grape/pull/2363): Replace autoload by zeitwerk - [@ericproulx](https://github.com/ericproulx). +* [#2425](https://github.com/ruby-grape/grape/pull/2425): Replace `{}` with `Rack::Header` or `Rack::Utils::HeaderHash` - [@dhruvCW](https://github.com/dhruvCW). * Your contribution here. #### Fixes diff --git a/lib/grape/dsl/headers.rb b/lib/grape/dsl/headers.rb index b84c4efe4d..a02bdd588e 100644 --- a/lib/grape/dsl/headers.rb +++ b/lib/grape/dsl/headers.rb @@ -12,7 +12,7 @@ def header(key = nil, val = nil) if key val ? header[key.to_s] = val : header.delete(key.to_s) else - @header ||= {} + @header ||= Grape::Util::Header.new end end alias headers header diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 121b1c4d68..a729216bb2 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -238,7 +238,7 @@ def equals?(e) def run ActiveSupport::Notifications.instrument('endpoint_run.grape', endpoint: self, env: env) do - @header = {} + @header = Grape::Util::Header.new @request = Grape::Request.new(env, build_params_with: namespace_inheritable(:build_params_with)) @params = @request.params @headers = @request.headers diff --git a/lib/grape/request.rb b/lib/grape/request.rb index 8b75da98ea..7093ab95dd 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -35,7 +35,7 @@ def grape_routing_args def build_headers Grape::Util::Lazy::Object.new do - env.each_pair.with_object({}) do |(k, v), headers| + env.each_pair.with_object(Grape::Util::Header.new) do |(k, v), headers| next unless k.to_s.start_with? HTTP_PREFIX transformed_header = Grape::Http::Headers::HTTP_HEADERS[k] || transform_header(k) @@ -44,14 +44,8 @@ def build_headers end end - if Grape::Http::Headers.lowercase? - def transform_header(header) - -header[5..].tr('_', '-').downcase - end - else - def transform_header(header) - -header[5..].split('_').map(&:capitalize).join('-') - end + def transform_header(header) + -header[5..].tr('_', '-').downcase end end end diff --git a/lib/grape/util/header.rb b/lib/grape/util/header.rb new file mode 100644 index 0000000000..632480798d --- /dev/null +++ b/lib/grape/util/header.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Grape + module Util + if Gem::Version.new(Rack.release) >= Gem::Version.new('3') + require 'rack/headers' + Header = Rack::Headers + else + require 'rack/utils' + Header = Rack::Utils::HeaderHash + end + end +end diff --git a/spec/grape/api/custom_validations_spec.rb b/spec/grape/api/custom_validations_spec.rb index 49571a803e..558da7576d 100644 --- a/spec/grape/api/custom_validations_spec.rb +++ b/spec/grape/api/custom_validations_spec.rb @@ -151,13 +151,13 @@ def validate(request) end def access_header - Grape::Http::Headers.lowercase? ? 'x-access-token' : 'X-Access-Token' + 'x-access-token' end end end let(:app) { Rack::Builder.new(subject) } - let(:x_access_token_header) { Grape::Http::Headers.lowercase? ? 'x-access-token' : 'X-Access-Token' } + let(:x_access_token_header) { 'x-access-token' } before { stub_const('Grape::Validations::Validators::AdminValidator', admin_validator) } diff --git a/spec/grape/dsl/headers_spec.rb b/spec/grape/dsl/headers_spec.rb index 9ced6304ff..154fbf82b2 100644 --- a/spec/grape/dsl/headers_spec.rb +++ b/spec/grape/dsl/headers_spec.rb @@ -11,8 +11,8 @@ class Dummy subject { HeadersSpec::Dummy.new } let(:header_data) do - { 'First Key' => 'First Value', - 'Second Key' => 'Second Value' } + { 'first key' => 'First Value', + 'second key' => 'Second Value' } end context 'when headers are set' do @@ -23,8 +23,8 @@ class Dummy describe 'get' do it 'returns a specifc value' do - expect(subject.header['First Key']).to eq 'First Value' - expect(subject.header['Second Key']).to eq 'Second Value' + expect(subject.header['first key']).to eq 'First Value' + expect(subject.header['second key']).to eq 'Second Value' end it 'returns all set headers' do @@ -35,15 +35,15 @@ class Dummy describe 'set' do it 'returns value' do - expect(subject.header('Third Key', 'Third Value')) - expect(subject.header['Third Key']).to eq 'Third Value' + expect(subject.header('third key', 'Third Value')) + expect(subject.header['third key']).to eq 'Third Value' end end describe 'delete' do it 'deletes a header key-value pair' do - expect(subject.header('First Key')).to eq header_data['First Key'] - expect(subject.header).not_to have_key('First Key') + expect(subject.header('first key')).to eq header_data['first key'] + expect(subject.header).not_to have_key('first key') end end end @@ -52,8 +52,8 @@ class Dummy context 'when no headers are set' do describe '#header' do it 'returns nil' do - expect(subject.header['First Key']).to be_nil - expect(subject.header('First Key')).to be_nil + expect(subject.header['first key']).to be_nil + expect(subject.header('first key')).to be_nil end end end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index c2443cbe7c..8b217af9e5 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -138,15 +138,18 @@ def app it 'includes request headers' do get '/headers' + cookie_header = Grape::Http::Headers.lowercase? ? 'cookie' : 'Cookie' + host_header = Grape::Http::Headers.lowercase? ? 'host' : 'Host' + expect(JSON.parse(last_response.body)).to include( - 'Host' => 'example.org', - 'Cookie' => '' + host_header => 'example.org', + cookie_header => '' ) end it 'includes additional request headers' do get '/headers', nil, 'HTTP_X_GRAPE_CLIENT' => '1' - x_grape_client_header = Grape::Http::Headers.lowercase? ? 'x-grape-client' : 'X-Grape-Client' + x_grape_client_header = 'x-grape-client' expect(JSON.parse(last_response.body)[x_grape_client_header]).to eq('1') end @@ -154,7 +157,7 @@ def app env = Rack::MockRequest.env_for('/headers') env[:HTTP_SYMBOL_HEADER] = 'Goliath passes symbols' body = read_chunks(subject.call(env)[2]).join - symbol_header = Grape::Http::Headers.lowercase? ? 'symbol-header' : 'Symbol-Header' + symbol_header = 'symbol-header' expect(JSON.parse(body)[symbol_header]).to eq('Goliath passes symbols') end end diff --git a/spec/grape/request_spec.rb b/spec/grape/request_spec.rb index 94d9f505a0..eaf9a6ac97 100644 --- a/spec/grape/request_spec.rb +++ b/spec/grape/request_spec.rb @@ -90,7 +90,7 @@ module Grape } end let(:x_grape_is_cool_header) do - Grape::Http::Headers.lowercase? ? 'x-grape-is-cool' : 'X-Grape-Is-Cool' + 'x-grape-is-cool' end it 'cuts HTTP_ prefix and capitalizes header name words' do @@ -120,7 +120,7 @@ module Grape default_env.merge(request_headers) end let(:grape_likes_symbolic_header) do - Grape::Http::Headers.lowercase? ? 'grape-likes-symbolic' : 'Grape-Likes-Symbolic' + 'grape-likes-symbolic' end it 'converts them to string' do