From 11a559f7c9ce21b040c3a9c0544169f4885b9fa6 Mon Sep 17 00:00:00 2001 From: Andreas Brandl Date: Sat, 3 Mar 2018 20:00:05 +0100 Subject: Singularize model name. --- app/models/event.rb | 2 +- app/models/user_interacted_project.rb | 59 +++++++++++++++++++++++++++ app/models/user_interacted_projects.rb | 59 --------------------------- spec/models/event_spec.rb | 12 +++--- spec/models/user_interacted_project_spec.rb | 60 ++++++++++++++++++++++++++++ spec/models/user_interacted_projects_spec.rb | 60 ---------------------------- 6 files changed, 126 insertions(+), 126 deletions(-) create mode 100644 app/models/user_interacted_project.rb delete mode 100644 app/models/user_interacted_projects.rb create mode 100644 spec/models/user_interacted_project_spec.rb delete mode 100644 spec/models/user_interacted_projects_spec.rb diff --git a/app/models/event.rb b/app/models/event.rb index 166c3f1b4b1..86a14044bc2 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -395,6 +395,6 @@ class Event < ActiveRecord::Base # 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). - UserInteractedProjects.track(self) if UserInteractedProjects.available? + UserInteractedProject.track(self) if UserInteractedProject.available? end end diff --git a/app/models/user_interacted_project.rb b/app/models/user_interacted_project.rb new file mode 100644 index 00000000000..5e7f6d5eeda --- /dev/null +++ b/app/models/user_interacted_project.rb @@ -0,0 +1,59 @@ +class UserInteractedProject < ActiveRecord::Base + belongs_to :user + belongs_to :project + + validates :project, presence: true + validates :user, presence: true + + CACHE_EXPIRY_TIME = 1.day + + # Schema version required for this model + REQUIRED_SCHEMA_VERSION = 20180223120443 + + class << self + def track(event) + # For events without a project, we simply don't care. + # An example of this is the creation of a snippet (which + # is not related to any project). + return unless event.project + + # This is a precaution because the cache lookup + # will work just fine without an author. + # + # However, this should never happen (tm). + raise 'event#author not present unexpectedly' unless event.author + + attributes = { + project_id: event.project_id, + user_id: event.author_id + } + + cached_exists?(attributes) do + begin + find_or_create_by!(attributes) + true # not caching the whole record here for now + rescue ActiveRecord::RecordNotUnique + retry + 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 + + # Flushes cached information about schema + def reset_column_information + @available_flag = nil + super + end + + private + + def cached_exists?(project_id:, user_id:, &block) + cache_key = "user_interacted_projects:#{project_id}:#{user_id}" + Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRY_TIME, &block) + end + end +end diff --git a/app/models/user_interacted_projects.rb b/app/models/user_interacted_projects.rb deleted file mode 100644 index ba6ed2bd2ff..00000000000 --- a/app/models/user_interacted_projects.rb +++ /dev/null @@ -1,59 +0,0 @@ -class UserInteractedProjects < ActiveRecord::Base - belongs_to :user - belongs_to :project - - validates :project, presence: true - validates :user, presence: true - - CACHE_EXPIRY_TIME = 1.day - - # Schema version required for this model - REQUIRED_SCHEMA_VERSION = 20180223120443 - - class << self - def track(event) - # For events without a project, we simply don't care. - # An example of this is the creation of a snippet (which - # is not related to any project). - return unless event.project - - # This is a precaution because the cache lookup - # will work just fine without an author. - # - # However, this should never happen (tm). - raise 'event#author not present unexpectedly' unless event.author - - attributes = { - project_id: event.project_id, - user_id: event.author_id - } - - cached_exists?(attributes) do - begin - find_or_create_by!(attributes) - true # not caching the whole record here for now - rescue ActiveRecord::RecordNotUnique - retry - 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 - - # Flushes cached information about schema - def reset_column_information - @available_flag = nil - super - end - - private - - def cached_exists?(project_id:, user_id:, &block) - cache_key = "user_interacted_projects:#{project_id}:#{user_id}" - Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRY_TIME, &block) - end - end -end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 780eacbb8dc..8ea92410022 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -53,15 +53,15 @@ describe Event do describe 'after_create :track_user_interacted_projects' do let(:event) { build(:push_event, project: project, author: project.owner) } - it 'passes event to UserInteractedProjects.track' do - expect(UserInteractedProjects).to receive(:available?).and_return(true) - expect(UserInteractedProjects).to receive(:track).with(event) + it 'passes event to UserInteractedProject.track' do + expect(UserInteractedProject).to receive(:available?).and_return(true) + expect(UserInteractedProject).to receive(:track).with(event) event.save end - it 'does not call UserInteractedProjects.track if its not yet available' do - expect(UserInteractedProjects).to receive(:available?).and_return(false) - expect(UserInteractedProjects).not_to receive(:track) + it 'does not call UserInteractedProject.track if its not yet available' do + expect(UserInteractedProject).to receive(:available?).and_return(false) + expect(UserInteractedProject).not_to receive(:track) event.save end end diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb new file mode 100644 index 00000000000..1428fc2d433 --- /dev/null +++ b/spec/models/user_interacted_project_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe UserInteractedProject do + describe '.track' do + subject { described_class.track(event) } + let(:event) { build(:event) } + + Event::ACTIONS.each do |action| + context "for all actions (event types)" do + let(:event) { build(:event, action: action) } + it 'creates a record' do + expect { subject }.to change { described_class.count }.from(0).to(1) + end + end + end + + it 'sets project accordingly' do + subject + expect(described_class.first.project).to eq(event.project) + end + + it 'sets user accordingly' do + subject + expect(described_class.first.user).to eq(event.author) + end + + it 'only creates a record once per user/project' do + expect do + subject + described_class.track(event) + end.to change { described_class.count }.from(0).to(1) + end + + describe 'with an event without a project' do + let(:event) { build(:event, project: nil) } + + it 'ignores the event' do + expect { subject }.not_to change { described_class.count } + 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(:user) } +end diff --git a/spec/models/user_interacted_projects_spec.rb b/spec/models/user_interacted_projects_spec.rb deleted file mode 100644 index 503d89e7f86..00000000000 --- a/spec/models/user_interacted_projects_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -require 'spec_helper' - -describe UserInteractedProjects do - describe '.track' do - subject { described_class.track(event) } - let(:event) { build(:event) } - - Event::ACTIONS.each do |action| - context "for all actions (event types)" do - let(:event) { build(:event, action: action) } - it 'creates a record' do - expect { subject }.to change { described_class.count }.from(0).to(1) - end - end - end - - it 'sets project accordingly' do - subject - expect(described_class.first.project).to eq(event.project) - end - - it 'sets user accordingly' do - subject - expect(described_class.first.user).to eq(event.author) - end - - it 'only creates a record once per user/project' do - expect do - subject - described_class.track(event) - end.to change { described_class.count }.from(0).to(1) - end - - describe 'with an event without a project' do - let(:event) { build(:event, project: nil) } - - it 'ignores the event' do - expect { subject }.not_to change { described_class.count } - 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(:user) } -end -- cgit v1.2.1