diff --git a/CHANGELOG.md b/CHANGELOG.md index e49b6d4c..6b561dfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Reuse normal and multipart Faraday connections across requests, adapted from [@khasinski](https://github.com/khasinski)'s [upstream work](https://github.com/alexrudall/ruby-openai/pull/636). + ## [9.0.0] - 2026-06-27 ### Changed diff --git a/lib/shopif_ai/client.rb b/lib/shopif_ai/client.rb index b3d7d918..68ecb227 100644 --- a/lib/shopif_ai/client.rb +++ b/lib/shopif_ai/client.rb @@ -3,7 +3,36 @@ module ShopifAi class Client include ShopifAi::HTTP - SENSITIVE_ATTRIBUTES = %i[@access_token @admin_token @organization_id @extra_headers].freeze + SENSITIVE_ATTRIBUTES = %i[ + @access_token + @admin_token + @conn + @extra_headers + @multipart_conn + @organization_id + ].freeze + MEMOIZED_ATTRIBUTES = %i[ + @assistants + @audio + @batches + @conn + @conversations + @files + @finetunes + @images + @messages + @models + @multipart_conn + @realtime + @responses + @run_steps + @runs + @threads + @usage + @vector_store_file_batches + @vector_store_files + @vector_stores + ].freeze CONFIG_KEYS = %i[access_token admin_token api_type api_version extra_headers log_errors organization_id request_timeout uri_base].freeze attr_reader(*CONFIG_KEYS, :faraday_middleware) @@ -19,6 +48,7 @@ def initialize(config = {}, &faraday_middleware) ) end @faraday_middleware = faraday_middleware + @connection_mutex = Mutex.new end def chat(parameters: {}, extra_headers: {}) @@ -139,6 +169,16 @@ def inspect "#<#{self.class}:#{object_id} #{vars.join(', ')}>" end + + private + + def initialize_dup(_original) + super + MEMOIZED_ATTRIBUTES.each do |attribute| + remove_instance_variable(attribute) if instance_variable_defined?(attribute) + end + @connection_mutex = Mutex.new + end end end # rubocop:enable Metrics/ClassLength diff --git a/lib/shopif_ai/http.rb b/lib/shopif_ai/http.rb index 1f7cc663..62317dca 100644 --- a/lib/shopif_ai/http.rb +++ b/lib/shopif_ai/http.rb @@ -56,6 +56,19 @@ def parse_json(response) end def conn(multipart: false) + connection = multipart ? @multipart_conn : @conn + return connection if connection + + @connection_mutex.synchronize do + if multipart + @multipart_conn ||= build_connection(multipart: true) + else + @conn ||= build_connection + end + end + end + + def build_connection(multipart: false) connection = Faraday.new do |f| f.options[:timeout] = @request_timeout f.request(:multipart) if multipart @@ -65,6 +78,8 @@ def conn(multipart: false) end @faraday_middleware&.call(connection) + # Faraday builds middleware lazily without synchronization, so publish a fully built stack. + connection.builder.app connection end diff --git a/spec/shopif_ai/client/client_spec.rb b/spec/shopif_ai/client/client_spec.rb index 836cd8fb..44fb52db 100644 --- a/spec/shopif_ai/client/client_spec.rb +++ b/spec/shopif_ai/client/client_spec.rb @@ -126,6 +126,28 @@ expect(admin_client.access_token).to eq(admin_token) expect(client.access_token).not_to eq(admin_token) # Original unchanged end + + it "rebuilds memoized endpoints for the admin client" do + authorization_headers = [] + client = ShopifAi::Client.new( + access_token: "user-token", + admin_token: admin_token, + api_version: "", + uri_base: "https://example.test" + ) do |faraday| + faraday.adapter :test do |stubs| + stubs.get("/threads/thread-id") do |env| + authorization_headers << env.request_headers["Authorization"] + [200, { "Content-Type" => "application/json" }, "{}"] + end + end + end + client.threads + + client.admin.threads.retrieve(id: "thread-id") + + expect(authorization_headers).to eq(["Bearer #{admin_token}"]) + end end context "when using both beta and admin" do @@ -155,6 +177,54 @@ end end + context "when building HTTP connections" do + let(:client) { ShopifAi::Client.new } + + it "reuses normal and multipart connections" do + connection = client.send(:conn) + multipart_connection = client.send(:conn, multipart: true) + + expect(client.send(:conn)).to equal(connection) + expect(client.send(:conn, multipart: true)).to equal(multipart_connection) + expect(multipart_connection).not_to equal(connection) + end + + it "builds the middleware stack once under concurrent first requests" do + build_count = 0 + count_mutex = Mutex.new + middleware_initialize = Faraday::Middleware.instance_method(:initialize) + middleware_class = Class.new(Faraday::Middleware) do + define_method(:initialize) do |app| + count_mutex.synchronize { build_count += 1 } + sleep 0.01 + middleware_initialize.bind(self).call(app) + end + end + concurrent_client = ShopifAi::Client.new(uri_base: "https://example.test") do |faraday| + faraday.use middleware_class + faraday.adapter :test do |stubs| + stubs.get("/v1/ping") { [200, { "Content-Type" => "application/json" }, "{}"] } + end + end + + responses = Array.new(8) do + Thread.new { concurrent_client.send(:get, path: "/ping") } + end.map(&:value) + + expect(responses).to all(eq({})) + expect(build_count).to eq(1) + end + + it "does not share connections with duplicated clients" do + connection = client.send(:conn) + multipart_connection = client.send(:conn, multipart: true) + duplicate = client.dup + + expect(duplicate.send(:conn)).not_to equal(connection) + expect(duplicate.send(:conn, multipart: true)).not_to equal(multipart_connection) + end + end + context "with a block" do let(:client) do ShopifAi::Client.new do |client| @@ -162,15 +232,18 @@ end end - it "sets the logger" do - connection = Faraday.new - client.faraday_middleware.call(connection) + it "applies the configured middleware to both connections" do + connection = client.send(:conn) + multipart_connection = client.send(:conn, multipart: true) + expect(connection.builder.handlers).to include Faraday::Response::Logger + expect(multipart_connection.builder.handlers).to include Faraday::Response::Logger end end context "when calling inspect" do let(:api_key) { "sk-123456789" } + let(:connection_header) { "Bearer connection-secret" } let(:organization_id) { "org-123456789" } let(:extra_headers) { { "Other-Auth": "key-123456789" } } let(:uri_base) { "https://example.com/" } @@ -182,13 +255,18 @@ access_token: api_key, organization_id: organization_id, extra_headers: extra_headers - ) + ) do |connection| + connection.headers["Authorization"] = connection_header + end end it "does not expose sensitive information" do + client.send(:conn) + client.send(:conn, multipart: true) expect(client.inspect).not_to include(api_key) expect(client.inspect).not_to include(organization_id) expect(client.inspect).not_to include(extra_headers[:"Other-Auth"]) + expect(client.inspect).not_to include(connection_header) end it "does expose non-sensitive information" do