Commit e0b589f1 authored by Yorick Peterse's avatar Yorick Peterse

Fix caching of future broadcast messages

This changes the caching mechanism so we cache both current _and_ future
broadcast messages, then manually filter out those we don't want to
display. This ensures we don't need any additional queries while still
being able to display the right messages at the right time.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/36661
parent 646aae3e
......@@ -19,11 +19,21 @@ class BroadcastMessage < ActiveRecord::Base
after_commit :flush_redis_cache
def self.current
Rails.cache.fetch(CACHE_KEY) do
where('ends_at > :now AND starts_at <= :now', now: Time.zone.now)
.reorder(id: :asc)
.to_a
messages = Rails.cache.fetch(CACHE_KEY) { current_and_future_messages.to_a }
return messages if messages.empty?
now_or_future = messages.select(&:now_or_future?)
# If there are cached entries but none are to be displayed we'll purge the
# cache so we don't keep running this code all the time.
Rails.cache.delete(CACHE_KEY) if now_or_future.empty?
now_or_future.select(&:now?)
end
def self.current_and_future_messages
where('ends_at > :now', now: Time.zone.now).reorder(id: :asc)
end
def active?
......@@ -38,6 +48,18 @@ class BroadcastMessage < ActiveRecord::Base
ends_at < Time.zone.now
end
def now?
(starts_at..ends_at).cover?(Time.zone.now)
end
def future?
starts_at > Time.zone.now
end
def now_or_future?
now? || future?
end
def flush_redis_cache
Rails.cache.delete(CACHE_KEY)
end
......
---
title: Fix caching of future broadcast messages
merge_request:
author:
type: fixed
......@@ -53,6 +53,29 @@ describe BroadcastMessage do
2.times { described_class.current }
end
it 'includes messages that need to be displayed in the future' do
create(:broadcast_message)
future = create(
:broadcast_message,
starts_at: Time.now + 10.minutes,
ends_at: Time.now + 20.minutes
)
expect(described_class.current.length).to eq(1)
Timecop.travel(future.starts_at) do
expect(described_class.current.length).to eq(2)
end
end
it 'does not clear the cache if only a future message should be displayed' do
create(:broadcast_message, :future)
expect(Rails.cache).not_to receive(:delete)
expect(described_class.current.length).to eq(0)
end
end
describe '#active?' 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