Skip to content

Commit e459e81

Browse files
ericproulxericproulx
committed
Use Rack constants instead of redefining it
Use Grape and Rack header constant in specs Drop self.lowercase? and rack2 integration since we're using Grape::Util::Header Change rack_3_0' integration tests to make sure we are returning lowercase headers
1 parent 96c2a91 commit e459e81

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+418
-373
lines changed

‎benchmark/large_model.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ def self.vrp_request_schedule(this)
233233
puts Grape::VERSION
234234

235235
options = {
236-
method: 'POST',
236+
method: Rack::POST,
237237
params: JSON.parse(File.read('benchmark/resource/vrp_example.json'))
238238
}
239239

‎benchmark/nested_params.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class API < Grape::API
2121
end
2222

2323
options = {
24-
method: 'POST',
24+
method: Rack::POST,
2525
params: {
2626
address: {
2727
street: 'Alexis Pl.',

‎benchmark/remounting.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class CommentAPI < Grape::API
2828
mount VotingApi
2929
end
3030

31-
env = Rack::MockRequest.env_for('/votes', method: 'GET')
31+
env = Rack::MockRequest.env_for('/votes', method: Rack::GET)
3232

3333
Benchmark.memory do |api|
3434
calls = 1000

‎benchmark/simple.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class API < Grape::API
1313
end
1414

1515
options = {
16-
method: 'GET'
16+
method: Rack::GET
1717
}
1818

1919
env = Rack::MockRequest.env_for('/api/v1', options)

‎docker-compose.yml

-2
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,3 @@ services:
1515
volumes:
1616
- .:/var/grape
1717
- gems:/usr/local/bundle
18-
environment:
19-
GEMFILE: multi_xml

‎lib/grape/api/instance.rb

+10-6
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,13 @@ def initialize
160160

161161
# Handle a request. See Rack documentation for what `env` is.
162162
def call(env)
163-
result = @router.call(env)
164-
result[1].delete(Grape::Http::Headers::X_CASCADE) unless cascade?
165-
result
163+
status, headers, response = @router.call(env)
164+
unless cascade?
165+
headers = Grape::Util::Header.new.merge(headers)
166+
headers.delete(Grape::Http::Headers::X_CASCADE)
167+
end
168+
169+
[status, headers, response]
166170
end
167171

168172
# Some requests may return a HTTP 404 error if grape cannot find a matching
@@ -201,11 +205,11 @@ def add_head_not_allowed_methods_and_options_methods
201205

202206
allowed_methods = config[:methods].dup
203207

204-
allowed_methods |= [Grape::Http::Headers::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Grape::Http::Headers::GET)
208+
allowed_methods |= [Rack::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Rack::GET)
205209

206-
allow_header = (self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Grape::Http::Headers::OPTIONS] | allowed_methods)
210+
allow_header = (self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Rack::OPTIONS] | allowed_methods)
207211

208-
config[:endpoint].options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Grape::Http::Headers::OPTIONS)
212+
config[:endpoint].options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Rack::OPTIONS)
209213

210214
attributes = config.merge(allowed_methods: allowed_methods, allow_header: allow_header)
211215
generate_not_allowed_method(config[:pattern], **attributes)

‎lib/grape/dsl/inside_route.rb

+8-5
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ def redirect(url, permanent: false, body: nil, **_options)
200200
if permanent
201201
status 301
202202
body_message ||= "This resource has been moved permanently to #{url}."
203-
elsif env[Grape::Http::Headers::HTTP_VERSION] == 'HTTP/1.1' && request.request_method.to_s.upcase != Grape::Http::Headers::GET
203+
elsif http_version == 'HTTP/1.1' && !request.get?
204204
status 303
205205
body_message ||= "An alternate resource is located at #{url}."
206206
else
@@ -226,10 +226,9 @@ def status(status = nil)
226226
when nil
227227
return @status if instance_variable_defined?(:@status) && @status
228228

229-
case request.request_method.to_s.upcase
230-
when Grape::Http::Headers::POST
229+
if request.post?
231230
201
232-
when Grape::Http::Headers::DELETE
231+
elsif request.delete?
233232
if instance_variable_defined?(:@body) && @body.present?
234233
200
235234
else
@@ -351,7 +350,7 @@ def stream(value = nil)
351350
return if value.nil? && @stream.nil?
352351

