From b157af6fbb4510c1063f0e8dbe15e307ad51b8d4 Mon Sep 17 00:00:00 2001
From: Mark Cooper <551470+mark-cooper@users.noreply.github.com>
Date: Fri, 24 Apr 2026 13:08:47 -0700
Subject: [PATCH] Some overdue cleanup

Configuration gets a timeout and added to request setup.
The timeout matches Net HTTP defaults so should be backwards compat.
Configuration uses accessor to set (no poking at internals).
Default headers made reasonable, no placeholder values.
Request sets basic_auth via options like everything (httparty is weird).
Adds a User-Agent for the client.
Fix passed in options (a reference) being mutated by Request.
---
 lib/collectionspace/client/client.rb        |  2 ++
 lib/collectionspace/client/configuration.rb |  9 +++---
 lib/collectionspace/client/request.rb       | 34 ++++++---------------
 spec/collectionspace/configuration_spec.rb  |  1 +
 spec/collectionspace/request_spec.rb        |  7 +++++
 5 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/lib/collectionspace/client/client.rb b/lib/collectionspace/client/client.rb
index 5224112..0c04bdf 100644
--- a/lib/collectionspace/client/client.rb
+++ b/lib/collectionspace/client/client.rb
@@ -7,6 +7,8 @@ class Client
 
     attr_reader :config
 
+    NAME = "CollectionSpaceClient"
+
     def initialize(config = Configuration.new)
       unless config.is_a? CollectionSpace::Configuration
         raise CollectionSpace::ArgumentError, "Invalid configuration object"
diff --git a/lib/collectionspace/client/configuration.rb b/lib/collectionspace/client/configuration.rb
index aea29d9..e6844d4 100644
--- a/lib/collectionspace/client/configuration.rb
+++ b/lib/collectionspace/client/configuration.rb
@@ -10,18 +10,19 @@ class Configuration
       page_size: 25,
       include_deleted: false,
       throttle: 0,
+      timeout: 60,
       verbose: false,
       verify_ssl: true
     }.freeze
 
-    attr_accessor :base_uri, :username, :password, :page_size, :include_deleted, :throttle, :verbose, :verify_ssl
+    attr_accessor :base_uri, :username, :password, :page_size, :include_deleted,
+      :throttle, :timeout, :verbose, :verify_ssl
 
     def initialize(settings = {})
-      settings = DEFAULTS.merge(settings)
-      settings.each do |property, value|
+      DEFAULTS.merge(settings).each do |property, value|
         next unless DEFAULTS.key?(property)
 
-        instance_variable_set(:"@#{property}", value)
+        send(:"#{property}=", value)
       end
     end
   end
diff --git a/lib/collectionspace/client/request.rb b/lib/collectionspace/client/request.rb
index 17e0ece..6d22467 100644
--- a/lib/collectionspace/client/request.rb
+++ b/lib/collectionspace/client/request.rb
@@ -7,38 +7,22 @@ class Request
 
     attr_reader :config, :headers, :method, :path, :options
 
-    def default_headers(method = :get)
-      headers = {
-        delete: {},
-        get: {},
-        post: {
-          "Content-Type" => "application/xml",
-          "Content-Length" => "nnnn"
-        },
-        put: {
-          "Content-Type" => "application/xml",
-          "Content-Length" => "nnnn"
-        }
-      }
-      headers[method]
-    end
+    DEFAULT_HEADERS = {"Content-Type" => "application/xml"}.freeze
 
     def initialize(config, method = "GET", path = "", options = {})
       @config = config
       @method = method.downcase.to_sym
-      @path = path.gsub(%r{^/}, "")
+      @path = path.gsub(%r{^/+}, "")
+
+      @options = options.dup
+      @options[:basic_auth] = {username: config.username, password: config.password}
 
-      @auth = {
-        username: config.username,
-        password: config.password
-      }
+      @options[:headers] = DEFAULT_HEADERS.merge(@options.fetch(:headers, {}))
+      @options[:headers]["User-Agent"] = "#{Client::NAME}/#{Client::VERSION}"
 
-      headers = default_headers(@method).merge(options.fetch(:headers, {}))
-      @options = options
-      @options[:basic_auth] = @auth
-      @options[:headers] = headers
+      @options[:query] = @options.fetch(:query, {})
+      @options[:timeout] = config.timeout
       @options[:verify] = config.verify_ssl
-      @options[:query] = options.fetch(:query, {})
 
       self.class.base_uri config.base_uri
       self.class.debug_output $stdout if config.verbose
diff --git a/spec/collectionspace/configuration_spec.rb b/spec/collectionspace/configuration_spec.rb
index 0073ab1..aff69b3 100644
--- a/spec/collectionspace/configuration_spec.rb
+++ b/spec/collectionspace/configuration_spec.rb
@@ -13,6 +13,7 @@
     expect(config.page_size).to eq 25
     expect(config.include_deleted).to eq false
     expect(config.throttle).to eq 0
+    expect(config.timeout).to eq 60
     expect(config.verbose).to eq false
     expect(config.verify_ssl).to eq true
   end
diff --git a/spec/collectionspace/request_spec.rb b/spec/collectionspace/request_spec.rb
index f691d57..bd27491 100644
--- a/spec/collectionspace/request_spec.rb
+++ b/spec/collectionspace/request_spec.rb
@@ -17,6 +17,13 @@
     }
   end
 
+  it "does not mutate the caller's options hash" do
+    opts = {query: {foo: "bar"}, headers: {"X-Custom" => "1"}}
+    caller_opts = Marshal.load(Marshal.dump(opts))
+    CollectionSpace::Request.new(default_config, "GET", "collectionobjects", opts)
+    expect(opts).to eq(caller_opts)
+  end
+
   it "can create a collectionobject" do
     VCR.use_cassette("request_collectionobjects_create") do
       response = client.post("collectionobjects", post_payload)