[GEM] Updates product validation

This update refactors the product validation in a few ways:
- Just check header, does not check the version of the server
- Warns only once when there's a general server error
- Removes the call to '/' (client.info) when doing the first request, checking on the first actual request from the client

Author: picandocodigo

elasticsearch/lib/elasticsearch.rb

@@ -43,6 +43,7 @@ class Client
     #                                                     if you're using X-Opaque-Id
     def initialize(arguments = {}, &block)
       @verified = false
+      @warned = false
       @opaque_id_prefix = arguments[:opaque_id_prefix] || nil
       api_key(arguments) if arguments[:api_key]
       if arguments[:cloud_id]
@@ -67,8 +68,11 @@ def method_missing(name, *args, &block)
           opaque_id = @opaque_id_prefix ? "#{@opaque_id_prefix}#{opaque_id}" : opaque_id
           args[4] = headers.merge('X-Opaque-Id' => opaque_id)
         end
-        verify_elasticsearch unless @verified
-        @transport.perform_request(*args, &block)
+        unless @verified
+          verify_elasticsearch(*args, &block)
+        else
+          @transport.perform_request(*args, &block)
+        end
       else
         @transport.send(name, *args, &block)
       end
@@ -80,42 +84,28 @@ def respond_to_missing?(method_name, *args)
 
     private
 
-    def verify_elasticsearch
+    def verify_elasticsearch(*args, &block)
       begin
-        response = elasticsearch_validation_request
+        response = @transport.perform_request(*args, &block)
       rescue Elastic::Transport::Transport::Errors::Unauthorized,
              Elastic::Transport::Transport::Errors::Forbidden,
-             Elastic::Transport::Transport::Errors::RequestEntityTooLarge
-        @verified = true
+             Elastic::Transport::Transport::Errors::RequestEntityTooLarge => e
         warn(SECURITY_PRIVILEGES_VALIDATION_WARNING)
-        return
-      rescue Elastic::Transport::Transport::Error
+        @verified = true
+        raise e
+      rescue Elastic::Transport::Transport::Error => e
+        unless @warned
+          warn(VALIDATION_WARNING)
+          @warned = true
+        end
+        raise e
+      rescue StandardError => e
         warn(VALIDATION_WARNING)
-        return
-      end
-
-      body = if response.headers['content-type'] == 'application/yaml'
-               require 'yaml'
-               YAML.safe_load(response.body)
-             else
-               response.body
-             end
-      version = body.dig('version', 'number')
-      verify_with_version_or_header(version, response.headers)
-    rescue StandardError => e
-      warn(VALIDATION_WARNING)
-      raise e
-    end
-
-    def verify_with_version_or_header(version, headers)
-      if version.nil? ||
-         Gem::Version.new(version) < Gem::Version.new('8.0.0.pre') && version != '8.0.0-SNAPSHOT' ||
-         headers['x-elastic-product'] != 'Elasticsearch'
-
-        raise Elasticsearch::UnsupportedProductError
+        raise e
       end
-
+      raise Elasticsearch::UnsupportedProductError unless response.headers['x-elastic-product'] == 'Elasticsearch'
       @verified = true
+      response
     end
 
     def setup_cloud_host(cloud_id, user, password, port)

elasticsearch/spec/unit/elasticsearch_product_validation_spec.rb

@@ -20,52 +20,28 @@
 
 describe 'Elasticsearch: Validation' do
   let(:host) { 'http://localhost:9200' }
-  let(:verify_request_stub) do
-    stub_request(:get, host)
-      .to_return(status: status, body: body, headers: headers)
-  end
   let(:count_request_stub) do
     stub_request(:get, "#{host}/_count")
-      .to_return(status: 200, body: nil, headers: {})
+      .to_return(status: status, body: nil, headers: headers)
   end
   let(:status) { 200 }
-  let(:body) { {}.to_json }
+  let(:body) { nil }
+  let(:headers) { {} }
   let(:client) { Elasticsearch::Client.new }
