Merge branch '59208-add-option-to-keep-cached-fields-during-serialization' into 'master'

Add option to whitelist _html fields from attributes on CacheMarkdownField

See merge request gitlab-org/gitlab-ce!26374

(cherry picked from commit f7fcfc7720c5149e2fa6f027900503ae3f215bf1)

3fd8e612 Add option to not exclude _html fields from attributes
bcc988a6 Does not exclude message_html from attributes
69dc893d Fix spec for Gitlab::JsonCache
6264694d Rename the hidden option to whitelisted

3 files changed, 39 insertions, 11 deletions

diff --git a/spec/lib/gitlab/json_cache_spec.rb b/spec/lib/gitlab/json_cache_spec.rb
index b7dc8234bdf..b82c09af306 100644
--- a/spec/lib/gitlab/json_cache_spec.rb
+++ b/spec/lib/gitlab/json_cache_spec.rb
@@ -146,6 +146,18 @@ describe Gitlab::JsonCache do
 
         expect(cache.read(key, BroadcastMessage)).to be_nil
       end
+
+      it 'gracefully handles excluded fields from attributes during serialization' do
+        allow(backend).to receive(:read)
+          .with(expanded_key)
+          .and_return(broadcast_message.attributes.except("message_html").to_json)
+
+        result = cache.read(key, BroadcastMessage)
+
+        BroadcastMessage.cached_markdown_fields.html_fields.each do |field|
+          expect(result.public_send(field)).to be_nil
+        end
+      end
     end
 
     context 'when the cached value is an array' do
@@ -327,7 +339,9 @@ describe Gitlab::JsonCache do
               .with(expanded_key)
               .and_return('{')
 
-            expect(cache.read(key, BroadcastMessage)).to be_nil
+            result = cache.fetch(key, as: BroadcastMessage) { 'block result' }
+
+            expect(result).to eq 'block result'
           end
 
           it 'gracefully handles an empty hash' do
@@ -335,7 +349,7 @@ describe Gitlab::JsonCache do
               .with(expanded_key)
               .and_return('{}')
 
-            expect(cache.read(key, BroadcastMessage)).to be_a(BroadcastMessage)
+            expect(cache.fetch(key, as: BroadcastMessage)).to be_a(BroadcastMessage)
           end
 
           it 'gracefully handles unknown attributes' do
@@ -343,17 +357,19 @@ describe Gitlab::JsonCache do
               .with(expanded_key)
               .and_return(broadcast_message.attributes.merge(unknown_attribute: 1).to_json)
 
-            expect(cache.read(key, BroadcastMessage)).to be_nil
+            result = cache.fetch(key, as: BroadcastMessage) { 'block result' }
+
+            expect(result).to eq 'block result'
           end
 
           it 'gracefully handles excluded fields from attributes during serialization' do
-            backend.write(expanded_key, broadcast_message.to_json)
+            allow(backend).to receive(:read)
+              .with(expanded_key)
+              .and_return(broadcast_message.attributes.except("message_html").to_json)
 
             result = cache.fetch(key, as: BroadcastMessage) { 'block result' }
 
-            excluded_fields = BroadcastMessage.cached_markdown_fields.html_fields
-
-            (excluded_fields + ['cached_markdown_version']).each do |field|
+            BroadcastMessage.cached_markdown_fields.html_fields.each do |field|
               expect(result.public_send(field)).to be_nil
             end
           end
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index 89839709131..30ca07d5d2c 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -95,6 +95,12 @@ describe BroadcastMessage do
     end
   end
 
+  describe '#attributes' do
+    it 'includes message_html field' do
+      expect(subject.attributes.keys).to include("cached_markdown_version", "message_html")
+    end
+  end
+
   describe '#active?' do
     it 'is truthy when started and not ended' do
       message = build(:broadcast_message)
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 447279f19a8..7d555f15e39 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -23,6 +23,7 @@ describe CacheMarkdownField do
     include CacheMarkdownField
     cache_markdown_field :foo
     cache_markdown_field :baz, pipeline: :single_line
+    cache_markdown_field :zoo, whitelisted: true
 
     def self.add_attr(name)
       self.attribute_names += [name]
@@ -35,7 +36,7 @@ describe CacheMarkdownField do
 
     add_attr :cached_markdown_version
 
-    [:foo, :foo_html, :bar, :baz, :baz_html].each do |name|
+    [:foo, :foo_html, :bar, :baz, :baz_html, :zoo, :zoo_html].each do |name|
       add_attr(name)
     end
 
@@ -84,8 +85,8 @@ describe CacheMarkdownField do
   end
 
   describe '.attributes' do
-    it 'excludes cache attributes' do
-      expect(thing.attributes.keys.sort).to eq(%w[bar baz foo])
+    it 'excludes cache attributes that is blacklisted by default' do
+      expect(thing.attributes.keys.sort).to eq(%w[bar baz cached_markdown_version foo zoo zoo_html])
     end
   end
 
@@ -297,7 +298,12 @@ describe CacheMarkdownField do
     it 'saves the changes using #update_columns' do
       expect(thing).to receive(:persisted?).and_return(true)
       expect(thing).to receive(:update_columns)
-        .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version)
+        .with(
+          "foo_html" => updated_html,
+          "baz_html" => "",
+          "zoo_html" => "",
+          "cached_markdown_version" => cache_version
+        )
 
       thing.refresh_markdown_cache!
     end