BigW Consortium Gitlab

Cache Appearance instances in Redis

This caches the result of Appearance.first in a similar fashion to how ApplicationSetting instances are cached. We also add some NOT NULL constraints to the table and correct the timestamp types. Fixes gitlab-org/gitlab-ce#36066, fixes gitlab-org/gitlab-ce#31698
parent 53ee3805
...@@ -45,7 +45,7 @@ class Admin::AppearancesController < Admin::ApplicationController ...@@ -45,7 +45,7 @@ class Admin::AppearancesController < Admin::ApplicationController
# Use callbacks to share common setup or constraints between actions. # Use callbacks to share common setup or constraints between actions.
def set_appearance def set_appearance
@appearance = Appearance.last || Appearance.new @appearance = Appearance.current || Appearance.new
end end
# Only allow a trusted parameter "white list" through. # Only allow a trusted parameter "white list" through.
......
...@@ -20,7 +20,7 @@ module AppearancesHelper ...@@ -20,7 +20,7 @@ module AppearancesHelper
end end
def brand_item def brand_item
@appearance ||= Appearance.first @appearance ||= Appearance.current
end end
def brand_header_logo def brand_header_logo
......
...@@ -8,7 +8,27 @@ class Appearance < ActiveRecord::Base ...@@ -8,7 +8,27 @@ class Appearance < ActiveRecord::Base
validates :logo, file_size: { maximum: 1.megabyte } validates :logo, file_size: { maximum: 1.megabyte }
validates :header_logo, file_size: { maximum: 1.megabyte } validates :header_logo, file_size: { maximum: 1.megabyte }
validate :single_appearance_row, on: :create
mount_uploader :logo, AttachmentUploader mount_uploader :logo, AttachmentUploader
mount_uploader :header_logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader
has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
CACHE_KEY = 'current_appearance'.freeze
after_commit :flush_redis_cache
def self.current
Rails.cache.fetch(CACHE_KEY) { first }
end
def flush_redis_cache
Rails.cache.delete(CACHE_KEY)
end
def single_appearance_row
if self.class.any?
errors.add(:single_appearance_row, 'Only 1 appearances row can exist')
end
end
end end
---
title: Cache Appearance instances in Redis
merge_request:
author:
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class CleanupAppearancesSchema < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
NOT_NULL_COLUMNS = %i[title description description_html created_at updated_at]
TIME_COLUMNS = %i[created_at updated_at]
def up
NOT_NULL_COLUMNS.each do |column|
change_column_null :appearances, column, false
end
TIME_COLUMNS.each do |column|
change_column :appearances, column, :datetime_with_timezone
end
end
def down
NOT_NULL_COLUMNS.each do |column|
change_column_null :appearances, column, true
end
TIME_COLUMNS.each do |column|
change_column :appearances, column, :datetime # rubocop: disable Migration/Datetime
end
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20170807160457) do ActiveRecord::Schema.define(version: 20170809142252) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -28,13 +28,13 @@ ActiveRecord::Schema.define(version: 20170807160457) do ...@@ -28,13 +28,13 @@ ActiveRecord::Schema.define(version: 20170807160457) do
end end
create_table "appearances", force: :cascade do |t| create_table "appearances", force: :cascade do |t|
t.string "title" t.string "title", null: false
t.text "description" t.text "description", null: false
t.string "header_logo" t.string "header_logo"
t.string "logo" t.string "logo"
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.text "description_html" t.text "description_html", null: false
t.integer "cached_markdown_version" t.integer "cached_markdown_version"
end end
......
...@@ -9,4 +9,39 @@ RSpec.describe Appearance do ...@@ -9,4 +9,39 @@ RSpec.describe Appearance do
it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_presence_of(:description) }
it { is_expected.to have_many(:uploads).dependent(:destroy) } it { is_expected.to have_many(:uploads).dependent(:destroy) }
describe '.current', :use_clean_rails_memory_store_caching do
let!(:appearance) { create(:appearance) }
it 'returns the current appearance row' do
expect(described_class.current).to eq(appearance)
end
it 'caches the result' do
expect(described_class).to receive(:first).once
2.times { described_class.current }
end
end
describe '#flush_redis_cache' do
it 'flushes the cache in Redis' do
appearance = create(:appearance)
expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY)
appearance.flush_redis_cache
end
end
describe '#single_appearance_row' do
it 'adds an error when more than 1 row exists' do
create(:appearance)
new_row = build(:appearance)
new_row.save
expect(new_row.valid?).to eq(false)
end
end
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