REFACTOR: Get bucket name from S3Helper.

lib/file_store/s3_store.rb

@@ -8,9 +8,8 @@ module FileStore
   class S3Store < BaseStore
     TOMBSTONE_PREFIX ||= "tombstone/"
 
-    def initialize(s3_helper=nil, s3_bucket=get_s3_bucket)
+    def initialize(s3_helper=nil)
-      @s3_bucket = s3_bucket
+      @s3_helper = s3_helper || S3Helper.new(s3_bucket, TOMBSTONE_PREFIX)
-      @s3_helper = s3_helper || S3Helper.new(@s3_bucket, TOMBSTONE_PREFIX)
     end
 
     def store_upload(file, upload, content_type = nil)
@@ -62,7 +61,7 @@ module FileStore
     end
 
     def absolute_base_url
-      bucket = @s3_bucket.split("/".freeze, 2).first
+      bucket = @s3_helper.s3_bucket_name
 
       # cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region
       @absolute_base_url ||= if SiteSetting.s3_region == "us-east-1"
@@ -103,7 +102,7 @@ module FileStore
       UserAvatar.external_avatar_url(user_id, avatar.upload_id, avatar.width)
     end
 
-    def get_s3_bucket
+    def s3_bucket
       raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank?
       SiteSetting.s3_upload_bucket.downcase
     end

lib/s3_helper.rb

@@ -4,10 +4,12 @@ class S3Helper
 
   class SettingMissing < StandardError; end
 
+  attr_reader :s3_bucket_name
+
   def initialize(s3_upload_bucket, tombstone_prefix='', options={})
     @s3_options = default_s3_options.merge(options)
 
-    @s3_bucket, @s3_bucket_folder_path = begin
+    @s3_bucket_name, @s3_bucket_folder_path = begin
       raise Discourse::InvalidParameters.new("s3_bucket") if s3_upload_bucket.blank?
       s3_upload_bucket.downcase.split("/".freeze, 2)
     end
@@ -36,7 +38,7 @@ class S3Helper
     if copy_to_tombstone && @tombstone_prefix.present?
       bucket
         .object(File.join(@tombstone_prefix, s3_filename))
-        .copy_from(copy_source: File.join(@s3_bucket, get_path_for_s3_upload(s3_filename)))
+        .copy_from(copy_source: File.join(@s3_bucket_name, get_path_for_s3_upload(s3_filename)))
     end
 
     # delete the file
@@ -49,7 +51,7 @@ class S3Helper
 
     # cf. http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html
     s3_resource.client.put_bucket_lifecycle({
-      bucket: @s3_bucket,
+      bucket: @s3_bucket_name,
       lifecycle_configuration: {
         rules: [
           {
@@ -86,7 +88,7 @@ class S3Helper
   end
 
   def s3_bucket
-    bucket = s3_resource.bucket(@s3_bucket)
+    bucket = s3_resource.bucket(@s3_bucket_name)
     bucket.create unless bucket.exists?
     bucket
   end

spec/components/file_store/s3_store_spec.rb

@@ -4,9 +4,8 @@ require 'file_store/local_store'
 
 describe FileStore::S3Store do
 
-  let(:s3_helper) { stub }
+  let(:store) { FileStore::S3Store.new }
-  let(:store) { FileStore::S3Store.new(s3_helper) }
+  let(:s3_helper) { store.instance_variable_get(:@s3_helper) }
-
   let(:upload) { Fabricate(:upload) }
   let(:uploaded_file) { file_from_fixtures("logo.png") }
 
@@ -205,7 +204,6 @@ describe FileStore::S3Store do
   end
 
   describe ".absolute_base_url" do
-
     it "returns a lowercase schemaless absolute url" do
       expect(store.absolute_base_url).to eq("//s3-upload-bucket.s3.amazonaws.com")
     end