diff options
| -rw-r--r-- | app/models/event.rb | 8 | ||||
| -rw-r--r-- | app/models/user_interacted_project.rb | 59 | ||||
| -rw-r--r-- | changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml | 5 | ||||
| -rw-r--r-- | db/migrate/20180223120443_create_user_interacted_projects_table.rb | 18 | ||||
| -rw-r--r-- | db/post_migrate/20180223124427_build_user_interacted_projects_table.rb | 124 | ||||
| -rw-r--r-- | db/schema.rb | 9 | ||||
| -rw-r--r-- | spec/models/event_spec.rb | 16 | ||||
| -rw-r--r-- | spec/models/user_interacted_project_spec.rb | 60 |
8 files changed, 299 insertions, 0 deletions
diff --git a/app/models/event.rb b/app/models/event.rb index be0fc7efa9a..17a198d52c7 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -65,6 +65,7 @@ class Event < ActiveRecord::Base # Callbacks after_create :reset_project_activity after_create :set_last_repository_updated_at, if: :push? + after_create :track_user_interacted_projects # Scopes scope :recent, -> { reorder(id: :desc) } @@ -389,4 +390,11 @@ class Event < ActiveRecord::Base Project.unscoped.where(id: project_id) .update_all(last_repository_updated_at: created_at) end + + def track_user_interacted_projects + # 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). + 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..dd55a6acb79 --- /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_id, presence: true + validates :user_id, 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_id + + attributes = { + project_id: event.project_id, + user_id: event.author_id + } + + cached_exists?(attributes) do + transaction(requires_new: true) do + begin + where(attributes).select(1).first || create!(attributes) + true # not caching the whole record here for now + rescue ActiveRecord::RecordNotUnique + # Note, above queries are not atomic and prone + # to race conditions (similar like #find_or_create!). + # In the case where we hit this, the record we want + # already exists - shortcut and return. + true + 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 + + # 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/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml b/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml new file mode 100644 index 00000000000..99b6ac76a3e --- /dev/null +++ b/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml @@ -0,0 +1,5 @@ +--- +title: Keep track of projects a user interacted with. +merge_request: 17327 +author: +type: other diff --git a/db/migrate/20180223120443_create_user_interacted_projects_table.rb b/db/migrate/20180223120443_create_user_interacted_projects_table.rb new file mode 100644 index 00000000000..20749940b1e --- /dev/null +++ b/db/migrate/20180223120443_create_user_interacted_projects_table.rb @@ -0,0 +1,18 @@ +class CreateUserInteractedProjectsTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + create_table :user_interacted_projects, id: false do |t| + t.references :user, null: false + t.references :project, null: false + end + end + + def down + drop_table :user_interacted_projects + end +end diff --git a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb new file mode 100644 index 00000000000..5e729b1aa53 --- /dev/null +++ b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb @@ -0,0 +1,124 @@ +class BuildUserInteractedProjectsTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + if Gitlab::Database.postgresql? + PostgresStrategy.new + else + MysqlStrategy.new + end.up + + unless index_exists?(:user_interacted_projects, [:project_id, :user_id]) + add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true + end + + unless foreign_key_exists?(:user_interacted_projects, :user_id) + add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade + end + + unless foreign_key_exists?(:user_interacted_projects, :project_id) + add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade + end + end + + def down + execute "TRUNCATE user_interacted_projects" + + if foreign_key_exists?(:user_interacted_projects, :user_id) + remove_foreign_key :user_interacted_projects, :users + end + + if foreign_key_exists?(:user_interacted_projects, :project_id) + remove_foreign_key :user_interacted_projects, :projects + end + + if index_exists_by_name?(:user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id') + remove_concurrent_index_by_name :user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id' + end + end + + private + + # Rails' index_exists? doesn't work when you only give it a table and index + # name. As such we have to use some extra code to check if an index exists for + # a given name. + def index_exists_by_name?(table, index) + indexes_for_table[table].include?(index) + end + + def indexes_for_table + @indexes_for_table ||= Hash.new do |hash, table_name| + hash[table_name] = indexes(table_name).map(&:name) + end + end + + def foreign_key_exists?(table, column) + foreign_keys(table).any? do |key| + key.options[:column] == column.to_s + end + end + + class PostgresStrategy < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + BATCH_SIZE = 100_000 + SLEEP_TIME = 5 + + def up + with_index(:events, [:author_id, :project_id], name: 'events_user_interactions_temp', where: 'project_id IS NOT NULL') do + iteration = 0 + records = 0 + begin + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}" + result = execute <<~SQL + INSERT INTO user_interacted_projects (user_id, project_id) + SELECT e.user_id, e.project_id + FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e + LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) + WHERE ucp.user_id IS NULL + LIMIT #{BATCH_SIZE} + SQL + iteration += 1 + records += result.cmd_tuples + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall" + Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0 + rescue ActiveRecord::InvalidForeignKey => e + Rails.logger.info "Retry on InvalidForeignKey: #{e}" + retry + end while result.cmd_tuples > 0 + end + + execute "ANALYZE user_interacted_projects" + + end + + private + + def with_index(*args) + add_concurrent_index(*args) unless index_exists?(*args) + yield + ensure + remove_concurrent_index(*args) if index_exists?(*args) + end + end + + class MysqlStrategy < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def up + execute <<~SQL + INSERT INTO user_interacted_projects (user_id, project_id) + SELECT e.user_id, e.project_id + FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e + LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) + WHERE ucp.user_id IS NULL + SQL + end + end + +end diff --git a/db/schema.rb b/db/schema.rb index d49bf022d0b..387b15f8f30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1843,6 +1843,13 @@ ActiveRecord::Schema.define(version: 20180307012445) do add_index "user_custom_attributes", ["key", "value"], name: "index_user_custom_attributes_on_key_and_value", using: :btree add_index "user_custom_attributes", ["user_id", "key"], name: "index_user_custom_attributes_on_user_id_and_key", unique: true, using: :btree + create_table "user_interacted_projects", id: false, force: :cascade do |t| + t.integer "user_id", null: false + t.integer "project_id", null: false + end + + add_index "user_interacted_projects", ["project_id", "user_id"], name: "index_user_interacted_projects_on_project_id_and_user_id", unique: true, using: :btree + create_table "user_synced_attributes_metadata", force: :cascade do |t| t.boolean "name_synced", default: false t.boolean "email_synced", default: false @@ -2115,6 +2122,8 @@ ActiveRecord::Schema.define(version: 20180307012445) do add_foreign_key "u2f_registrations", "users" add_foreign_key "user_callouts", "users", on_delete: :cascade add_foreign_key "user_custom_attributes", "users", on_delete: :cascade + add_foreign_key "user_interacted_projects", "projects", name: "fk_722ceba4f7", on_delete: :cascade + add_foreign_key "user_interacted_projects", "users", name: "fk_0894651f08", on_delete: :cascade add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 67f49348acb..8ea92410022 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -49,6 +49,22 @@ describe Event do end end end + + describe 'after_create :track_user_interacted_projects' do + let(:event) { build(:push_event, project: project, author: project.owner) } + + 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 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 end describe "Push event" do diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb new file mode 100644 index 00000000000..cb4bb3372d4 --- /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_id) } + it { is_expected.to validate_presence_of(:user_id) } +end |