FIX: Multisite DB was leaving old data in test mode

This commit introduces a new helper to enable transactional fixtures
when testing multisite. This would show up as tests that passed the
first time then failed the second time due to stale data being leftover.

spec/lib/backup_restore/shared_examples_for_backup_store.rb

@@ -57,7 +57,7 @@ shared_examples "backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           expect(store.files).to eq([backup5, backup4])
         end
       end
@@ -74,7 +74,7 @@ shared_examples "backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           expect(store.latest_file).to eq(backup5)
         end
       end
@@ -110,7 +110,7 @@ shared_examples "backup store" do
       it "works with multisite", type: :multisite do
         SiteSetting.maximum_backups = 1
 
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           store.delete_old
           expect(store.files).to eq([backup5])
         end
@@ -132,7 +132,7 @@ shared_examples "backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           file = store.file(backup4.filename, include_download_source: true)
           expect(file.source).to match(source_regex("second", backup4.filename, multisite: true))
         end
@@ -153,7 +153,7 @@ shared_examples "backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           expect(store.files).to include(backup5)
           store.delete_file(backup5.filename)
           expect(store.files).to_not include(backup5)
@@ -182,7 +182,7 @@ shared_examples "backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           expect(store.files).to include(backup5)
           store.delete_file(backup5.filename)
           expect(store.files).to_not include(backup5)
@@ -239,7 +239,7 @@ shared_examples "remote backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           upload_file
         end
       end
@@ -266,7 +266,7 @@ shared_examples "remote backup store" do
       end
 
       it "works with multisite", type: :multisite do
-        RailsMultisite::ConnectionManagement.with_connection("second") do
+        test_multisite_connection("second") do
           filename = "foo.tar.gz"
           url = store.generate_upload_url(filename)
 

spec/multisite/distributed_cache_spec.rb

@@ -1,8 +1,6 @@
 require 'rails_helper'
 
 RSpec.describe 'Multisite SiteSettings', type: :multisite do
-  let(:conn) { RailsMultisite::ConnectionManagement }
-
   def cache(name, namespace: true)
     DistributedCache.new(name, namespace: namespace)
   end
@@ -15,7 +13,7 @@ RSpec.describe 'Multisite SiteSettings', type: :multisite do
 
       expect(cache1.hash).to eq('default' => true)
 
-      conn.with_connection('second') do
+      test_multisite_connection('second') do
         message = MessageBus.track_publish(DistributedCache::Manager::CHANNEL_NAME) do
           cache1['second'] = true
         end.first

spec/multisite/s3_store_spec.rb

@@ -2,9 +2,9 @@ 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) { Fabricate(:upload, sha1: Digest::SHA1.hexdigest(File.read(uploaded_file))) }
+  let(:upload_sha1) { Digest::SHA1.hexdigest(File.read(uploaded_file)) }
+  let(:upload) { Fabricate(:upload, sha1: upload_sha1) }
 
   context 'uploading to s3' do
     before(:each) do
@@ -20,15 +20,17 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
       let(:store) { FileStore::S3Store.new(s3_helper) }
 
       it "returns the correct url for default and second multisite db" do
-        conn.with_connection('default') do
-          expect(store.store_upload(uploaded_file, upload)).to eq(
+        test_multisite_connection('default') do
+          test_upload = Fabricate(:upload, sha1: upload_sha1)
+          expect(store.store_upload(uploaded_file, test_upload)).to eq(
             "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/uploads/default/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
           )
           expect(upload.etag).to eq("ETag")
         end
 
-        conn.with_connection('second') do
-          expect(store.store_upload(uploaded_file, upload)).to eq(
+        test_multisite_connection('second') do
+          test_upload = Fabricate(:upload, sha1: upload_sha1)
+          expect(store.store_upload(uploaded_file, test_upload)).to eq(
             "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/uploads/second/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
           )
           expect(upload.etag).to eq("ETag")
@@ -54,7 +56,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
       let(:s3_helper) { store.s3_helper }
 
       it "removes the file from s3 on multisite" do
-        conn.with_connection('default') do
+        test_multisite_connection('default') do
           store.expects(:get_depth_for).with(upload.id).returns(0)
           s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
           upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/default/original/1X/#{upload.sha1}.png")
@@ -70,7 +72,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
       end
 
       it "removes the file from s3 on another multisite db" do
-        conn.with_connection('second') do
+        test_multisite_connection('second') do
           store.expects(:get_depth_for).with(upload.id).returns(0)
           s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
           upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/uploads/second/original/1X/#{upload.sha1}.png")
@@ -91,7 +93,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
         end
 
         it "removes the file from s3 on multisite" do
-          conn.with_connection('default') do
+          test_multisite_connection('default') do
             store.expects(:get_depth_for).with(upload.id).returns(0)
             s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
             upload.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/discourse-uploads/uploads/default/original/1X/#{upload.sha1}.png")

spec/multisite/site_settings_spec.rb

@@ -1,34 +1,28 @@
 require 'rails_helper'
 
 RSpec.describe 'Multisite SiteSettings', type: :multisite do
-  let(:conn) { RailsMultisite::ConnectionManagement }
-
   before do
     @original_provider = SiteSetting.provider
     SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting)
   end
 
   after do
-    ['default', 'second'].each do |db|
-      conn.with_connection(db) { SiteSetting.where(name: 'default_locale').destroy_all }
-    end
-
     SiteSetting.provider = @original_provider
   end
 
   describe '#default_locale' do
     it 'should return the right locale' do
-      conn.with_connection('default') do
+      test_multisite_connection('default') do
         expect(SiteSetting.default_locale).to eq('en')
       end
 
-      conn.with_connection('second') do
+      test_multisite_connection('second') do
         SiteSetting.default_locale = 'zh_TW'
 
         expect(SiteSetting.default_locale).to eq('zh_TW')
       end
 
-      conn.with_connection('default') do
+      test_multisite_connection('default') do
         expect(SiteSetting.default_locale).to eq('en')
 
         SiteSetting.default_locale = 'ja'
@@ -36,7 +30,7 @@ RSpec.describe 'Multisite SiteSettings', type: :multisite do
         expect(SiteSetting.default_locale).to eq('ja')
       end
 
-      conn.with_connection('second') do
+      test_multisite_connection('second') do
         expect(SiteSetting.default_locale).to eq('zh_TW')
       end
     end

spec/rails_helper.rb

@@ -202,6 +202,22 @@ RSpec.configure do |config|
     end
   end
 
+  # Normally we `use_transactional_fixtures` to clear out a database after a test
+  # runs. However, this does not apply to tests done for multisite. The second time
+  # a test runs you can end up with stale data that breaks things. This method will
+  # force a rollback after using a multisite connection.
+  def test_multisite_connection(name)
+    RailsMultisite::ConnectionManagement.with_connection(name) do
+      ActiveRecord::Base.transaction do
+        begin
+          yield
+        ensure
+          throw raise ActiveRecord::Rollback
+        end
+      end
+    end
+  end
+
 end
 
 class TrackTimeStub