From 3b7f5db5ba9d4db23593fe116499f9583fed271f Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 18 Dec 2019 11:21:57 +0530 Subject: [PATCH] FIX: parallel spec system needs a dedicated upload folder for each worker. (#8547) --- app/services/inline_uploads.rb | 18 ++--- config/routes.rb | 3 + lib/backup_restore/backuper.rb | 4 +- lib/backup_restore/restorer.rb | 15 ++-- lib/discourse.rb | 4 + lib/disk_space.rb | 2 +- lib/file_store/base_store.rb | 6 +- lib/s3_inventory.rb | 2 +- spec/components/cooked_post_processor_spec.rb | 75 ++++++++++--------- .../components/file_store/local_store_spec.rb | 27 +++---- spec/components/url_helper_spec.rb | 2 +- spec/fabricators/post_fabricator.rb | 38 +++++----- .../helpers/user_notifications_helper_spec.rb | 6 +- spec/jobs/pull_hotlinked_images_spec.rb | 5 +- spec/models/post_spec.rb | 18 +++-- spec/multisite/s3_store_spec.rb | 38 +++++----- 16 files changed, 142 insertions(+), 121 deletions(-) diff --git a/app/services/inline_uploads.rb b/app/services/inline_uploads.rb index 8168543b243..30727ec2b8e 100644 --- a/app/services/inline_uploads.rb +++ b/app/services/inline_uploads.rb @@ -54,10 +54,8 @@ class InlineUploads raw_matches << [match, href, replacement, index] end - db = RailsMultisite::ConnectionManagement.current_db - regexps = [ - /(https?:\/\/[a-zA-Z0-9\.\/-]+\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/, + /(https?:\/\/[a-zA-Z0-9\.\/-]+\/#{Discourse.store.upload_path}#{UPLOAD_REGEXP_PATTERN})/, ] if Discourse.store.external? @@ -219,28 +217,28 @@ class InlineUploads end def self.matched_uploads(node) - db = RailsMultisite::ConnectionManagement.current_db + upload_path = Discourse.store.upload_path base_url = Discourse.base_url.sub(/https?:\/\//, "(https?://)") regexps = [ /(upload:\/\/([a-zA-Z0-9]+)[a-zA-Z0-9\.]*)/, /(\/uploads\/short-url\/([a-zA-Z0-9]+)[a-zA-Z0-9\.]*)/, /(#{base_url}\/uploads\/short-url\/([a-zA-Z0-9]+)[a-zA-Z0-9\.]*)/, - /(#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/, - /(#{base_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/, + /(#{GlobalSetting.relative_url_root}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/, + /(#{base_url}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/, ] if GlobalSetting.cdn_url && (cdn_url = GlobalSetting.cdn_url.sub(/https?:\/\//, "(https?://)")) - regexps << /(#{cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /(#{cdn_url}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/ if GlobalSetting.relative_url_root.present? - regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /(#{cdn_url}#{GlobalSetting.relative_url_root}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/ end end if Discourse.store.external? if Rails.configuration.multisite - regexps << /((https?:)?#{SiteSetting.Upload.s3_base_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ - regexps << /(#{SiteSetting.Upload.s3_cdn_url}\/uploads\/#{db}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /((https?:)?#{SiteSetting.Upload.s3_base_url}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/ + regexps << /(#{SiteSetting.Upload.s3_cdn_url}\/#{upload_path}#{UPLOAD_REGEXP_PATTERN})/ else regexps << /((https?:)?#{SiteSetting.Upload.s3_base_url}#{UPLOAD_REGEXP_PATTERN})/ regexps << /(#{SiteSetting.Upload.s3_cdn_url}#{UPLOAD_REGEXP_PATTERN})/ diff --git a/config/routes.rb b/config/routes.rb index 21e536ce9c6..feaf544f18e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -507,6 +507,9 @@ Discourse::Application.routes.draw do get "uploads/short-url/:base62(.:extension)" => "uploads#show_short", constraints: { site: /\w+/, base62: /[a-zA-Z0-9]+/, extension: /[a-z0-9\.]+/i }, as: :upload_short # used to download attachments get "uploads/:site/original/:tree:sha(.:extension)" => "uploads#show", constraints: { site: /\w+/, tree: /([a-z0-9]+\/)+/i, sha: /\h{40}/, extension: /[a-z0-9\.]+/i } + if Discourse.is_parallel_test? + get "uploads/:site/:index/original/:tree:sha(.:extension)" => "uploads#show", constraints: { site: /\w+/, index: /\d+/, tree: /([a-z0-9]+\/)+/i, sha: /\h{40}/, extension: /[a-z0-9\.]+/i } + end # used to download attachments (old route) get "uploads/:site/:id/:sha" => "uploads#show", constraints: { site: /\w+/, id: /\d+/, sha: /\h{16}/, format: /.*/ } get "secure-media-uploads/*path(.:extension)" => "uploads#show_secure", constraints: { extension: /[a-z0-9\.]+/i } diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb index 0770af2c86f..3068ecb2a49 100644 --- a/lib/backup_restore/backuper.rb +++ b/lib/backup_restore/backuper.rb @@ -265,7 +265,7 @@ module BackupRestore def add_local_uploads_to_archive(tar_filename) log "Archiving uploads..." - upload_directory = "uploads/" + @current_db + upload_directory = Discourse.store.upload_path if File.directory?(File.join(Rails.root, "public", upload_directory)) exclude_optimized = SiteSetting.include_thumbnails_in_backups ? '' : "--exclude=#{upload_directory}/optimized" @@ -289,7 +289,7 @@ module BackupRestore log "Downloading uploads from S3. This may take a while..." store = FileStore::S3Store.new - upload_directory = File.join("uploads", @current_db) + upload_directory = Discourse.store.upload_path count = 0 Upload.find_each do |upload| diff --git a/lib/backup_restore/restorer.rb b/lib/backup_restore/restorer.rb index c76685c6595..6d1e1f0c179 100644 --- a/lib/backup_restore/restorer.rb +++ b/lib/backup_restore/restorer.rb @@ -431,36 +431,36 @@ module BackupRestore log "Extracting uploads..." public_uploads_path = File.join(Rails.root, "public") + upload_path = Discourse.store.upload_path FileUtils.mkdir_p(File.join(public_uploads_path, "uploads")) tmp_uploads_path = Dir.glob(File.join(@tmp_directory, "uploads", "*")).first return if tmp_uploads_path.blank? previous_db_name = BackupMetadata.value_for("db_name") || File.basename(tmp_uploads_path) - current_db_name = RailsMultisite::ConnectionManagement.current_db optimized_images_exist = File.exist?(File.join(tmp_uploads_path, 'optimized')) Discourse::Utils.execute_command( - 'rsync', '-avp', '--safe-links', "#{tmp_uploads_path}/", "uploads/#{current_db_name}/", + 'rsync', '-avp', '--safe-links', "#{tmp_uploads_path}/", "#{upload_path}/", failure_message: "Failed to restore uploads.", chdir: public_uploads_path ) - remap_uploads(previous_db_name, current_db_name) + remap_uploads(previous_db_name, upload_path) if SiteSetting.Upload.enable_s3_uploads migrate_to_s3 - remove_local_uploads(File.join(public_uploads_path, "uploads/#{current_db_name}")) + remove_local_uploads(File.join(public_uploads_path, upload_path)) end generate_optimized_images unless optimized_images_exist end - def remap_uploads(previous_db_name, current_db_name) + def remap_uploads(previous_db_name, upload_path) log "Remapping uploads..." was_multisite = BackupMetadata.value_for("multisite") == "t" - uploads_folder = was_multisite ? "/" : "/uploads/#{current_db_name}/" + uploads_folder = was_multisite ? "/" : "/#{upload_path}/" if (old_base_url = BackupMetadata.value_for("base_url")) && old_base_url != Discourse.base_url remap(old_base_url, Discourse.base_url) @@ -490,8 +490,9 @@ module BackupRestore remap(old_host, new_host) end + current_db_name = RailsMultisite::ConnectionManagement.current_db if previous_db_name != current_db_name - remap("uploads/#{previous_db_name}", "uploads/#{current_db_name}") + remap("uploads/#{previous_db_name}", upload_path) end rescue => ex diff --git a/lib/discourse.rb b/lib/discourse.rb index d4b6b4eb68c..f4819a210e6 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -837,6 +837,10 @@ module Discourse def self.redis $redis end + + def self.is_parallel_test? + ENV['RAILS_ENV'] == "test" && ENV['TEST_ENV_NUMBER'] + end end # rubocop:enable Style/GlobalVars diff --git a/lib/disk_space.rb b/lib/disk_space.rb index c817c68f3bb..96121b0cc73 100644 --- a/lib/disk_space.rb +++ b/lib/disk_space.rb @@ -20,7 +20,7 @@ class DiskSpace end def self.uploads_path - "#{Rails.root}/public/uploads/#{RailsMultisite::ConnectionManagement.current_db}" + "#{Rails.root}/public/#{Discourse.store.upload_path}" end private_class_method :uploads_path end diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 0c4c14d2be5..8fbf45d5b7c 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -31,7 +31,11 @@ module FileStore end def upload_path - File.join("uploads", RailsMultisite::ConnectionManagement.current_db) + path = File.join("uploads", RailsMultisite::ConnectionManagement.current_db) + return path unless Discourse.is_parallel_test? + + n = ENV['TEST_ENV_NUMBER'].presence || '1' + File.join(path, n) end def has_been_uploaded?(url) diff --git a/lib/s3_inventory.rb b/lib/s3_inventory.rb index b98e2e99094..3f3735b2323 100644 --- a/lib/s3_inventory.rb +++ b/lib/s3_inventory.rb @@ -39,7 +39,7 @@ class S3Inventory decompress_inventory_file(file) end - multisite_prefix = "uploads/#{RailsMultisite::ConnectionManagement.current_db}/" + multisite_prefix = Discourse.store.upload_path ActiveRecord::Base.transaction do begin connection.exec("CREATE TEMP TABLE #{table_name}(url text UNIQUE, etag text, PRIMARY KEY(etag, url))") diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index b9627b6cdb1..25749d133a3 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -17,6 +17,7 @@ end describe CookedPostProcessor do fab!(:upload) { Fabricate(:upload) } + let(:upload_path) { Discourse.store.upload_path } context "#post_process" do fab!(:post) do @@ -290,7 +291,7 @@ describe CookedPostProcessor do # fake some optimized images OptimizedImage.create!( - url: '/uploads/default/666x500.jpg', + url: "/#{upload_path}/666x500.jpg", width: 666, height: 500, upload_id: upload.id, @@ -302,7 +303,7 @@ describe CookedPostProcessor do # fake 3x optimized image, we lose 2 pixels here over original due to rounding on downsize OptimizedImage.create!( - url: '/uploads/default/1998x1500.jpg', + url: "/#{upload_path}/1998x1500.jpg", width: 1998, height: 1500, upload_id: upload.id, @@ -313,7 +314,7 @@ describe CookedPostProcessor do # Fake a loading image _optimized_image = OptimizedImage.create!( - url: '/uploads/default/10x10.png', + url: "/#{upload_path}/10x10.png", width: CookedPostProcessor::LOADING_SIZE, height: CookedPostProcessor::LOADING_SIZE, upload_id: upload.id, @@ -329,10 +330,10 @@ describe CookedPostProcessor do html = cpp.html - expect(html).to include(%Q|data-small-upload="//test.localhost/uploads/default/10x10.png"|) + expect(html).to include(%Q|data-small-upload="//test.localhost/#{upload_path}/10x10.png"|) # 1.5x is skipped cause we have a missing thumb - expect(html).to include('srcset="//test.localhost/uploads/default/666x500.jpg, //test.localhost/uploads/default/1998x1500.jpg 3x"') - expect(html).to include('src="//test.localhost/uploads/default/666x500.jpg"') + expect(html).to include("srcset=\"//test.localhost/#{upload_path}/666x500.jpg, //test.localhost/#{upload_path}/1998x1500.jpg 3x\"") + expect(html).to include("src=\"//test.localhost/#{upload_path}/666x500.jpg\"") # works with CDN set_cdn_url("http://cdn.localhost") @@ -343,9 +344,9 @@ describe CookedPostProcessor do html = cpp.html - expect(html).to include(%Q|data-small-upload="//cdn.localhost/uploads/default/10x10.png"|) - expect(html).to include('srcset="//cdn.localhost/uploads/default/666x500.jpg, //cdn.localhost/uploads/default/1998x1500.jpg 3x"') - expect(html).to include('src="//cdn.localhost/uploads/default/666x500.jpg"') + expect(html).to include(%Q|data-small-upload="//cdn.localhost/#{upload_path}/10x10.png"|) + expect(html).to include("srcset=\"//cdn.localhost/#{upload_path}/666x500.jpg, //cdn.localhost/#{upload_path}/1998x1500.jpg 3x\"") + expect(html).to include("src=\"//cdn.localhost/#{upload_path}/666x500.jpg\"") end it "doesn't include response images for cropped images" do @@ -450,7 +451,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

logo.png1750×2000 1.21 KB

HTML @@ -463,41 +464,41 @@ describe CookedPostProcessor do let(:post) { Fabricate(:post, raw: url) } before do - Oneboxer.stubs(:onebox).with(url, anything).returns("") + Oneboxer.stubs(:onebox).with(url, anything).returns("") end it 'should not add lightbox' do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

HTML end end describe 'when image is an svg' do fab!(:post) do - Fabricate(:post, raw: '') + Fabricate(:post, raw: "") end it 'should not add lightbox' do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

HTML end describe 'when image src is an URL' do let(:post) do - Fabricate(:post, raw: '') + Fabricate(:post, raw: "") end it 'should not add lightbox' do SiteSetting.crawl_images = true cpp.post_process - expect(cpp.html).to match_html("

") + expect(cpp.html).to match_html("

") end end end @@ -589,7 +590,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

logo.png1125×2436 1.21 KB

HTML @@ -623,7 +624,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html). to match_html <<~HTML -

+

logo.png1750×2000 1.21 KB

HTML @@ -636,7 +637,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

&gt;&lt;img src=x onerror=alert(&#39;haha&#39;)&gt;.png1750×2000 1.21 KB

HTML @@ -665,7 +666,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

RED
+

RED
WAT1750×2000 1.21 KB

HTML @@ -696,7 +697,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

+

WAT1750×2000 1.21 KB

HTML @@ -727,7 +728,7 @@ describe CookedPostProcessor do cpp.post_process expect(cpp.html).to match_html <<~HTML -

RED
+

RED
RED1750×2000 1.21 KB

HTML @@ -1137,11 +1138,11 @@ describe CookedPostProcessor do it "uses schemaless url for uploads" do cpp.optimize_urls expect(cpp.html).to match_html <<~HTML -

Link
-
+

Link
+
Google

- text.txt (20 Bytes)
+ text.txt (20 Bytes)
:smile:

HTML end @@ -1152,11 +1153,11 @@ describe CookedPostProcessor do Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") cpp.optimize_urls expect(cpp.html).to match_html <<~HTML -

Link
-
+

Link
+
Google

- text.txt (20 Bytes)
+ text.txt (20 Bytes)
:smile:

HTML end @@ -1165,11 +1166,11 @@ describe CookedPostProcessor do Rails.configuration.action_controller.stubs(:asset_host).returns("https://my.cdn.com") cpp.optimize_urls expect(cpp.html).to match_html <<~HTML -

Link
-
+

Link
+
Google

- text.txt (20 Bytes)
+ text.txt (20 Bytes)
:smile:

HTML end @@ -1179,11 +1180,11 @@ describe CookedPostProcessor do Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") cpp.optimize_urls expect(cpp.html).to match_html <<~HTML -

Link
-
+

Link
+
Google

- text.txt (20 Bytes)
+ text.txt (20 Bytes)
:smile:

HTML end @@ -1193,11 +1194,11 @@ describe CookedPostProcessor do Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") cpp.optimize_urls expect(cpp.html).to match_html <<~HTML -

Link
-
+

Link
+
Google

- text.txt (20 Bytes)
+ text.txt (20 Bytes)
:smile:

HTML end diff --git a/spec/components/file_store/local_store_spec.rb b/spec/components/file_store/local_store_spec.rb index 680deb2204f..f601dec73dc 100644 --- a/spec/components/file_store/local_store_spec.rb +++ b/spec/components/file_store/local_store_spec.rb @@ -9,6 +9,7 @@ describe FileStore::LocalStore do fab!(:upload) { Fabricate(:upload) } let(:uploaded_file) { file_from_fixtures("logo.png") } + let(:upload_path) { Discourse.store.upload_path } fab!(:optimized_image) { Fabricate(:optimized_image) } @@ -16,7 +17,7 @@ describe FileStore::LocalStore do it "returns a relative url" do store.expects(:copy_file) - expect(store.store_upload(uploaded_file, upload)).to match(/\/uploads\/default\/original\/.+#{upload.sha1}\.png/) + expect(store.store_upload(uploaded_file, upload)).to match(/\/#{upload_path}\/original\/.+#{upload.sha1}\.png/) end end @@ -25,7 +26,7 @@ describe FileStore::LocalStore do it "returns a relative url" do store.expects(:copy_file) - expect(store.store_optimized_image({}, optimized_image)).to match(/\/uploads\/default\/optimized\/.+#{optimized_image.upload.sha1}_#{OptimizedImage::VERSION}_100x200\.png/) + expect(store.store_optimized_image({}, optimized_image)).to match(/\/#{upload_path}\/optimized\/.+#{optimized_image.upload.sha1}_#{OptimizedImage::VERSION}_100x200\.png/) end end @@ -92,24 +93,24 @@ describe FileStore::LocalStore do describe "#has_been_uploaded?" do it "identifies relatives urls" do - expect(store.has_been_uploaded?("/uploads/default/42/0123456789ABCDEF.jpg")).to eq(true) + expect(store.has_been_uploaded?("/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(true) end it "identifies local urls" do Discourse.stubs(:base_url_no_prefix).returns("http://discuss.site.com") - expect(store.has_been_uploaded?("http://discuss.site.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(true) - expect(store.has_been_uploaded?("//discuss.site.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(true) + expect(store.has_been_uploaded?("http://discuss.site.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(true) + expect(store.has_been_uploaded?("//discuss.site.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(true) end it "identifies local urls when using a CDN" do Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") - expect(store.has_been_uploaded?("http://my.cdn.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(true) - expect(store.has_been_uploaded?("//my.cdn.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(true) + expect(store.has_been_uploaded?("http://my.cdn.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(true) + expect(store.has_been_uploaded?("//my.cdn.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(true) end it "does not match dummy urls" do - expect(store.has_been_uploaded?("http://domain.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(false) - expect(store.has_been_uploaded?("//domain.com/uploads/default/42/0123456789ABCDEF.jpg")).to eq(false) + expect(store.has_been_uploaded?("http://domain.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(false) + expect(store.has_been_uploaded?("//domain.com/#{upload_path}/42/0123456789ABCDEF.jpg")).to eq(false) end end @@ -117,12 +118,12 @@ describe FileStore::LocalStore do describe "#absolute_base_url" do it "is present" do - expect(store.absolute_base_url).to eq("http://test.localhost/uploads/default") + expect(store.absolute_base_url).to eq("http://test.localhost/#{upload_path}") end it "supports subfolder" do set_subfolder "/forum" - expect(store.absolute_base_url).to eq("http://test.localhost/forum/uploads/default") + expect(store.absolute_base_url).to eq("http://test.localhost/forum/#{upload_path}") end end @@ -130,12 +131,12 @@ describe FileStore::LocalStore do describe "#relative_base_url" do it "is present" do - expect(store.relative_base_url).to eq("/uploads/default") + expect(store.relative_base_url).to eq("/#{upload_path}") end it "supports subfolder" do set_subfolder "/forum" - expect(store.relative_base_url).to eq("/forum/uploads/default") + expect(store.relative_base_url).to eq("/forum/#{upload_path}") end end diff --git a/spec/components/url_helper_spec.rb b/spec/components/url_helper_spec.rb index 8cdf7af1385..3dd4360a6f4 100644 --- a/spec/components/url_helper_spec.rb +++ b/spec/components/url_helper_spec.rb @@ -121,7 +121,7 @@ describe UrlHelper do end describe "#local_cdn_url" do - let(:url) { "/uploads/default/1X/575bcc2886bf7a39684b57ca90be85f7d399bbc7.png" } + let(:url) { "/#{Discourse.store.upload_path}/1X/575bcc2886bf7a39684b57ca90be85f7d399bbc7.png" } let(:asset_host) { "//my.awesome.cdn" } it "should return correct cdn url for local relative urls" do diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb index 33c7598568f..cd18b78967e 100644 --- a/spec/fabricators/post_fabricator.rb +++ b/spec/fabricators/post_fabricator.rb @@ -48,26 +48,26 @@ end Fabricator(:post_with_plenty_of_images, from: :post) do cooked <<~HTML - -
-
+ +
+

With an emoji! smile

HTML end Fabricator(:post_with_uploaded_image, from: :post) do - raw '' + raw "" end Fabricator(:post_with_an_attachment, from: :post) do - raw 'archive.zip' + raw "archive.zip" end Fabricator(:post_with_unsized_images, from: :post) do - raw ' - - -' + raw " + + +" end Fabricator(:post_with_image_urls, from: :post) do @@ -78,31 +78,31 @@ Fabricator(:post_with_image_urls, from: :post) do end Fabricator(:post_with_large_image, from: :post) do - raw '' + raw "" end Fabricator(:post_with_large_image_and_title, from: :post) do - raw '' + raw "" end Fabricator(:post_with_large_image_on_subfolder, from: :post) do - raw '' + raw "" end Fabricator(:post_with_uploads, from: :post) do - raw ' -Link - -' + raw " +Link + +" end Fabricator(:post_with_uploads_and_links, from: :post) do raw <<~RAW - Link - + Link + Google - text.txt (20 Bytes) + text.txt (20 Bytes) :smile: RAW end diff --git a/spec/helpers/user_notifications_helper_spec.rb b/spec/helpers/user_notifications_helper_spec.rb index 7610495b258..b72dcf82c0c 100644 --- a/spec/helpers/user_notifications_helper_spec.rb +++ b/spec/helpers/user_notifications_helper_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe UserNotificationsHelper do + let(:upload_path) { Discourse.store.upload_path } + describe '#email_excerpt' do let(:paragraphs) { [ "

This is the first paragraph, but you should read more.

", @@ -97,7 +99,7 @@ describe UserNotificationsHelper do it 'should return the right URL' do expect(helper.logo_url).to eq( - "http://test.localhost/uploads/default/original/1X/somesha1.png" + "http://test.localhost/#{upload_path}/original/1X/somesha1.png" ) end @@ -110,7 +112,7 @@ describe UserNotificationsHelper do it 'should return the right URL' do expect(helper.logo_url).to eq( - "https://some.localcdn.com/uploads/default/original/1X/somesha1.png" + "https://some.localcdn.com/#{upload_path}/original/1X/somesha1.png" ) end end diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index dd194f3b897..fa37da185c6 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -10,6 +10,7 @@ describe Jobs::PullHotlinkedImages do let(:large_image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat3.png" } let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") } let(:large_png) { Base64.decode64("iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAIAAAAlC+aJAAAK10lEQVR42r3aeVRTVx4H8Oc2atWO7Sw9OnM6HWvrOON0aFlcAZ3RopZWOyqgoCACKqPWBUVQi4gIqAVllciiKPu+JOyGnQQSNgkIIQgoKljAYVARCZnf4yXhkeXlJmDP+f4hOUF+n3fvffe++y5W0i4qJqWoDU8hKQUPxWFKcq9VnHxJ8gTi5EqS0yJOtiRZfHEyJWE0i0MnJaMJTzopaQ/wpJKS0ogneTQYABANTDlDvpxBCsiu72eUP0zPq8Fzr45e8TircRDFQAAy5ABpcgDCgJV2iCbRQM+rinU/E26ie9NgfrDO1GBtTBy96SH/WhBhaxwfGEjndmfKGeiaGsYAJXIANQyCkfR05u3dhuOKVhLamnmRzocyKp9mNo9QG9IRDDiAiMaG3Nqfo45aoJROzk3DDxNCbjGahBM0yAKoDfIDOpNZE/bNYrVKJyfylB2D91pdA3lAjwE0MDAyS+BCalw9kdu2xvT6AY0NWBkJoNaAzsrj4CN1YtUTidi/hdH4BvGmJGPAAYgGMuMery/U6ONJqZ5I1PlTjNExre7kgJU/EqEbJC0gjDpiiv9hnSkJ2z+t9dzxwNcSUudlUuuxnXP+W/bZTWWO64uO6hccWQ0pPm4IP1a6GFe5bYXvNF7f0xxg3XrzgCDYjn1m4+218/D/SndaYnSqBpMDDlDXkHYnMlh7Srj+HLanxfOsyyOVN0ScYI0zkOeVZvYZGEI2/DFDMkWgTw7jAGWUA5owMOt7QtcvDF09qybA/mGC6zA7aCLVExkq9U3895/wm9LpgyonBxmDGKDQoHBySPQ8B5e/zM2kJdalN/fqxKsn8oLhFr5mdvDyX6UVNqqcpMmDAWNJACjtUMDrDVn7m6SdS/kxPwrizg+zAycLAKm5tA0a4a7DPpSFhmIAxWAgDKm0IJrutBr/g3D5n9E9J7F6oiNFGf2WtnI2vboH3YADEA0AuG2ml2i2BC4/AAYKr00uAHL/ihk0QnxQMPqKFWM/FiEamFWPYMHD8tgF1UMmZfjKZLDIJ1z/vQibzTKrbop2wAGIhoxbt8IN5zZHnoHqO5LdJr16IkXHDG4afJDJG0B8chADUAxxTnbp1trE5Z/0ASDN09hTcJdLy+EoawQZgyyAwhCxcznr0k4C0JNz5R0BYFqM3PBhQugtxKdQrEICUGFoE4ZtWPAg4jQBeJHv/Y4AkBKHdTHuZ8lP0hSDAQdQGwhAUUNv4s6/EvcfSD/T590B2u8cj3SwltkNUGaQBSgbDAXc9pxTW4jqIf8ruAa37efJLg/DfuBd21ftYU7OA387+QXSk2gHWMmRw/M2F9D2d8WffsW8Sv5+X/mtyBN7s+V2NBQasMpOEYqhuLG3MimMqL4h/GTu4fW01b/z05qrMKEGC96W+8sA8g/qKX281JuWafX350lniG++rIpOTcknb8lQGHAAoqG+pgqqr7hqE2K4kCg0bO3CJDMthvVKInTrlUmm/4j+9vO7mxYNlfrJAJiHVsYaL0g1XZy194scmy+JMCyXxWz+CAD4anTFjLrLpiMVQW+4t1G2lQiDGIBiuF/NLbmwM1B3PpQe892SFtqh4fIAhZ14mBUo34WE7ECFC29hRdDz5LO5dtrwdAGM0pP/HKoMzWsZRtwakwVQGPJjo/2/ej9Q74N8xy19o+tQYcWNzjT3mJNmR/W/uPi9fobr3ifpl6hXeG9Zge1JF5LPWvz4zYoTa7VSzu0mniggMEigNcBQ7GjE5A9Kt/eoOxLGkQBUGkoyGeEbPqnys2+OPlcbdir80PdOX+usmDFdG8OIwCc3bI0vm657WeSrsPouhuelbQZh/9nqY7FB+lsGc2ad27w86oTJo5SLrwu9s/dpVXuYFPEHELcocQC1QXpjhS4EpcMwiPhh2/U9XzfedYYFhe7UKdJSqkNOIt4oMy/uIwP68n6C3/WzMmIFHIUeJawMLm7ul9lmVdYOYgCKob6aK72NEo8yQ+UBtl99BkXoTMFcv1sF3UNaIpd24vCqvykDvCr2PbJ6GQFwNtKFrjhuCHFCCvmvcuW2ihUaMO4TWYCyAU0GSJcSsCblRTjDSJAZoFnuNiafLqReMrQlukKTylQvBZC3iikMOIDCQGaQAT9nq1gLqQRQBABFLa9U7tcTBjEApR3IALh1/DIAlQZZAIWBDOjO9HrXAMT3JliVBKCyHciALsYvAUAx4IAqOYDCmxKPBFD5QDNBQHHLS2XvfmQMYgCKgQx4muGhFmCw1B8dIOTQyvj9FO+vyDclrPqpLECZgVczBoAlA3URMCubLv6D9I657ZOP0lws1QJQv4OTGnAAogEdAF+A+TXHw3b0R5qoszLLyx4+gc8RAeUt/SrfIxIGMYDCoBDwONVdaQ9mB+3XWeK87kvJ1EYTDfYLn9XDgsdO+3NYKSACUN6FQsYAKg2IgIqgY6tnzmi6bP8y2X2EmGUbkkWCPJitV82cURfuqPq5nhPM4vchvpDGauQAygxkAMW+ULCdsfWSj/tCTr8IdeqPdBnK94FnFCEr8DXd68CyRXeObkfpRWx+D+JLdRxANlC0QwMaINHZfP37c4oczQkDnjDnvlCnMuc9RvPnxp/ehQKokAAoOlIeGUDdDvKAtsQLyv72mzJ/P6uN+rNnHtf5S7GjRVeQQ6nTbge9pdB/vEzWDso9aqoEUBuw2mciZY0gY0AEEBHEuZzZqAdFG743c/n0aQ7rtBruOKO/y+HwnyMebsABiIbG2jFAa7wryh4bPDaUXD+swWuoKv5TxMMNYgCFgQSoIgHOv7uNLbgLcfldiAc0xgAqDbVtLwTJXgQAeojmLzLKAzjBxyl257vqcgsfChUeDJA3YHUkgEpDQz2vJU7cCDJTEnQSWOHBDK0wMACgL0U7mLptXWO/fGmCk7myGW2gOra09Q36aSUcoIahc4Rfmi59JBi3H5j3k5fJOs8dhgoTYL0Jqi/1PfyMTrUKHOKGcwS9Kg9okA1iALqh+tGggBFIGJRtn2gWWEHwmlsRD5lIDdj9LpG8gXpyuN/yRJBwEQCwRYWytkEcuB28iuK2EXVPXOEAqaEW2dBUzZI+HE/wTT2RnjpGSZtQg1NjYoDa7dA50sKMIgywyTPB6l9VRbPaXmt28m0MQNEOCgdDbXu/IM17tCO5TaQjveWG1Qi6NT75htWTAOoaeA/4gnhXlF0Wiq7f3NSk1okrGQMO0NzQOdLMziU60usSPw2q7+SVlnWMlE3g1BjG6xZNxFDe1s2OO0Z0JHhxBuMBJlroUSgju682ldUxTH24QaVhDFAvB1Bp4HS+PRO/5ZDP7xtjnaXLJGKlBMtVeGqDuRk2If97z/tl0XVYZg+T3nF0F3tcjN1W2vFWrdNK8gYcgGiQvykFFl7a7oFBvG5o5UfvVRQrRuQu+mjgH5lRu7JjLPISLAtTrJ1pf94dj4U0+mhw4opsEAPU6kiEIZ1XYnZlFgFQKzu8MYtYzKYUs63E7Lnz0ls5iKeVFBrGAGq1A6uj1zZw0XZPzPwuZhqE7biiqm4vzNQP/7JVFmZbgdlxxnKienFBe4/G7YA1kADI7TDilmQJZVlE41cRirBlYdZMzIqB7UnGdseRkohZZmDW+ZhNmfibEHvuzAOcaWTD5XpLuBepdfKtiAxQ1xDPTdnhOdXUH7Nlj7uWKDnAme7bvPlI1a/Hfz4ljp+BfnqPPKD/DzQWIVWNoUiJAAAAAElFTkSuQmCC") } + let(:upload_path) { Discourse.store.upload_path } before do stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" }) @@ -18,12 +19,12 @@ describe Jobs::PullHotlinkedImages do stub_request( :get, - "#{Discourse.base_url}/uploads/default/original/1X/f59ea56fe8ebe42048491d43a19d9f34c5d0f8dc.gif" + "#{Discourse.base_url}/#{upload_path}/original/1X/f59ea56fe8ebe42048491d43a19d9f34c5d0f8dc.gif" ) stub_request( :get, - "#{Discourse.base_url}/uploads/default/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" + "#{Discourse.base_url}/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" ) SiteSetting.crawl_images = true diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index e82c21d87a9..760252adf52 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -5,6 +5,8 @@ require 'rails_helper' describe Post do before { Oneboxer.stubs :onebox } + let(:upload_path) { Discourse.store.upload_path } + describe '#hidden_reasons' do context "verify enum sequence" do before do @@ -313,8 +315,8 @@ describe Post do describe "maximum attachments" do fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) } let(:post_no_attachments) { Fabricate.build(:post, post_args.merge(user: newuser)) } - let(:post_one_attachment) { post_with_body('file.txt', newuser) } - let(:post_two_attachments) { post_with_body('errors.log model.3ds', newuser) } + let(:post_one_attachment) { post_with_body("file.txt", newuser) } + let(:post_two_attachments) { post_with_body("errors.log model.3ds", newuser) } it "returns 0 attachments for an empty post" do expect(Fabricate.build(:post).attachment_count).to eq(0) @@ -729,11 +731,11 @@ describe Post do end describe 'before save' do - let(:cooked) { "

\nSword reworks.gif1000x400 1000 KB\n

" } + let(:cooked) { "

\nSword reworks.gif1000x400 1000 KB\n

" } let(:post) do Fabricate(:post, - raw: "", + raw: "", cooked: cooked ) end @@ -1282,7 +1284,7 @@ describe Post do context "#link_post_uploads" do it "finds all the uploads in the post" do post.custom_fields[Post::DOWNLOADED_IMAGES] = { - "/uploads/default/original/1X/1/1234567890123456.csv": attachment_upload.id + "/#{upload_path}/original/1X/1/1234567890123456.csv": attachment_upload.id } post.save_custom_fields @@ -1433,10 +1435,10 @@ describe Post do context "have_uploads" do it "should find all posts with the upload" do ids = [] - ids << Fabricate(:post, cooked: "A post with upload ").id - ids << Fabricate(:post, cooked: "A post with optimized image ").id + ids << Fabricate(:post, cooked: "A post with upload ").id + ids << Fabricate(:post, cooked: "A post with optimized image ").id Fabricate(:post) - ids << Fabricate(:post, cooked: "A post with upload ").id + ids << Fabricate(:post, cooked: "A post with upload ").id ids << Fabricate(:post, cooked: "A post with upload link ").id ids << Fabricate(:post, cooked: "A post with optimized image ").id Fabricate(:post, cooked: "A post with external link ") diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb index 1c84beefe57..f2107a4b0b5 100644 --- a/spec/multisite/s3_store_spec.rb +++ b/spec/multisite/s3_store_spec.rb @@ -6,6 +6,7 @@ require 'file_store/s3_store' RSpec.describe 'Multisite s3 uploads', type: :multisite do let(:uploaded_file) { file_from_fixtures("smallest.png") } let(:upload_sha1) { Digest::SHA1.hexdigest(File.read(uploaded_file)) } + let(:upload_path) { Discourse.store.upload_path } def build_upload Fabricate.build(:upload, sha1: upload_sha1, id: 1) @@ -28,15 +29,16 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do test_multisite_connection('default') do upload = build_upload expect(store.store_upload(uploaded_file, upload)).to eq( - "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/uploads/default/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" + "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" ) expect(upload.etag).to eq("ETag") end test_multisite_connection('second') do + upload_path = Discourse.store.upload_path upload = build_upload 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" + "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" ) expect(upload.etag).to eq("ETag") end @@ -65,12 +67,12 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload = build_upload store.expects(:get_depth_for).with(upload.id).returns(0) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/default/original/1X/#{upload.sha1}.png") + upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.png") s3_object = stub s3_bucket.expects(:object).with("uploads/tombstone/default/original/1X/#{upload.sha1}.png").returns(s3_object) - s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/uploads/default/original/1X/#{upload.sha1}.png") - s3_bucket.expects(:object).with("uploads/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/#{upload_path}/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("#{upload_path}/original/1X/#{upload.sha1}.png").returns(s3_object) s3_object.expects(:delete) store.remove_upload(upload) @@ -82,12 +84,12 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload = build_upload store.expects(:get_depth_for).with(upload.id).returns(0) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/second/original/1X/#{upload.sha1}.png") + upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.png") s3_object = stub s3_bucket.expects(:object).with("uploads/tombstone/second/original/1X/#{upload.sha1}.png").returns(s3_object) - s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/uploads/second/original/1X/#{upload.sha1}.png") - s3_bucket.expects(:object).with("uploads/second/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/#{upload_path}/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("#{upload_path}/original/1X/#{upload.sha1}.png").returns(s3_object) s3_object.expects(:delete) store.remove_upload(upload) @@ -104,12 +106,12 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload = build_upload store.expects(:get_depth_for).with(upload.id).returns(0) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png") + upload.update!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/discourse-uploads/#{upload_path}/original/1X/#{upload.sha1}.png") s3_object = stub s3_bucket.expects(:object).with("discourse-uploads/uploads/tombstone/default/original/1X/#{upload.sha1}.png").returns(s3_object) - s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png") - s3_bucket.expects(:object).with("discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png").returns(s3_object) + s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/discourse-uploads/#{upload_path}/original/1X/#{upload.sha1}.png") + s3_bucket.expects(:object).with("discourse-uploads/#{upload_path}/original/1X/#{upload.sha1}.png").returns(s3_object) s3_object.expects(:delete) store.remove_upload(upload) @@ -147,11 +149,11 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - s3_bucket.expects(:object).with("uploads/default/original/1X/#{upload.sha1}.pdf").returns(s3_object).at_least_once + s3_bucket.expects(:object).with("#{upload_path}/original/1X/#{upload.sha1}.pdf").returns(s3_object).at_least_once s3_object.expects(:presigned_url).with(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS) expect(store.store_upload(uploaded_file, upload)).to eq( - "//some-really-cool-bucket.s3.dualstack.us-east-1.amazonaws.com/uploads/default/original/1X/#{upload.sha1}.pdf" + "//some-really-cool-bucket.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.pdf" ) expect(store.url_for(upload)).not_to eq(upload.url) @@ -173,15 +175,16 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do signed_url = Discourse.store.signed_url_for_path(upload.url) expect(signed_url).to match(/Amz-Expires/) - expect(signed_url).to match("uploads/default") + expect(signed_url).to match("#{upload_path}") end test_multisite_connection('second') do + upload_path = Discourse.store.upload_path upload = Fabricate.build(:upload_s3, sha1: upload_sha1, id: 1) signed_url = Discourse.store.signed_url_for_path(upload.url) expect(signed_url).to match(/Amz-Expires/) - expect(signed_url).to match("uploads/second") + expect(signed_url).to match("#{upload_path}") end end end @@ -193,7 +196,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - s3_bucket.expects(:object).with("uploads/default/original/1X/#{upload.sha1}.pdf").returns(s3_object) + s3_bucket.expects(:object).with("#{upload_path}/original/1X/#{upload.sha1}.pdf").returns(s3_object) s3_object.expects(:acl).returns(s3_object) s3_object.expects(:put).with(acl: "private").returns(s3_object) @@ -201,11 +204,12 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do end test_multisite_connection('second') do + upload_path = Discourse.store.upload_path upload = build_upload upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true) s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once - s3_bucket.expects(:object).with("uploads/second/original/1X/#{upload.sha1}.pdf").returns(s3_object) + s3_bucket.expects(:object).with("#{upload_path}/original/1X/#{upload.sha1}.pdf").returns(s3_object) s3_object.expects(:acl).returns(s3_object) s3_object.expects(:put).with(acl: "private").returns(s3_object)