DEV: Clean up S3 specs, stubs, and helpers

DEV: Clean up S3 specs, stubs, and helpers

Extracted commonly used spec helpers into spec/support/uploads_helpers.rb, removed unused stubs and let definitions. Makes it easier to write new S3-related specs without copy and pasting setup steps from other specs.

diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index 143536b..dc5842f 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -4,17 +4,6 @@ require "rails_helper"
 require "cooked_post_processor"
 require "file_store/s3_store"
 
-def s3_setup
-  Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com")
-
-  SiteSetting.s3_upload_bucket = "some-bucket-on-s3"
-  SiteSetting.s3_access_key_id = "s3-access-key-id"
-  SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-  SiteSetting.s3_cdn_url = "https://s3.cdn.com"
-  SiteSetting.enable_s3_uploads = true
-  SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
-end
-
 describe CookedPostProcessor do
   fab!(:upload) { Fabricate(:upload) }
   let(:upload_path) { Discourse.store.upload_path }
@@ -483,17 +472,16 @@ describe CookedPostProcessor do
 
         context "s3_uploads" do
           let(:upload) { Fabricate(:secure_upload_s3) }
+
           before do
-            s3_setup
+            setup_s3
+            SiteSetting.s3_cdn_url = "https://s3.cdn.com"
+            SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
+
             stored_path = Discourse.store.get_path_for_upload(upload)
             upload.update_column(:url, "#{SiteSetting.Upload.absolute_base_url}/#{stored_path}")
 
-            stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-            stub_request(
-              :put,
-              "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/optimized/1X/#{upload.sha1}_2_#{optimized_size}.#{upload.extension}"
-            )
-            stub_request(:get, /#{SiteSetting.s3_upload_bucket}\.s3\.amazonaws\.com/)
+            stub_upload(upload)
 
             SiteSetting.login_required = true
             SiteSetting.secure_media = true
@@ -1049,11 +1037,11 @@ describe CookedPostProcessor do
 
       context "when the post is with_secure_media and the upload is secure and secure media is enabled" do
         before do
+          setup_s3
           upload.update(secure: true)
+
           SiteSetting.login_required = true
-          s3_setup
           SiteSetting.secure_media = true
-          stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
         end
 
         it "does not use the direct URL, uses the cooked URL instead (because of the private ACL preventing w/h fetch)" do
@@ -1269,7 +1257,11 @@ describe CookedPostProcessor do
 
       context "s3_uploads" do
         before do
-          s3_setup
+          Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com")
+
+          setup_s3
+          SiteSetting.s3_cdn_url = "https://s3.cdn.com"
+          SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
 
           uploaded_file = file_from_fixtures("smallest.png")
           upload_sha1 = Digest::SHA1.hexdigest(File.read(uploaded_file))
@@ -1546,9 +1538,7 @@ describe CookedPostProcessor do
       end
 
       it "doesn't disable download_remote_images_to_local if site uses S3" do
-        SiteSetting.s3_access_key_id = "s3-access-key-id"
-        SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
         cpp.disable_if_low_on_disk_space
 
         expect(SiteSetting.download_remote_images_to_local).to eq(true)
diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb
index 38d4f27..54b30ce 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -408,25 +408,10 @@ describe Email::Sender do
     end
 
     context "when secure media enabled" do
-      def enable_s3_uploads
-        SiteSetting.enable_s3_uploads = true
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secrets3_region key"
-        stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-        stub_request(
-          :put,
-          "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image.sha1}.#{image.extension}?acl"
-        )
-        store = FileStore::S3Store.new
-        s3_helper = store.instance_variable_get(:@s3_helper)
-        client = Aws::S3::Client.new(stub_responses: true)
-        s3_helper.stubs(:s3_client).returns(client)
-        Discourse.stubs(:store).returns(store)
-      end
-
       before do
-        enable_s3_uploads
+        setup_s3
+        stub_s3_store
+
         SiteSetting.secure_media = true
         SiteSetting.login_required = true
         SiteSetting.email_total_attachment_size_limit_kb = 14_000
diff --git a/spec/components/file_store/base_store_spec.rb b/spec/components/file_store/base_store_spec.rb
index 708235d..4771b28 100644
--- a/spec/components/file_store/base_store_spec.rb
+++ b/spec/components/file_store/base_store_spec.rb
@@ -47,12 +47,7 @@ RSpec.describe FileStore::BaseStore do
 
   describe '#download' do
     before do
-      SiteSetting.enable_s3_uploads = true
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secret key"
-      SiteSetting.s3_region = "us-east-1"
-
+      setup_s3
       stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world")
     end
 
@@ -78,7 +73,7 @@ RSpec.describe FileStore::BaseStore do
     end
 
     it "should return the file when s3 cdn enabled" do
-      SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com"
+      SiteSetting.s3_cdn_url = "https://cdn.s3.#{SiteSetting.s3_region}.amazonaws.com"
       stub_request(:get, Discourse.store.cdn_url(upload_s3.url)).to_return(status: 200, body: "Hello world")
 
       file = store.download(upload_s3)
@@ -90,7 +85,7 @@ RSpec.describe FileStore::BaseStore do
       SiteSetting.login_required = true
       SiteSetting.secure_media = true
 
-      stub_request(:head, "https://s3-upload-bucket.s3.amazonaws.com/")
+      stub_request(:head, "https://s3-upload-bucket.s3.#{SiteSetting.s3_region}.amazonaws.com/")
       signed_url = Discourse.store.signed_url_for_path(upload_s3.url)
       stub_request(:get, signed_url).to_return(status: 200, body: "Hello world")
 
diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb
index d5bbceb..e9e4f7c 100644
--- a/spec/components/file_store/s3_store_spec.rb
+++ b/spec/components/file_store/s3_store_spec.rb
@@ -5,42 +5,26 @@ require 'file_store/s3_store'
 require 'file_store/local_store'
 
 describe FileStore::S3Store do
-
   let(:store) { FileStore::S3Store.new }
   let(:s3_helper) { store.instance_variable_get(:@s3_helper) }
-  fab!(:upload) { Fabricate(:upload) }
-  let(:uploaded_file) { file_from_fixtures("logo.png") }
+  let(:client) { Aws::S3::Client.new(stub_responses: true) }
+  let(:resource) { Aws::S3::Resource.new(client: client) }
+  let(:s3_bucket) { resource.bucket("s3-upload-bucket") }
+  let(:s3_object) { stub }
 
   fab!(:optimized_image) { Fabricate(:optimized_image) }
   let(:optimized_image_file) { file_from_fixtures("logo.png") }
-
-  before(:each) do
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "s3-access-key-id"
-    SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-    SiteSetting.enable_s3_uploads = true
+  let(:uploaded_file) { file_from_fixtures("logo.png") }
+  fab!(:upload) do
+    Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string'))
   end
 
-  shared_context 's3 helpers' do
-    fab!(:upload) do
-      Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string'))
-    end
-
-    let(:store) { FileStore::S3Store.new }
-    let(:client) { Aws::S3::Client.new(stub_responses: true) }

[... diff too long, it was truncated ...]

GitHub sha: e00abbe1

This commit appears in #10756 which was approved by danielwaterworth and danielwaterworth. It was merged by danielwaterworth.