From ff1ae47daa3697e147c21144b3cadc8bcf7ad015 Mon Sep 17 00:00:00 2001 From: Fanis Tharropoulos Date: Mon, 18 Nov 2024 12:49:31 +0200 Subject: [PATCH 1/5] refactor(api)!: migrate from typhoeus to faraday http client * replace typhoeus request handling with faraday for improved http connections * update response code checks to use response.status instead of response.code * add integration tests for collections api with real typesense server * update gemspec to include faraday dependency * this is a breaking change and should be marked as a major release --- lib/typesense/api_call.rb | 59 ++++++++-------- spec/typesense/collections_spec.rb | 106 +++++++++++++++++++++++++++++ typesense.gemspec | 2 +- 3 files changed, 139 insertions(+), 28 deletions(-) diff --git a/lib/typesense/api_call.rb b/lib/typesense/api_call.rb index c500e76..19a3f47 100644 --- a/lib/typesense/api_call.rb +++ b/lib/typesense/api_call.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'typhoeus' +require 'faraday' require 'oj' module Typesense @@ -69,23 +69,25 @@ def perform_request(method, endpoint, query_parameters: nil, body_parameters: ni @logger.debug "Attempting #{method.to_s.upcase} request Try ##{num_tries} to Node #{node[:index]}" begin - request_options = { - method: method, - timeout: @connection_timeout_seconds, - headers: default_headers.merge(additional_headers) - } - request_options.merge!(params: query_parameters) unless query_parameters.nil? - - unless body_parameters.nil? - body = body_parameters - body = Oj.dump(body_parameters, mode: :compat) if request_options[:headers]['Content-Type'] == 'application/json' - request_options.merge!(body: body) + conn = Faraday.new(uri_for(endpoint, node)) do |f| + f.options.timeout = @connection_timeout_seconds + f.options.open_timeout = @connection_timeout_seconds end - response = Typhoeus::Request.new(uri_for(endpoint, node), request_options).run - set_node_healthcheck(node, is_healthy: true) if response.code >= 1 && response.code <= 499 + headers = default_headers.merge(additional_headers) - @logger.debug "Request #{method}:#{uri_for(endpoint, node)} to Node #{node[:index]} was successfully made (at the network layer). Response Code was #{response.code}." + response = conn.send(method) do |req| + req.headers = headers + req.params = query_parameters unless query_parameters.nil? + unless body_parameters.nil? + body = body_parameters + body = Oj.dump(body_parameters, mode: :compat) if headers['Content-Type'] == 'application/json' + req.body = body + end + end + set_node_healthcheck(node, is_healthy: true) if response.status >= 1 && response.status <= 499 + + @logger.debug "Request #{method}:#{uri_for(endpoint, node)} to Node #{node[:index]} was successfully made (at the network layer). response.status was #{response.status}." parsed_response = if response.headers && (response.headers['content-type'] || '').include?('application/json') Oj.load(response.body, mode: :compat) @@ -94,13 +96,15 @@ def perform_request(method, endpoint, query_parameters: nil, body_parameters: ni end # If response is 2xx return the object, else raise the response as an exception - return parsed_response if response.code >= 200 && response.code <= 299 + return parsed_response if response.status >= 200 && response.status <= 299 exception_message = (parsed_response && parsed_response['message']) || 'Error' raise custom_exception_klass_for(response), exception_message - rescue Errno::EINVAL, Errno::ENETDOWN, Errno::ENETUNREACH, Errno::ENETRESET, Errno::ECONNABORTED, Errno::ECONNRESET, - Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTDOWN, Errno::EHOSTUNREACH, - Typesense::Error::TimeoutError, Typesense::Error::ServerError, Typesense::Error::HTTPStatus0Error => e + rescue Faraday::ConnectionFailed, Faraday::TimeoutError, + Errno::EINVAL, Errno::ENETDOWN, Errno::ENETUNREACH, Errno::ENETRESET, + Errno::ECONNABORTED, Errno::ECONNRESET, Errno::ETIMEDOUT, + Errno::ECONNREFUSED, Errno::EHOSTDOWN, Errno::EHOSTUNREACH, + Typesense::Error::ServerError, Typesense::Error::HTTPStatus0Error => e # Rescue network layer exceptions and HTTP 5xx errors, so the loop can continue. # Using loops for retries instead of rescue...retry to maintain consistency with client libraries in # other languages that might not support the same construct. @@ -176,23 +180,24 @@ def set_node_healthcheck(node, is_healthy:) end def custom_exception_klass_for(response) - if response.code == 400 + if response.status == 400 Typesense::Error::RequestMalformed.new(response: response) - elsif response.code == 401 + elsif response.status == 401 Typesense::Error::RequestUnauthorized.new(response: response) - elsif response.code == 404 + elsif response.status == 404 Typesense::Error::ObjectNotFound.new(response: response) - elsif response.code == 409 + elsif response.status == 409 Typesense::Error::ObjectAlreadyExists.new(response: response) - elsif response.code == 422 + elsif response.status == 422 Typesense::Error::ObjectUnprocessable.new(response: response) - elsif response.code >= 500 && response.code <= 599 + elsif response.status >= 500 && response.status <= 599 Typesense::Error::ServerError.new(response: response) - elsif response.timed_out? + elsif response.respond_to?(:timed_out?) && response.timed_out? Typesense::Error::TimeoutError.new(response: response) - elsif response.code.zero? + elsif response.status.zero? Typesense::Error::HTTPStatus0Error.new(response: response) else + # This will handle both 300-level responses and any other unhandled status codes Typesense::Error::HTTPError.new(response: response) end end diff --git a/spec/typesense/collections_spec.rb b/spec/typesense/collections_spec.rb index 96ed430..ded43c6 100644 --- a/spec/typesense/collections_spec.rb +++ b/spec/typesense/collections_spec.rb @@ -50,6 +50,112 @@ expect(result).to eq(company_schema) end + + context 'with integration', :integration do + let(:integration_schema) do + { + 'name' => 'integration_companies', + 'fields' => [ + { + 'name' => 'company_name', + 'type' => 'string', + 'facet' => false + }, + { + 'name' => 'num_employees', + 'type' => 'int32', + 'facet' => false + }, + { + 'name' => 'country', + 'type' => 'string', + 'facet' => true + } + ], + 'default_sorting_field' => 'num_employees' + } + end + + let(:integration_client) do + Typesense::Client.new( + nodes: [{ + host: 'localhost', + port: '8108', + protocol: 'http' + }], + api_key: 'xyz', + connection_timeout_seconds: 10 + ) + end + + let(:expected_fields) do + [ + { + 'name' => 'company_name', + 'type' => 'string', + 'facet' => false, + 'index' => true, + 'infix' => false, + 'locale' => '', + 'optional' => false, + 'sort' => false, + 'stem' => false, + 'store' => true + }, + { + 'name' => 'num_employees', + 'type' => 'int32', + 'facet' => false, + 'index' => true, + 'infix' => false, + 'locale' => '', + 'optional' => false, + 'sort' => true, + 'stem' => false, + 'store' => true + }, + { + 'name' => 'country', + 'type' => 'string', + 'facet' => true, + 'index' => true, + 'infix' => false, + 'locale' => '', + 'optional' => false, + 'sort' => false, + 'stem' => false, + 'store' => true + } + ] + end + + before do + WebMock.disable! + begin + integration_client.collections['integration_companies'].delete + rescue Typesense::Error::ObjectNotFound + # Collection doesn't exist, which is fine + end + end + + after do + begin + integration_client.collections['integration_companies'].delete + rescue Typesense::Error::ObjectNotFound + # Collection doesn't exist, which is fine + end + WebMock.enable! + end + + it 'creates a collection on a real Typesense server' do + result = integration_client.collections.create(integration_schema) + + expect(result['name']).to eq('integration_companies') + expect(result['fields']).to eq(expected_fields) + expect(result['default_sorting_field']).to eq(integration_schema['default_sorting_field']) + expect(result['num_documents']).to eq(0) + end + end end describe '#retrieve' do diff --git a/typesense.gemspec b/typesense.gemspec index 02f226a..6b7cb31 100644 --- a/typesense.gemspec +++ b/typesense.gemspec @@ -27,6 +27,6 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_dependency 'oj', '~> 3.16' - spec.add_dependency 'typhoeus', '~> 1.4' + spec.add_dependency 'faraday', '~> 2.12' spec.metadata['rubygems_mfa_required'] = 'true' end From 32dbb79429dd4709e067e6b581237a4c654329d7 Mon Sep 17 00:00:00 2001 From: Fanis Tharropoulos Date: Tue, 19 Nov 2024 10:29:24 +0200 Subject: [PATCH 2/5] chore: add erb to deps --- Gemfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Gemfile b/Gemfile index abfce55..b18290a 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } gem 'awesome_print', '~> 1.8' gem 'bundler', '~> 2.0' gem 'codecov', '~> 0.1' +gem 'erb' gem 'guard', '~> 2.16' gem 'guard-rubocop', '~> 1.3' gem 'rake', '~> 13.0' From 02aa0eb726cae320d62e869dae13a9f5fb39f900 Mon Sep 17 00:00:00 2001 From: Fanis Tharropoulos Date: Tue, 19 Nov 2024 10:29:37 +0200 Subject: [PATCH 3/5] chore: fix linting issue with gemspec dependencies --- typesense.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typesense.gemspec b/typesense.gemspec index 6b7cb31..76ab87c 100644 --- a/typesense.gemspec +++ b/typesense.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_dependency 'oj', '~> 3.16' spec.add_dependency 'faraday', '~> 2.12' + spec.add_dependency 'oj', '~> 3.16' spec.metadata['rubygems_mfa_required'] = 'true' end From 0ceb3d95f3c4bb5f81d9e3b777b7241f326d2f77 Mon Sep 17 00:00:00 2001 From: Fanis Tharropoulos Date: Tue, 19 Nov 2024 10:29:46 +0200 Subject: [PATCH 4/5] ci: add integration testing to ci --- .github/workflows/tests.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 53b2e17..55e8919 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,8 +11,25 @@ jobs: strategy: matrix: ruby-version: ['2.7', '3.0', '3.2'] + services: + typesense: + image: typesense/typesense:27.1 + ports: + - 8108:8108 + volumes: + - /tmp/typesense-data:/data + - /tmp/typesense-analytics:/analytics + env: + TYPESENSE_API_KEY: xyz + TYPESENSE_DATA_DIR: /data + TYPESENSE_ENABLE_CORS: true + TYPESENSE_ANALYTICS_DIR: /analytics + TYPESENSE_ENABLE_SEARCH_ANALYTICS: true steps: + - name: Wait for Typesense + run: | + timeout 20 bash -c 'while [[ "$(curl -s -o /dev/null -w ''%{http_code}'' localhost:8108/health)" != "200" ]]; do sleep 1; done' || false - uses: actions/checkout@v3 - uses: ruby/setup-ruby@v1 with: From f3e97ff534e3529f8f40ffecdcba5b7f9d69c10b Mon Sep 17 00:00:00 2001 From: Fanis Tharropoulos Date: Wed, 20 Nov 2024 17:53:03 +0200 Subject: [PATCH 5/5] chore: downgrade faraday to latest ruby 2.7 supported version --- typesense.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typesense.gemspec b/typesense.gemspec index 76ab87c..ea507aa 100644 --- a/typesense.gemspec +++ b/typesense.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_dependency 'faraday', '~> 2.12' + spec.add_dependency 'faraday', '~> 2.8' spec.add_dependency 'oj', '~> 3.16' spec.metadata['rubygems_mfa_required'] = 'true' end