Commit cc5095ed authored by Stan Hu's avatar Stan Hu

Merge branch...

Merge branch '59208-fix-error-500-on-every-page-when-active-broadcast-message-present-after-upgrading-to-11-9-0' into 'master'

Gracefully handles excluded fields from attributes during serialization on JsonCache

Closes #59208

See merge request gitlab-org/gitlab-ce!26368
parents a6fe183e 1c7a2c05
---
title: Gracefully handles excluded fields from attributes during serialization on
JsonCache
merge_request: 26368
author:
type: fixed
...@@ -80,8 +80,23 @@ module Gitlab ...@@ -80,8 +80,23 @@ module Gitlab
# when the new_record? method incorrectly returns false. # when the new_record? method incorrectly returns false.
# #
# See https://gitlab.com/gitlab-org/gitlab-ee/issues/9903#note_145329964 # See https://gitlab.com/gitlab-org/gitlab-ee/issues/9903#note_145329964
attributes = klass.attributes_builder.build_from_database(raw, {}) klass
klass.allocate.init_with("attributes" => attributes, "new_record" => new_record?(raw, klass)) .allocate
.init_with(
"attributes" => attributes_for(klass, raw),
"new_record" => new_record?(raw, klass)
)
end
def attributes_for(klass, raw)
# We have models that leave out some fields from the JSON export for
# security reasons, e.g. models that include the CacheMarkdownField.
# The ActiveRecord::AttributeSet we build from raw does know about
# these columns so we need manually set them.
missing_attributes = (klass.columns.map(&:name) - raw.keys)
missing_attributes.each { |column| raw[column] = nil }
klass.attributes_builder.build_from_database(raw, {})
end end
def new_record?(raw, klass) def new_record?(raw, klass)
......
...@@ -7,7 +7,7 @@ describe Gitlab::JsonCache do ...@@ -7,7 +7,7 @@ describe Gitlab::JsonCache do
let(:namespace) { 'geo' } let(:namespace) { 'geo' }
let(:key) { 'foo' } let(:key) { 'foo' }
let(:expanded_key) { "#{namespace}:#{key}:#{Rails.version}" } let(:expanded_key) { "#{namespace}:#{key}:#{Rails.version}" }
let(:broadcast_message) { create(:broadcast_message) } set(:broadcast_message) { create(:broadcast_message) }
subject(:cache) { described_class.new(namespace: namespace, backend: backend) } subject(:cache) { described_class.new(namespace: namespace, backend: backend) }
...@@ -321,6 +321,42 @@ describe Gitlab::JsonCache do ...@@ -321,6 +321,42 @@ describe Gitlab::JsonCache do
expect(result).to be_new_record expect(result).to be_new_record
end end
it 'gracefully handles bad cached entry' do
allow(backend).to receive(:read)
.with(expanded_key)
.and_return('{')
expect(cache.read(key, BroadcastMessage)).to be_nil
end
it 'gracefully handles an empty hash' do
allow(backend).to receive(:read)
.with(expanded_key)
.and_return('{}')
expect(cache.read(key, BroadcastMessage)).to be_a(BroadcastMessage)
end
it 'gracefully handles unknown attributes' do
allow(backend).to receive(:read)
.with(expanded_key)
.and_return(broadcast_message.attributes.merge(unknown_attribute: 1).to_json)
expect(cache.read(key, BroadcastMessage)).to be_nil
end
it 'gracefully handles excluded fields from attributes during serialization' do
backend.write(expanded_key, broadcast_message.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|
expect(result.public_send(field)).to be_nil
end
end
end end
it "returns the result of the block when 'as' option is nil" do it "returns the result of the block when 'as' option is nil" do
......
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