Commit 375a5c9f authored by Andreas Brandl's avatar Andreas Brandl

Only track contributions if table is available.

This is due to the problem that the callback can be called while running
an earlier database schema version (for example during earlier
migrations). We work around this by checking the current schema version
and only track contributions if the table is available.
parent 8dde0301
...@@ -392,6 +392,9 @@ class Event < ActiveRecord::Base ...@@ -392,6 +392,9 @@ class Event < ActiveRecord::Base
end end
def track_user_contributed_projects def track_user_contributed_projects
UserContributedProjects.track(self) # Note the call to .available? is due to earlier migrations
# that would otherwise conflict with the call to .track
# (because the table does not exist yet).
UserContributedProjects.track(self) if UserContributedProjects.available?
end end
end end
...@@ -7,6 +7,9 @@ class UserContributedProjects < ActiveRecord::Base ...@@ -7,6 +7,9 @@ class UserContributedProjects < ActiveRecord::Base
CACHE_EXPIRY_TIME = 1.day CACHE_EXPIRY_TIME = 1.day
# Schema version required for this model
REQUIRED_SCHEMA_VERSION = 20180223120443
class << self class << self
def track(event) def track(event)
# For events without a project, we simply don't care. # For events without a project, we simply don't care.
...@@ -35,6 +38,11 @@ class UserContributedProjects < ActiveRecord::Base ...@@ -35,6 +38,11 @@ class UserContributedProjects < ActiveRecord::Base
end end
end end
# Check if we can safely call .track (table exists)
def available?
@available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION # rubocop:disable Gitlab/PredicateMemoization
end
private private
def cached_exists?(project_id:, user_id:, &block) def cached_exists?(project_id:, user_id:, &block)
......
...@@ -51,11 +51,19 @@ describe Event do ...@@ -51,11 +51,19 @@ describe Event do
end end
describe 'after_create :track_user_contributed_projects' do describe 'after_create :track_user_contributed_projects' do
let(:event) { build(:push_event, project: project, author: project.owner) }
it 'passes event to UserContributedProjects.track' do it 'passes event to UserContributedProjects.track' do
event = build(:push_event, project: project, author: project.owner) expect(UserContributedProjects).to receive(:available?).and_return(true)
expect(UserContributedProjects).to receive(:track).with(event) expect(UserContributedProjects).to receive(:track).with(event)
event.save event.save
end end
it 'does not call UserContributedProjects.track if its not yet available' do
expect(UserContributedProjects).to receive(:available?).and_return(false)
expect(UserContributedProjects).not_to receive(:track)
event.save
end
end end
end end
......
...@@ -40,6 +40,21 @@ describe UserContributedProjects do ...@@ -40,6 +40,21 @@ describe UserContributedProjects do
end end
end end
describe '.available?' do
before do
described_class.instance_variable_set('@available_flag', nil)
end
it 'checks schema version and properly caches positive result' do
expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION - 1 - rand(1000))
expect(described_class.available?).to be_falsey
expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION + rand(1000))
expect(described_class.available?).to be_truthy
expect(ActiveRecord::Migrator).not_to receive(:current_version)
expect(described_class.available?).to be_truthy # cached response
end
end
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:user) }
end end
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