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

Respect the params_encoder in Faraday::Adapter::Test #1316

Merged
merged 3 commits into from Aug 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions examples/client_spec.rb
Expand Up @@ -81,4 +81,17 @@ def sushi(jname, params: {})
stubs.verify_stubbed_calls
end
end

context 'When the Faraday is configured with FlatParamsEncoder' do
yykamei marked this conversation as resolved.
Show resolved Hide resolved
let(:conn) { Faraday.new(request: { params_encoder: Faraday::FlatParamsEncoder }) { |b| b.adapter(:test, stubs) } }

it 'handles the same multiple URL parameters' do
stubs.get('/ebi?a=x&a=y&a=z') { [200, { 'Content-Type' => 'application/json' }, '{"name": "shrimp"}'] }

# uncomment to raise Stubs::NotFound
# expect(client.sushi('ebi', params: { a: %w[x y] })).to eq('shrimp')
expect(client.sushi('ebi', params: { a: %w[x y z] })).to eq('shrimp')
stubs.verify_stubbed_calls
end
end
end
43 changes: 41 additions & 2 deletions examples/client_test.rb
Expand Up @@ -13,8 +13,8 @@ def initialize(conn)
@conn = conn
end

def sushi(jname)
res = @conn.get("/#{jname}")
def sushi(jname, params: {})
res = @conn.get("/#{jname}", params)
data = JSON.parse(res.body)
data['name']
end
Expand Down Expand Up @@ -70,6 +70,45 @@ def test_sushi_exception
stubs.verify_stubbed_calls
end

def test_strict_mode
stubs = Faraday::Adapter::Test::Stubs.new(strict_mode: true)
stubs.get('/ebi?abc=123') do
[
200,
{ 'Content-Type': 'application/javascript' },
'{"name": "shrimp"}'
]
end

cli = client(stubs)
assert_equal 'shrimp', cli.sushi('ebi', params: { abc: 123 })

# uncomment to raise Stubs::NotFound
# assert_equal 'shrimp', cli.sushi('ebi', params: { abc: 123, foo: 'Kappa' })
stubs.verify_stubbed_calls
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to add this test in #1298 😅


def test_non_default_params_encoder
stubs = Faraday::Adapter::Test::Stubs.new(strict_mode: true)
stubs.get('/ebi?a=x&a=y&a=z') do
[
200,
{ 'Content-Type': 'application/javascript' },
'{"name": "shrimp"}'
]
end
conn = Faraday.new(request: { params_encoder: Faraday::FlatParamsEncoder }) do |builder|
builder.adapter :test, stubs
end

cli = Client.new(conn)
assert_equal 'shrimp', cli.sushi('ebi', params: { a: %w[x y z] })

# uncomment to raise Stubs::NotFound
# assert_equal 'shrimp', cli.sushi('ebi', params: { a: %w[x y] })
stubs.verify_stubbed_calls
end

def client(stubs)
conn = Faraday.new do |builder|
builder.adapter :test, stubs
Expand Down
73 changes: 30 additions & 43 deletions lib/faraday/adapter/test.rb
Expand Up @@ -62,18 +62,20 @@ def empty?
@stack.empty?
end

def match(request_method, host, path, headers, body)
# @param env [Faraday::Env]
def match(env)
request_method = env[:method]
return false unless @stack.key?(request_method)

stack = @stack[request_method]
consumed = (@consumed[request_method] ||= [])

stub, meta = matches?(stack, host, path, headers, body)
stub, meta = matches?(stack, env)
if stub
consumed << stack.delete(stub)
return stub, meta
end
matches?(consumed, host, path, headers, body)
matches?(consumed, env)
end

def get(path, headers = {}, &block)
Expand Down Expand Up @@ -142,51 +144,41 @@ def new_stub(request_method, path, headers = {}, body = nil, &block)
Faraday::Utils.URI(path).host
]
end

path, query = normalized_path.respond_to?(:split) ? normalized_path.split('?') : normalized_path
headers = Utils::Headers.new(headers)
stub = Stub.new(host, normalized_path, headers, body, @strict_mode, block)

stub = Stub.new(host, path, query, headers, body, @strict_mode, block)
(@stack[request_method] ||= []) << stub
end

def matches?(stack, host, path, headers, body)
# @param stack [Hash]
# @param env [Faraday::Env]
def matches?(stack, env)
stack.each do |stub|
match_result, meta = stub.matches?(host, path, headers, body)
match_result, meta = stub.matches?(env)
return stub, meta if match_result
end
nil
end
end

