From 0d129529f774584a6ab5e8fc6217a3cf0ee7a88b Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez Date: Tue, 24 Oct 2023 14:08:27 -0300 Subject: [PATCH 1/2] Enable elasticsearch-transport v7 to use Faraday >=2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attempts to close #2228. All credit for this goes to @picandocodigo, as these are the same changes made in https://github.com/elastic/elastic-transport-ruby/pull/30 but for version 7.17 of `elastic-transport` – I just copy-pasted that code over. --- .github/workflows/7.17.yml | 6 +- elasticsearch-transport/Gemfile | 4 + .../Gemfile-faraday1.gemfile | 47 ++++++ elasticsearch-transport/Rakefile | 38 +++++ .../elasticsearch-transport.gemspec | 10 +- .../spec/elasticsearch/transport/base_spec.rb | 6 +- .../elasticsearch/transport/client_spec.rb | 16 ++- .../transport/meta_header_spec.rb | 4 +- elasticsearch-transport/spec/spec_helper.rb | 4 + .../test/integration/transport_test.rb | 134 ++++++++++++------ elasticsearch-transport/test/test_helper.rb | 4 + .../test/unit/adapters_test.rb | 88 ++++++++++++ 12 files changed, 301 insertions(+), 60 deletions(-) create mode 100644 elasticsearch-transport/Gemfile-faraday1.gemfile create mode 100644 elasticsearch-transport/test/unit/adapters_test.rb diff --git a/.github/workflows/7.17.yml b/.github/workflows/7.17.yml index fa367a4f4c..c54e5a88af 100644 --- a/.github/workflows/7.17.yml +++ b/.github/workflows/7.17.yml @@ -39,7 +39,11 @@ jobs: rake bundle:install - name: elasticsearch run: cd elasticsearch && bundle exec rake test:all - - name: elasticsearch-transport + - name: elasticsearch-transport faraday1 + run: cd elasticsearch-transport && bundle install && bundle exec rake test:all + env: + BUNDLE_GEMFILE: 'Gemfile-faraday1.gemfile' + - name: elasticsearch-transport faraday2 run: cd elasticsearch-transport && bundle exec rake test:all - name: elasticsearch-api run: cd elasticsearch-api && bundle exec rake test:spec diff --git a/elasticsearch-transport/Gemfile b/elasticsearch-transport/Gemfile index f53a01b2d9..d5cb7d7791 100644 --- a/elasticsearch-transport/Gemfile +++ b/elasticsearch-transport/Gemfile @@ -29,6 +29,10 @@ if File.exist? File.expand_path('../elasticsearch/elasticsearch.gemspec', __dir_ end group :development, :test do + gem 'faraday-httpclient' + gem 'faraday-net_http_persistent' + gem 'faraday-patron' unless defined? JRUBY_VERSION + gem 'faraday-typhoeus' gem 'rspec' if defined?(JRUBY_VERSION) gem 'pry-nav' diff --git a/elasticsearch-transport/Gemfile-faraday1.gemfile b/elasticsearch-transport/Gemfile-faraday1.gemfile new file mode 100644 index 0000000000..c23a26d963 --- /dev/null +++ b/elasticsearch-transport/Gemfile-faraday1.gemfile @@ -0,0 +1,47 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +source 'https://rubygems.org' + +# Usage: +# +# $ BUNDLE_GEMFILE=./Gemfile-faraday1.gemfile bundle install +# $ BUNDLE_GEMFILE=./Gemfile-faraday1.gemfile bundle exec rake test:faraday1:unit + +gem 'faraday', '~> 1' +gemspec path: './' + +if File.exist? File.expand_path('../elasticsearch-api/elasticsearch-api.gemspec', __dir__) + gem 'elasticsearch-api', path: File.expand_path('../elasticsearch-api', __dir__), require: false +end + +if File.exist? File.expand_path('../elasticsearch/elasticsearch.gemspec', __dir__) + gem 'elasticsearch', path: File.expand_path('../elasticsearch', __dir__), require: false +end + +group :development, :test do + gem 'httpclient' + gem 'net-http-persistent' + gem 'patron' unless defined? JRUBY_VERSION + gem 'typhoeus' + gem 'rspec' + if defined?(JRUBY_VERSION) + gem 'pry-nav' + else + gem 'pry-byebug' + end +end diff --git a/elasticsearch-transport/Rakefile b/elasticsearch-transport/Rakefile index 2b625eb789..f2dcb3f726 100644 --- a/elasticsearch-transport/Rakefile +++ b/elasticsearch-transport/Rakefile @@ -25,6 +25,15 @@ task :test => 'test:unit' require 'rake/testtask' require 'rspec/core/rake_task' +FARADAY1_GEMFILE = 'Gemfile-faraday1.gemfile'.freeze +GEMFILES = ['Gemfile', FARADAY1_GEMFILE].freeze + +task :install do + GEMFILES.each do |gemfile| + gemfile = File.expand_path("../#{gemfile}", __FILE__) + sh "bundle install --gemfile #{gemfile}" + end +end namespace :test do desc 'Wait for Elasticsearch to be in a green state' @@ -52,6 +61,7 @@ namespace :test do desc 'Run all tests' task :all do Rake::Task['test:unit'].invoke + Rake::Task['test:spec'].invoke Rake::Task['test:integration'].invoke end @@ -60,6 +70,34 @@ namespace :test do test.test_files = FileList['test/profile/**/*_test.rb'] end + namespace :faraday1 do + desc 'Faraday 1: Run RSpec with dependency on Faraday 1' + task :spec do + sh "BUNDLE_GEMFILE=#{FARADAY1_GEMFILE} bundle exec rspec" + end + + desc 'Faraday 1: Run unit tests with dependency on Faraday 1' + task :unit do + Dir.glob('./test/unit/**/**.rb').each do |test| + sh "BUNDLE_GEMFILE=#{FARADAY1_GEMFILE} ruby -Ilib:test #{test}" + end + end + + desc 'Faraday 1: Run integration tests with dependency on Faraday 1' + task :integration do + Dir.glob('./test/integration/**/**.rb').each do |test| + sh "BUNDLE_GEMFILE=#{FARADAY1_GEMFILE} ruby -Ilib:test #{test}" + end + end + + desc 'Faraday 1: Run all tests' + task :all do + Rake::Task['test:faraday1:unit'].invoke + Rake::Task['test:faraday1:spec'].invoke + Rake::Task['test:faraday1:integration'].invoke + end + end + namespace :cluster do desc "Start Elasticsearch nodes for tests" task :start do diff --git a/elasticsearch-transport/elasticsearch-transport.gemspec b/elasticsearch-transport/elasticsearch-transport.gemspec index d82bffe782..12e10a1172 100644 --- a/elasticsearch-transport/elasticsearch-transport.gemspec +++ b/elasticsearch-transport/elasticsearch-transport.gemspec @@ -45,21 +45,20 @@ Gem::Specification.new do |s| s.required_ruby_version = '>= 2.4' s.add_dependency 'multi_json' - s.add_dependency 'faraday', '~> 1' + s.add_dependency 'faraday', '>= 1', '< 3' + # Faraday Adapters + s.add_development_dependency 'manticore' if defined? JRUBY_VERSION + s.add_development_dependency 'curb' unless defined? JRUBY_VERSION s.add_development_dependency 'ansi' s.add_development_dependency 'bundler' s.add_development_dependency 'cane' - s.add_development_dependency 'curb' unless defined? JRUBY_VERSION s.add_development_dependency 'elasticsearch', ['>= 7', '< 8.0.0'] s.add_development_dependency 'elasticsearch-extensions' s.add_development_dependency 'hashie' - s.add_development_dependency 'httpclient' - s.add_development_dependency 'manticore' if defined? JRUBY_VERSION s.add_development_dependency 'minitest' s.add_development_dependency 'minitest-reporters' s.add_development_dependency 'mocha' - s.add_development_dependency 'net-http-persistent' s.add_development_dependency 'patron' unless defined? JRUBY_VERSION s.add_development_dependency 'pry' s.add_development_dependency 'rake', '~> 13' @@ -68,7 +67,6 @@ Gem::Specification.new do |s| s.add_development_dependency 'shoulda-context' s.add_development_dependency 'simplecov' s.add_development_dependency 'test-unit', '~> 2' - s.add_development_dependency 'typhoeus', '~> 1.4' s.add_development_dependency 'yard' s.description = <<-DESC.gsub(/^ /, '') diff --git a/elasticsearch-transport/spec/elasticsearch/transport/base_spec.rb b/elasticsearch-transport/spec/elasticsearch/transport/base_spec.rb index 3151ac415b..f09d8dac73 100644 --- a/elasticsearch-transport/spec/elasticsearch/transport/base_spec.rb +++ b/elasticsearch-transport/spec/elasticsearch/transport/base_spec.rb @@ -145,7 +145,8 @@ let(:arguments) do { hosts: ['http://unavailable:9200', 'http://unavailable:9201'], - retry_on_failure: 2 + retry_on_failure: 2, + adapter: :net_http } end @@ -169,7 +170,8 @@ let(:arguments) do { hosts: ELASTICSEARCH_HOSTS, - retry_on_status: ['404'] + retry_on_status: ['404'], + adapter: :net_http } end diff --git a/elasticsearch-transport/spec/elasticsearch/transport/client_spec.rb b/elasticsearch-transport/spec/elasticsearch/transport/client_spec.rb index c0608d9ddd..ac73f6a0af 100644 --- a/elasticsearch-transport/spec/elasticsearch/transport/client_spec.rb +++ b/elasticsearch-transport/spec/elasticsearch/transport/client_spec.rb @@ -234,8 +234,8 @@ it 'uses Faraday NetHttp' do expect(adapter).to eq Faraday::Adapter::NetHttp end - end unless jruby? - end + end + end unless jruby? context 'when the adapter is patron' do let(:adapter) do @@ -247,9 +247,10 @@ end it 'uses Faraday with the adapter' do + require 'faraday/patron' expect(adapter).to eq Faraday::Adapter::Patron end - end + end unless jruby? context 'when the adapter is typhoeus' do let(:adapter) do @@ -257,6 +258,8 @@ end let(:client) do + require 'faraday/typhoeus' if is_faraday_v2? + described_class.new(adapter: :typhoeus, enable_meta_header: false) end @@ -277,7 +280,7 @@ it 'uses Faraday with the adapter' do expect(adapter).to eq Faraday::Adapter::Patron end - end + end unless jruby? context 'when the adapter can be detected', unless: jruby? do @@ -319,7 +322,7 @@ it 'sets the logger' do expect(handlers).to include(Faraday::Response::Logger) end - end + end unless jruby? end context 'when cloud credentials are provided' do @@ -1587,6 +1590,8 @@ end context 'when the Faraday adapter is set in the block' do + require 'faraday/net_http_persistent' if is_faraday_v2? + let(:client) do described_class.new(host: ELASTICSEARCH_HOSTS.first, logger: logger) do |client| client.adapter(:net_http_persistent) @@ -1747,6 +1752,7 @@ end context 'when using the HTTPClient adapter' do + require 'faraday/httpclient' let(:client) do described_class.new(hosts: ELASTICSEARCH_HOSTS, compression: true, adapter: :httpclient, enable_meta_header: false) diff --git a/elasticsearch-transport/spec/elasticsearch/transport/meta_header_spec.rb b/elasticsearch-transport/spec/elasticsearch/transport/meta_header_spec.rb index c1a5e7aaf4..e84e977277 100644 --- a/elasticsearch-transport/spec/elasticsearch/transport/meta_header_spec.rb +++ b/elasticsearch-transport/spec/elasticsearch/transport/meta_header_spec.rb @@ -172,7 +172,7 @@ def meta_version expect(headers).to include('x-elastic-client-meta' => meta) Typhoeus = @klass if was_required - end unless jruby? + end it 'sets adapter in the meta header' do require 'typhoeus' @@ -180,7 +180,7 @@ def meta_version meta = "#{meta_header},ty=#{Typhoeus::VERSION}" expect(headers).to include('x-elastic-client-meta' => meta) end - end + end unless jruby? unless jruby? let(:adapter) { :patron } diff --git a/elasticsearch-transport/spec/spec_helper.rb b/elasticsearch-transport/spec/spec_helper.rb index 8e4b272acd..1c8967b43f 100644 --- a/elasticsearch-transport/spec/spec_helper.rb +++ b/elasticsearch-transport/spec/spec_helper.rb @@ -75,6 +75,10 @@ def default_client $client ||= Elasticsearch::Client.new(hosts: ELASTICSEARCH_HOSTS) end +def is_faraday_v2? + Gem::Version.new(Faraday::VERSION) >= Gem::Version.new(2) +end + module Config def self.included(context) # Get the hosts to use to connect an elasticsearch client. diff --git a/elasticsearch-transport/test/integration/transport_test.rb b/elasticsearch-transport/test/integration/transport_test.rb index b17c27d081..3fe8af5726 100644 --- a/elasticsearch-transport/test/integration/transport_test.rb +++ b/elasticsearch-transport/test/integration/transport_test.rb @@ -17,7 +17,7 @@ require 'test_helper' -class Elasticsearch::Transport::ClientIntegrationTest < Elasticsearch::Test::IntegrationTestCase +class Elasticsearch::Transport::Transport::ClientIntegrationTest < Elasticsearch::Test::IntegrationTestCase startup do Elasticsearch::Extensions::Test::Cluster.start(number_of_nodes: 2) if ENV['SERVER'] and not Elasticsearch::Extensions::Test::Cluster.running?(number_of_nodes: 2) end @@ -30,69 +30,115 @@ class Elasticsearch::Transport::ClientIntegrationTest < Elasticsearch::Test::Int setup do @host, @port = ELASTICSEARCH_HOSTS.first.split(':') @hosts = { hosts: [ { host: @host, port: @port } ] } - begin; Object.send(:remove_const, :Patron); rescue NameError; end end - should "allow to customize the Faraday adapter to Typhoeus" do - require 'typhoeus' - require 'typhoeus/adapters/faraday' - + should "use the default Faraday adapter" do transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| f.response :logger - f.adapter :typhoeus end client = Elasticsearch::Transport::Client.new transport: transport + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::NetHttp) client.perform_request 'GET', '' - end unless jruby? + end + + unless jruby? + should "allow to customize the Faraday adapter to Typhoeus" do + if is_faraday_v2? + require 'faraday/typhoeus' + else + require 'typhoeus' + end + + transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| + f.response :logger + f.adapter :typhoeus + end + + client = Elasticsearch::Transport::Client.new transport: transport + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::Typhoeus) + client.perform_request 'GET', '' + end - should "allow to customize the Faraday adapter to NetHttpPersistent" do - require 'net/http/persistent' + should "use the Curb client" do + require 'curb' + require 'elasticsearch/transport/transport/http/curb' + transport = Elasticsearch::Transport::Transport::HTTP::Curb.new(@hosts) do |curl| + curl.verbose = true + end - transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| - f.response :logger - f.adapter :net_http_persistent + client = Elasticsearch::Transport::Client.new transport: transport + assert_equal(client.transport.class, Elasticsearch::Transport::Transport::HTTP::Curb) + client.perform_request 'GET', '' end - client = Elasticsearch::Transport::Client.new transport: transport - client.perform_request 'GET', '' - end + should "deserialize JSON responses in the Curb client" do + require 'curb' + require 'elasticsearch/transport/transport/http/curb' + transport = Elasticsearch::Transport::Transport::HTTP::Curb.new(@hosts) do |curl| + curl.verbose = true + end - should "allow to define connection parameters and pass them" do - transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new( - hosts: [ { host: @host, port: @port } ], - options: { transport_options: { params: { :format => 'yaml' } } } - ) - client = Elasticsearch::Transport::Client.new transport: transport - response = client.perform_request 'GET', '' + client = Elasticsearch::Transport::Client.new transport: transport + response = client.perform_request 'GET', '' - assert response.body.start_with?("---\n"), "Response body should be YAML: #{response.body.inspect}" - end + assert_respond_to(response.body, :to_hash) + assert_not_nil response.body['name'] + assert_equal 'application/json', response.headers['content-type'] + end - should "use the Curb client" do - require 'curb' - require 'elasticsearch/transport/transport/http/curb' - transport = Elasticsearch::Transport::Transport::HTTP::Curb.new(@hosts) do |curl| - curl.verbose = true + should 'allow to customize the Faraday adapter to Patron' do + if is_faraday_v2? + require 'faraday/patron' + else + require 'patron' + end + transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| + f.response :logger + f.adapter :patron + end + + client = Elasticsearch::Transport::Client.new(transport: transport) + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::Patron) + client.perform_request 'GET', '' end - client = Elasticsearch::Transport::Client.new transport: transport - client.perform_request 'GET', '' - end unless jruby? + should "allow to customize the Faraday adapter to NetHttpPersistent" do + require 'faraday/net_http_persistent' - should "deserialize JSON responses in the Curb client" do - require 'curb' - require 'elasticsearch/transport/transport/http/curb' - transport = Elasticsearch::Transport::Transport::HTTP::Curb.new(@hosts) do |curl| - curl.verbose = true + transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| + f.response :logger + f.adapter :net_http_persistent + end + + client = Elasticsearch::Transport::Client.new transport: transport + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::NetHttpPersistent) + client.perform_request 'GET', '' end - client = Elasticsearch::Transport::Client.new transport: transport - response = client.perform_request 'GET', '' + should 'allow to customize the Faraday adapter to HTTPClient' do + require 'faraday/httpclient' + + transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new(@hosts) do |f| + f.response :logger + f.adapter :httpclient + end - assert_respond_to(response.body, :to_hash) - assert_not_nil response.body['name'] - assert_equal 'application/json', response.headers['content-type'] - end unless jruby? + client = Elasticsearch::Transport::Client.new(transport: transport) + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::HTTPClient) + client.perform_request 'GET', '' + end + + should "allow to define connection parameters and pass them" do + transport = Elasticsearch::Transport::Transport::HTTP::Faraday.new( + hosts: [ { host: @host, port: @port } ], + options: { transport_options: { params: { :format => 'yaml' } } } + ) + client = Elasticsearch::Transport::Client.new transport: transport + response = client.perform_request 'GET', '' + + assert response.body.start_with?("---\n"), "Response body should be YAML: #{response.body.inspect}" + end + end end end diff --git a/elasticsearch-transport/test/test_helper.rb b/elasticsearch-transport/test/test_helper.rb index 909674c328..13b3ed8d19 100644 --- a/elasticsearch-transport/test/test_helper.rb +++ b/elasticsearch-transport/test/test_helper.rb @@ -97,6 +97,10 @@ def assert_block(*msgs) end end +def is_faraday_v2? + Gem::Version.new(Faraday::VERSION) >= Gem::Version.new(2) +end + Minitest::Reporters.use! FixedMinitestSpecReporter.new module Elasticsearch diff --git a/elasticsearch-transport/test/unit/adapters_test.rb b/elasticsearch-transport/test/unit/adapters_test.rb new file mode 100644 index 0000000000..fda6b74cc7 --- /dev/null +++ b/elasticsearch-transport/test/unit/adapters_test.rb @@ -0,0 +1,88 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +require 'test_helper' + +class Elasticsearch::Transport::Transport::ClientAdaptersUnitTest < Minitest::Test + context 'Adapters' do + setup do + begin + Object.send(:remove_const, :Patron) + rescue NameError + end + end + + should 'use the default Faraday adapter' do + fork do + client = Elasticsearch::Transport::Client.new + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::NetHttp) + end + end + + should 'use Patron Faraday adapter' do + fork do + if is_faraday_v2? + require 'faraday/patron' + else + require 'patron' + end + + client = Elasticsearch::Transport::Client.new + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::Patron) + end + end + + should 'use Typhoeus Faraday adapter' do + fork do + if is_faraday_v2? + require 'faraday/typhoeus' + else + require 'typhoeus' + end + + client = Elasticsearch::Transport::Client.new + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::Typhoeus) + end + end + + should 'use NetHttpPersistent Faraday adapter' do + fork do + if is_faraday_v2? + require 'faraday/net_http_persistent' + else + require 'net/http/persistent' + end + + client = Elasticsearch::Transport::Client.new + assert_equal(client.transport.connections.first.connection.adapter, Faraday::Adapter::NetHttpPersistent) + end + end + + should 'use HTTPClient Faraday adapter' do + fork do + if is_faraday_v2? + require 'faraday/httpclient' + else + require 'httpclient' + end + + client = Elasticsearch::Transport::Client.new + assert_equal(Faraday::Adapter::HTTPClient, client.transport.connections.first.connection.adapter) + end + end + end unless jruby? +end From 6888ced8777b7150bf27b09c77b569512a15d325 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Thu, 26 Oct 2023 12:45:05 -0300 Subject: [PATCH 2/2] Explicitly call `test:faraday1:all` Co-authored-by: Fernando Briano --- .github/workflows/7.17.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/7.17.yml b/.github/workflows/7.17.yml index c54e5a88af..e24a85e97d 100644 --- a/.github/workflows/7.17.yml +++ b/.github/workflows/7.17.yml @@ -40,7 +40,7 @@ jobs: - name: elasticsearch run: cd elasticsearch && bundle exec rake test:all - name: elasticsearch-transport faraday1 - run: cd elasticsearch-transport && bundle install && bundle exec rake test:all + run: cd elasticsearch-transport && bundle install && bundle exec rake test:faraday1:all env: BUNDLE_GEMFILE: 'Gemfile-faraday1.gemfile' - name: elasticsearch-transport faraday2