From baeac8f105cff8bbe70db67789bc96883cb185bc Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 6 Nov 2023 16:36:20 +1000 Subject: [PATCH] FIX: Do not log client site setting deprecations (#24237) For deprecated site settings, we log out a warning when the old setting is used. However when we convert all the client settings to JSON, we are creating a lot of log noise like this: > Deprecation notice: `SiteSetting.anonymous_posting_min_trust_level` has been deprecated. We don't need to do this because we are just dumping the JSON. --- lib/site_setting_extension.rb | 14 +++++-- spec/lib/site_setting_extension_spec.rb | 54 +++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 35312387995..fec41e8c001 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -127,11 +127,12 @@ module SiteSettingExtension end end + def deprecated_settings + @deprecated_settings ||= SiteSettings::DeprecatedSettings::SETTINGS.map(&:first).to_set + end + def settings_hash result = {} - deprecated_settings = Set.new - - SiteSettings::DeprecatedSettings::SETTINGS.each { |s| deprecated_settings << s[0] } defaults.all.keys.each do |s| result[s] = if deprecated_settings.include?(s.to_s) @@ -157,7 +158,12 @@ module SiteSettingExtension Hash[ *@client_settings .map do |name| - value = self.public_send(name) + value = + if deprecated_settings.include?(name.to_s) + public_send(name, warn: false) + else + public_send(name) + end type = type_supervisor.get_type(name) value = value.to_s if type == :upload value = value.map(&:to_s).join("|") if type == :uploaded_image_list diff --git a/spec/lib/site_setting_extension_spec.rb b/spec/lib/site_setting_extension_spec.rb index e2bd93a4291..298b9c0cbd8 100644 --- a/spec/lib/site_setting_extension_spec.rb +++ b/spec/lib/site_setting_extension_spec.rb @@ -832,6 +832,60 @@ RSpec.describe SiteSettingExtension do expect(client_settings["with_html"]).to eq("rest") end + + context "for deprecated settings" do + before do + @orig_logger = Rails.logger + Rails.logger = @fake_logger = FakeLogger.new + end + + after { Rails.logger = @orig_logger } + + it "does not log deprecation warnings" do + begin + original_settings = SiteSettings::DeprecatedSettings::SETTINGS.dup + SiteSettings::DeprecatedSettings::SETTINGS.clear + SiteSettings::DeprecatedSettings::SETTINGS.push( + ["use_https", "force_https", true, "0.0.1"], + ) + SiteSetting.setup_deprecated_methods + + SiteSetting.client_settings_json_uncached + expect(@fake_logger.warnings).to eq([]) + ensure + SiteSettings::DeprecatedSettings::SETTINGS.clear + SiteSettings::DeprecatedSettings::SETTINGS.concat(original_settings) + end + end + end + end + + describe ".settings_hash" do + context "for deprecated settings" do + before do + @orig_logger = Rails.logger + Rails.logger = @fake_logger = FakeLogger.new + end + + after { Rails.logger = @orig_logger } + + it "does not log deprecation warnings" do + begin + original_settings = SiteSettings::DeprecatedSettings::SETTINGS.dup + SiteSettings::DeprecatedSettings::SETTINGS.clear + SiteSettings::DeprecatedSettings::SETTINGS.push( + ["use_https", "force_https", true, "0.0.1"], + ) + SiteSetting.setup_deprecated_methods + + SiteSetting.settings_hash + expect(@fake_logger.warnings).to eq([]) + ensure + SiteSettings::DeprecatedSettings::SETTINGS.clear + SiteSettings::DeprecatedSettings::SETTINGS.concat(original_settings) + end + end + end end describe ".setup_methods" do