-  let(:headers) do
-    { 'content-type' => 'application/json' }
-  end
-
-  def error_requests_and_expectations(message = Elasticsearch::NOT_ELASTICSEARCH_WARNING)
-    expect { client.count }.to raise_error Elasticsearch::UnsupportedProductError, message
-    assert_requested :get, host
-    assert_not_requested :post, "#{host}/_count"
-    expect { client.cluster.health }.to raise_error Elasticsearch::UnsupportedProductError, message
-    expect(client.instance_variable_get('@verified')).to be false
-    expect { client.cluster.health }.to raise_error Elasticsearch::UnsupportedProductError, message
-  end
-
-  def valid_requests_and_expectations
-    expect(client.instance_variable_get('@verified')).to be false
-    assert_not_requested :get, host
-
-    client.count
-    expect(client.instance_variable_get('@verified'))
-    assert_requested :get, host
-    assert_requested :get, "#{host}/_count"
-  end
 
   context 'When Elasticsearch replies with status 401' do
     let(:status) { 401 }
-    let(:body) { {}.to_json }
 
     it 'Verifies the request but shows a warning' do
       stderr      = $stderr
       fake_stderr = StringIO.new
       $stderr     = fake_stderr
-
-      verify_request_stub
+      expect(client.instance_variable_get('@verified')).to be false
       count_request_stub
-      valid_requests_and_expectations
+      expect do
+        client.count
+      end.to raise_error Elastic::Transport::Transport::Errors::Unauthorized
+      expect(client.instance_variable_get('@verified')).to be true
 
       fake_stderr.rewind
       expect(fake_stderr.string).to eq("#{Elasticsearch::SECURITY_PRIVILEGES_VALIDATION_WARNING}\n")
@@ -76,16 +52,18 @@ def valid_requests_and_expectations
 
   context 'When Elasticsearch replies with status 403' do
     let(:status) { 403 }
-    let(:body) { {}.to_json }
 
     it 'Verifies the request but shows a warning' do
       stderr      = $stderr
       fake_stderr = StringIO.new
       $stderr     = fake_stderr
 
-      verify_request_stub
+      expect(client.instance_variable_get('@verified')).to be false
       count_request_stub
-      valid_requests_and_expectations
+      expect do
+        client.count
+      end.to raise_error Elastic::Transport::Transport::Errors::Forbidden
+      expect(client.instance_variable_get('@verified')).to be true
 
       fake_stderr.rewind
       expect(fake_stderr.string).to eq("#{Elasticsearch::SECURITY_PRIVILEGES_VALIDATION_WARNING}\n")
@@ -96,17 +74,17 @@ def valid_requests_and_expectations
 
   context 'When Elasticsearch replies with status 413' do
     let(:status) { 413 }
-    let(:body) { {}.to_json }
 
     it 'Verifies the request and shows a warning' do
       stderr      = $stderr
       fake_stderr = StringIO.new
       $stderr     = fake_stderr
 
       expect(client.instance_variable_get('@verified')).to be false
-      assert_not_requested :get, host
-      verify_request_stub
-      expect { client.info }.to raise_error Elastic::Transport::Transport::Errors::RequestEntityTooLarge
+      count_request_stub
+      expect do
+        client.count
+      end.to raise_error Elastic::Transport::Transport::Errors::RequestEntityTooLarge
       expect(client.instance_variable_get('@verified')).to be true
 
       fake_stderr.rewind
@@ -127,10 +105,10 @@ def valid_requests_and_expectations
       $stderr     = fake_stderr
 
       expect(client.instance_variable_get('@verified')).to be false
-      assert_not_requested :get, host
-      verify_request_stub
-      expect { client.info }.to raise_error Elastic::Transport::Transport::Errors::ServiceUnavailable
-      assert_not_requested :post, "#{host}/_count"
+      count_request_stub
+      expect do
+        client.count
+      end.to raise_error Elastic::Transport::Transport::Errors::ServiceUnavailable
       expect(client.instance_variable_get('@verified')).to be false
 
       fake_stderr.rewind
@@ -147,156 +125,24 @@ def valid_requests_and_expectations
     end
   end
 
+  context 'When the header is present' do
+    let(:headers) { { 'X-Elastic-Product' => 'Elasticsearch' } }
 
