[WIP] Use separate memcahed key for session ttl

app/controllers/application_controller.rb

@@ -1612,7 +1612,8 @@ def set_global_session_data
     session[:miq_compare] = @compare.nil? ? (@keep_compare ? session[:miq_compare] : nil) : Marshal.dump(@compare)
     session[:miq_compressed] = @compressed unless @compressed.nil?
     session[:miq_exists_mode] = @exists_mode unless @exists_mode.nil?
-    session[:last_trans_time] = Time.now
+    # Update user activity time in separate memcache key
+    SessionActivityService.update_last_transaction_time(current_user.id) if current_user
 
     # Set timelines hash, if it is in the session for the running controller
     set_tl_session_data

app/services/privilege_checker_service.rb

@@ -1,10 +1,10 @@
 class PrivilegeCheckerService
   def valid_session?(session, current_user)
-    user_signed_in?(current_user) && session_active?(session) && server_ready?(current_user)
+    user_signed_in?(current_user) && session_active?(current_user.id) && server_ready?(current_user)
   end
 
   def user_session_timed_out?(session, current_user)
-    user_signed_in?(current_user) && !session_active?(session)
+    user_signed_in?(current_user) && !session_active?(current_user.id)
   end
 
   private
@@ -13,8 +13,9 @@ def user_signed_in?(current_user)
     !!current_user
   end
 
-  def session_active?(session)
-    Time.current - (session[:last_trans_time] || Time.current) <= ::Settings.session.timeout
+  def session_active?(user_id)
+    return false unless user_id
+    SessionActivityService.session_active?(user_id)
   end
 
   def server_ready?(current_user)

app/services/session_activity_service.rb

@@ -0,0 +1,78 @@
+class SessionActivityService
+  # Key prefix for storing session activity data in memcache
+  SESSION_ACTIVITY_PREFIX = "user_activity:".freeze
+
+  # Class variables to cache the Dalli client instance
+  @@dalli_client = nil
+  @@dalli_client_mutex = Mutex.new
+
+  # Get the last transaction time for a user
+  # @param user_id [String, Integer] The user ID
+  # @return [Time, nil] The last transaction time or nil if not found
+  def self.get_last_transaction_time(user_id)
+    return nil unless user_id
+
+    value = dalli_client.get(activity_key(user_id))
+    value ? Time.parse(value) : nil
+  end
+
+  # Update the last transaction time for a user
+  # @param user_id [String, Integer] The user ID
+  # @param time [Time] The time to set (defaults to current time)
+  def self.update_last_transaction_time(user_id, time = Time.current)
+    return unless user_id
+
+    dalli_client.set(activity_key(user_id), time.iso8601, ttl)
+  end
+
+  # Check if a user session is active based on last transaction time
+  # @param user_id [String, Integer] The user ID
+  # @return [Boolean] True if the session is active, false otherwise
+  def self.session_active?(user_id)
+    last_time = get_last_transaction_time(user_id)
+    return false unless last_time
+
+    Time.current - last_time <= ::Settings.session.timeout
+  end
+
+  # Generate the key for storing user activity data
+  # @param user_id [String, Integer] The user ID
+  # @return [String] The key for memcache
+  def self.activity_key(user_id)
+    "#{SESSION_ACTIVITY_PREFIX}#{user_id}"
+  end
+
+  # Get a cached Dalli client for interacting with memcache
+  # Thread-safe implementation using a mutex to ensure only one client is created
+  # @return [Dalli::Client] A configured Dalli client
+  def self.dalli_client
+    # Return the cached client if it exists
+    return @@dalli_client if @@dalli_client
+
+    # Use a mutex to ensure thread safety when creating the client
+    @@dalli_client_mutex.synchronize do
+      # Check again inside the mutex in case another thread created the client
+      return @@dalli_client if @@dalli_client
+
+      require 'dalli'
+      options = {
+        :namespace => "MIQ:USER_ACTIVITY",
+        :expires_in => ttl
+      }
+
+      @@dalli_client = MiqMemcached.client(options)
+    end
+
+    @@dalli_client
+  end
+
+  # Get the TTL for user activity data
+  # @return [Integer] The TTL in seconds
+  def self.ttl
+    # Set TTL to double the session timeout to ensure we keep the data
+    # long enough for proper timeout detection
+    (::Settings.session.timeout * 2).to_i
+  end
+
+  private_class_method :activity_key, :dalli_client, :ttl
+end

spec/services/privilege_checker_service_spec.rb

@@ -1,93 +1,74 @@
 describe PrivilegeCheckerService do
-  let(:privilege_checker) { described_class.new }
+  let(:service) { described_class.new }
+  let(:user) { double("User", :id => 123, :super_admin_user? => false) }
+  let(:session) { {} }
+  let(:server) { double("MiqServer", :logon_status => :ready) }
 
-  describe "#valid_session?" do
-    shared_examples_for "PrivilegeCheckerService#valid_session? that returns false" do
-      it "returns false" do
-        expect(privilege_checker.valid_session?(session, user)).to be_falsey
-      end
-    end
+  before do
+    allow(MiqServer).to receive(:my_server).and_return(server)
+  end
 
-    let(:session) do
-      {
-        :last_trans_time => last_trans_time
-      }
-    end
+  describe '#valid_session?' do
+    context 'when user is signed in' do
+      context 'when session is active' do
+        before do
+          allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(true)
+        end
 
-    context "when the user is signed out" do
-      let(:user) { nil }
-      let(:last_trans_time) { nil }
+        it 'returns true when server is ready' do
+          expect(service.valid_session?(session, user)).to be true
+        end
 
-      it_behaves_like "PrivilegeCheckerService#valid_session? that returns false"
-    end
+        it 'returns true when user is super admin' do
+          allow(user).to receive(:super_admin_user?).and_return(true)
+          allow(server).to receive(:logon_status).and_return(:not_ready)
 
-    context "when the user is signed in" do
-      let(:user) { FactoryBot.create(:user) }
+          expect(service.valid_session?(session, user)).to be true
+        end
 
-      context "when the session is timed out" do
-        let(:last_trans_time) { 2.hours.ago }
+        it 'returns false when server is not ready' do
+          allow(server).to receive(:logon_status).and_return(:not_ready)
 
-        it_behaves_like "PrivilegeCheckerService#valid_session? that returns false"
+          expect(service.valid_session?(session, user)).to be false
+        end
       end
 
-      context "when the session has not timed out" do
-        let(:last_trans_time) { Time.current }
-        let(:server) { double("MiqServer", :logon_status => logon_status) }
-
+      context 'when session is not active' do
         before do
-          allow(MiqServer).to receive(:my_server).and_return(server)
+          allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(false)
         end
 
-        context "when the server is not ready" do
-          let(:logon_status) { :not_ready }
-
-          it_behaves_like "PrivilegeCheckerService#valid_session? that returns false"
-        end
-
-        context "when the server is ready" do
-          let(:logon_status) { :ready }
-
-          it "returns true" do
-            expect(privilege_checker.valid_session?(session, user)).to be_truthy
-          end
+        it 'returns false' do
+          expect(service.valid_session?(session, user)).to be false
         end
       end
     end
-  end
 
-  describe "#user_session_timed_out?" do
-    let(:session) do
-      {
-        :last_trans_time => last_trans_time
-      }
+    context 'when user is not signed in' do
+      it 'returns false' do
+        expect(service.valid_session?(session, nil)).to be false
+      end
     end
+  end
 
-    context "when a user exists" do
-      let(:user) { FactoryBot.create(:user) }
-
-      context "when the session is timed out" do
-        let(:last_trans_time) { 2.hours.ago }
+  describe '#user_session_timed_out?' do
+    context 'when user is signed in' do
+      it 'returns true when session is not active' do
+        allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(false)
 
