DEV: Improve multisite testing (#14884)

This commit adds the RailsMultisite middleware in test mode when Rails.configuration.multisite is true. This allows for much more realistic integration testing. The `multisite_spec.rb` file is rewritten to avoid needing to simulate a middleware stack.

config/initializers/200-first_middlewares.rb

@@ -20,7 +20,18 @@ if Rails.env != 'development' || ENV['TRACK_REQUESTS']
   end
 end
 
-if Rails.configuration.multisite
+if Rails.env.test?
+  # In test mode we can't insert/remove middlewares
+  # Therefore we insert a small helper which effectively switches the multisite
+  # middleware on/off based on the Rails.configuration.multisite value
+  class TestMultisiteMiddleware < RailsMultisite::Middleware
+    def call(env)
+      return @app.call(env) if !Rails.configuration.multisite
+      super(env)
+    end
+  end
+  Rails.configuration.middleware.unshift TestMultisiteMiddleware, RailsMultisite::DiscoursePatches.config
+elsif Rails.configuration.multisite
   assets_hostnames = GlobalSetting.cdn_hostnames
 
   if assets_hostnames.empty?

spec/integration/multisite_spec.rb

@@ -2,62 +2,33 @@
 
 require 'rails_helper'
 
-describe 'multisite', type: :multisite do
+describe 'multisite', type: [:multisite, :request] do
-  class DBNameMiddleware
-    def initialize(app, config = {})
-      @app = app
-    end
-
-    def call(env)
-      # note current_db is already being ruined on boot cause its not multisite
-      [200, {}, [RailsMultisite::ConnectionManagement.current_hostname]]
-    end
-  end
-
-  let :session do
-    stack = ActionDispatch::MiddlewareStack.new
-    stack.use RailsMultisite::Middleware, RailsMultisite::DiscoursePatches.config
-    stack.use DBNameMiddleware
-
-    routes = ActionDispatch::Routing::RouteSet.new
-    stack.build(routes)
-  end
-
   it "should always allow /srv/status through" do
-    headers = {
+    get "http://unknown.com/srv/status"
-      "HTTP_HOST" => "unknown.com",
+    expect(response.status).to eq(200)
-      "REQUEST_METHOD" => "GET",
+    expect(request.env["HTTP_HOST"]).to eq("test.localhost") # Rewritten by EnforceHostname middleware
-      "PATH_INFO" => "/srv/status",
-      "rack.input" => StringIO.new
-    }
-
-    code, _, body = session.call(headers)
-    expect(code).to eq(200)
-    expect(body.join).to eq("test.localhost")
   end
 
-  it "should 404 on unknown routes" do
+  it "should 404 for unknown domains" do
-    headers = {
+    get "http://unknown.com/about.json"
-      "HTTP_HOST" => "unknown.com",
+    expect(response.status).to eq(404)
-      "REQUEST_METHOD" => "GET",
-      "PATH_INFO" => "/topics",
-      "rack.input" => StringIO.new
-    }
-
-    code, _ = session.call(headers)
-    expect(code).to eq(404)
   end
 
-  it "should hit correct site elsewise" do
+  it "should hit correct site otherwise" do
-    headers = {
+    site_1_url = Fabricate(:topic, title: "Site 1 Topic Title", user: Discourse.system_user).relative_url
-      "HTTP_HOST" => "test2.localhost",
-      "REQUEST_METHOD" => "GET",
-      "PATH_INFO" => "/topics",
-      "rack.input" => StringIO.new
-    }
 
-    code, _, body = session.call(headers)
+    test_multisite_connection('second') do
-    expect(code).to eq(200)
+      site_2_url = Fabricate(:topic, title: "Site 2 Topic Title", user: Discourse.system_user).relative_url
-    expect(body.join).to eq("test2.localhost")
+
+      get "http://test.localhost/#{site_1_url}.json"
+      expect(request.env["RAILS_MULTISITE_HOST"]).to eq("test.localhost")
+      expect(response.status).to eq(200)
+      expect(response.parsed_body["title"]).to eq("Site 1 Topic Title")
+
+      get "http://test2.localhost/#{site_2_url}.json"
+      expect(response.status).to eq(200)
+      expect(request.env["RAILS_MULTISITE_HOST"]).to eq("test2.localhost")
+      expect(response.parsed_body["title"]).to eq("Site 2 Topic Title")
+    end
   end
 end

spec/lib/backup_restore/uploads_restorer_spec.rb

@@ -99,7 +99,7 @@ describe BackupRestore::UploadsRestorer do
     let!(:multisite) { { name: "multisite", value: true } }
     let!(:no_multisite) { { name: "multisite", value: false } }
     let!(:source_db_name) { { name: "db_name", value: "foo" } }
-    let!(:base_url) { { name: "base_url", value: "https://www.example.com/forum" } }
+    let!(:base_url) { { name: "base_url", value: "https://test.localhost/forum" } }
     let!(:no_cdn_url)  { { name: "cdn_url", value: nil } }
     let!(:cdn_url)  { { name: "cdn_url", value: "https://some-cdn.example.com" } }
     let(:target_site_name) { target_site_type == multisite ? "second" : "default" }
@@ -187,7 +187,7 @@ describe BackupRestore::UploadsRestorer do
         expect_remap(
           target_site_name: target_site_name,
           metadata: [source_site_type, base_url],
-          from: "https://www.example.com/forum",
+          from: "https://test.localhost/forum",
           to: "http://localhost"
         )
       end

spec/rails_helper.rb

@@ -291,6 +291,10 @@ RSpec.configure do |config|
     DB.test_transaction = ActiveRecord::Base.connection.current_transaction
   end
 
+  # Match the request hostname to the value in `database.yml`
+  config.before(:all, type: [:request, :multisite]) { host! "test.localhost" }
+  config.before(:each, type: [:request, :multisite]) { host! "test.localhost" }
+
   config.before(:each, type: :multisite) do
     Rails.configuration.multisite = true # rubocop:disable Discourse/NoDirectMultisiteManipulation