From d9c494e8a930639918135688ad98ead769acc188 Mon Sep 17 00:00:00 2001 From: Kazuhiro NISHIYAMA Date: Tue, 28 Nov 2023 15:59:16 +0900 Subject: [PATCH 1/6] `ruby2_keywords` is not an instance method --- elasticsearch-model/lib/elasticsearch/model/proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/proxy.rb b/elasticsearch-model/lib/elasticsearch/model/proxy.rb index 6fdf66322..69a6059d2 100644 --- a/elasticsearch-model/lib/elasticsearch/model/proxy.rb +++ b/elasticsearch-model/lib/elasticsearch/model/proxy.rb @@ -115,7 +115,7 @@ def initialize(target) @target = target end - def ruby2_keywords(*) # :nodoc: + def self.ruby2_keywords(*) # :nodoc: end if RUBY_VERSION < "2.7" # Delegate methods to `@target`. As per [the Ruby 3.0 explanation for keyword arguments](https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/), the only way to work on Ruby <2.7, and 2.7, and 3.0+ is to use `ruby2_keywords`. From c1ccb4dc3d152b01a16d28ef2ee1ff0b8273428d Mon Sep 17 00:00:00 2001 From: Colin MacKenzie IV Date: Fri, 12 Apr 2024 15:47:21 -0400 Subject: [PATCH 2/6] Handle security config for ES8 --- Rakefile | 18 ++++++++++-------- .../spec/elasticsearch/model/indexing_spec.rb | 2 +- elasticsearch-model/spec/spec_helper.rb | 3 ++- elasticsearch-persistence/spec/spec_helper.rb | 3 ++- elasticsearch-rails/spec/spec_helper.rb | 3 ++- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Rakefile b/Rakefile index 4d49a8f34..55c294486 100644 --- a/Rakefile +++ b/Rakefile @@ -38,14 +38,16 @@ def admin_client if test_suite == 'security' transport_options.merge!(:ssl => { verify: false, - ca_path: CERT_DIR }) + ca_path: defined?(CERT_DIR) ? CERT_DIR : nil + }.compact) password = ENV['ELASTIC_PASSWORD'] user = ENV['ELASTIC_USER'] || 'elastic' - url = "https://#{user}:#{password}@#{host}:#{port}" + url = "https://#{user}:#{password}@#{host || 'localhost'}:#{port || 9200}" else url = "http://#{host || 'localhost'}:#{port || 9200}" end + ENV['ELASTICSEARCH_URL'] ||= url Elasticsearch::Client.new(host: url, transport_options: transport_options) end end @@ -140,7 +142,7 @@ namespace :test do end desc "Run all tests in all subprojects" - task all: :wait_for_green do + task all: :wait_for_green_or_yellow do subprojects.each do |project| puts '-'*80 sh "cd #{project} && " + @@ -151,20 +153,20 @@ namespace :test do end end -desc "Wait for elasticsearch cluster to be in green state" -task :wait_for_green do +desc "Wait for elasticsearch cluster to be in green or yellow state" +task :wait_for_green_or_yellow do require 'elasticsearch' ready = nil 5.times do |i| begin - puts "Attempting to wait for green status: #{i + 1}" - if admin_client.cluster.health(wait_for_status: 'green', timeout: '50s') + puts "Attempting to wait for green or yellow status: #{i + 1}" + if admin_client.cluster.health(wait_for_status: 'yellow', timeout: '50s') ready = true break end rescue Elastic::Transport::Transport::Errors::RequestTimeout => ex - puts "Couldn't confirm green status.\n#{ex.inspect}." + puts "Couldn't confirm green or yellow status.\n#{ex.inspect}." rescue Faraday::ConnectionFailed => ex puts "Couldn't connect to Elasticsearch.\n#{ex.inspect}." sleep(30) diff --git a/elasticsearch-model/spec/elasticsearch/model/indexing_spec.rb b/elasticsearch-model/spec/elasticsearch/model/indexing_spec.rb index 68fc2c242..b4c0d299d 100644 --- a/elasticsearch-model/spec/elasticsearch/model/indexing_spec.rb +++ b/elasticsearch-model/spec/elasticsearch/model/indexing_spec.rb @@ -591,7 +591,7 @@ class ::DummyIndexingModelForRecreate context 'when the index is not found' do let(:logger) { nil } - let(:client) { Elasticsearch::Client.new(logger: logger) } + let(:client) { Elasticsearch::Client.new(logger: logger, transport_options: { ssl: { verify: false } }) } before do expect(DummyIndexingModelForRecreate).to receive(:client).at_most(3).times.and_return(client) diff --git a/elasticsearch-model/spec/spec_helper.rb b/elasticsearch-model/spec/spec_helper.rb index d3a1e303f..921c1cfa2 100644 --- a/elasticsearch-model/spec/spec_helper.rb +++ b/elasticsearch-model/spec/spec_helper.rb @@ -45,7 +45,8 @@ tracer.formatter = lambda { |s, d, p, m| "#{m.gsub(/^.*$/) { |n| ' ' + n }.ansi(:faint)}\n" } Elasticsearch::Model.client = Elasticsearch::Client.new( host: ELASTICSEARCH_URL, - tracer: (ENV['QUIET'] ? nil : tracer) + tracer: (ENV['QUIET'] ? nil : tracer), + transport_options: { :ssl => { verify: false } } ) puts "Elasticsearch Version: #{Elasticsearch::Model.client.info['version']}" diff --git a/elasticsearch-persistence/spec/spec_helper.rb b/elasticsearch-persistence/spec/spec_helper.rb index e577d3808..bf9ac3e07 100644 --- a/elasticsearch-persistence/spec/spec_helper.rb +++ b/elasticsearch-persistence/spec/spec_helper.rb @@ -36,7 +36,8 @@ # # @since 6.0.0 DEFAULT_CLIENT = Elasticsearch::Client.new(host: ELASTICSEARCH_URL, - tracer: (ENV['QUIET'] ? nil : ::Logger.new(STDERR))) + tracer: (ENV['QUIET'] ? nil : ::Logger.new(STDERR)), + transport_options: { :ssl => { verify: false } }) class MyTestRepository include Elasticsearch::Persistence::Repository diff --git a/elasticsearch-rails/spec/spec_helper.rb b/elasticsearch-rails/spec/spec_helper.rb index e0a299964..ccc28f345 100644 --- a/elasticsearch-rails/spec/spec_helper.rb +++ b/elasticsearch-rails/spec/spec_helper.rb @@ -36,7 +36,8 @@ tracer = ::Logger.new(STDERR) tracer.formatter = lambda { |s, d, p, m| "#{m.gsub(/^.*$/) { |n| ' ' + n }.ansi(:faint)}\n" } Elasticsearch::Model.client = Elasticsearch::Client.new host: ELASTICSEARCH_URL, - tracer: (ENV['QUIET'] ? nil : tracer) + tracer: (ENV['QUIET'] ? nil : tracer), + transport_options: { :ssl => { verify: false } } puts "Elasticsearch Version: #{Elasticsearch::Model.client.info['version']}" unless ActiveRecord::Base.connected? From 07482cbf2a66cb9abdde6ea804b980a5d2778285 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie IV Date: Fri, 12 Apr 2024 15:47:49 -0400 Subject: [PATCH 3/6] Use extend here instead of include --- elasticsearch-model/lib/elasticsearch/model/proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/proxy.rb b/elasticsearch-model/lib/elasticsearch/model/proxy.rb index 63645ffdc..bdd9f2583 100644 --- a/elasticsearch-model/lib/elasticsearch/model/proxy.rb +++ b/elasticsearch-model/lib/elasticsearch/model/proxy.rb @@ -61,7 +61,7 @@ def self.__elasticsearch__ &block # Mix the importing module into the `ClassMethodsProxy` self.__elasticsearch__.class_eval do - include Adapter.from_class(base).importing_mixin + extend Adapter.from_class(base).importing_mixin end # Register a callback for storing changed attributes for models which implement From c5bff9f0bc8aaaa9db6be636c9c74c9e2dfa03bd Mon Sep 17 00:00:00 2001 From: Colin MacKenzie IV Date: Fri, 12 Apr 2024 19:08:33 -0400 Subject: [PATCH 4/6] Revert "Use extend here instead of include" This reverts commit 07482cbf2a66cb9abdde6ea804b980a5d2778285. --- elasticsearch-model/lib/elasticsearch/model/proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/proxy.rb b/elasticsearch-model/lib/elasticsearch/model/proxy.rb index 3a02cd2af..8a93bc359 100644 --- a/elasticsearch-model/lib/elasticsearch/model/proxy.rb +++ b/elasticsearch-model/lib/elasticsearch/model/proxy.rb @@ -61,7 +61,7 @@ def self.__elasticsearch__ &block # Mix the importing module into the `ClassMethodsProxy` self.__elasticsearch__.class_eval do - extend Adapter.from_class(base).importing_mixin + include Adapter.from_class(base).importing_mixin end # Register a callback for storing changed attributes for models which implement From 7615ed37a5fba283a37fd8406de641fe91bd6a92 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie IV Date: Fri, 12 Apr 2024 19:11:44 -0400 Subject: [PATCH 5/6] Test that the model namespace isn't polluted. The goal of ClassMethodsProxy is to avoid polluting the target's namespace, but it was possible to do this by accident when calling `class_eval` before ActiveSupport was completely loaded. This test ensures the namespace isn't polluted, regardless of the load state of ActiveSupport. --- .../elasticsearch/model/adapters/active_record/import_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/elasticsearch-model/spec/elasticsearch/model/adapters/active_record/import_spec.rb b/elasticsearch-model/spec/elasticsearch/model/adapters/active_record/import_spec.rb index a23e58c37..73be18c59 100644 --- a/elasticsearch-model/spec/elasticsearch/model/adapters/active_record/import_spec.rb +++ b/elasticsearch-model/spec/elasticsearch/model/adapters/active_record/import_spec.rb @@ -52,6 +52,10 @@ it 'imports all documents' do expect(ImportArticle.search('*').results.total).to eq(10) end + + it "does not pollute the model's namespace" do + expect(ImportArticle.methods).not_to include(:__transform) + end end context 'when batch size is specified' do From 5479306c877cdba0758fc0ab5bcb4158f99dc3fe Mon Sep 17 00:00:00 2001 From: Colin MacKenzie IV Date: Fri, 12 Apr 2024 19:12:33 -0400 Subject: [PATCH 6/6] Require activesupport/all to more closely mirror a production Rails app. ActiveSupport patches Kernel to add `class_eval` but this behavior wasn't loaded in the test environment. This created a discrepancy between test and prod, causing tests to fail that should have passed and vice versa. Fully loading ActiveSupport makes the test environment more accurate. --- elasticsearch-model/spec/spec_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/elasticsearch-model/spec/spec_helper.rb b/elasticsearch-model/spec/spec_helper.rb index 921c1cfa2..7c4f21a4a 100644 --- a/elasticsearch-model/spec/spec_helper.rb +++ b/elasticsearch-model/spec/spec_helper.rb @@ -31,6 +31,10 @@ require 'yaml' require 'active_record' +# Load all of ActiveSupport to be sure of complete compatibility - +# see https://github.com/elastic/elasticsearch-rails/pull/1075 for details +require 'active_support/all' + unless defined?(ELASTICSEARCH_URL) ELASTICSEARCH_URL = ENV['ELASTICSEARCH_URL'] || "localhost:#{(ENV['TEST_CLUSTER_PORT'] || 9200)}" end