Merge pull request #38 from tharropoulos/faraday

Breaking: Migrate HTTP Client from Typhoeus to Faraday and Add Integration Tests

Author: jasonbosco

.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:

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'

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

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

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.8'
   spec.add_dependency 'oj', '~> 3.16'
-  spec.add_dependency 'typhoeus', '~> 1.4'
   spec.metadata['rubygems_mfa_required'] = 'true'
 end