Fix cname cache scoping and tighten error handling

Author: vipulnsward

lib/uploadcare/cname_generator.rb

@@ -11,7 +11,9 @@ class CnameGenerator
 
     class << self
       def cdn_base_postfix
-        @cdn_base_postfix ||= begin
+        @cdn_base_postfix_cache ||= {}
+        key = [Uploadcare.configuration.cdn_base_postfix, Uploadcare.configuration.public_key]
+        @cdn_base_postfix_cache[key] ||= begin
           uri = URI.parse(Uploadcare.configuration.cdn_base_postfix)
           uri.host = "#{generate_cname}.#{uri.host}"
           uri.to_s
@@ -28,10 +30,11 @@ def generate_cname
 
       # Generate CNAME prefix
       def custom_cname
-        @custom_cname ||= begin
-          public_key = Uploadcare.configuration.public_key
-          raise Uploadcare::Exception::ConfigurationError, "Invalid public_key: #{public_key}" if public_key.nil?
+        @custom_cname_cache ||= {}
+        public_key = Uploadcare.configuration.public_key
+        raise Uploadcare::Exception::ConfigurationError, "Invalid public_key: #{public_key}" if public_key.nil?
 
+        @custom_cname_cache[public_key] ||= begin
           sha256_hex = Digest::SHA256.hexdigest(public_key)
           sha256_hex = sha256_hex.to_i(16)
           sha256_base36 = sha256_hex.to_s(36)

lib/uploadcare/error_handler.rb

@@ -22,6 +22,8 @@ module ErrorHandler
     # @raise [Uploadcare::Exception::RequestError] for other error statuses
     def handle_error(error)
       response = error.response
+      return raise Exception::RequestError, error.message if response.nil?
+
       catch_upload_errors(response)
 
       error_message = extract_error_message(response)

lib/uploadcare/param/upload/upload_params_generator.rb

@@ -33,6 +33,7 @@ def store_value(store)
 
           def metadata(options:)
             return {} if options[:metadata].nil?
+            raise ArgumentError, 'metadata must be a hash' unless options[:metadata].is_a?(Hash)
 
             options[:metadata].each_with_object({}) do |(k, v), res|
               res.merge!("metadata[#{k}]" => v.to_s)

spec/uploadcare/cname_generator_spec.rb

@@ -5,8 +5,8 @@
 RSpec.describe Uploadcare::CnameGenerator do
   before do
     # Reset memoized variables between tests
-    described_class.instance_variable_set(:@custom_cname, nil)
-    described_class.instance_variable_set(:@cdn_base_postfix, nil)
+    described_class.instance_variable_set(:@custom_cname_cache, nil)
+    described_class.instance_variable_set(:@cdn_base_postfix_cache, nil)
     # Reset configuration
     Uploadcare.instance_variable_set(:@configuration, nil)
   end
@@ -33,7 +33,7 @@
     end
 
     it 'handles different CDN bases' do
-      described_class.instance_variable_set(:@cdn_base_postfix, nil)
+      described_class.instance_variable_set(:@cdn_base_postfix_cache, nil)
       allow(Uploadcare.configuration).to receive(:cdn_base_postfix).and_return('https://example.com')
       allow(described_class).to receive(:custom_cname).and_return('xyz789')
 
@@ -50,7 +50,7 @@
     end
 
     it 'handles CDN base with path' do
-      described_class.instance_variable_set(:@cdn_base_postfix, nil)
+      described_class.instance_variable_set(:@cdn_base_postfix_cache, nil)
       allow(Uploadcare.configuration).to receive(:cdn_base_postfix).and_return('https://cdn.example.com/path/')
       allow(described_class).to receive(:custom_cname).and_return('prefix123')
 
@@ -72,7 +72,7 @@
 
       invalid_urls.each do |invalid_url|
         # Reset memoization for each test
-        described_class.instance_variable_set(:@cdn_base_postfix, nil)
+        described_class.instance_variable_set(:@cdn_base_postfix_cache, nil)
         allow(Uploadcare.configuration).to receive(:cdn_base_postfix).and_return(invalid_url)
         allow(described_class).to receive(:generate_cname).and_return('test123')
 
@@ -110,7 +110,7 @@
       result1 = described_class.send(:custom_cname)
 
       # Reset memoization
-      described_class.instance_variable_set(:@custom_cname, nil)
+      described_class.instance_variable_set(:@custom_cname_cache, nil)
 
       allow(Uploadcare.configuration).to receive(:public_key).and_return('key2')
       result2 = described_class.send(:custom_cname)
@@ -213,7 +213,7 @@
         ]
 
         test_cases.each do |cdn_base|
-          described_class.instance_variable_set(:@cdn_base_postfix, nil)
+          described_class.instance_variable_set(:@cdn_base_postfix_cache, nil)
           allow(Uploadcare.configuration).to receive(:cdn_base_postfix).and_return(cdn_base)
           allow(described_class).to receive(:custom_cname).and_return('test123')
 

spec/uploadcare/param/upload/upload_params_generator_spec.rb

@@ -53,6 +53,6 @@
 
     expect do
       described_class.call(options: { metadata: 'nope' }, config: config)
-    end.to raise_error(NoMethodError)
+    end.to raise_error(ArgumentError, 'metadata must be a hash')
   end
 end