From d40e33fcc5899a91ceab10f8bd86a1455846917a Mon Sep 17 00:00:00 2001
From: bogdanvlviv <bogdanvlviv@gmail.com>
Date: Mon, 10 Sep 2018 12:41:14 +0300
Subject: DRY `activerecord/lib/active_record/core.rb` and fix tests

- Move
  ```
  filter_attributes = self.filter_attributes.map(&:to_s).to_set

  filter_attributes.include?(attribute_name) && !read_attribute(attribute_name).nil?
  ```
  to private method.

- Fix tests in `activerecord/test/cases/filter_attributes_test.rb`
  - Ensure that `teardown` sets `ActiveRecord::Base.filter_attributes` to
    previous state.

  - Ensure that `Admin::Account.filter_attributes` is set to previous
    state in the "filter_attributes could be overwritten by models" test.

Follow up #33756
---
 activerecord/lib/active_record/core.rb            | 15 ++++++-----
 activerecord/test/cases/filter_attributes_test.rb | 32 +++++++++++++----------
 2 files changed, 27 insertions(+), 20 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 4f10cddc87..d5d2c70a8a 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -493,13 +493,12 @@ module ActiveRecord
 
     # Returns the contents of the record as a nicely formatted string.
     def inspect
-      filter_attributes = self.filter_attributes.map(&:to_s).to_set
       # We check defined?(@attributes) not to issue warnings if the object is
       # allocated but not initialized.
       inspection = if defined?(@attributes) && @attributes
         self.class.attribute_names.collect do |name|
           if has_attribute?(name)
-            if filter_attributes.include?(name) && !read_attribute(name).nil?
+            if filter_attribute?(name)
               "#{name}: #{ActiveRecord::Core::FILTERED}"
             else
               "#{name}: #{attribute_for_inspect(name)}"
@@ -517,21 +516,19 @@ module ActiveRecord
     # when pp is required.
     def pretty_print(pp)
       return super if custom_inspect_method_defined?
-      filter_attributes = self.filter_attributes.map(&:to_s).to_set
       pp.object_address_group(self) do
         if defined?(@attributes) && @attributes
           column_names = self.class.column_names.select { |name| has_attribute?(name) || new_record? }
           pp.seplist(column_names, proc { pp.text "," }) do |column_name|
-            column_value = read_attribute(column_name)
             pp.breakable " "
             pp.group(1) do
               pp.text column_name
               pp.text ":"
               pp.breakable
-              if filter_attributes.include?(column_name) && !column_value.nil?
+              if filter_attribute?(column_name)
                 pp.text ActiveRecord::Core::FILTERED
               else
-                pp.pp column_value
+                pp.pp read_attribute(column_name)
               end
             end
           end
@@ -583,5 +580,11 @@ module ActiveRecord
       def custom_inspect_method_defined?
         self.class.instance_method(:inspect).owner != ActiveRecord::Base.instance_method(:inspect).owner
       end
+
+      def filter_attribute?(attribute_name)
+        filter_attributes = self.filter_attributes.map(&:to_s).to_set
+
+        filter_attributes.include?(attribute_name) && !read_attribute(attribute_name).nil?
+      end
   end
 end
diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb
index f88cecfe2b..af5badd87d 100644
--- a/activerecord/test/cases/filter_attributes_test.rb
+++ b/activerecord/test/cases/filter_attributes_test.rb
@@ -10,11 +10,12 @@ class FilterAttributesTest < ActiveRecord::TestCase
   fixtures :"admin/users", :"admin/accounts"
 
   setup do
+    @previous_filter_attributes = ActiveRecord::Base.filter_attributes
     ActiveRecord::Base.filter_attributes = [:name]
   end
 
   teardown do
-    ActiveRecord::Base.filter_attributes = []
+    ActiveRecord::Base.filter_attributes = @previous_filter_attributes
   end
 
   test "filter_attributes" do
@@ -35,20 +36,23 @@ class FilterAttributesTest < ActiveRecord::TestCase
       assert_equal 1, account.inspect.scan("[FILTERED]").length
     end
 
-    Admin::Account.filter_attributes = []
-
-    # Above changes should not impact other models
-    Admin::User.all.each do |user|
-      assert_includes user.inspect, "name: [FILTERED]"
-      assert_equal 1, user.inspect.scan("[FILTERED]").length
+    begin
+      previous_account_filter_attributes = Admin::Account.filter_attributes
+      Admin::Account.filter_attributes = []
+
+      # Above changes should not impact other models
+      Admin::User.all.each do |user|
+        assert_includes user.inspect, "name: [FILTERED]"
+        assert_equal 1, user.inspect.scan("[FILTERED]").length
+      end
+
+      Admin::Account.all.each do |account|
+        assert_not_includes account.inspect, "name: [FILTERED]"
+        assert_equal 0, account.inspect.scan("[FILTERED]").length
+      end
+    ensure
+      Admin::Account.filter_attributes = previous_account_filter_attributes
     end
-
-    Admin::Account.all.each do |account|
-      assert_not_includes account.inspect, "name: [FILTERED]"
-      assert_equal 0, account.inspect.scan("[FILTERED]").length
-    end
-
-    Admin::Account.filter_attributes = [:name]
   end
 
   test "filter_attributes should not filter nil value" do
-- 
cgit v1.2.3