Commit 5c67aeaf authored by Stan Hu's avatar Stan Hu Committed by GitLab Release Tools Bot

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 f7fcfc77)

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
parent 2b16e92b
...@@ -4,7 +4,7 @@ class BroadcastMessage < ActiveRecord::Base ...@@ -4,7 +4,7 @@ class BroadcastMessage < ActiveRecord::Base
include CacheMarkdownField include CacheMarkdownField
include Sortable include Sortable
cache_markdown_field :message, pipeline: :broadcast_message cache_markdown_field :message, pipeline: :broadcast_message, whitelisted: true
validates :message, presence: true validates :message, presence: true
validates :starts_at, presence: true validates :starts_at, presence: true
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
# cache_markdown_field :foo # cache_markdown_field :foo
# cache_markdown_field :bar # cache_markdown_field :bar
# cache_markdown_field :baz, pipeline: :single_line # cache_markdown_field :baz, pipeline: :single_line
# cache_markdown_field :baz, whitelisted: true
# #
# Corresponding foo_html, bar_html and baz_html fields should exist. # Corresponding foo_html, bar_html and baz_html fields should exist.
module CacheMarkdownField module CacheMarkdownField
...@@ -37,7 +38,15 @@ module CacheMarkdownField ...@@ -37,7 +38,15 @@ module CacheMarkdownField
end end
def html_fields def html_fields
markdown_fields.map {|field| html_field(field) } markdown_fields.map { |field| html_field(field) }
end
def html_fields_whitelisted
markdown_fields.each_with_object([]) do |field, fields|
if @data[field].fetch(:whitelisted, false)
fields << html_field(field)
end
end
end end
end end
...@@ -149,13 +158,18 @@ module CacheMarkdownField ...@@ -149,13 +158,18 @@ module CacheMarkdownField
alias_method :attributes_before_markdown_cache, :attributes alias_method :attributes_before_markdown_cache, :attributes
def attributes def attributes
attrs = attributes_before_markdown_cache attrs = attributes_before_markdown_cache
html_fields = cached_markdown_fields.html_fields
whitelisted = cached_markdown_fields.html_fields_whitelisted
exclude_fields = html_fields - whitelisted
attrs.delete('cached_markdown_version') exclude_fields.each do |field|
cached_markdown_fields.html_fields.each do |field|
attrs.delete(field) attrs.delete(field)
end end
if whitelisted.empty?
attrs.delete('cached_markdown_version')
end
attrs attrs
end end
......
...@@ -146,6 +146,18 @@ describe Gitlab::JsonCache do ...@@ -146,6 +146,18 @@ describe Gitlab::JsonCache do
expect(cache.read(key, BroadcastMessage)).to be_nil expect(cache.read(key, BroadcastMessage)).to be_nil
end 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 end
context 'when the cached value is an array' do context 'when the cached value is an array' do
...@@ -327,7 +339,9 @@ describe Gitlab::JsonCache do ...@@ -327,7 +339,9 @@ describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return('{') .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 end
it 'gracefully handles an empty hash' do it 'gracefully handles an empty hash' do
...@@ -335,7 +349,7 @@ describe Gitlab::JsonCache do ...@@ -335,7 +349,7 @@ describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return('{}') .and_return('{}')
expect(cache.read(key, BroadcastMessage)).to be_a(BroadcastMessage) expect(cache.fetch(key, as: BroadcastMessage)).to be_a(BroadcastMessage)
end end
it 'gracefully handles unknown attributes' do it 'gracefully handles unknown attributes' do
...@@ -343,17 +357,19 @@ describe Gitlab::JsonCache do ...@@ -343,17 +357,19 @@ describe Gitlab::JsonCache do
.with(expanded_key) .with(expanded_key)
.and_return(broadcast_message.attributes.merge(unknown_attribute: 1).to_json) .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 end
it 'gracefully handles excluded fields from attributes during serialization' do 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' } result = cache.fetch(key, as: BroadcastMessage) { 'block result' }
excluded_fields = BroadcastMessage.cached_markdown_fields.html_fields BroadcastMessage.cached_markdown_fields.html_fields.each do |field|
(excluded_fields + ['cached_markdown_version']).each do |field|
expect(result.public_send(field)).to be_nil expect(result.public_send(field)).to be_nil
end end
end end
......
...@@ -95,6 +95,12 @@ describe BroadcastMessage do ...@@ -95,6 +95,12 @@ describe BroadcastMessage do
end end
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 describe '#active?' do
it 'is truthy when started and not ended' do it 'is truthy when started and not ended' do
message = build(:broadcast_message) message = build(:broadcast_message)
......
...@@ -23,6 +23,7 @@ describe CacheMarkdownField do ...@@ -23,6 +23,7 @@ describe CacheMarkdownField do
include CacheMarkdownField include CacheMarkdownField
cache_markdown_field :foo cache_markdown_field :foo
cache_markdown_field :baz, pipeline: :single_line cache_markdown_field :baz, pipeline: :single_line
cache_markdown_field :zoo, whitelisted: true
def self.add_attr(name) def self.add_attr(name)
self.attribute_names += [name] self.attribute_names += [name]
...@@ -35,7 +36,7 @@ describe CacheMarkdownField do ...@@ -35,7 +36,7 @@ describe CacheMarkdownField do
add_attr :cached_markdown_version 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) add_attr(name)
end end
...@@ -84,8 +85,8 @@ describe CacheMarkdownField do ...@@ -84,8 +85,8 @@ describe CacheMarkdownField do
end end
describe '.attributes' do describe '.attributes' do
it 'excludes cache attributes' do it 'excludes cache attributes that is blacklisted by default' do
expect(thing.attributes.keys.sort).to eq(%w[bar baz foo]) expect(thing.attributes.keys.sort).to eq(%w[bar baz cached_markdown_version foo zoo zoo_html])
end end
end end
...@@ -297,7 +298,12 @@ describe CacheMarkdownField do ...@@ -297,7 +298,12 @@ describe CacheMarkdownField do
it 'saves the changes using #update_columns' do it 'saves the changes using #update_columns' do
expect(thing).to receive(:persisted?).and_return(true) expect(thing).to receive(:persisted?).and_return(true)
expect(thing).to receive(:update_columns) 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! thing.refresh_markdown_cache!
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment