Delete UserActivities and related workers

10 files changed, 45 insertions, 248 deletions

diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 7c00652317b..8e25b61e2f1 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -50,8 +50,6 @@ describe SessionsController do
       end
 
       context 'when using valid password', :clean_gitlab_redis_shared_state do
-        include UserActivitiesHelpers
-
         let(:user) { create(:user) }
         let(:user_params) { { login: user.username, password: user.password } }
 
@@ -77,7 +75,7 @@ describe SessionsController do
         it 'updates the user activity' do
           expect do
             post(:create, user: user_params)
-          end.to change { user_activity(user) }
+          end.to change { user.reload.last_activity_on }.to(Date.today)
         end
       end
 
diff --git a/spec/lib/gitlab/user_activities_spec.rb b/spec/lib/gitlab/user_activities_spec.rb
deleted file mode 100644
index 6bce2ee13cf..00000000000
--- a/spec/lib/gitlab/user_activities_spec.rb
+++ /dev/null
@@ -1,127 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::UserActivities, :clean_gitlab_redis_shared_state do
-  let(:now) { Time.now }
-
-  describe '.record' do
-    context 'with no time given' do
-      it 'uses Time.now and records an activity in SharedState' do
-        Timecop.freeze do
-          now # eager-load now
-          described_class.record(42)
-        end
-
-        Gitlab::Redis::SharedState.with do |redis|
-          expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
-        end
-      end
-    end
-
-    context 'with a time given' do
-      it 'uses the given time and records an activity in SharedState' do
-        described_class.record(42, now)
-
-        Gitlab::Redis::SharedState.with do |redis|
-          expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
-        end
-      end
-    end
-  end
-
-  describe '.delete' do
-    context 'with a single key' do
-      context 'and key exists' do
-        it 'removes the pair from SharedState' do
-          described_class.record(42, now)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
-          end
-
-          subject.delete(42)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
-          end
-        end
-      end
-
-      context 'and key does not exist' do
-        it 'removes the pair from SharedState' do
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
-          end
-
-          subject.delete(42)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
-          end
-        end
-      end
-    end
-
-    context 'with multiple keys' do
-      context 'and all keys exist' do
-        it 'removes the pair from SharedState' do
-          described_class.record(41, now)
-          described_class.record(42, now)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['41', now.to_i.to_s], ['42', now.to_i.to_s]]])
-          end
-
-          subject.delete(41, 42)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
-          end
-        end
-      end
-
-      context 'and some keys does not exist' do
-        it 'removes the existing pair from SharedState' do
-          described_class.record(42, now)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
-          end
-
-          subject.delete(41, 42)
-
-          Gitlab::Redis::SharedState.with do |redis|
-            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
-          end
-        end
-      end
-    end
-  end
-
-  describe 'Enumerable' do
-    before do
-      described_class.record(40, now)
-      described_class.record(41, now)
-      described_class.record(42, now)
-    end
-
-    it 'allows to read the activities sequentially' do
-      expected = { '40' => now.to_i.to_s, '41' => now.to_i.to_s, '42' => now.to_i.to_s }
-
-      actual = described_class.new.each_with_object({}) do |(key, time), actual|
-        actual[key] = time
-      end
-
-      expect(actual).to eq(expected)
-    end
-
-    context 'with many records' do
-      before do
-        1_000.times { |i| described_class.record(i, now) }
-      end
-
-      it 'is possible to loop through all the records' do
-        expect(described_class.new.count).to eq(1_000)
-      end
-    end
-  end
-end
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index a56b913198c..a2cfa706f58 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -279,7 +279,7 @@ describe API::Internal do
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq('/')
           expect(json_response["gl_repository"]).to eq("wiki-#{project.id}")
-          expect(user).not_to have_an_activity_record
+          expect(user.reload.last_activity_on).to be_nil
         end
       end
 
@@ -291,7 +291,7 @@ describe API::Internal do
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq('/')
           expect(json_response["gl_repository"]).to eq("wiki-#{project.id}")
-          expect(user).to have_an_activity_record
+          expect(user.reload.last_activity_on).to eql(Date.today)
         end
       end
 
@@ -309,7 +309,7 @@ describe API::Internal do
           expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
           expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
           expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-          expect(user).to have_an_activity_record
+          expect(user.reload.last_activity_on).to eql(Date.today)
         end
       end
 
@@ -328,7 +328,7 @@ describe API::Internal do
             expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
             expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
             expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-            expect(user).not_to have_an_activity_record
