Merge branch 'appearances-caching-and-schema' into 'master'

Cache Appearance instances in Redis

Closes #36066 and #31698

See merge request !13433

app/controllers/admin/appearances_controller.rb

@@ -45,7 +45,7 @@ class Admin::AppearancesController < Admin::ApplicationController
 
   # Use callbacks to share common setup or constraints between actions.
   def set_appearance
-    @appearance = Appearance.last || Appearance.new
+    @appearance = Appearance.current || Appearance.new
   end
 
   # Only allow a trusted parameter "white list" through.

app/helpers/appearances_helper.rb

@@ -20,7 +20,7 @@ module AppearancesHelper
   end
 
   def brand_item
-    @appearance ||= Appearance.first
+    @appearance ||= Appearance.current
   end
 
   def brand_header_logo

app/models/appearance.rb

@@ -8,7 +8,27 @@ class Appearance < ActiveRecord::Base
   validates :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 :header_logo,  AttachmentUploader
   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

changelogs/unreleased/appearances-caching-and-schema.yml

+---
+title: Cache Appearance instances in Redis
+merge_request:
+author:

db/migrate/20170809142252_cleanup_appearances_schema.rb

+# 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

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # 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
   enable_extension "plpgsql"
@@ -28,13 +28,13 @@ ActiveRecord::Schema.define(version: 20170807160457) do
   end
 
   create_table "appearances", force: :cascade do |t|
-    t.string "title"
-    t.text "description"
+    t.string "title", null: false
+    t.text "description", null: false
     t.string "header_logo"
     t.string "logo"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.text "description_html"
+    t.text "description_html", null: false
     t.integer "cached_markdown_version"
   end
 

spec/models/appearance_spec.rb

@@ -9,4 +9,39 @@ RSpec.describe Appearance do
   it { is_expected.to validate_presence_of(:description) }
 
   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