From 52729064439d28b1fa3e95c441c3a54d3c14f96f Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 17:51:42 +0000 Subject: [PATCH 1/6] Fix dropped values from queries by using FlatParamsEncoder The `uri_template` gem, which hyperclient uses to build urls, implements RFC6570, that expands a parameter in the `{?foo*}` format as `?foo=a&foo=b&foo=c`. Unfortunately, faraday's default argument parser (`Faraday::Utils.default_params_encoder`) [is set to the](https://github.com/lostisland/faraday/blob/master/lib/faraday/utils.rb#L213) [`NestedParamsEncoder`](https://github.com/lostisland/faraday/blob/master/lib/faraday/parameters.rb#L4) which only considers parameters to be arrays if they have `[]` in the key. The result is that values for a parameter are dropped in this process: ```ruby require 'faraday' require 'uri_template' Faraday::Utils.default_params_encoder.decode( URITemplate.new('http://example.com/{?foo*}').expand(foo: ['a', 'b']).split('?').last ) => {"foo"=>"b"} ``` And thus the resulting faraday request will be missing some of the values. The solution is to use faraday's other parameter encoder (the `FlatParamsEncoder`), that correctly encodes such requests. Fixes #114 --- .rubocop_todo.yml | 2 +- lib/hyperclient/entry_point.rb | 1 + test/hyperclient/entry_point_test.rb | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b15948e..5279cc9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -25,7 +25,7 @@ Metrics/MethodLength: # Offense count: 3 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 391 + Max: 398 # Offense count: 1 Style/AsciiComments: diff --git a/lib/hyperclient/entry_point.rb b/lib/hyperclient/entry_point.rb index 18b731f..eadf32c 100644 --- a/lib/hyperclient/entry_point.rb +++ b/lib/hyperclient/entry_point.rb @@ -138,6 +138,7 @@ def default_faraday_block connection.request :hal_json connection.response :hal_json, content_type: /\bjson$/ connection.adapter :net_http + connection.options.params_encoder = Faraday::FlatParamsEncoder end end diff --git a/test/hyperclient/entry_point_test.rb b/test/hyperclient/entry_point_test.rb index c6226eb..cecf551 100644 --- a/test/hyperclient/entry_point_test.rb +++ b/test/hyperclient/entry_point_test.rb @@ -33,11 +33,14 @@ module Hyperclient it 'creates a Faraday connection with the default block' do handlers = entry_point.connection.builder.handlers + handlers.must_include Faraday::Response::RaiseError handlers.must_include FaradayMiddleware::FollowRedirects handlers.must_include FaradayMiddleware::EncodeHalJson handlers.must_include FaradayMiddleware::ParseHalJson handlers.must_include Faraday::Adapter::NetHttp + + entry_point.connection.options.params_encoder.must_equal Faraday::FlatParamsEncoder end it 'raises a ConnectionAlreadyInitializedError if attempting to modify headers' do @@ -172,12 +175,15 @@ module Hyperclient it 'creates a Faraday connection with the default block plus any additional handlers' do handlers = entry_point.connection.builder.handlers + handlers.must_include Faraday::Request::OAuth handlers.must_include Faraday::Response::RaiseError handlers.must_include FaradayMiddleware::FollowRedirects handlers.must_include FaradayMiddleware::EncodeHalJson handlers.must_include FaradayMiddleware::ParseHalJson handlers.must_include Faraday::Adapter::NetHttp + + entry_point.connection.options.params_encoder.must_equal Faraday::FlatParamsEncoder end end end From 7ce4faf90545579cebd321147a6ae519efff6cdf Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 17:56:30 +0000 Subject: [PATCH 2/6] Fix warning: instance variable @resource not initialized This happens because EntryPoint extends Link, but does not call super in its initializer so @resource is never defined. --- lib/hyperclient/entry_point.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/hyperclient/entry_point.rb b/lib/hyperclient/entry_point.rb index eadf32c..9e747da 100644 --- a/lib/hyperclient/entry_point.rb +++ b/lib/hyperclient/entry_point.rb @@ -40,6 +40,7 @@ def initialize(url, &_block) @entry_point = self @options = { async: true } @connection = nil + @resource = nil yield self if block_given? end From 557585afc8e9138627da016a21f476d612eb7476 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 18:04:33 +0000 Subject: [PATCH 3/6] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f02ef3..1bd0770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 0.8.3 (Next) +* [#115](https://github.com/codegram/hyperclient/pull/115): Fix dropped values from queries by using FlatParamsEncoder - [@ivoanjo](https://github.com/ivoanjo). * Your contribution here. ### 0.8.2 (December 31, 2016) From 6febe87589836909ff6a5de2423d00f11f049d57 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 21:43:33 +0000 Subject: [PATCH 4/6] Add test scenario for dropped values for same parameter --- features/api_navigation.feature | 5 +++++ features/steps/api_navigation.rb | 8 ++++++++ features/support/api.rb | 2 ++ features/support/fixtures.rb | 1 + 4 files changed, 16 insertions(+) diff --git a/features/api_navigation.feature b/features/api_navigation.feature index 40dd6b0..da8f4d0 100644 --- a/features/api_navigation.feature +++ b/features/api_navigation.feature @@ -12,6 +12,11 @@ Feature: API navigation When I search for a post with a templated link Then the API should receive the request with all the params + Scenario: Templated links with multiple values + Given I connect to the API + When I search for posts by tag with a templated link + Then the API should receive the request for posts by tag with all the params + Scenario: Attributes Given I connect to the API When I load a single post diff --git a/features/steps/api_navigation.rb b/features/steps/api_navigation.rb index bd05389..e52ad95 100644 --- a/features/steps/api_navigation.rb +++ b/features/steps/api_navigation.rb @@ -17,6 +17,14 @@ class Spinach::Features::ApiNavigation < Spinach::FeatureSteps assert_requested :get, 'http://api.example.org/search?q=something' end + step 'I search for posts by tag with a templated link' do + api._links.tagged._expand(tags: %w(foo bar))._resource + end + + step 'the API should receive the request for posts by tag with all the params' do + assert_requested :get, 'http://api.example.org/search?tags=foo&tags=bar' + end + step 'I load a single post' do @post = api._links.posts._links.last_post end diff --git a/features/support/api.rb b/features/support/api.rb index 81560ee..0108363 100644 --- a/features/support/api.rb +++ b/features/support/api.rb @@ -5,6 +5,8 @@ module API include Spinach::Fixtures before do + WebMock::Config.instance.query_values_notation = :flat_array + stub_request(:any, %r{api.example.org*}).to_return(body: root_response, headers: { 'Content-Type' => 'application/hal+json' }) stub_request(:get, 'api.example.org/posts').to_return(body: posts_response, headers: { 'Content-Type' => 'application/hal+json' }) stub_request(:get, 'api.example.org/posts/1').to_return(body: post_response, headers: { 'Content-Type' => 'application/hal+json' }) diff --git a/features/support/fixtures.rb b/features/support/fixtures.rb index 37e50bd..2c91cce 100644 --- a/features/support/fixtures.rb +++ b/features/support/fixtures.rb @@ -8,6 +8,7 @@ def root_response "self": { "href": "/" }, "posts": { "href": "/posts" }, "search": { "href": "/search{?q}", "templated": true }, + "tagged": { "href": "/search{?tags*}", "templated": true }, "api:authors": { "href": "/authors" }, "next": { "href": "/page2" } } From 1779fee9988b371e7758ce5ddd3f5173f053bead Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 21:50:35 +0000 Subject: [PATCH 5/6] Update system gem in travis to avoid issue with rainbow gem See https://github.com/sickill/rainbow/issues/48 for more details. --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 7a12322..732daa0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,4 +19,8 @@ matrix: - rvm: jruby-head - rvm: rbx-2 +before_install: + - gem update --system + - gem install bundler + bundler_args: --without development From 7cecb97f838a556ae8536f77d62a4e155e97762f Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 17 Feb 2017 21:55:16 +0000 Subject: [PATCH 6/6] Add ruby 2.4.0 and jruby 9.1.7.0 to travis configuration --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 732daa0..acacb33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,10 +10,11 @@ matrix: - rvm: 2.3.1 - rvm: 2.3.0 - rvm: 2.2.5 + - rvm: 2.4.0 - rvm: rbx-2 - rvm: ruby-head - rvm: jruby-head - - rvm: jruby-9.1.6.0 + - rvm: jruby-9.1.7.0 allow_failures: - rvm: ruby-head - rvm: jruby-head