mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
FIX: Bump rails_multisite and update usage (#42684)
`with_connection`, `with_hostname` and `establish_connection` no longer silently fallback to the default db. When we want that fallback behavior, we have to specifically opt-in. For example: - Most hostname-based lookups in Discourse should fallback to the default site, so that we match the existing behavior for any sites which may have misconfigured hostnames - Loops like CachedCounting, keep_readonly_mode and sidekiq-pausable should defensively skip attempts to connect to sites which are now missing, so that they don't get blocked by an exception for one missing site Depends on https://github.com/discourse/rails_multisite/pull/40
This commit is contained in:
+4
-4
@@ -535,9 +535,9 @@ GEM
|
||||
activerecord (>= 6.1, < 9.0)
|
||||
concurrent-ruby
|
||||
railties (>= 6.1, < 9.0)
|
||||
rails_multisite (7.0.0)
|
||||
activerecord (>= 7.1)
|
||||
railties (>= 7.1)
|
||||
rails_multisite (9.0.0)
|
||||
activerecord (>= 7.2)
|
||||
railties (>= 7.2)
|
||||
railties (8.0.5.1)
|
||||
actionpack (= 8.0.5.1)
|
||||
activesupport (= 8.0.5.1)
|
||||
@@ -1218,7 +1218,7 @@ CHECKSUMS
|
||||
rails-dom-testing (2.3.0) sha256=8acc7953a7b911ca44588bf08737bc16719f431a1cc3091a292bca7317925c1d
|
||||
rails-html-sanitizer (1.7.1) sha256=e797a7c9b01e567307e317c576b49ab4168017e63eea4dba9ce3cb587e2f22c2
|
||||
rails_failover (2.3.0) sha256=eed6ea0674fd6f9f6b070ad297ad2ead121ecf9202920f6068b6a4f29d9491c9
|
||||
rails_multisite (7.0.0) sha256=7aacf364ed86d2bee73fb679cbfe6c343ce89067b9746b3d5857fffc57f036f2
|
||||
rails_multisite (9.0.0) sha256=2bc88f0405cb88d1055712095a11c4eb8ce1d4a36599d1226c6779946be495e6
|
||||
railties (8.0.5.1) sha256=da1958e1d9dab04691a2f8721b3ff7fab323715d37f103c19972dedfd644d5c7
|
||||
rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a
|
||||
raindrops (0.20.1) sha256=aa0eb9ff6834f2d9e232ba688bd49cb30be893bc5a3452e74722c94c1fab4730
|
||||
|
||||
@@ -13,7 +13,10 @@ class HighlightJsController < ApplicationController
|
||||
def show
|
||||
no_cookies
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
|
||||
RailsMultisite::ConnectionManagement.with_hostname(
|
||||
params[:hostname],
|
||||
raise_on_missing: false,
|
||||
) do
|
||||
current_version = HighlightJs.version(SiteSetting.highlighted_languages)
|
||||
|
||||
return redirect_to path(HighlightJs.path) if current_version != params[:version]
|
||||
|
||||
@@ -18,7 +18,10 @@ class SvgSpriteController < ApplicationController
|
||||
def show
|
||||
no_cookies
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
|
||||
RailsMultisite::ConnectionManagement.with_hostname(
|
||||
params[:hostname],
|
||||
raise_on_missing: false,
|
||||
) do
|
||||
theme_id = params[:theme_id].to_i if params[:theme_id].present?
|
||||
|
||||
if SvgSprite.version(theme_id) != params[:version]
|
||||
@@ -70,7 +73,10 @@ class SvgSpriteController < ApplicationController
|
||||
def svg_icon
|
||||
no_cookies
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
|
||||
RailsMultisite::ConnectionManagement.with_hostname(
|
||||
params[:hostname],
|
||||
raise_on_missing: false,
|
||||
) do
|
||||
params.permit(:color)
|
||||
name = params.require(:name)
|
||||
icon = SvgSprite.search(name)
|
||||
|
||||
@@ -87,9 +87,10 @@ class UserAvatarsController < ApplicationController
|
||||
is_asset_path
|
||||
|
||||
# we need multisite support to keep a single origin pull for CDNs
|
||||
RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
|
||||
hijack { show_in_site(params[:hostname]) }
|
||||
end
|
||||
RailsMultisite::ConnectionManagement.with_hostname(
|
||||
params[:hostname],
|
||||
raise_on_missing: false,
|
||||
) { hijack { show_in_site(params[:hostname]) } }
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
@@ -128,6 +128,10 @@ module CachedCounting
|
||||
# could be a race condition in test
|
||||
if val > 0
|
||||
klass_name, db, date, local_key = key.split(",", 4)
|
||||
|
||||
# the site may have been deleted since the count was recorded
|
||||
next if !RailsMultisite::ConnectionManagement.has_db?(db)
|
||||
|
||||
date = Date.strptime(date, "%Y%m%d")
|
||||
klass = Module.const_get(klass_name)
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ def setup_message_bus_env(env)
|
||||
end
|
||||
|
||||
host = RailsMultisite::ConnectionManagement.host(env)
|
||||
RailsMultisite::ConnectionManagement.with_hostname(host) do
|
||||
RailsMultisite::ConnectionManagement.with_hostname(host, raise_on_missing: false) do
|
||||
cors_origin = Discourse.base_url_no_prefix
|
||||
|
||||
if GlobalSetting.enable_cors && SiteSetting.cors_origins.present?
|
||||
|
||||
@@ -840,6 +840,11 @@ module Discourse
|
||||
|
||||
@mutex.synchronize do
|
||||
@dbs.each do |db|
|
||||
if !RailsMultisite::ConnectionManagement.has_db?(db)
|
||||
@dbs.delete(db)
|
||||
next
|
||||
end
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_connection(db) do
|
||||
@dbs.delete(db) if !Discourse.redis.expire(key, ttl)
|
||||
end
|
||||
|
||||
@@ -77,6 +77,11 @@ class SidekiqPauser
|
||||
|
||||
@mutex.synchronize do
|
||||
@dbs.each do |db|
|
||||
if !RailsMultisite::ConnectionManagement.has_db?(db)
|
||||
@dbs.delete(db)
|
||||
next
|
||||
end
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_connection(db) do
|
||||
if !Discourse.redis.expire(PAUSED_KEY, TTL)
|
||||
# if it was unpaused in another process we got to remove the
|
||||
@@ -135,8 +140,9 @@ class Sidekiq::Pausable
|
||||
private
|
||||
|
||||
def sidekiq_paused?(msg)
|
||||
if site_id = msg["args"]&.first&.dig("current_site_id")
|
||||
RailsMultisite::ConnectionManagement.with_connection(site_id) { Sidekiq.paused? }
|
||||
end
|
||||
site_id = msg["args"]&.first&.dig("current_site_id")
|
||||
return false if !site_id || !RailsMultisite::ConnectionManagement.has_db?(site_id)
|
||||
|
||||
RailsMultisite::ConnectionManagement.with_connection(site_id) { Sidekiq.paused? }
|
||||
end
|
||||
end
|
||||
|
||||
@@ -48,6 +48,20 @@ RSpec.describe CachedCounting do
|
||||
|
||||
expect(TestCachedCounting.data).to eq({ "a,a" => 2, "b" => 1 })
|
||||
end
|
||||
|
||||
it "drops counts for a site that no longer exists" do
|
||||
Discourse.redis.without_namespace.hincrby(
|
||||
CachedCounting::COUNTER_REDIS_HASH,
|
||||
"TestCachedCounting,gone-site,#{Time.zone.now.strftime("%Y%m%d")},a",
|
||||
1,
|
||||
)
|
||||
CachedCounting.queue("b", TestCachedCounting)
|
||||
|
||||
CachedCounting.flush_in_memory
|
||||
CachedCounting.flush_to_db
|
||||
|
||||
expect(TestCachedCounting.data).to eq({ "b" => 1 })
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -120,6 +120,13 @@ RSpec.describe Scheduler::Defer do
|
||||
let!(:ivar) { Concurrent::IVar.new }
|
||||
let!(:responses) { Thread::Queue.new }
|
||||
|
||||
before do
|
||||
# "site1"/"site2" here are fairness-queue tags, not real dbs
|
||||
allow(RailsMultisite::ConnectionManagement).to receive(:with_connection) do |_db, &blk|
|
||||
blk.call
|
||||
end
|
||||
end
|
||||
|
||||
def later(db, current_user, request)
|
||||
@defer.later(nil, db, current_user: current_user) do
|
||||
ivar.value
|
||||
|
||||
@@ -65,5 +65,14 @@ RSpec.describe Sidekiq::Pausable, type: :multisite do
|
||||
expect(called2).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
it "runs a job for a site this process does not know about" do
|
||||
Sidekiq.pause!
|
||||
|
||||
called = false
|
||||
call_middleware("unknown-site") { called = true }
|
||||
|
||||
expect(called).to eq(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
# The asset routes below take the site's hostname as a path segment so a CDN can
|
||||
# pull every site through a single origin. The segment is unvalidated, so an
|
||||
# unknown one has to fall back to the default site rather than error.
|
||||
RSpec.describe "Asset routes with a hostname segment", type: %i[multisite request] do
|
||||
it "serves svg sprites for an unknown hostname" do
|
||||
get "/svg-sprite/unknown.example.com/svg--#{SecureRandom.hex(20)}.js"
|
||||
|
||||
expect(response.status).to eq(302)
|
||||
end
|
||||
|
||||
it "serves svg icons for an unknown hostname" do
|
||||
get "/svg-sprite/unknown.example.com/icon/heart.svg"
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
it "serves highlight.js for an unknown hostname" do
|
||||
get "/highlight-js/unknown.example.com/stale.js"
|
||||
|
||||
expect(response.status).to eq(302)
|
||||
end
|
||||
|
||||
it "serves avatars for an unknown hostname" do
|
||||
user = Fabricate(:user)
|
||||
|
||||
get "/user_avatar/unknown.example.com/#{user.username}/45/1.png"
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user