From 05a4f3fb51201c1c5bf647140e19c31ba25eb971 Mon Sep 17 00:00:00 2001
From: Rishabh <rishabh.nambiar@discourse.org>
Date: Thu, 29 Nov 2018 09:41:48 +0530
Subject: [PATCH] FEATURE: Multisite support for S3 image stores (#6689)

* FEATURE: Multisite support for S3 image stores

* Use File.join to concatenate all paths & fix linting on multisite/s3_store_spec.rb
---
 lib/file_store/base_store.rb    |  4 ++++
 lib/file_store/local_store.rb   | 14 ++++-------
 lib/file_store/s3_store.rb      |  7 ++++--
 spec/multisite/s3_store_spec.rb | 42 +++++++++++++++++++++++++++++++++
 4 files changed, 55 insertions(+), 12 deletions(-)
 create mode 100644 spec/multisite/s3_store_spec.rb

diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb
index 2cd9dad0e31..bdc48562448 100644
--- a/lib/file_store/base_store.rb
+++ b/lib/file_store/base_store.rb
@@ -28,6 +28,10 @@ module FileStore
       not_implemented
     end
 
+    def upload_path
+      File.join("uploads", RailsMultisite::ConnectionManagement.current_db)
+    end
+
     def has_been_uploaded?(url)
       not_implemented
     end
diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb
index 63ae443aad6..fc24c767bd5 100644
--- a/lib/file_store/local_store.rb
+++ b/lib/file_store/local_store.rb
@@ -32,10 +32,6 @@ module FileStore
       "#{Discourse.asset_host}#{relative_base_url}"
     end
 
-    def upload_path
-      "/uploads/#{RailsMultisite::ConnectionManagement.current_db}"
-    end
-
     def relative_base_url
       "#{Discourse.base_uri}#{upload_path}"
     end
@@ -46,7 +42,7 @@ module FileStore
 
     def download_url(upload)
       return unless upload
-      "#{relative_base_url}/#{upload.sha1}"
+      File.join(relative_base_url, upload.sha1)
     end
 
     def cdn_url(url)
@@ -68,7 +64,7 @@ module FileStore
     end
 
     def get_path_for(type, upload_id, sha, extension)
-      "#{upload_path}/#{super(type, upload_id, sha, extension)}"
+      File.join("/", upload_path, super(type, upload_id, sha, extension))
     end
 
     def copy_file(file, path)
@@ -90,7 +86,7 @@ module FileStore
     end
 
     def public_dir
-      "#{Rails.root}/public"
+      File.join(Rails.root, "public")
     end
 
     def tombstone_dir
@@ -105,15 +101,13 @@ module FileStore
     private
 
     def list_missing(model)
-      public_directory = "#{Rails.root}/public"
-
       count = 0
       model.find_each do |upload|
 
         # could be a remote image
         next unless upload.url =~ /^\/[^\/]/
 
-        path = "#{public_directory}#{upload.url}"
+        path = "#{public_dir}#{upload.url}"
         bad = true
         begin
           bad = false if File.size(path) != 0
diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb
index 08d26001375..35fc3249819 100644
--- a/lib/file_store/s3_store.rb
+++ b/lib/file_store/s3_store.rb
@@ -41,9 +41,12 @@ module FileStore
       # add a "content disposition" header for "attachments"
       options[:content_disposition] = "attachment; filename=\"#{filename}\"" unless FileHelper.is_supported_image?(filename)
       # if this fails, it will throw an exception
+
+      path.prepend(File.join(upload_path, "/")) if RailsMultisite::ConnectionManagement.current_db != "default"
       path = @s3_helper.upload(file, path, options)
+
       # return the upload url
-      "#{absolute_base_url}/#{path}"
+      File.join(absolute_base_url, path)
     end
 
     def remove_file(url, path)
@@ -93,7 +96,7 @@ module FileStore
       return url if SiteSetting.Upload.s3_cdn_url.blank?
       schema = url[/^(https?:)?\/\//, 1]
       folder = @s3_helper.s3_bucket_folder_path.nil? ? "" : "#{@s3_helper.s3_bucket_folder_path}/"
-      url.sub("#{schema}#{absolute_base_url}/#{folder}", "#{SiteSetting.Upload.s3_cdn_url}/")
+      url.sub(File.join("#{schema}#{absolute_base_url}", folder), File.join(SiteSetting.Upload.s3_cdn_url, "/"))
     end
 
     def cache_avatar(avatar, user_id)
diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb
new file mode 100644
index 00000000000..e3e85428a85
--- /dev/null
+++ b/spec/multisite/s3_store_spec.rb
@@ -0,0 +1,42 @@
+require 'rails_helper'
+require 'file_store/s3_store'
+
+RSpec.describe 'Multisite s3 uploads', type: :multisite do
+  let(:conn) { RailsMultisite::ConnectionManagement }
+  let(:uploaded_file) { file_from_fixtures("smallest.png") }
+
+  let(:upload) do
+    Fabricate(:upload, sha1: Digest::SHA1.hexdigest(File.read(uploaded_file)))
+  end
+
+  let(:s3_client) { Aws::S3::Client.new(stub_responses: true) }
+
+  let(:s3_helper) do
+    S3Helper.new(SiteSetting.s3_upload_bucket, '', client: s3_client)
+  end
+
+  let(:store) { FileStore::S3Store.new(s3_helper) }
+
+  context 'uploading to s3' do
+    before(:each) do
+      SiteSetting.s3_upload_bucket = "some-really-cool-bucket"
+      SiteSetting.s3_access_key_id = "s3-access-key-id"
+      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
+      SiteSetting.enable_s3_uploads = true
+    end
+
+    describe "#store_upload" do
+      it "returns the correct url for default and second multisite db" do
+        expect(store.store_upload(uploaded_file, upload)).to eq(
+          "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
+        )
+
+        conn.with_connection('second') do
+          expect(store.store_upload(uploaded_file, upload)).to eq(
+            "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/uploads/second/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
+          )
+        end
+      end
+    end
+  end
+end