improve caching of ignored_columns

refs AE-124

Change-Id: Ia458e233a64a9d4fbd878537d39418a1349660db
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/311728
QA-Review: Aaron Ogata <aogata@instructure.com>
Product-Review: Aaron Ogata <aogata@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>

config/initializers/active_record.rb

@@ -2213,11 +2213,36 @@ if Rails.version >= "6.1"
 end
 
 module AdditionalIgnoredColumns
-  def ignored_columns
-    return super unless superclass <= ActiveRecord::Base && !abstract_class?
+  def self.included(klass)
+    klass.extend(ClassMethods)
+    klass.prepend(InstanceMethods)
 
-    columns_to_ignore = DynamicSettings.find("activerecord/ignored_columns", tree: :store)[table_name]&.split(",") || []
-    super + columns_to_ignore
+    klass.reset_ignored_columns!
+
+    ::Canvas::Reloader.on_reload do
+      klass.reset_ignored_columns!
+    end
+  end
+
+  module InstanceMethods
+    def ignored_columns
+      return super unless superclass <= ActiveRecord::Base && !abstract_class?
+
+      cache_class = ActiveRecord::Base.singleton_class
+      return super unless cache_class.columns_to_ignore_enabled
+
+      cache_class.columns_to_ignore_cache[table_name] ||= DynamicSettings.find("activerecord/ignored_columns", tree: :store)[table_name]&.split(",") || []
+      super + cache_class.columns_to_ignore_cache[table_name]
+    end
+  end
+
+  module ClassMethods
+    attr_accessor :columns_to_ignore_cache, :columns_to_ignore_enabled
+
+    def reset_ignored_columns!
+      @columns_to_ignore_cache = {}
+      @columns_to_ignore_enabled = !DynamicSettings.find("activerecord", tree: :store)["ignored_columns_disabled"]
+    end
   end
 end
 ActiveRecord::Base.singleton_class.include(AdditionalIgnoredColumns)

spec/initializers/active_record_spec.rb

@@ -340,27 +340,54 @@ module ActiveRecord
     end
 
     describe ".ignored_columns" do
-      it "ignores additional columns specified in Consul" do
+      before do
+        allow(DynamicSettings).to receive(:find).with(any_args).and_call_original
+
         # If this test is the first one to run that requires User - preload User so that the correct
         # accessors (getters / setters) already exist since the "ensure" block won't create them. In
         # a real situation, we would first perform a rolling restart after having unset this key and
         # finished pre-deploy migrations everywhere.
         User.create!(name: "user u1")
+      end
 
-        allow(DynamicSettings).to receive(:find).with(any_args).and_call_original
-        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_return(
+      after do
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_call_original
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns_disabled", tree: :store).and_call_original
+
+        reset_cache!
+        User.create!(name: "user u2")
+      end
+
+      def reset_cache!
+        Canvas::Reloader.reload!
+        User.reset_column_information
+      end
+
+      def set_ignored_columns_state!(columns, enabled)
+        allow(DynamicSettings).to receive(:find).with("activerecord", tree: :store).and_return(
           {
-            "users" => "name"
+            "ignored_columns_disabled" => !enabled
           }
         )
 
-        User.reset_column_information
-        expect { User.create!(name: "user u2") }.to raise_exception(ActiveModel::UnknownAttributeError)
-      ensure
-        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_call_original
+        allow(DynamicSettings).to receive(:find).with("activerecord/ignored_columns", tree: :store).and_return(
+          {
+            "users" => columns
+          }
+        )
 
-        User.reset_column_information
-        User.create!(name: "user u2")
+        reset_cache!
+      end
+
+      it "ignores additional columns specified in Consul" do
+        set_ignored_columns_state!("name", true)
+        expect { User.create!(name: "user u2") }.to raise_exception(ActiveModel::UnknownAttributeError)
+      end
+
+      it "does not ignore additional columns if disabled" do
+        set_ignored_columns_state!("name", false)
+        expect(DynamicSettings).not_to receive(:find).with("activerecord/ignored_columns")
+        expect { User.create!(name: "user u2") }.not_to raise_exception
       end
     end
   end