+            expect(user.reload.last_activity_on).to be_nil
           end
         end
       end
@@ -345,7 +345,7 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user).not_to have_an_activity_record
+          expect(user.reload.last_activity_on).to be_nil
         end
       end
 
@@ -355,7 +355,7 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user).not_to have_an_activity_record
+          expect(user.reload.last_activity_on).to be_nil
         end
       end
     end
@@ -373,7 +373,7 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user).not_to have_an_activity_record
+          expect(user.reload.last_activity_on).to be_nil
         end
       end
 
@@ -383,7 +383,7 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user).not_to have_an_activity_record
+          expect(user.reload.last_activity_on).to be_nil
         end
       end
     end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index b030d9862c6..0f3e7157e14 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -5,7 +5,6 @@ describe 'Git HTTP requests' do
   include TermsHelper
   include GitHttpHelpers
   include WorkhorseHelpers
-  include UserActivitiesHelpers
 
   shared_examples 'pulls require Basic HTTP Authentication' do
     context "when no credentials are provided" do
@@ -440,10 +439,10 @@ describe 'Git HTTP requests' do
                 end
 
                 it 'updates the user last activity', :clean_gitlab_redis_shared_state do
-                  expect(user_activity(user)).to be_nil
+                  expect(user.last_activity_on).to be_nil
 
                   download(path, env) do |response|
-                    expect(user_activity(user)).to be_present
+                    expect(user.reload.last_activity_on).to eql(Date.today)
                   end
                 end
               end
diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb
index 13395a7cac3..68e310b0506 100644
--- a/spec/services/event_create_service_spec.rb
+++ b/spec/services/event_create_service_spec.rb
@@ -1,8 +1,6 @@
 require 'spec_helper'
 
 describe EventCreateService do
-  include UserActivitiesHelpers
-
   let(:service) { described_class.new }
 
   describe 'Issues' do
@@ -146,7 +144,7 @@ describe EventCreateService do
 
     it 'updates user last activity' do
       expect { service.push(project, user, push_data) }
-        .to change { user_activity(user) }
+        .to change {  user.last_activity_on }.to(Date.today)
     end
 
     it 'caches the last push event for the user' do
diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb
index 17eabad73be..f20849e6924 100644
--- a/spec/services/users/activity_service_spec.rb
+++ b/spec/services/users/activity_service_spec.rb
@@ -1,60 +1,61 @@
 require 'spec_helper'
 
 describe Users::ActivityService do
-  include UserActivitiesHelpers
+  include ExclusiveLeaseHelpers
 
-  let(:user) { create(:user) }
+  let(:user) { create(:user, last_activity_on: last_activity_on) }
 
-  subject(:service) { described_class.new(user, 'type') }
+  subject { described_class.new(user, 'type') }
 
   describe '#execute', :clean_gitlab_redis_shared_state do
     context 'when last activity is nil' do
-      before do
-        service.execute
-      end
+      let(:last_activity_on) { nil }
 
-      it 'sets the last activity timestamp for the user' do
-        expect(last_hour_user_ids).to eq([user.id])
+      it 'updates last_activity_on for the user' do
+        expect { subject.execute }
+          .to change(user, :last_activity_on).from(last_activity_on).to(Date.today)
       end
+    end
 
-      it 'updates the same user' do
-        service.execute
+    context 'when last activity is in the past' do
+      let(:last_activity_on) { Date.today - 1.week }
 
-        expect(last_hour_user_ids).to eq([user.id])
-      end
-
-      it 'updates the timestamp of an existing user' do
-        Timecop.freeze(Date.tomorrow) do
-          expect { service.execute }.to change { user_activity(user) }.to(Time.now.to_i.to_s)
-        end
+      it 'updates last_activity_on for the user' do
+        expect { subject.execute }
+          .to change(user, :last_activity_on)
+                .from(last_activity_on)
+                .to(Date.today)
       end
+    end
 
-      describe 'other user' do
-        it 'updates other user' do
-          other_user = create(:user)
-          described_class.new(other_user, 'type').execute
+    context 'when last activity is today' do
+      let(:last_activity_on) { Date.today }
 
-          expect(last_hour_user_ids).to match_array([user.id, other_user.id])
-        end
+      it 'does not update last_activity_on' do
+        expect { subject.execute }.not_to change(user, :last_activity_on)
       end
     end
 
     context 'when in GitLab read-only instance' do
+      let(:last_activity_on) { nil }
+
       before do
         allow(Gitlab::Database).to receive(:read_only?).and_return(true)
       end
 
