From 05cdcbaf9d68abce0f728239a347d7c440d1065a Mon Sep 17 00:00:00 2001
From: Michael Mayer <michael@photoprism.app>
Date: Mon, 3 Oct 2022 23:39:36 +0200
Subject: [PATCH] Sessions: Cache pointers to improve performance #98 #782

Signed-off-by: Michael Mayer <michael@photoprism.app>
---
 internal/api/auth_session.go               |  2 +-
 internal/entity/auth_session.go            |  2 +-
 internal/entity/auth_session_cache.go      | 48 +++++++++++-----------
 internal/entity/auth_session_cache_test.go |  2 +-
 internal/session/session_get.go            |  4 +-
 5 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/internal/api/auth_session.go b/internal/api/auth_session.go
index e847670d6..990adc1f6 100644
--- a/internal/api/auth_session.go
+++ b/internal/api/auth_session.go
@@ -36,5 +36,5 @@ func Session(id string) *entity.Session {
 		return nil
 	}
 
-	return &s
+	return s
 }
diff --git a/internal/entity/auth_session.go b/internal/entity/auth_session.go
index fe6add33f..01624a5c3 100644
--- a/internal/entity/auth_session.go
+++ b/internal/entity/auth_session.go
@@ -139,7 +139,7 @@ func (m *Session) CacheDuration(d time.Duration) {
 		return
 	}
 
-	sessionCache.Set(m.ID, *m, d)
+	sessionCache.Set(m.ID, m, d)
 }
 
 // Cache caches the session with the default expiration duration.
diff --git a/internal/entity/auth_session_cache.go b/internal/entity/auth_session_cache.go
index 75bd8b373..976a86a2a 100644
--- a/internal/entity/auth_session_cache.go
+++ b/internal/entity/auth_session_cache.go
@@ -29,37 +29,35 @@ func DeleteFromSessionCache(id string) {
 }
 
 // FindSession returns an existing session or nil if not found.
-func FindSession(id string) (s Session, err error) {
-	s = Session{}
+func FindSession(id string) (*Session, error) {
+	found := &Session{}
 
 	// Valid id?
 	if !rnd.IsSessionID(id) {
-		return s, fmt.Errorf("id %s is invalid", clean.LogQuote(id))
+		return found, fmt.Errorf("id %s is invalid", clean.LogQuote(id))
 	}
 
-	// Find cached session.
-	if cacheData, ok := sessionCache.Get(id); ok {
-		s = cacheData.(Session)
-		s.LastActive = UnixTime()
-		return s, nil
-	}
-
-	// Search database and return session if found.
-	if r := Db().First(&s, "id = ?", id); r.RecordNotFound() {
-		return s, fmt.Errorf("not found")
-	} else if r.Error != nil {
-		return s, r.Error
-	} else if !rnd.IsSessionID(s.ID) {
-		return s, fmt.Errorf("has invalid id %s", clean.LogQuote(s.ID))
-	} else if s.Expired() {
-		if err = s.Delete(); err != nil {
-			event.AuditErr([]string{s.IP(), "session %s", "failed to delete after expiration", "%s"}, s.RefID, err)
+	// Find the session in the cache with a fallback to the database.
+	if cacheData, ok := sessionCache.Get(id); ok && cacheData != nil {
+		if cached := cacheData.(*Session); !cached.Expired() {
+			cached.LastActive = UnixTime()
+			return cached, nil
+		} else if err := cached.Delete(); err != nil {
+			event.AuditErr([]string{cached.IP(), "session %s", "failed to delete after expiration", "%s"}, cached.RefID, err)
 		}
-		return s, fmt.Errorf("expired")
-	} else {
-		s.UpdateLastActive()
-		sessionCache.SetDefault(s.ID, s)
+	} else if res := Db().First(&found, "id = ?", id); res.RecordNotFound() {
+		return found, fmt.Errorf("not found")
+	} else if res.Error != nil {
+		return found, res.Error
+	} else if !rnd.IsSessionID(found.ID) {
+		return found, fmt.Errorf("has invalid id %s", clean.LogQuote(found.ID))
+	} else if !found.Expired() {
+		found.UpdateLastActive()
+		sessionCache.SetDefault(found.ID, found)
+		return found, nil
+	} else if err := found.Delete(); err != nil {
+		event.AuditErr([]string{found.IP(), "session %s", "failed to delete after expiration", "%s"}, found.RefID, err)
 	}
 
-	return s, err
+	return found, fmt.Errorf("expired")
 }
diff --git a/internal/entity/auth_session_cache_test.go b/internal/entity/auth_session_cache_test.go
index ee937f7df..0881d2deb 100644
--- a/internal/entity/auth_session_cache_test.go
+++ b/internal/entity/auth_session_cache_test.go
@@ -14,7 +14,7 @@ func TestFlushSessionCache(t *testing.T) {
 	})
 }
 
-func TestCachedSession(t *testing.T) {
+func TestFindSession(t *testing.T) {
 	t.Run("EmptyID", func(t *testing.T) {
 		if _, err := FindSession(""); err == nil {
 			t.Fatal("error expected")
diff --git a/internal/session/session_get.go b/internal/session/session_get.go
index 61eb275fb..97bbcd51e 100644
--- a/internal/session/session_get.go
+++ b/internal/session/session_get.go
@@ -7,9 +7,9 @@ import (
 )
 
 // Get returns an existing client session.
-func (s *Session) Get(id string) (m entity.Session, err error) {
+func (s *Session) Get(id string) (m *entity.Session, err error) {
 	if id == "" {
-		return entity.Session{}, fmt.Errorf("new")
+		return &entity.Session{}, fmt.Errorf("new")
 	}
 
 	return entity.FindSession(id)