-  context 'When the Elasticsearch version is >= 8.0.0' do
-    context 'With a valid Elasticsearch response' do
-      let(:body) { { 'version' => { 'number' => '8.0.0' } }.to_json }
-      let(:headers) do
-        {
-          'X-Elastic-Product' => 'Elasticsearch',
-          'content-type' => 'json'
-        }
-      end
-
-      it 'Makes requests and passes validation' do
-        verify_request_stub
-        count_request_stub
-
-        valid_requests_and_expectations
-      end
-    end
-
-    context 'When the header is not present' do
-      it 'Fails validation' do
-        verify_request_stub
-
-        expect(client.instance_variable_get('@verified')).to be false
-        assert_not_requested :get, host
-
-        error_requests_and_expectations
-      end
-    end
-  end
-
-  context 'When the Elasticsearch version is >= 8.1.0' do
-    context 'With a valid Elasticsearch response' do
-      let(:body) { { 'version' => { 'number' => '8.1.0' } }.to_json }
-      let(:headers) do
-        {
-          'X-Elastic-Product' => 'Elasticsearch',
-          'content-type' => 'json'
-        }
-      end
-
-      it 'Makes requests and passes validation' do
-        verify_request_stub
-        count_request_stub
-
-        valid_requests_and_expectations
-      end
-    end
-
-    context 'When the header is not present' do
-      it 'Fails validation' do
-        verify_request_stub
-
-        expect(client.instance_variable_get('@verified')).to be false
-        assert_not_requested :get, host
-
-        error_requests_and_expectations
-      end
-    end
-  end
-
-
-  context 'When the Elasticsearch version is 8.0.0.pre' do
-    context 'With a valid Elasticsearch response' do
-      let(:body) { { 'version' => { 'number' => '8.0.0.pre' } }.to_json }
-      let(:headers) do
-        {
-          'X-Elastic-Product' => 'Elasticsearch',
-          'content-type' => 'json'
-        }
-      end
-
-      it 'Makes requests and passes validation' do
-        verify_request_stub
-        count_request_stub
-
-        valid_requests_and_expectations
-      end
-    end
-
-    context 'When the header is not present' do
-      it 'Fails validation' do
-        verify_request_stub
-
-        expect(client.instance_variable_get('@verified')).to be false
-        assert_not_requested :get, host
-
-        error_requests_and_expectations
-      end
-    end
-  end
-
-  context 'When the version is 8.0.0-SNAPSHOT' do
-    let(:body) { { 'version' => { 'number' => '8.0.0-SNAPSHOT' } }.to_json }
-
-    context 'When the header is not present' do
-      it 'Fails validation' do
-        verify_request_stub
-        count_request_stub
-
-        error_requests_and_expectations
-      end
-    end
-
-    context 'With a valid Elasticsearch response' do
-      let(:headers) do
-        {
-          'X-Elastic-Product' => 'Elasticsearch',
-          'content-type' => 'json'
-        }
-      end
-
-      it 'Makes requests and passes validation' do
-        verify_request_stub
-        count_request_stub
-
-        valid_requests_and_expectations
-      end
-    end
-  end
-
-  context 'When Elasticsearch version is < 8.0.0' do
-    let(:body) { { 'version' => { 'number' => '7.16.0' } }.to_json }
-
-    it 'Raises an exception and client doesnae work' do
-      verify_request_stub
-      error_requests_and_expectations
-    end
-  end
-
-  context 'When there is no version data' do
-    let(:body) { {}.to_json }
-    it 'Raises an exception and client doesnae work' do
-      verify_request_stub
-      error_requests_and_expectations
+    it 'Makes requests and passes validation' do
+      expect(client.instance_variable_get('@verified')).to be false
+      count_request_stub
+      client.count
+      expect(client.instance_variable_get('@verified')).to be true
     end
   end
 
-  context 'When doing a yaml content-type request' do
-    let(:client) do
-      Elasticsearch::Client.new(transport_options: {headers: { accept: 'application/yaml', content_type: 'application/yaml' }})
-    end
-
-    let(:headers) { { 'content-type' => 'application/yaml', 'X-Elastic-Product' => 'Elasticsearch' } }
-    let(:body) { "---\nversion:\n  number: \"8.0.0-SNAPSHOT\"\n" }
-
-    it 'validates' do
-      verify_request_stub
-      count_request_stub
-      valid_requests_and_expectations
+  context 'When the header is not present' do
+    it 'Fails validation' do
+      expect(client.instance_variable_get('@verified')).to be false
+      stub_request(:get, "#{host}/_cluster/health")
+        .to_return(status: status, body: nil, headers: {})
+      expect { client.cluster.health }.to raise_error Elasticsearch::UnsupportedProductError, Elasticsearch::NOT_ELASTICSEARCH_WARNING
+      expect(client.instance_variable_get('@verified')).to be false
     end
   end
 end