-      it 'does not update last_activity_at' do
-        service.execute
-
-        expect(last_hour_user_ids).to eq([])
+      it 'does not update last_activity_on' do
+        expect { subject.execute }.not_to change(user, :last_activity_on)
       end
     end
-  end
 
-  def last_hour_user_ids
-    Gitlab::UserActivities.new
-      .select { |k, v| v >= 1.hour.ago.to_i.to_s }
-      .map { |k, _| k.to_i }
+    context 'when a lease could not be obtained' do
+      let(:last_activity_on) { nil }
+
+      it 'does not update last_activity_on' do
+        stub_exclusive_lease_taken("acitvity_service:#{user.id}", timeout: 1.minute.to_i)
+
+        expect { subject.execute }.not_to change(user, :last_activity_on)
+      end
+    end
   end
 end
diff --git a/spec/support/helpers/user_activities_helpers.rb b/spec/support/helpers/user_activities_helpers.rb
deleted file mode 100644
index 44feb104644..00000000000
--- a/spec/support/helpers/user_activities_helpers.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-module UserActivitiesHelpers
-  def user_activity(user)
-    Gitlab::UserActivities.new
-      .find { |k, _| k == user.id.to_s }&.
-      second
-  end
-end
diff --git a/spec/support/matchers/user_activity_matchers.rb b/spec/support/matchers/user_activity_matchers.rb
deleted file mode 100644
index ce3b683b6d2..00000000000
--- a/spec/support/matchers/user_activity_matchers.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-RSpec::Matchers.define :have_an_activity_record do |expected|
-  match do |user|
-    expect(Gitlab::UserActivities.new.find { |k, _| k == user.id.to_s }).to be_present
-  end
-end
diff --git a/spec/workers/schedule_update_user_activity_worker_spec.rb b/spec/workers/schedule_update_user_activity_worker_spec.rb
deleted file mode 100644
index 32c59381b01..00000000000
--- a/spec/workers/schedule_update_user_activity_worker_spec.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-require 'spec_helper'
-
-describe ScheduleUpdateUserActivityWorker, :clean_gitlab_redis_shared_state do
-  let(:now) { Time.now }
-
-  before do
-    Gitlab::UserActivities.record('1', now)
-    Gitlab::UserActivities.record('2', now)
-  end
-
-  it 'schedules UpdateUserActivityWorker once' do
-    expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s, '2' => now.to_i.to_s })
-
-    subject.perform
-  end
-
-  context 'when specifying a batch size' do
-    it 'schedules UpdateUserActivityWorker twice' do
-      expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s })
-      expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '2' => now.to_i.to_s })
-
-      subject.perform(1)
-    end
-  end
-end
diff --git a/spec/workers/update_user_activity_worker_spec.rb b/spec/workers/update_user_activity_worker_spec.rb
deleted file mode 100644
index 268ca1d81f2..00000000000
--- a/spec/workers/update_user_activity_worker_spec.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-require 'spec_helper'
-
-describe UpdateUserActivityWorker, :clean_gitlab_redis_shared_state do
-  let(:user_active_2_days_ago) { create(:user, current_sign_in_at: 10.months.ago) }
-  let(:user_active_yesterday_1) { create(:user) }
-  let(:user_active_yesterday_2) { create(:user) }
-  let(:user_active_today) { create(:user) }
-  let(:data) do
-    {
-      user_active_2_days_ago.id.to_s => 2.days.ago.at_midday.to_i.to_s,
-      user_active_yesterday_1.id.to_s => 1.day.ago.at_midday.to_i.to_s,
-      user_active_yesterday_2.id.to_s => 1.day.ago.at_midday.to_i.to_s,
-      user_active_today.id.to_s => Time.now.to_i.to_s
-    }
-  end
-
-  it 'updates users.last_activity_on' do
-    subject.perform(data)
-
-    aggregate_failures do
-      expect(user_active_2_days_ago.reload.last_activity_on).to eq(2.days.ago.to_date)
-      expect(user_active_yesterday_1.reload.last_activity_on).to eq(1.day.ago.to_date)
-      expect(user_active_yesterday_2.reload.last_activity_on).to eq(1.day.ago.to_date)
-      expect(user_active_today.reload.reload.last_activity_on).to eq(Date.today)
-    end
-  end
-
-  it 'deletes the pairs from SharedState' do
-    data.each { |id, time| Gitlab::UserActivities.record(id, time) }
-
-    subject.perform(data)
-
-    expect(Gitlab::UserActivities.new.to_a).to be_empty
-  end
-end