-        it "returns true" do
-          expect(privilege_checker.user_session_timed_out?(session, user)).to be_truthy
-        end
+        expect(service.user_session_timed_out?(session, user)).to be true
       end
 
-      context "when the session has not timed out" do
-        let(:last_trans_time) { Time.current }
+      it 'returns false when session is active' do
+        allow(SessionActivityService).to receive(:session_active?).with(user.id).and_return(true)
 
-        it "returns false" do
-          expect(privilege_checker.user_session_timed_out?(session, user)).to be_falsey
-        end
+        expect(service.user_session_timed_out?(session, user)).to be false
       end
     end
 
-    context "when a user does not exist" do
-      let(:user) { nil }
-      let(:last_trans_time) { nil }
-
-      it "returns false" do
-        expect(privilege_checker.user_session_timed_out?(session, user)).to be_falsey
+    context 'when user is not signed in' do
+      it 'returns false' do
+        expect(service.user_session_timed_out?(session, nil)).to be false
       end
     end
   end

spec/services/session_activity_service_spec.rb

@@ -0,0 +1,88 @@
+describe SessionActivityService do
+  let(:user_id) { 123 }
+  let(:current_time) { Time.zone.local(2025, 10, 30, 14, 30, 0) }
+  let(:mock_client) { double("Dalli::Client") }
+
+  before do
+    Timecop.freeze(current_time)
+    # Reset the cached client before each test
+    SessionActivityService.instance_variable_set(:@dalli_client, nil)
+    # Mock the Dalli client to avoid actual memcache calls in tests
+    allow(MiqMemcached).to receive(:client).and_return(mock_client)
+  end
+
+  after do
+    Timecop.return
+  end
+
+  describe '.update_last_transaction_time' do
+    it 'stores the current time for a user' do
+      expect(mock_client).to receive(:set).with(
+        "user_activity:#{user_id}",
+        current_time.iso8601,
+        (::Settings.session.timeout * 2).to_i
+      )
+
+      described_class.update_last_transaction_time(user_id)
+    end
+
+    it 'does nothing when user_id is nil' do
+      expect(mock_client).not_to receive(:set)
+      described_class.update_last_transaction_time(nil)
+    end
+  end
+
+  describe '.get_last_transaction_time' do
+    it 'retrieves the stored time for a user' do
+      stored_time = current_time - 5.minutes
+      expect(mock_client).to receive(:get).with("user_activity:#{user_id}").and_return(stored_time.iso8601)
+
+      result = described_class.get_last_transaction_time(user_id)
+      expect(result).to be_within(1.second).of(stored_time)
+    end
+
+    it 'returns nil when no time is stored' do
+      expect(mock_client).to receive(:get).with("user_activity:#{user_id}").and_return(nil)
+
+      result = described_class.get_last_transaction_time(user_id)
+      expect(result).to be_nil
+    end
+
+    it 'returns nil when user_id is nil' do
+      expect(mock_client).not_to receive(:get)
+
+      result = described_class.get_last_transaction_time(nil)
+      expect(result).to be_nil
+    end
+  end
+
+  describe '.session_active?' do
+    context 'when session is active' do
+      it 'returns true when last activity is within timeout period' do
+        stored_time = current_time - (::Settings.session.timeout / 2)
+        allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(stored_time)
+
+        expect(described_class.session_active?(user_id)).to be true
+      end
+    end
+
+    context 'when session is inactive' do
+      it 'returns false when last activity is beyond timeout period' do
+        stored_time = current_time - (::Settings.session.timeout + 1.minute)
+        allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(stored_time)
+
+        expect(described_class.session_active?(user_id)).to be false
+      end
+
+      it 'returns false when no activity is recorded' do
+        allow(described_class).to receive(:get_last_transaction_time).with(user_id).and_return(nil)
+
+        expect(described_class.session_active?(user_id)).to be false
+      end
+
+      it 'returns false when user_id is nil' do
+        expect(described_class.session_active?(nil)).to be false
+      end
+    end
+  end
+end