353352
header Rack::CONTENT_LENGTH, nil
354-
header Grape::Http::Headers::TRANSFER_ENCODING, nil
353+
header Rack::TRANSFER_ENCODING, nil
355354
header Rack::CACHE_CONTROL, 'no-cache' # Skips ETag generation (reading the response up front)
356355
if value.is_a?(String)
357356
file_body = Grape::ServeStream::FileBody.new(value)
@@ -458,6 +457,10 @@ def entity_representation_for(entity_class, object, options)
458457
embeds[:version] = env[Grape::Env::API_VERSION] if env.key?(Grape::Env::API_VERSION)
459458
entity_class.represent(object, **embeds.merge(options))
460459
end
460+
461+
def http_version
462+
env['HTTP_VERSION'] || env[Rack::SERVER_PROTOCOL]
463+
end
461464
end
462465
end
463466
end

‎lib/grape/endpoint.rb

+3-2
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def mount_in(router)
151151
reset_routes!
152152
routes.each do |route|
153153
methods = [route.request_method]
154-
methods << Grape::Http::Headers::HEAD if !namespace_inheritable(:do_not_route_head) && route.request_method == Grape::Http::Headers::GET
154+
methods << Rack::HEAD if !namespace_inheritable(:do_not_route_head) && route.request_method == Rack::GET
155155
methods.each do |method|
156156
route = Grape::Router::Route.new(method, route.origin, **route.attributes.to_h) unless route.request_method == method
157157
router.append(route.apply(self))
@@ -280,6 +280,7 @@ def build_stack(helpers)
280280
stack = Grape::Middleware::Stack.new
281281

282282
stack.use Rack::Head
283+
stack.use Rack::Lint
283284
stack.use Class.new(Grape::Middleware::Error),
284285
helpers: helpers,
285286
format: namespace_inheritable(:format),
@@ -401,7 +402,7 @@ def validations
401402

402403
def options?
403404
options[:options_route_enabled] &&
404-
env[Grape::Http::Headers::REQUEST_METHOD] == Grape::Http::Headers::OPTIONS
405+
env[Rack::REQUEST_METHOD] == Rack::OPTIONS
405406
end
406407

407408
def method_missing(name, *_args)

‎lib/grape/env.rb

-5
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,6 @@ module Env
1111
API_VENDOR = 'api.vendor'
1212
API_FORMAT = 'api.format'
1313

14-
RACK_INPUT = 'rack.input'
15-
RACK_REQUEST_QUERY_HASH = 'rack.request.query_hash'
16-
RACK_REQUEST_FORM_HASH = 'rack.request.form_hash'
17-
RACK_REQUEST_FORM_INPUT = 'rack.request.form_input'
18-
1914
GRAPE_REQUEST = 'grape.request'
2015
GRAPE_REQUEST_HEADERS = 'grape.request.headers'
2116
GRAPE_REQUEST_PARAMS = 'grape.request.params'

‎lib/grape/http/headers.rb

+17-36
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,25 @@
33
module Grape
44
module Http
55
module Headers
6-
# https://github.com/rack/rack/blob/master/lib/rack.rb
7-
HTTP_VERSION = 'HTTP_VERSION'
8-
PATH_INFO = 'PATH_INFO'
9-
REQUEST_METHOD = 'REQUEST_METHOD'
10-
QUERY_STRING = 'QUERY_STRING'
11-
12-
def self.lowercase?
13-
Rack::CONTENT_TYPE == 'content-type'
14-
end
15-
16-
if lowercase?
17-
ALLOW = 'allow'
18-
LOCATION = 'location'
19-
TRANSFER_ENCODING = 'transfer-encoding'
20-
X_CASCADE = 'x-cascade'
21-
else
22-
ALLOW = 'Allow'
23-
LOCATION = 'Location'
24-
TRANSFER_ENCODING = 'Transfer-Encoding'
25-
X_CASCADE = 'X-Cascade'
26-
end
27-
28-
GET = 'GET'
29-
POST = 'POST'
30-
PUT = 'PUT'
31-
PATCH = 'PATCH'
32-
DELETE = 'DELETE'
33-
HEAD = 'HEAD'
34-
OPTIONS = 'OPTIONS'
35-
36-
SUPPORTED_METHODS = [GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS].freeze
37-
SUPPORTED_METHODS_WITHOUT_OPTIONS = Grape::Util::Lazy::Object.new { [GET, POST, PUT, PATCH, DELETE, HEAD].freeze }
38-
39-
HTTP_ACCEPT_VERSION = 'HTTP_ACCEPT_VERSION'
6+
HTTP_ACCEPT_VERSION = 'HTTP_ACCEPT_VERSION'
7+
HTTP_ACCEPT = 'HTTP_ACCEPT'
408
HTTP_TRANSFER_ENCODING = 'HTTP_TRANSFER_ENCODING'
41-
HTTP_ACCEPT = 'HTTP_ACCEPT'
429

43-
FORMAT = 'format'
10+
ALLOW = 'Allow'
11+
LOCATION = 'Location'
12+
X_CASCADE = 'X-Cascade'
13+
14+
SUPPORTED_METHODS = [
15+
Rack::GET,
16+
Rack::POST,
17+
Rack::PUT,
18+
Rack::PATCH,
19+
Rack::DELETE,
20+
Rack::HEAD,
21+
Rack::OPTIONS
22+
].freeze
23+
24+
SUPPORTED_METHODS_WITHOUT_OPTIONS = (SUPPORTED_METHODS - [Rack::OPTIONS]).freeze
4425

4526
HTTP_HEADERS = Grape::Util::Lazy::Object.new do
4627
common_http_headers = %w[

‎lib/grape/middleware/error.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def call!(env)
4040

4141
def rack_response(status, headers, message)
4242
message = Rack::Utils.escape_html(message) if headers[Rack::CONTENT_TYPE] == TEXT_HTML
43-
Rack::Response.new(Array.wrap(message), Rack::Utils.status_code(status), headers)
43+
Rack::Response.new(Array.wrap(message), Rack::Utils.status_code(status), Grape::Util::Header.new.merge(headers))
4444
end
4545

4646
def format_message(message, backtrace, original_exception = nil)

‎lib/grape/middleware/formatter.rb

+9-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module Grape
44
module Middleware
55
class Formatter < Base
66
CHUNKED = 'chunked'
7+
FORMAT = 'format'
78

89
def default_options
910
{
@@ -80,7 +81,7 @@ def read_body_input
8081
!request.parseable_data? &&
8182
(request.content_length.to_i.positive? || request.env[Grape::Http::Headers::HTTP_TRANSFER_ENCODING] == CHUNKED)
8283

83-
return unless (input = env[Grape::Env::RACK_INPUT])
84+
return unless (input = env[Rack::RACK_INPUT])
8485

8586
input.rewind
8687
body = env[Grape::Env::API_REQUEST_INPUT] = input.read
@@ -101,12 +102,12 @@ def read_rack_input(body)
101102
begin
102103
body = (env[Grape::Env::API_REQUEST_BODY] = parser.call(body, env))
103104
if body.is_a?(Hash)
104-
env[Grape::Env::RACK_REQUEST_FORM_HASH] = if env.key?(Grape::Env::RACK_REQUEST_FORM_HASH)
105-
env[Grape::Env::RACK_REQUEST_FORM_HASH].merge(body)
106-
else
107-
body
108-
end
109-
env[Grape::Env::RACK_REQUEST_FORM_INPUT] = env[Grape::Env::RACK_INPUT]
105+
env[Rack::RACK_REQUEST_FORM_HASH] = if env.key?(Rack::RACK_REQUEST_FORM_HASH)
106+
env[Rack::RACK_REQUEST_FORM_HASH].merge(body)
107+
else
108+
body
109+
end
110+
env[Rack::RACK_REQUEST_FORM_INPUT] = env[Rack::RACK_INPUT]
110111
end
111112
rescue Grape::Exceptions::Base => e
112113
raise e
@@ -139,7 +140,7 @@ def format_from_extension
139140
end
140141

141142
def format_from_params
142-
fmt = Rack::Utils.parse_nested_query(env[Grape::Http::Headers::QUERY_STRING])[Grape::Http::Headers::FORMAT]
143+
fmt = Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[FORMAT]
143144
# avoid symbol memory leak on an unknown format
144145
return fmt.to_sym if content_type_for(fmt)
145146

‎lib/grape/middleware/globals.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ def before
77
request = Grape::Request.new(@env, build_params_with: @options[:build_params_with])
88
@env[Grape::Env::GRAPE_REQUEST] = request
99
@env[Grape::Env::GRAPE_REQUEST_HEADERS] = request.headers
10-
@env[Grape::Env::GRAPE_REQUEST_PARAMS] = request.params if @env[Grape::Env::RACK_INPUT]
10+
@env[Grape::Env::GRAPE_REQUEST_PARAMS] = request.params if @env[Rack::RACK_INPUT]
1111
end
1212
end
1313
end

‎lib/grape/middleware/versioner/param.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ def default_options
2828
end
2929

3030
def before
31-
potential_version = Rack::Utils.parse_nested_query(env[Grape::Http::Headers::QUERY_STRING])[paramkey]
31+
potential_version = Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[paramkey]
3232
return if potential_version.nil?
3333

3434
throw :error, status: 404, message: '404 API Version Not Found', headers: { Grape::Http::Headers::X_CASCADE => 'pass' } if options[:versions] && !options[:versions].find { |v| v.to_s == potential_version }
3535
env[Grape::Env::API_VERSION] = potential_version
36-
env[Grape::Env::RACK_REQUEST_QUERY_HASH].delete(paramkey) if env.key? Grape::Env::RACK_REQUEST_QUERY_HASH
36+
env[Rack::RACK_REQUEST_QUERY_HASH].delete(paramkey) if env.key? Rack::RACK_REQUEST_QUERY_HASH
3737
end
3838

3939
private

‎lib/grape/middleware/versioner/path.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def default_options
2424
end
2525

2626
def before
27-
path = env[Grape::Http::Headers::PATH_INFO].dup
27+
path = env[Rack::PATH_INFO].dup
2828
path.sub!(mount_path, '') if mounted_path?(path)
2929

3030
if prefix && path.index(prefix) == 0 # rubocop:disable all

‎lib/grape/router.rb

+3-3
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ def transaction(env)
9696

9797
# If last_neighbor_route exists and request method is OPTIONS,
9898
# return response by using #call_with_allow_headers.
99-
return call_with_allow_headers(env, last_neighbor_route) if last_neighbor_route && method == Grape::Http::Headers::OPTIONS && !cascade
99+
return call_with_allow_headers(env, last_neighbor_route) if last_neighbor_route && method == Rack::OPTIONS && !cascade
100100

101101
route = match?(input, '*')
102102

@@ -123,8 +123,8 @@ def make_routing_args(default_args, route, input)
123123
end
124124

125125
def extract_input_and_method(env)
126-
input = string_for(env[Grape::Http::Headers::PATH_INFO])
127-
method = env[Grape::Http::Headers::REQUEST_METHOD]
126+
input = string_for(env[Rack::PATH_INFO])
127+
method = env[Rack::REQUEST_METHOD]
128128
[input, method]
129129
end
130130

‎spec/grape/api/custom_validations_spec.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def validate_param!(attr_name, params)
7171
let(:in_body_validator) do
7272
Class.new(Grape::Validations::Validators::PresenceValidator) do
7373
def validate(request)
74-
validate!(request.env['api.request.body'])
74+
validate!(request.env[Grape::Env::API_REQUEST_BODY])
7575
end
7676
end
7777
end

‎spec/grape/api/defines_boolean_in_params_spec.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
end
2525

2626
context 'Params endpoint type' do
27-
subject { app.new.router.map['POST'].first.options[:params]['message'][:type] }
27+
subject { app.new.router.map[Rack::POST].first.options[:params]['message'][:type] }
2828

2929
it 'params type is a boolean' do
3030
expect(subject).to eq 'Grape::API::Boolean'

‎spec/grape/api/patch_method_helpers_spec.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@ def app
4949

5050
context 'patch' do
5151
it 'public' do
52-
patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
52+
patch '/', {}, Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.grape-public-v1+json'
5353
expect(last_response.status).to eq 405
5454
end
5555

5656
it 'private' do
57-
patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
57+
patch '/', {}, Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.grape-private-v1+json'
5858
expect(last_response.status).to eq 405
5959
end
6060

@@ -66,13 +66,13 @@ def app
6666

6767
context 'default' do
6868
it 'public' do
69-
get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
69+
get '/', {}, Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.grape-public-v1+json'
7070
expect(last_response.status).to eq 200
7171
expect(last_response.body).to eq({ ok: 'public' }.to_json)
7272
end
7373

7474
it 'private' do
75-
get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
75+
get '/', {}, Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.grape-private-v1+json'
7676
expect(last_response.status).to eq 200
7777
expect(last_response.body).to eq({ ok: 'private' }.to_json)
7878
end

0 commit comments

Comments
 (0)