-
Notifications
You must be signed in to change notification settings - Fork 323
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
add support for users to provide custom uri normalizer #533
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# frozen_string_literal: true | ||
|
||
module HTTP | ||
module Features | ||
class NormalizeUri < Feature | ||
attr_reader :normalizer | ||
|
||
def initialize(normalizer: Normalizer) | ||
@normalizer = normalizer | ||
end | ||
|
||
def normalize_uri(uri) | ||
normalizer.call(uri) | ||
end | ||
|
||
module Normalizer | ||
def self.call(uri) | ||
HTTP::URI::NORMALIZER.call(uri) | ||
end | ||
end | ||
|
||
HTTP::Options.register_feature(:normalize_uri, self) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,20 +66,24 @@ class UnsupportedSchemeError < RequestError; end | |
# Scheme is normalized to be a lowercase symbol e.g. :http, :https | ||
attr_reader :scheme | ||
|
||
attr_reader :uri_normalizer | ||
|
||
# "Request URI" as per RFC 2616 | ||
# http://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html | ||
attr_reader :uri | ||
attr_reader :proxy, :body, :version | ||
|
||
# @option opts [String] :version | ||
# @option opts [#to_s] :verb HTTP request method | ||
# @option opts [#to_s] :uri_normalizer uri normalizer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. description is redundant. It's obvious from the key name. |
||
# @option opts [HTTP::URI, #to_s] :uri | ||
# @option opts [Hash] :headers | ||
# @option opts [Hash] :proxy | ||
# @option opts [String, Enumerable, IO, nil] :body | ||
def initialize(opts) | ||
@verb = opts.fetch(:verb).to_s.downcase.to_sym | ||
@uri = normalize_uri(opts.fetch(:uri)) | ||
@uri_normalizer = opts[:normalize_uri] ? opts.fetch(:normalize_uri) : HTTP::URI::NORMALIZER | ||
@uri = @uri_normalizer.call(opts.fetch(:uri)) | ||
mamoonraja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@scheme = @uri.scheme.to_s.downcase.to_sym if @uri.scheme | ||
|
||
raise(UnsupportedMethodError, "unknown method: #{verb}") unless METHODS.include?(@verb) | ||
|
@@ -212,18 +216,5 @@ def port | |
def default_host_header_value | ||
PORTS[@scheme] != port ? "#{host}:#{port}" : host | ||
end | ||
|
||
# @return [HTTP::URI] URI with all componentes but query being normalized. | ||
def normalize_uri(uri) | ||
uri = HTTP::URI.parse uri | ||
|
||
HTTP::URI.new( | ||
:scheme => uri.normalized_scheme, | ||
:authority => uri.normalized_authority, | ||
:path => uri.normalized_path, | ||
:query => uri.query, | ||
:fragment => uri.normalized_fragment | ||
) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -430,6 +430,31 @@ def setsockopt(*args) | |||||
expect(response.to_s).to eq("#{body}-deflated") | ||||||
end | ||||||
end | ||||||
|
||||||
context "with :normalize_uri" do | ||||||
module Normalizer | ||||||
def self.call(uri) | ||||||
uri | ||||||
end | ||||||
end | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't declare module/class dynamically. You basically don't need this at all. |
||||||
|
||||||
it "Use the defaul Uri normalizer when user does not use uri normalizer" do | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. URI should be all caps. And topic of example should start with lowercase. Also typo in defaul. Line 21 in 1eae155
or here: Line 466 in 1eae155
And should look like: it "normalizes URI" do
response = HTTP.get "#{dummy.endpoint}/hello world"
expect(response.to_s).to eq("hello world")
end others examples are OK to be left in here, just make them less "wordy"... |
||||||
response = HTTP.get "#{dummy.endpoint}/hello world" | ||||||
expect(response.to_s).to eq("hello world") | ||||||
end | ||||||
|
||||||
it "Use the custom Uri Normalizer method" do | ||||||
client = HTTP.use(:normalize_uri => {:normalizer => Normalizer}) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said above you don't need
Suggested change
|
||||||
response = client.get("#{dummy.endpoint}/hello world") | ||||||
expect(response.status).to eq(400) | ||||||
end | ||||||
|
||||||
it "Use the default Uri normalizer when user does not specify custom uri normalizer" do | ||||||
client = HTTP.use :normalize_uri | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth to add expectation that default normalizer will be called in here: expect(HTTP::URI::NORMALIZER).to receive(:call).and_call_original |
||||||
response = client.get("#{dummy.endpoint}/hello world") | ||||||
expect(response.to_s).to eq("hello world") | ||||||
end | ||||||
end | ||||||
end | ||||||
|
||||||
it "unifies socket errors into HTTP::ConnectionError" do | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to expose it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am exposing it because we are using it in
wrap_request
method inauto_deflate