diff --git a/app/controllers/admin/appearances_controller.rb b/app/controllers/admin/appearances_controller.rb index 4b0ec54b3f4f8623cc5d71752e339966dbdc7f74..92df1c8dff07457dc8053464eeaaf5fef2c3c895 100644 --- a/app/controllers/admin/appearances_controller.rb +++ b/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. diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index 16136d025304660408bfa13c733ab44ee4f26b28..cdf5fa5d4b7b34b5d4e63b584604843e493d6d44 100644 --- a/app/helpers/appearances_helper.rb +++ b/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 diff --git a/app/models/appearance.rb b/app/models/appearance.rb index f9c48482be74537531f7c5ff72646e2ccb4497a6..ff15689ecac3d3cf58b5bac155c04aafb8f551ad 100644 --- a/app/models/appearance.rb +++ b/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 diff --git a/changelogs/unreleased/appearances-caching-and-schema.yml b/changelogs/unreleased/appearances-caching-and-schema.yml new file mode 100644 index 0000000000000000000000000000000000000000..5743f6e0f2dd78b3046af263616eaea85dc0fdda --- /dev/null +++ b/changelogs/unreleased/appearances-caching-and-schema.yml @@ -0,0 +1,4 @@ +--- +title: Cache Appearance instances in Redis +merge_request: +author: diff --git a/db/migrate/20170809142252_cleanup_appearances_schema.rb b/db/migrate/20170809142252_cleanup_appearances_schema.rb new file mode 100644 index 0000000000000000000000000000000000000000..90d12925ba29996c715c0d2927748788492a82a9 --- /dev/null +++ b/db/migrate/20170809142252_cleanup_appearances_schema.rb @@ -0,0 +1,33 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index ed3cf70bcdd5632e7257026e4087b76fc4be955d..ec92b185be2c225ff18ef2c4eaf7e80a9266e6b4 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 7cd3a84d59206310b5ad90222edd6bc343440178..b5d5d58697bac2936eef1a9d42ce80e635034f91 100644 --- a/spec/models/appearance_spec.rb +++ b/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