summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-03-07 12:55:18 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2018-03-07 12:55:18 +0000
commit0bae567369e51c9a8590101633adf435066ac18d (patch)
tree010a32e78504fb08048ca344d42226f5a43672e8
parent001a28d04203feac77840b90024b769b442130b2 (diff)
parent9f7767079ef061e8ddce32976b1e4a5bef1822d7 (diff)
downloadgitlab-ce-0bae567369e51c9a8590101633adf435066ac18d.tar.gz
Merge branch '43460-track-projects-a-user-contributed-to' into 'master'
Keep track of projects a user interacted with Closes #43460 See merge request gitlab-org/gitlab-ce!17327
-rw-r--r--app/models/event.rb8
-rw-r--r--app/models/user_interacted_project.rb59
-rw-r--r--changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml5
-rw-r--r--db/migrate/20180223120443_create_user_interacted_projects_table.rb18
-rw-r--r--db/post_migrate/20180223124427_build_user_interacted_projects_table.rb124
-rw-r--r--db/schema.rb9
-rw-r--r--spec/models/event_spec.rb16
-rw-r--r--spec/models/user_interacted_project_spec.rb60
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