# Stub request
# rubocop:disable Style/StructInheritance
class Stub < Struct.new(:host, :path, :params, :headers, :body, :strict_mode, :block)
# rubocop:enable Style/StructInheritance
def initialize(host, full, headers, body, strict_mode, block) # rubocop:disable Metrics/ParameterLists
path, query = full.respond_to?(:split) ? full.split('?') : full
params =
if query
Faraday::Utils.parse_nested_query(query)
else
{}
end

super(host, path, params, headers, body, strict_mode, block)
end
class Stub < Struct.new(:host, :path, :query, :headers, :body, :strict_mode, :block) # rubocop:disable Style/StructInheritance
# @param env [Faraday::Env]
def matches?(env)
request_host = env[:url].host
request_path = Faraday::Utils.normalize_path(env[:url].path)
params_encoder = env.request.params_encoder || Faraday::Utils.default_params_encoder
request_params = params_encoder.decode(env[:url].query) || {}
request_headers = env.request_headers
request_body = env[:body]

def matches?(request_host, request_uri, request_headers, request_body)
request_path, request_query = request_uri.split('?')
request_params =
if request_query
Faraday::Utils.parse_nested_query(request_query)
else
{}
end
# meta is a hash used as carrier
# that will be yielded to consumer block
meta = {}
[(host.nil? || host == request_host) &&
path_match?(request_path, meta) &&
params_match?(request_params) &&
params_match?(params_encoder, request_params) &&
(body.to_s.size.zero? || request_body == body) &&
headers_match?(request_headers), meta]
end
Expand All @@ -199,7 +191,9 @@ def path_match?(request_path, meta)
end
end

def params_match?(request_params)
def params_match?(params_encoder, request_params)
params = params_encoder.decode(query) || {}

if strict_mode
return Set.new(params) == Set.new(request_params)
end
Expand Down Expand Up @@ -239,26 +233,19 @@ def configure
yield(stubs)
end

# @param env [Faraday::Env]
def call(env)
super
host = env[:url].host
normalized_path = Faraday::Utils.normalize_path(env[:url])
params_encoder = env.request.params_encoder ||
Faraday::Utils.default_params_encoder

stub, meta = stubs.match(env[:method], host, normalized_path,
env.request_headers, env[:body])
stub, meta = stubs.match(env)

unless stub
normalized_path = Faraday::Utils.normalize_path(env[:url])
raise Stubs::NotFound, "no stubbed request for #{env[:method]} "\
"#{normalized_path} #{env[:body]}"
end

env[:params] = if (query = env[:url].query)
params_encoder.decode(query)
else
{}
end
params_encoder = env.request.params_encoder || Faraday::Utils.default_params_encoder
env[:params] = params_encoder.decode(env[:url].query) || {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmmh I really like how much simpler the matches? methods signature are now, thanks to the env being passed, but I notice there's a lot a of repeated code between here and there. These two lines for example, and line 242 as well.

I wonder if instead of reusing env it might make sense to introduce a new Struct, internal and only used in the Test adapter, that holds this information so we only need to calculate it once. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your quick review! It makes sense 👍 I'll try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to avoid similar codes with 91689f0. How about the change?

block_arity = stub.block.arity
status, headers, body =
if block_arity >= 0
Expand Down
32 changes: 32 additions & 0 deletions spec/faraday/adapter/test_spec.rb
Expand Up @@ -258,6 +258,38 @@ def make_request
end
end

describe 'for request with non default params encoder' do
let(:connection) do
Faraday.new(request: { params_encoder: Faraday::FlatParamsEncoder }) do |builder|
builder.adapter :test, stubs
end
end
let(:stubs) do
described_class::Stubs.new do |stubs|
stubs.get('/path?a=x&a=y&a=z') { [200, {}, 'a'] }
end
end

context 'when all flat param values are correctly set' do
subject(:request) { connection.get('/path?a=x&a=y&a=z') }

it { expect(request.status).to eq 200 }
end

shared_examples 'raise NotFound when params do not satisfy the flat param values' do |params|
subject(:request) { connection.get('/path', params) }

context "with #{params.inspect}" do
it { expect { request }.to raise_error described_class::Stubs::NotFound }
end
end

it_behaves_like 'raise NotFound when params do not satisfy the flat param values', { a: %w[x] }
it_behaves_like 'raise NotFound when params do not satisfy the flat param values', { a: %w[x y] }
it_behaves_like 'raise NotFound when params do not satisfy the flat param values', { a: %w[x z y] } # NOTE: The order of the value is also compared.
it_behaves_like 'raise NotFound when params do not satisfy the flat param values', { b: %w[x y z] }
end

describe 'strict_mode' do
let(:stubs) do
described_class::Stubs.new(strict_mode: true) do |stubs|
Expand Down