Expire the stats cache once per day
When we first started caching the stats, generating them was a process
that took several minutes, so we never expired the cache.
However, there have been cases where we run into issues where the stats
shown on the screen were outdated. That's why we introduced a task to
manually expire the cache.
But now, generating the stats only takes a few seconds, so we can
automatically expire them every day, remove all the logic needed to
manually expire them, and get rid of most of the issues related to the
cache being outdated.
We're expiring them every day because it's the same day we were doing in
public stats (which we removed in commit 631b48f58), only we're using
`expires_at:` to set the expiration time, in order to simplify the code.
Note that, in the test, we're using `travel_to(time)` so the test passes
even when it starts an instant before midnight. We aren't using
`:with_frozen_time` because, in similar cases (although not in this
case, but I'm not sure whether that's intentional), `travel_to` shows
this error:
> Calling `travel_to` with a block, when we have previously already made
> a call to `travel_to`, can lead to confusing time stubbing.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
class Budget < ApplicationRecord
|
||||
include Measurable
|
||||
include Sluggable
|
||||
include StatsVersionable
|
||||
include Reportable
|
||||
include Imageable
|
||||
|
||||
|
||||
@@ -193,6 +193,6 @@ class Budget::Stats
|
||||
stats_cache(*stats_methods)
|
||||
|
||||
def full_cache_key_for(key)
|
||||
"budgets_stats/#{budget.id}/#{phases.join}/#{key}/#{version}"
|
||||
"budgets_stats/#{budget.id}/#{phases.join}/#{key}"
|
||||
end
|
||||
end
|
||||
|
||||
@@ -149,10 +149,6 @@ module Statisticable
|
||||
(fraction * 100.0 / total).round(3)
|
||||
end
|
||||
|
||||
def version
|
||||
"v#{resource.find_or_create_stats_version.updated_at.to_i}"
|
||||
end
|
||||
|
||||
def advanced?
|
||||
resource.advanced_stats_enabled?
|
||||
end
|
||||
@@ -231,6 +227,6 @@ module Statisticable
|
||||
end
|
||||
|
||||
def stats_cache(key, &)
|
||||
Rails.cache.fetch(full_cache_key_for(key), &)
|
||||
Rails.cache.fetch(full_cache_key_for(key), expires_at: Date.current.end_of_day, &)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,11 +0,0 @@
|
||||
module StatsVersionable
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
has_one :stats_version, as: :process, inverse_of: :process
|
||||
end
|
||||
|
||||
def find_or_create_stats_version
|
||||
stats_version || create_stats_version
|
||||
end
|
||||
end
|
||||
@@ -7,7 +7,6 @@ class Poll < ApplicationRecord
|
||||
include Notifiable
|
||||
include Searchable
|
||||
include Sluggable
|
||||
include StatsVersionable
|
||||
include Reportable
|
||||
include SDG::Relatable
|
||||
|
||||
|
||||
@@ -122,6 +122,6 @@ class Poll::Stats
|
||||
stats_cache(*stats_methods)
|
||||
|
||||
def full_cache_key_for(key)
|
||||
"polls_stats/#{poll.id}/#{key}/#{version}"
|
||||
"polls_stats/#{poll.id}/#{key}"
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,5 +0,0 @@
|
||||
class StatsVersion < ApplicationRecord
|
||||
validates :process, presence: true
|
||||
|
||||
belongs_to :process, polymorphic: true
|
||||
end
|
||||
12
db/migrate/20240408154814_drop_stats_versions.rb
Normal file
12
db/migrate/20240408154814_drop_stats_versions.rb
Normal file
@@ -0,0 +1,12 @@
|
||||
class DropStatsVersions < ActiveRecord::Migration[7.0]
|
||||
def change
|
||||
drop_table :stats_versions, id: :serial do |t|
|
||||
t.string :process_type
|
||||
t.integer :process_id
|
||||
t.datetime :created_at, precision: nil, null: false
|
||||
t.datetime :updated_at, precision: nil, null: false
|
||||
|
||||
t.index ["process_type", "process_id"], name: "index_stats_versions_on_process_type_and_process_id"
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -1515,14 +1515,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_04_24_013913) do
|
||||
t.string "locale"
|
||||
end
|
||||
|
||||
create_table "stats_versions", id: :serial, force: :cascade do |t|
|
||||
t.string "process_type"
|
||||
t.integer "process_id"
|
||||
t.datetime "created_at", precision: nil, null: false
|
||||
t.datetime "updated_at", precision: nil, null: false
|
||||
t.index ["process_type", "process_id"], name: "index_stats_versions_on_process_type_and_process_id"
|
||||
end
|
||||
|
||||
create_table "taggings", id: :serial, force: :cascade do |t|
|
||||
t.integer "tag_id"
|
||||
t.string "taggable_type"
|
||||
|
||||
@@ -1,10 +0,0 @@
|
||||
namespace :stats do
|
||||
desc "Expires stats cache"
|
||||
task expire_cache: :environment do
|
||||
Tenant.run_on_each do
|
||||
[Budget, Poll].each do |model_class|
|
||||
model_class.find_each { |record| record.find_or_create_stats_version.touch }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -299,32 +299,4 @@ describe Poll::Stats do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "#version", :with_frozen_time do
|
||||
context "record with no stats" do
|
||||
it "returns a string based on the current time" do
|
||||
expect(stats.version).to eq "v#{Time.current.to_i}"
|
||||
end
|
||||
|
||||
it "doesn't overwrite the timestamp when called multiple times" do
|
||||
time = Time.current
|
||||
|
||||
expect(stats.version).to eq "v#{time.to_i}"
|
||||
|
||||
unfreeze_time
|
||||
|
||||
travel_to 2.seconds.from_now do
|
||||
expect(stats.version).to eq "v#{time.to_i}"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "record with stats" do
|
||||
before { poll.create_stats_version(updated_at: 1.day.ago) }
|
||||
|
||||
it "returns the version of the existing stats" do
|
||||
expect(stats.version).to eq "v#{1.day.ago.to_i}"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -4,6 +4,12 @@ describe Statisticable do
|
||||
before do
|
||||
dummy_stats = Class.new do
|
||||
include Statisticable
|
||||
attr_accessor :total
|
||||
stats_cache :total
|
||||
|
||||
def full_cache_key_for(key)
|
||||
"dummy_stats/#{object_id}/#{key}"
|
||||
end
|
||||
|
||||
def participants
|
||||
User.all
|
||||
@@ -221,4 +227,28 @@ describe Statisticable do
|
||||
end.each(&:join)
|
||||
end
|
||||
end
|
||||
|
||||
describe "cache" do
|
||||
it "expires the cache at the end of the day", :with_cache do
|
||||
time = Time.current
|
||||
|
||||
travel_to(time) do
|
||||
stats.total = 6
|
||||
|
||||
expect(stats.total).to eq 6
|
||||
|
||||
stats.total = 7
|
||||
|
||||
expect(stats.total).to eq 6
|
||||
end
|
||||
|
||||
travel_to(time.end_of_day) do
|
||||
expect(stats.total).to eq 6
|
||||
end
|
||||
|
||||
travel_to(time.end_of_day + 1.second) do
|
||||
expect(stats.total).to eq 7
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,13 +0,0 @@
|
||||
require "rails_helper"
|
||||
|
||||
describe StatsVersion do
|
||||
describe "validations" do
|
||||
it "is valid with a process" do
|
||||
expect(StatsVersion.new(process: Budget.new)).to be_valid
|
||||
end
|
||||
|
||||
it "is not valid without a process" do
|
||||
expect(StatsVersion.new(process: nil)).not_to be_valid
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user