From da058148b448144ae7f9cebed35a46632f168a4e Mon Sep 17 00:00:00 2001 From: Mattia Giuffrida Date: Mon, 8 Aug 2022 08:52:07 +0100 Subject: [PATCH 1/5] Add `bake-test-external` and test `faraday-net_http` as part of the CI --- .github/workflows/ci.yml | 8 +++++++- .gitignore | 1 + Gemfile | 1 + config/external.yaml | 3 +++ 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 config/external.yaml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44a53959d..ec2c8f559 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,7 +49,13 @@ jobs: ruby-version: ${{ matrix.ruby }} bundler-cache: true - - name: Test + - name: RSpec continue-on-error: ${{ matrix.experimental }} run: bundle exec rake + - name: Test External Adapters + if: ${{ matrix.ruby != '2.6' }} + continue-on-error: ${{ matrix.experimental }} + run: bundle exec bake test:external + + diff --git a/.gitignore b/.gitignore index eea05d17b..205c532ea 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ tmp .bundle Gemfile.lock vendor/bundle +external ## PROJECT::SPECIFIC .rbx diff --git a/Gemfile b/Gemfile index 0ffb9c69b..b6645427e 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ source 'https://rubygems.org' gem 'jruby-openssl', '~> 0.11.0', platforms: :jruby group :development, :test do + gem 'bake-test-external' gem 'coveralls_reborn', require: false gem 'pry' gem 'rack', '~> 2.2' diff --git a/config/external.yaml b/config/external.yaml new file mode 100644 index 000000000..fbc232921 --- /dev/null +++ b/config/external.yaml @@ -0,0 +1,3 @@ +faraday-net_http: + url: https://github.com/lostisland/faraday-net_http.git + command: bundle exec rspec From 4b5cb71ad3902582b76b33d4de36dd18b9d2d164 Mon Sep 17 00:00:00 2001 From: Mattia Giuffrida Date: Mon, 8 Aug 2022 09:20:12 +0100 Subject: [PATCH 2/5] Temporarily use edge rubocop-packaging --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index b6645427e..f0d177ced 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ end group :development, :lint do gem 'rubocop' - gem 'rubocop-packaging', '~> 0.5' + gem 'rubocop-packaging', github: 'utkarsh2102/rubocop-packaging' # '~> 0.5' gem 'rubocop-performance', '~> 1.0' gem 'yard-junk' end From 982165bf00f0999ea0983bb4193a03f9a7dc9a4f Mon Sep 17 00:00:00 2001 From: Mattia Giuffrida Date: Mon, 8 Aug 2022 09:22:18 +0100 Subject: [PATCH 3/5] Introduce new streaming API: * Backwards-compatible * Allow adapters to provide response info to on_call block * Provide `stream_response` helper method --- .rubocop.yml | 3 +++ lib/faraday/adapter.rb | 4 ++-- lib/faraday/options/env.rb | 18 ++++++++++++++++++ spec/support/shared_examples/request_method.rb | 18 +++++++++++++++--- 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index b285bdc90..5b216ee12 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -14,6 +14,9 @@ Metrics/BlockLength: - spec/**/*.rb - examples/**/*.rb +Metrics/ParameterLists: + Max: 6 + Layout/EmptyLinesAroundAttributeAccessor: # (0.83) Enabled: true diff --git a/lib/faraday/adapter.rb b/lib/faraday/adapter.rb index 39bb5fae1..f3e6af7bf 100644 --- a/lib/faraday/adapter.rb +++ b/lib/faraday/adapter.rb @@ -59,7 +59,7 @@ def call(env) private - def save_response(env, status, body, headers = nil, reason_phrase = nil) + def save_response(env, status, body, headers = nil, reason_phrase = nil, finished: true) env.status = status env.body = body env.reason_phrase = reason_phrase&.to_s&.strip @@ -68,7 +68,7 @@ def save_response(env, status, body, headers = nil, reason_phrase = nil) yield(response_headers) if block_given? end - env.response.finish(env) unless env.parallel? + env.response.finish(env) unless env.parallel? || !finished env.response end diff --git a/lib/faraday/options/env.rb b/lib/faraday/options/env.rb index d7dac7178..1446f67f2 100644 --- a/lib/faraday/options/env.rb +++ b/lib/faraday/options/env.rb @@ -157,6 +157,24 @@ def inspect %(#<#{self.class}#{attrs.join(' ')}>) end + def stream_response? + request.stream_response? + end + + def stream_response(&block) + size = 0 + yielded = false + block_result = block.call do |chunk| # rubocop:disable Performance/RedundantBlockCall + if chunk.bytesize.positive? || size.positive? + yielded = true + size += chunk.bytesize + request.on_data.call(chunk, size, self) + end + end + request.on_data.call(+'', 0, self) unless yielded + block_result + end + # @private def custom_members @custom_members ||= {} diff --git a/spec/support/shared_examples/request_method.rb b/spec/support/shared_examples/request_method.rb index b6336549a..772ee301a 100644 --- a/spec/support/shared_examples/request_method.rb +++ b/spec/support/shared_examples/request_method.rb @@ -153,12 +153,18 @@ let(:streamed) { [] } context 'when response is empty' do - it do + it 'handles streaming' do + env = nil conn.public_send(http_method, '/') do |req| - req.options.on_data = proc { |*args| streamed << args } + req.options.on_data = proc do |chunk, size, block_env| + streamed << [chunk, size] + env ||= block_env + end end expect(streamed).to eq([['', 0]]) + expect(env).to be_a(Faraday::Env) + expect(env.status).to eq(200) end end @@ -166,12 +172,18 @@ before { request_stub.to_return(body: big_string) } it 'handles streaming' do + env = nil response = conn.public_send(http_method, '/') do |req| - req.options.on_data = proc { |*args| streamed << args } + req.options.on_data = proc do |chunk, size, block_env| + streamed << [chunk, size] + env ||= block_env + end end expect(response.body).to eq('') check_streaming_response(streamed, chunk_size: 16 * 1024) + expect(env).to be_a(Faraday::Env) + expect(env.status).to eq(200) end end end From f3290e267b8560e6349e7cb7913e70c6d2b375a0 Mon Sep 17 00:00:00 2001 From: Mattia Giuffrida Date: Mon, 8 Aug 2022 09:51:55 +0100 Subject: [PATCH 4/5] Update CI matrix: * Make `truffleruby-head` experimental * Add `ruby-head` as experimental --- .github/workflows/ci.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec2c8f559..5c35c55c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,13 @@ jobs: strategy: fail-fast: false matrix: - ruby: [ '2.6', '2.7', '3.0', '3.1', truffleruby-head ] + ruby: [ '2.6', '2.7', '3.0', '3.1' ] + experimental: [false] + include: + - ruby: head + experimental: true + - ruby: truffleruby-head + experimental: true steps: - uses: actions/checkout@v3 From fd622e8f4414af42f6f8b9da9737eb7c3738dd41 Mon Sep 17 00:00:00 2001 From: Mattia Giuffrida Date: Mon, 8 Aug 2022 09:59:55 +0100 Subject: [PATCH 5/5] Disable new streaming API specific tests --- spec/support/shared_examples/request_method.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/support/shared_examples/request_method.rb b/spec/support/shared_examples/request_method.rb index 772ee301a..afa337677 100644 --- a/spec/support/shared_examples/request_method.rb +++ b/spec/support/shared_examples/request_method.rb @@ -163,8 +163,9 @@ end expect(streamed).to eq([['', 0]]) - expect(env).to be_a(Faraday::Env) - expect(env.status).to eq(200) + # TODO: enable this after updating all existing adapters to the new streaming API + # expect(env).to be_a(Faraday::Env) + # expect(env.status).to eq(200) end end @@ -182,8 +183,9 @@ expect(response.body).to eq('') check_streaming_response(streamed, chunk_size: 16 * 1024) - expect(env).to be_a(Faraday::Env) - expect(env.status).to eq(200) + # TODO: enable this after updating all existing adapters to the new streaming API + # expect(env).to be_a(Faraday::Env) + # expect(env.status).to eq(200) end end end