Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Hash with Rack::Header/Rack::Utils::HeaderHash for Grape Headers #2425

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/dsl/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 3 additions & 9 deletions lib/grape/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
13 changes: 13 additions & 0 deletions lib/grape/util/header.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions spec/grape/api/custom_validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down
20 changes: 10 additions & 10 deletions spec/grape/dsl/headers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
11 changes: 7 additions & 4 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,23 +138,26 @@ 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

it 'includes headers passed as symbols' do
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
Expand Down
4 changes: 2 additions & 2 deletions spec/grape/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading