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

Rack 3 support #277

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ unless ENV["CI"]
gem "yard-junk"
end

gem "hanami-devtools", github: "hanami/devtools", branch: "main"
gem "hanami-devtools", github: "kyleplump/devtools", branch: "rack3"
# gem "hanami-devtools", github: "hanami/devtools", branch: "main"
gem "rexml"
4 changes: 2 additions & 2 deletions hanami-router.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ Gem::Specification.new do |spec|
spec.metadata["rubygems_mfa_required"] = "true"
spec.required_ruby_version = ">= 3.1"

spec.add_dependency "rack", "~> 2.0"
spec.add_dependency "rack", ">= 2.0"
spec.add_dependency "mustermann", "~> 3.0"
spec.add_dependency "mustermann-contrib", "~> 3.0"

spec.add_development_dependency "bundler", ">= 1.6", "< 3"
spec.add_development_dependency "rake", "~> 13"
spec.add_development_dependency "rack-test", "~> 1.0"
spec.add_development_dependency "rack-test", "~> 2.0"
spec.add_development_dependency "rspec", "~> 3.8"

spec.add_development_dependency "rubocop", "~> 1.0"
Expand Down
6 changes: 3 additions & 3 deletions lib/hanami/middleware/body_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def initialize(app, parsers)
end

def call(env)
body = env[RACK_INPUT].read
return @app.call(env) if body.empty?
body = env[RACK_INPUT]&.read
return @app.call(env) if body.nil? || body.empty?

env[RACK_INPUT].rewind # somebody might try to read this stream
Rack::RewindableInput.new(env[RACK_INPUT]).rewind # somebody might try to read this stream

if (parser = @parsers[media_type(env)])
env[Router::ROUTER_PARSED_BODY] = parser.parse(body, env)
Expand Down
1 change: 1 addition & 0 deletions lib/hanami/middleware/body_parser/form_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def self.mime_types
# @since 2.0.1
# @api private
def parse(*, env)
env[::Rack::RACK_INPUT].rewind if env[::Rack::RACK_INPUT].respond_to?(:rewind)
::Rack::Multipart.parse_multipart(env)
rescue StandardError => exception
raise BodyParsingError.new(exception.message)
Expand Down
17 changes: 13 additions & 4 deletions lib/hanami/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class Router
require "hanami/router/url_helpers"
require "hanami/router/globbed_path"
require "hanami/router/mounted_path"
require "hanami/router/rack_utils"

# URL helpers for other Hanami integrations
#
Expand Down Expand Up @@ -739,7 +740,11 @@ def env_for(env, params = {}, options = {})

# @since 2.0.0
# @api private
HTTP_HEADER_LOCATION = "Location"
HTTP_HEADER_LOCATION = modern_rack? ? "location" : "Location"

# @since 2.2.0
# @api private
HTTP_HEADER_ALLOW = modern_rack? ? "allow" : "Allow"

# @since 2.0.0
# @api private
Expand All @@ -762,7 +767,7 @@ def env_for(env, params = {}, options = {})
HTTP_STATUS_NOT_ALLOWED,
{
::Rack::CONTENT_LENGTH => HTTP_BODY_NOT_ALLOWED_LENGTH,
"Allow" => allowed_http_methods.join(", ")
HTTP_HEADER_ALLOW => allowed_http_methods.join(", ")
},
[HTTP_BODY_NOT_ALLOWED]
]
Expand All @@ -774,12 +779,13 @@ def env_for(env, params = {}, options = {})
# @since 2.0.0
NOT_FOUND = ->(*) {
[HTTP_STATUS_NOT_FOUND, {::Rack::CONTENT_LENGTH => HTTP_BODY_NOT_FOUND_LENGTH}, [HTTP_BODY_NOT_FOUND]]
}.freeze
}

# @since 2.0.0
# @api private
def lookup(env)
endpoint = fixed(env)

return [endpoint, {}] if endpoint

variable(env) || globbed_or_mounted(env)
Expand Down Expand Up @@ -916,15 +922,18 @@ def _redirect(to, code)
def _params(env, params)
params ||= {}
env[PARAMS] ||= {}
input = Rack::RewindableInput.new(env[::Rack::RACK_INPUT]) if env[::Rack::RACK_INPUT]

if !env.key?(ROUTER_PARSED_BODY) && (input = env[::Rack::RACK_INPUT]) and input.rewind
if !env.key?(ROUTER_PARSED_BODY) && input
env[PARAMS].merge!(::Rack::Utils.parse_nested_query(input.read))
input.rewind
env[::Rack::RACK_INPUT] = input
end

env[PARAMS].merge!(::Rack::Utils.parse_nested_query(env[::Rack::QUERY_STRING]))
env[PARAMS].merge!(params)
env[PARAMS] = Params.deep_symbolize(env[PARAMS])

env
end

Expand Down
4 changes: 4 additions & 0 deletions lib/hanami/router/block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ def params
# @since 2.0.0
def call
body = instance_exec(&@blk)

# explicitly set 'content-length' header
headers[::Rack::CONTENT_LENGTH] = body.size.to_s

[status, headers, [body]]
end
end
Expand Down
11 changes: 11 additions & 0 deletions lib/hanami/router/rack_utils.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

module Hanami
class Router
# @since 2.2.0
# @api private
def self.modern_rack?
defined?(Rack::Headers)
end
end
end
8 changes: 4 additions & 4 deletions spec/integration/hanami/router/mount_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
Hanami::Router.new do
mount Api::App.new, at: "/api"
mount Backend::App, at: "/backend"
mount ->(*) { [200, {"Content-Length" => "4"}, ["proc"]] }, at: "/proc"
mount ->(*) { [200, {"Content-Length" => "8"}, ["trailing"]] }, at: "/trailing/"
mount ->(*) { [200, RSpec::Support::HTTP.headers({"Content-Length" => "4"}), ["proc"]] }, at: "/proc"
mount ->(*) { [200, RSpec::Support::HTTP.headers({"Content-Length" => "8"}), ["trailing"]] }, at: "/trailing/"
mount Api::App.new, at: "/"
end
end
Expand Down Expand Up @@ -64,7 +64,7 @@
Hanami::Router.new do
mount Api::App.new, at: "/api"

get "/*any", to: ->(*) { [200, {"Content-Length" => "4"}, ["home"]] }
get "/*any", to: ->(*) { [200, RSpec::Support::HTTP.headers({"Content-Length" => "4"}), ["home"]] }
end
end
let(:app) { Rack::MockRequest.new(router) }
Expand All @@ -76,7 +76,7 @@
context "with more-specific glob before root-level mount" do
let(:router) do
Hanami::Router.new do
get "/home/*any", to: ->(*) { [200, {"Content-Length" => "4"}, ["home"]] }
get "/home/*any", to: ->(*) { [200, RSpec::Support::HTTP.headers({"Content-Length" => "4"}), ["home"]] }

mount Api::App.new, at: "/"
end
Expand Down
1 change: 1 addition & 0 deletions spec/integration/hanami/router/params_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def multipart_fixture(filename, boundary = Rack::Multipart::MULTIPART_BOUNDARY)
path = fixture_path(filename)
file = Rack::Multipart::UploadedFile.new(path)
data = Rack::Multipart.build_multipart("file" => file)

env = Rack::MockRequest.env_for(
"/submit",
"CONTENT_TYPE" => "multipart/form-data; boundary=#{boundary}",
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/hanami/router/pass_on_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
RSpec.describe "Pass on response" do
let(:app) { Rack::MockRequest.new(routes) }
let(:routes) do
Hanami::Router.new { get "/", to: ->(*) { [200, {"Content-Length" => "2"}, ["OK"]] } }
Hanami::Router.new { get "/", to: ->(*) { [200, RSpec::Support::HTTP.headers({"Content-Length" => "2"}), ["OK"]] } }
end

it "is successful" do
Expand Down
8 changes: 7 additions & 1 deletion spec/integration/hanami/router/prefix_option_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,14 @@
env = Rack::MockRequest.env_for("/admin/redirect")
status, headers, = subject.call(env)

location_header = if Hanami::Router.modern_rack?
headers.fetch("location")
else
headers["Location"]
end

expect(status).to eq(301)
expect(headers["Location"]).to eq("/admin/redirect_destination")
expect(location_header).to eq("/admin/redirect_destination")
end
end
end
36 changes: 18 additions & 18 deletions spec/integration/hanami/router/routing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,55 +27,55 @@

context "path recognition" do
context "root trailing slash" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "14"}, "Trailing root!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "14"}), "Trailing root!") }

it "recognizes" do
expect(app.request(verb.upcase, "/", lint: true)).to eq_response(response)
end
end

context "trailing slash" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "9"}, "Trailing!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "9"}), "Trailing!") }

it "recognizes" do
expect(app.request(verb.upcase, "/trailing/", lint: true)).to eq_response(response)
end
end

context "fixed string" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "6"}, "Fixed!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "6"}), "Fixed!") }

it "recognizes" do
expect(app.request(verb.upcase, "/hanami", lint: true)).to eq_response(response)
end
end

context "moving parts string" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "7"}, "Moving!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "7"}), "Moving!") }

it "recognizes" do
expect(app.request(verb.upcase, "/hanami/23", lint: true)).to eq_response(response)
end
end

context "globbing string" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "9"}, "Globbing!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "9"}), "Globbing!") }

it "recognizes" do
expect(app.request(verb.upcase, "/hanami/all", lint: true)).to eq_response(response)
end
end

context "format string" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "7"}, "Format!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "7"}), "Format!") }

it "recognizes" do
expect(app.request(verb.upcase, "/hanami/all.json", lint: true)).to eq_response(response)
end
end

context "block" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "6"}, "Block!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "6"}), "Block!") }

it "recognizes" do
expect(app.request(verb.upcase, "/block", lint: true)).to eq_response(response)
Expand All @@ -84,7 +84,7 @@
end

describe "constraints" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "24"}, "Moving with constraints!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "24"}), "Moving with constraints!") }

it "recognize when called with matching constraints" do
expect(app.request(verb.upcase, "/books/23", lint: true)).to eq_response(response)
Expand All @@ -94,7 +94,7 @@

context "named routes" do
context "symbol" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "12"}, "Named route!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "12"}), "Named route!") }

it "recognizes by the given symbol" do
expect(router.path(:"#{verb}_named_route")).to eq("/named_route")
Expand All @@ -103,7 +103,7 @@
end

context "compiled variables" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "13"}, "Named %route!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "13"}), "Named %route!") }

it "recognizes" do
expect(router.path(:"#{verb}_named_route_var", var: "route")).to eq("/named_route")
Expand All @@ -112,7 +112,7 @@
end

context "custom url parts" do
let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "30"}, "Named route with custom parts!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "30"}), "Named route with custom parts!") }

it "recognizes" do
r = response
Expand All @@ -127,7 +127,7 @@

context "#HEAD" do
let(:app) { Rack::MockRequest.new(Rack::Head.new(router)) }
let(:response) { Rack::MockResponse.new(405, {"Content-Length" => "18", "Allow" => verb.upcase}, []) }
let(:response) { Rack::MockResponse.new(405, RSpec::Support::HTTP.headers({"Content-Length" => "18", "Allow" => verb.upcase}), []) }

context "path recognition" do
context "fixed string" do
Expand Down Expand Up @@ -186,14 +186,14 @@
end
end

let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "6"}, "Fixed!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "6"}), "Fixed!") }

it "recognizes" do
actual = app.request("GET", "/", lint: true)

expect(actual.status).to eq(response.status)
expect(actual.header).to eq(response.header)
expect(actual.body).to eq(response.body)
expect(actual.original_headers).to eq(response.original_headers)
expect(actual.body).to eq(response.body)
end

it "recognizes by :root" do
Expand All @@ -211,14 +211,14 @@
end
end

let(:response) { Rack::MockResponse.new(200, {"Content-Length" => "6"}, "Block!") }
let(:response) { Rack::MockResponse.new(200, RSpec::Support::HTTP.headers({"Content-Length" => "6"}), "Block!") }

it "recognizes" do
actual = app.request("GET", "/", lint: true)

expect(actual.status).to eq(response.status)
expect(actual.header).to eq(response.header)
expect(actual.body).to eq(response.body)
expect(actual.original_headers).to eq(response.original_headers)
expect(actual.body).to eq(response.body)
end
end
end
Expand Down
14 changes: 14 additions & 0 deletions spec/support/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ def self.testable?(mounted, requested)
(mounted == requested) ||
(mounted == "get" && requested == "head")
end

def self.headers(expected)
if Hanami::Router.modern_rack?
headers = Rack::Headers.new

expected.each do |k, v|
headers[k] = v
end

headers
else
expected
end
end
end
end
end
2 changes: 1 addition & 1 deletion spec/unit/hanami/middleware/trie_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

RSpec.describe Hanami::Middleware::Trie do
subject { described_class.new(app) }
let(:app) { -> (*) { [200, {"content-length" => "2"}, ["OK"]] } }
let(:app) { -> (*) { [200, {"Content-Length" => "2"}, ["OK"]] } }

describe "#initialize" do
it "returns an instance of #{described_class}" do
Expand Down
Loading