Makes a private method public, and adds tests

This method (or at least the logic of it) is used in a rake task. The
rake task is written to re-use the
`ActiveSession.cleaned_up_lookup_entries` method, and tests are added to
specify the behavior.

app/models/active_session.rb

@@ -274,7 +274,9 @@ class ActiveSession
   # Cleans up the lookup set by removing any session IDs that are no longer present.
   #
   # Returns an array of marshalled ActiveModel objects that are still active.
-  private_class_method def self.cleaned_up_lookup_entries(redis, user)
+  # Records removed keys in the optional `removed` argument array.
+  def self.cleaned_up_lookup_entries(redis, user, removed = [])
+    lookup_key = lookup_key_name(user.id)
     session_ids = session_ids_for_user(user.id)
     session_ids_and_entries = raw_active_session_entries(redis, session_ids, user.id)
 
@@ -283,7 +285,10 @@ class ActiveSession
     # lookup entries in the set need to be removed manually.
     redis.pipelined do |pipeline|
       session_ids_and_entries.each do |session_id, entry|
-        pipeline.srem(lookup_key_name(user.id), session_id) unless entry
+        next if entry
+
+        pipeline.srem(lookup_key, session_id)
+        removed << session_id
       end
     end
 

lib/tasks/gitlab/cleanup.rake

@@ -121,26 +121,16 @@ namespace :gitlab do
               last_save_check = Time.now
             end
 
+            user = Struct.new(:id)
+
             keys.each do |key|
               user_id = key.split(':').last
 
-              lookup_key_count = redis.scard(key)
-
-              session_ids = ActiveSession.session_ids_for_user(user_id)
-              session_ids_and_entries = ActiveSession.raw_active_session_entries(redis, session_ids, user_id)
-
-              inactive_session_ids = session_ids_and_entries.map do |session_id, session|
-                session_id if session.nil?
-              end.compact
-
-              redis.pipelined do |conn|
-                inactive_session_ids.each do |session_id|
-                  conn.srem(key, session_id)
-                end
-              end
+              removed = []
+              active = ActiveSession.cleaned_up_lookup_entries(redis, user.new(user_id), removed)
 
-              if inactive_session_ids
-                puts "deleted #{inactive_session_ids.count} out of #{lookup_key_count} lookup keys for User ##{user_id}"
+              if removed.any?
+                puts "deleted #{removed.count} out of #{active.count + removed.count} lookup keys for User ##{user_id}"
               end
             end
           end while cursor.to_i != 0

spec/models/active_session_spec.rb

@@ -569,4 +569,86 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do
       it_behaves_like 'cleaning up'
     end
   end
+
+  describe '.cleaned_up_lookup_entries' do
+    before do
+      stub_const("ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS", 5)
+    end
+
+    shared_examples 'cleaning up lookup entries' do
+      let(:current_session_id) { '6919a6f1bb119dd7396fadc38fd18d0d' }
+      let(:active_count) { 3 }
+
+      before do
+        Gitlab::Redis::SharedState.with do |redis|
+          active_count.times do |number|
+            redis.set(
+              key_name(user.id, number),
+              dump_session(ActiveSession.new(session_id: number.to_s, updated_at: number.days.ago))
+            )
+
+            redis.sadd(lookup_key, number.to_s)
+          end
+
+          redis.sadd(lookup_key, (active_count + 1).to_s)
+          redis.sadd(lookup_key, (active_count + 2).to_s)
+        end
+      end
+
+      it 'removes obsolete lookup entries' do
+        active = Gitlab::Redis::SharedState.with do |redis|
+          ActiveSession.cleaned_up_lookup_entries(redis, user)
+        end
+
+        expect(active.count).to eq(active_count)
+
+        Gitlab::Redis::SharedState.with do |redis|
+          lookup_entries = redis.smembers(lookup_key)
+
+          expect(lookup_entries.count).to eq(active_count)
+          expect(lookup_entries).not_to include(
+            (active_count + 1).to_s,
+            (active_count + 2).to_s
+          )
+        end
+      end
+
+      it 'reports the removed entries' do
+        removed = []
+        Gitlab::Redis::SharedState.with do |redis|
+          ActiveSession.cleaned_up_lookup_entries(redis, user, removed)
+        end
+
+        expect(removed.count).to eq(2)
+      end
+    end
+
+    context 'with legacy sessions' do
+      let(:session_key) { described_class.key_name_v1(user.id, current_session_id) }
+
+      def key_name(user_id, session_id)
+        described_class.key_name_v1(user_id, session_id)
+      end
+
+      def dump_session(session)
+        Marshal.dump(session)
+      end
+
+      it_behaves_like 'cleaning up lookup entries'
+    end
+
+    context 'with new sessions' do
+      let(:session_key) { described_class.key_name(user.id, current_session_id) }
+
+      def key_name(user_id, session_id)
+        described_class.key_name(user_id, session_id)
+      end
+
+      def dump_session(session)
+        session.dump
+      end
+
+      it_behaves_like 'cleaning up lookup entries'
+    end
+  end
 end