From 04bfab3f00b8290965e72fd944a8beb0477c5e8e Mon Sep 17 00:00:00 2001
From: Mitsuhiro Shibuya <mit.shibuya@gmail.com>
Date: Sun, 1 Dec 2024 18:37:20 +0900
Subject: [PATCH] Follow-up for #2763

- Raises an error instead of returning nil
- Reorganize spec/storage/fog_helper.rb so it doesn't have high cyclomatic complexity
---
 .rubocop.yml                   |  2 +-
 lib/carrierwave/storage/fog.rb | 11 +++--------
 spec/storage/fog_helper.rb     | 10 +++++-----
 spec/storage/fog_spec.rb       |  2 +-
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index d0d6efcdf..aec22bbf0 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -90,7 +90,7 @@ Metrics/ClassLength:
   Enabled: false
 
 Metrics/CyclomaticComplexity:
-  Max: 28
+  Max: 18
 
 Metrics/MethodLength:
   Enabled: false
diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb
index 7e97c0bb3..d837261a6 100644
--- a/lib/carrierwave/storage/fog.rb
+++ b/lib/carrierwave/storage/fog.rb
@@ -376,12 +376,8 @@ def public_url
             when 'AWS'
               # check if some endpoint is set in fog_credentials
               if @uploader.fog_credentials.has_key?(:endpoint)
-                if !@uploader.fog_aws_fips
-                  "#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
-                else
-                  warn 'Use of options :endpoint and :fog_aws_fips=true together will fail, as FIPS endpoints do not support path-style URLs.'
-                  nil
-                end
+                raise 'fog_aws_fips = true is incompatible with :endpoint, as FIPS endpoints do not support path-style URLs.' if @uploader.fog_aws_fips
+                "#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
               else
                 protocol = @uploader.fog_use_ssl_for_aws ? "https" : "http"
 
@@ -389,8 +385,6 @@ def public_url
                 # To use the virtual-hosted style, the bucket name needs to be representable as a subdomain
                 use_virtual_hosted_style = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./)
 
-                return nil if !use_virtual_hosted_style && @uploader.fog_aws_fips # FIPS Endpoints can only be used with Virtual Hosted-Style addressing.
-
                 region = @uploader.fog_credentials[:region].to_s
                 regional_host = 's3.amazonaws.com' # used for DEFAULT_S3_REGION or no region set
                 if @uploader.fog_aws_fips
@@ -403,6 +397,7 @@ def public_url
                   regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate
                   "#{protocol}://#{@uploader.fog_directory}.#{regional_host}/#{encoded_path}"
                 else # directory is not a valid subdomain, so use path style for access
+                  raise 'FIPS Endpoints can only be used with Virtual Hosted-Style addressing.' if @uploader.fog_aws_fips
                   "#{protocol}://#{regional_host}/#{@uploader.fog_directory}/#{encoded_path}"
                 end
               end
diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb
index 43c554439..0163234b2 100644
--- a/spec/storage/fog_helper.rb
+++ b/spec/storage/fog_helper.rb
@@ -1,4 +1,4 @@
-def fog_tests(fog_credentials)
+shared_examples "Fog storage" do |fog_credentials|
   describe "with #{fog_credentials[:provider]} provider", with_retry: !Fog.mocking do
     before do
       WebMock.disable! unless Fog.mocking?
@@ -506,11 +506,11 @@ def check_file
             expect(@fog_file.public_url).to include("https://#{CARRIERWAVE_DIRECTORY}.s3-accelerate.amazonaws.com")
           end
 
-          it 'returns nil when both :endpoint and :fog_aws_fips=true' do
+          it 'raises an error when both :endpoint and :fog_aws_fips=true' do
             allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com'))
             allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')
             allow(@uploader).to receive(:fog_aws_fips).and_return(true)
-            expect(@fog_file.url).to be nil
+            expect { @fog_file.url }.to raise_error RuntimeError, /incompatible/
           end
 
           it 'returns endpoint+bucket when :endpoint and !:fog_aws_fips' do
@@ -562,13 +562,13 @@ def check_file
               'us-east-2',
               'us-gov-west-1'
             ].each do |region|
-              it "public_url should be nil" do
+              it "raises an error" do
                 allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true)
                 allow(@uploader).to receive(:fog_directory).and_return('foo.bar')
                 allow(@uploader).to receive(:fog_aws_fips).and_return(true)
                 allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region))
 
-                expect(@fog_file.public_url).to be_nil
+                expect { @fog_file.url }.to raise_error RuntimeError, /Virtual Hosted-Style/
               end
             end
           end
diff --git a/spec/storage/fog_spec.rb b/spec/storage/fog_spec.rb
index 6957e67e2..10671dded 100644
--- a/spec/storage/fog_spec.rb
+++ b/spec/storage/fog_spec.rb
@@ -17,7 +17,7 @@
 
 describe CarrierWave::Storage::Fog do
   FOG_CREDENTIALS.each do |credential|
-    fog_tests(credential)
+    include_examples 'Fog storage', credential
   end
 
   describe '.eager_load' do