summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/user.rb10
-rw-r--r--app/models/user_activity.rb19
-rw-r--r--app/services/users/activity_service.rb6
-rw-r--r--db/post_migrate/20161128170531_drop_user_activities_table.rb23
-rw-r--r--db/schema.rb8
-rw-r--r--lib/api/helpers/internal_helpers.rb7
-rw-r--r--spec/controllers/sessions_controller_spec.rb6
-rw-r--r--spec/factories/user_activities.rb6
-rw-r--r--spec/requests/api/internal_spec.rb20
-rw-r--r--spec/services/event_create_service_spec.rb28
-rw-r--r--spec/services/users/activity_service_spec.rb34
-rw-r--r--spec/support/user_activities_helpers.rb17
12 files changed, 111 insertions, 73 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index 1dde5c89699..83e17bcb04e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -101,7 +101,6 @@ class User < ActiveRecord::Base
has_many :assigned_issues, dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue"
has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest"
- has_one :user_activity, dependent: :destroy
# Issues that a user owns are expected to be moved to the "ghost" user before
# the user is destroyed. If the user owns any issues during deletion, this
@@ -159,7 +158,6 @@ class User < ActiveRecord::Base
alias_attribute :private_token, :authentication_token
delegate :path, to: :namespace, allow_nil: true, prefix: true
- delegate :last_activity_at, to: :user_activity, allow_nil: true
state_machine :state, initial: :active do
event :block do
@@ -951,6 +949,14 @@ class User < ActiveRecord::Base
end
end
+ def record_activity
+ Gitlab::Redis.with do |redis|
+ redis.zadd('user/activities', Time.now.to_i, self.username)
+ end
+ end
+
+ private
+
def access_level=(new_level)
new_level = new_level.to_s
return unless %w(admin regular).include?(new_level)
diff --git a/app/models/user_activity.rb b/app/models/user_activity.rb
deleted file mode 100644
index c5fdbff0feb..00000000000
--- a/app/models/user_activity.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-class UserActivity < ActiveRecord::Base
- belongs_to :user, inverse_of: :user_activity
-
- validates :user, uniqueness: true, presence: true
- validates :last_activity_at, presence: true
-
- # Updated version of http://apidock.com/rails/ActiveRecord/Timestamp/touch
- # That accepts a new record.
- def touch
- current_time = current_time_from_proper_timezone
-
- if persisted?
- update_column(:last_activity_at, current_time)
- else
- self.last_activity_at = current_time
- save!(validate: false)
- end
- end
-end
diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb
index b81f947cd01..483821b7f01 100644
--- a/app/services/users/activity_service.rb
+++ b/app/services/users/activity_service.rb
@@ -14,13 +14,9 @@ module Users
private
def record_activity
- user_activity.touch
+ @author.record_activity
Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username}")
end
-
- def user_activity
- UserActivity.find_or_initialize_by(user: @author)
- end
end
end
diff --git a/db/post_migrate/20161128170531_drop_user_activities_table.rb b/db/post_migrate/20161128170531_drop_user_activities_table.rb
new file mode 100644
index 00000000000..3ece0722821
--- /dev/null
+++ b/db/post_migrate/20161128170531_drop_user_activities_table.rb
@@ -0,0 +1,23 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class DropUserActivitiesTable < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ # When using the methods "add_concurrent_index" or "add_column_with_default"
+ # you must disable the use of transactions as these methods can not run in an
+ # existing transaction. When using "add_concurrent_index" make sure that this
+ # method is the _only_ method called in the migration, any other changes
+ # should go in a separate migration. This ensures that upon failure _only_ the
+ # index creation fails and can be retried or reverted easily.
+ #
+ # To disable transactions uncomment the following line and remove these
+ # comments:
+ # disable_ddl_transaction!
+
+ def change
+ drop_table :user_activities
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3ed28a02de2..1c592dd5d6d 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1230,13 +1230,6 @@ ActiveRecord::Schema.define(version: 20170408033905) do
add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree
add_index "uploads", ["path"], name: "index_uploads_on_path", using: :btree
- create_table "user_activities", force: :cascade do |t|
- t.integer "user_id"
- t.datetime "last_activity_at", null: false
- end
-
- add_index "user_activities", ["user_id"], name: "index_user_activities_on_user_id", unique: true, using: :btree
-
create_table "user_agent_details", force: :cascade do |t|
t.string "user_agent", null: false
t.string "ip_address", null: false
@@ -1396,5 +1389,4 @@ ActiveRecord::Schema.define(version: 20170408033905) do
add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
add_foreign_key "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users"
- add_foreign_key "user_activities", "users", on_delete: :cascade
end
diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb
index 810e5063996..8f25bcf2f54 100644
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -60,6 +60,13 @@ module API
rescue JSON::ParserError
{}
end
+
+ def log_user_activity(actor)
+ commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS +
+ Gitlab::GitAccess::GIT_ANNEX_COMMANDS
+
+ ::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action])
+ end
end
end
end
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 3459f30ef42..d817099394a 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -16,7 +16,9 @@ describe SessionsController do
end
end
- context 'when using valid password' do
+ context 'when using valid password', :redis do
+ include UserActivitiesHelpers
+
let(:user) { create(:user) }
it 'authenticates user correctly' do
@@ -41,7 +43,7 @@ describe SessionsController do
it 'updates the user activity' do
expect do
post(:create, user: { login: user.username, password: user.password })
- end.to change { user.reload.last_activity_at }.from(nil)
+ end.to change { user_score }.from(0)
end
end
end
diff --git a/spec/factories/user_activities.rb b/spec/factories/user_activities.rb
deleted file mode 100644
index 32ad8c6a3b2..00000000000
--- a/spec/factories/user_activities.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-FactoryGirl.define do
- factory :user_activity do
- last_activity_at { Time.now }
- user
- end
-end
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 63f566da7a8..eee8bd51bff 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -147,7 +147,9 @@ describe API::Internal, api: true do
end
end
- describe "POST /internal/allowed" do
+ describe "POST /internal/allowed", :redis do
+ include UserActivitiesHelpers
+
context "access granted" do
before do
project.team << [user, :developer]
@@ -181,7 +183,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo)
- expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i)
+ expect(user_score).to be_zero
end
end
@@ -192,7 +194,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo)
- expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i)
+ expect(user_score).not_to be_zero
end
end
@@ -203,7 +205,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
- expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i)
+ expect(user_score).not_to be_zero
end
end
@@ -214,7 +216,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
- expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i)
+ expect(user_score).to be_zero
end
context 'project as /namespace/project' do
@@ -250,7 +252,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey
- expect(key.user.reload.last_activity_at).to be_nil
+ expect(user_score).to be_zero
end
end
@@ -260,7 +262,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey
- expect(key.user.reload.last_activity_at).to be_nil
+ expect(user_score).to be_zero
end
end
end
@@ -278,7 +280,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey
- expect(key.user.reload.last_activity_at).to be_nil
+ expect(user_score).to be_zero
end
end
@@ -288,7 +290,7 @@ describe API::Internal, api: true do
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey
- expect(key.user.reload.last_activity_at).to be_nil
+ expect(user_score).to be_zero
end
end
end
diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb
index 54e5c0b236b..13c0aac2363 100644
--- a/spec/services/event_create_service_spec.rb
+++ b/spec/services/event_create_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe EventCreateService, services: true do
+ include UserActivitiesHelpers
+
let(:service) { EventCreateService.new }
describe 'Issues' do
@@ -111,6 +113,19 @@ describe EventCreateService, services: true do
end
end
+ describe '#push', :redis do
+ let(:project) { create(:empty_project) }
+ let(:user) { create(:user) }
+
+ it 'creates a new event' do
+ expect { service.push(project, user, {}) }.to change { Event.count }
+ end
+
+ it 'updates user last activity' do
+ expect { service.push(project, user, {}) }.to change { user_score }
+ end
+ end
+
describe 'Project' do
let(:user) { create :user }
let(:project) { create(:empty_project) }
@@ -129,17 +144,4 @@ describe EventCreateService, services: true do
it { expect { subject }.to change { Event.count }.from(0).to(1) }
end
end
-
- describe '#push' do
- let(:project) { create(:empty_project) }
- let(:user) { create(:user) }
-
- it 'creates a new event' do
- expect { service.push(project, user, {}) }.to change { Event.count }
- end
-
- it 'updates user last activity' do
- expect { service.push(project, user, {}) }.to change { user.last_activity_at }
- end
- end
end
diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb
index 68399118579..07715ad4ca0 100644
--- a/spec/services/users/activity_service_spec.rb
+++ b/spec/services/users/activity_service_spec.rb
@@ -1,24 +1,40 @@
require 'spec_helper'
describe Users::ActivityService, services: true do
+ include UserActivitiesHelpers
+
let(:user) { create(:user) }
+
subject(:service) { described_class.new(user, 'type') }
- describe '#execute' do
+ describe '#execute', :redis do
context 'when last activity is nil' do
- it 'sets the last activity timestamp' do
+ before do
service.execute
+ end
- expect(user.last_activity_at).not_to be_nil
+ it 'sets the last activity timestamp for the user' do
+ expect(last_hour_members).to eq([user.username])
+ end
+
+ it 'updates the same user' do
+ service.execute
+
+ expect(last_hour_members).to eq([user.username])
+ end
+
+ it 'updates the timestamp of an existing user' do
+ Timecop.freeze(Date.tomorrow) do
+ expect { service.execute }.to change { user_score }.to(Time.now.to_i)
+ end
end
- end
- context 'when activity_at is not nil' do
- it 'updates the activity multiple times' do
- activity = create(:user_activity, user: user)
+ describe 'other user' do
+ it 'updates other user' do
+ other_user = create(:user)
+ described_class.new(other_user, 'type').execute
- Timecop.travel(activity.last_activity_at + 1.minute) do
- expect { service.execute }.to change { user.reload.last_activity_at }
+ expect(last_hour_members).to match_array([user.username, other_user.username])
end
end
end
diff --git a/spec/support/user_activities_helpers.rb b/spec/support/user_activities_helpers.rb
new file mode 100644
index 00000000000..ef88dab6c91
--- /dev/null
+++ b/spec/support/user_activities_helpers.rb
@@ -0,0 +1,17 @@
+module UserActivitiesHelpers
+ def last_hour_members
+ Gitlab::Redis.with do |redis|
+ redis.zrangebyscore(user_activities_key, 1.hour.ago.to_i, Time.now.to_i)
+ end
+ end
+
+ def user_score
+ Gitlab::Redis.with do |redis|
+ redis.zscore(user_activities_key, user.username).to_i
+ end
+ end
+
+ def user_activities_key
+ 'user/activities'
+ end
+end
View Lorry Controller Status