diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-08-22 11:45:51 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-08-22 11:45:51 +0000 |
commit | e6f20e52f6f77f77942c047dca91d3e325cce8b2 (patch) | |
tree | e28a101bcd091c34ed896244bb4e6e16c53cacd0 | |
parent | dd0681d772e49933b74a06812d11fff6afad81ad (diff) | |
parent | e0b589f16166a88d18e58fbc154e0cf165732acb (diff) | |
download | gitlab-ce-e6f20e52f6f77f77942c047dca91d3e325cce8b2.tar.gz |
Merge branch 'fix-broadcast-message-caching' into 'master'
Fix caching of future broadcast messages
Closes #36661
See merge request !13667
-rw-r--r-- | app/models/broadcast_message.rb | 32 | ||||
-rw-r--r-- | changelogs/unreleased/fix-broadcast-message-caching.yml | 5 | ||||
-rw-r--r-- | spec/models/broadcast_message_spec.rb | 23 |
3 files changed, 55 insertions, 5 deletions
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb index 3692bcc680d..fdc5a2adea0 100644 --- a/app/models/broadcast_message.rb +++ b/app/models/broadcast_message.rb @@ -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 - end + 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 diff --git a/changelogs/unreleased/fix-broadcast-message-caching.yml b/changelogs/unreleased/fix-broadcast-message-caching.yml new file mode 100644 index 00000000000..58ec1766cfd --- /dev/null +++ b/changelogs/unreleased/fix-broadcast-message-caching.yml @@ -0,0 +1,5 @@ +--- +title: Fix caching of future broadcast messages +merge_request: +author: +type: fixed diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 3369aef1d3e..461e754dc1f 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -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 |