feat: only "touch" the session if no value updated (#91)

.github/workflows/go.yml

@@ -33,7 +33,7 @@ jobs:
     name: Test
     strategy:
       matrix:
-        go-version: [ 1.19.x, 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-latest, macos-latest, windows-latest ]
     runs-on: ${{ matrix.platform }}
     steps:
@@ -55,7 +55,7 @@ jobs:
     name: Postgres
     strategy:
       matrix:
-        go-version: [ 1.19.x, 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-latest ]
     runs-on: ${{ matrix.platform }}
     services:
@@ -95,7 +95,7 @@ jobs:
     name: Redis
     strategy:
       matrix:
-        go-version: [ 1.19.x, 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-latest ]
     runs-on: ${{ matrix.platform }}
     services:
@@ -130,7 +130,7 @@ jobs:
     name: MySQL
     strategy:
       matrix:
-        go-version: [ 1.19.x, 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-20.04 ]
     runs-on: ${{ matrix.platform }}
     steps:
@@ -159,7 +159,7 @@ jobs:
     name: Mongo
     strategy:
       matrix:
-        go-version: [ 1.19.x, 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-latest ]
     runs-on: ${{ matrix.platform }}
     services:
@@ -196,7 +196,7 @@ jobs:
     name: SQLite
     strategy:
       matrix:
-        go-version: [ 1.20.x ]
+        go-version: [ 1.20.x, 1.21.x ]
         platform: [ ubuntu-latest ]
     runs-on: ${{ matrix.platform }}
     steps:

.github/workflows/lsif.yml

@@ -13,7 +13,7 @@ jobs:
     if: github.repository == 'flamego/session'
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: Generate LSIF data
         uses: sourcegraph/lsif-go-action@master
       - name: Upload LSIF data to sourcegraph.com

file.go

@@ -104,6 +104,19 @@ func (s *fileStore) Destroy(_ context.Context, sid string) error {
 	return os.Remove(s.filename(sid))
 }
 
+func (s *fileStore) Touch(_ context.Context, sid string) error {
+	filename := s.filename(sid)
+	if !isFile(filename) {
+		return nil
+	}
+
+	err := os.Chtimes(filename, s.nowFunc(), s.nowFunc())
+	if err != nil {
+		return errors.Wrap(err, "change times")
+	}
+	return nil
+}
+
 func (s *fileStore) Save(_ context.Context, sess Session) error {
 	if len(sess.ID()) < minimumSIDLength {
 		return ErrMinimumSIDLength
@@ -114,10 +127,16 @@ func (s *fileStore) Save(_ context.Context, sess Session) error {
 		return errors.Wrap(err, "encode")
 	}
 
-	err = os.WriteFile(s.filename(sess.ID()), binary, 0600)
+	filename := s.filename(sess.ID())
+	err = os.WriteFile(filename, binary, 0600)
 	if err != nil {
 		return errors.Wrap(err, "write file")
 	}
+
+	err = os.Chtimes(filename, s.nowFunc(), s.nowFunc())
+	if err != nil {
+		return errors.Wrap(err, "change times")
+	}
 	return nil
 }
 
@@ -145,7 +164,7 @@ func (s *fileStore) GC(ctx context.Context) error {
 		}
 		return os.Remove(path)
 	})
-	if err != nil && err != ctx.Err() {
+	if err != nil && !errors.Is(err, ctx.Err()) {
 		return err
 	}
 	return nil

file_test.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/flamego/flamego"
 )
@@ -57,7 +58,7 @@ func TestFileStore(t *testing.T) {
 
 	resp := httptest.NewRecorder()
 	req, err := http.NewRequest("GET", "/set", nil)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	f.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
@@ -66,15 +67,15 @@ func TestFileStore(t *testing.T) {
 
 	resp = httptest.NewRecorder()
 	req, err = http.NewRequest("GET", "/get", nil)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	req.Header.Set("Cookie", cookie)
 	f.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
 
 	resp = httptest.NewRecorder()
 	req, err = http.NewRequest("GET", "/destroy", nil)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	req.Header.Set("Cookie", cookie)
 	f.ServeHTTP(resp, req)
@@ -91,48 +92,65 @@ func TestFileStore_GC(t *testing.T) {
 			Lifetime: time.Second,
 		},
 	)
-	assert.Nil(t, err)
-
-	setModTime := func(sid string) {
-		t.Helper()
-
-		err := os.Chtimes(store.(*fileStore).filename(sid), now, now)
-		assert.Nil(t, err)
-	}
+	require.Nil(t, err)
 
 	sess1, err := store.Read(ctx, "111")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 	err = store.Save(ctx, sess1)
-	assert.Nil(t, err)
-	setModTime("111")
+	require.Nil(t, err)
 
 	now = now.Add(-2 * time.Second)
 	sess2, err := store.Read(ctx, "222")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	sess2.Set("name", "flamego")
 	err = store.Save(ctx, sess2)
-	assert.Nil(t, err)
-	setModTime("222")
+	require.Nil(t, err)
 
 	// Read on an expired session should wipe data but preserve the record
 	now = now.Add(2 * time.Second)
 	tmp, err := store.Read(ctx, "222")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 	assert.Nil(t, tmp.Get("name"))
 
 	now = now.Add(-2 * time.Second)
 	sess3, err := store.Read(ctx, "333")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 	err = store.Save(ctx, sess3)
-	assert.Nil(t, err)
-	setModTime("333")
+	require.Nil(t, err)
 
 	now = now.Add(2 * time.Second)
 	err = store.GC(ctx) // sess3 should be recycled
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	assert.True(t, store.Exist(ctx, "111"))
 	assert.False(t, store.Exist(ctx, "222"))
 	assert.False(t, store.Exist(ctx, "333"))
 }
+
+func TestFileStore_Touch(t *testing.T) {
+	ctx := context.Background()
+	now := time.Now()
+	store, err := FileIniter()(ctx,
+		FileConfig{
+			nowFunc:  func() time.Time { return now },
+			RootDir:  filepath.Join(os.TempDir(), "sessions"),
+			Lifetime: time.Second,
+		},
+	)
+	require.Nil(t, err)
+
+	sess, err := store.Read(ctx, "111")
+	require.Nil(t, err)
+	err = store.Save(ctx, sess)
+	require.Nil(t, err)
+
+	now = now.Add(2 * time.Second)
+	// Touch should keep the session alive
+	err = store.Touch(ctx, sess.ID())
+	require.Nil(t, err)
+
+	err = store.GC(ctx)
+	require.Nil(t, err)
+	assert.True(t, store.Exist(ctx, sess.ID()))
+}

manager.go

@@ -24,6 +24,9 @@ type Store interface {
 	Read(ctx context.Context, sid string) (Session, error)
 	// Destroy deletes session with given ID from the session store completely.
 	Destroy(ctx context.Context, sid string) error
+	// Touch updates the expiry time of the session with given ID. It does nothing
+	// if there is no session associated with the ID.
+	Touch(ctx context.Context, sid string) error
 	// Save persists session data to the session store.
 	Save(ctx context.Context, session Session) error
 	// GC performs a GC operation on the session store.

manager_test.go

@@ -10,12 +10,13 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestIsValidSessionID(t *testing.T) {
 	for i := 0; i < 10; i++ {
 		s, err := randomChars(16)
-		assert.Nil(t, err)
+		require.Nil(t, err)
 		assert.True(t, isValidSessionID(s, 16))
 	}
 

memory.go

@@ -155,10 +155,22 @@ func (s *memoryStore) Destroy(_ context.Context, sid string) error {
 	return nil
 }
 
-func (s *memoryStore) Save(context.Context, Session) error {
+func (s *memoryStore) Touch(_ context.Context, sid string) error {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	sess, ok := s.index[sid]
+	if !ok {
+		return nil
+	}
+
+	sess.SetLastAccessedAt(s.nowFunc())
+	heap.Fix(s, sess.index)
 	return nil
 }
 
+func (s *memoryStore) Save(context.Context, Session) error { return nil }
+
 func (s *memoryStore) GC(ctx context.Context) error {
 	// Removing expired sessions from top of the heap until there is no more expired
 	// sessions found.

memory_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/flamego/flamego"
 )
@@ -46,25 +47,25 @@ func TestMemoryStore(t *testing.T) {
 	})
 
 	resp := httptest.NewRecorder()
-	req, err := http.NewRequest("GET", "/set", nil)
-	assert.Nil(t, err)
+	req, err := http.NewRequest(http.MethodGet, "/set", nil)
+	require.Nil(t, err)
 
 	f.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
 
 	cookie := resp.Header().Get("Set-Cookie")
 
 	resp = httptest.NewRecorder()
-	req, err = http.NewRequest("GET", "/get", nil)
-	assert.Nil(t, err)
+	req, err = http.NewRequest(http.MethodGet, "/get", nil)
+	require.Nil(t, err)
 
 	req.Header.Set("Cookie", cookie)
 	f.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
 
 	resp = httptest.NewRecorder()
-	req, err = http.NewRequest("GET", "/destroy", nil)
-	assert.Nil(t, err)
+	req, err = http.NewRequest(http.MethodGet, "/destroy", nil)
+	require.Nil(t, err)
 
 	req.Header.Set("Cookie", cookie)
 	f.ServeHTTP(resp, req)
@@ -82,29 +83,29 @@ func TestMemoryStore_GC(t *testing.T) {
 	)
 
 	sess1, err := store.Read(ctx, "1")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	now = now.Add(-2 * time.Second)
 	sess2, err := store.Read(ctx, "2")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	sess2.Set("name", "flamego")
 	err = store.Save(ctx, sess2)
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	// Read on an expired session should wipe data but preserve the record
 	now = now.Add(2 * time.Second)
 	tmp, err := store.Read(ctx, "2")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 	assert.Nil(t, tmp.Get("name"))
 
 	now = now.Add(-2 * time.Second)
 	_, err = store.Read(ctx, "3")
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	now = now.Add(2 * time.Second)
 	err = store.GC(ctx) // sess3 should be recycled
-	assert.Nil(t, err)
+	require.Nil(t, err)
 
 	wantHeap := []*memorySession{sess2.(*memorySession), sess1.(*memorySession)}
 	assert.Equal(t, wantHeap, store.heap)
@@ -115,3 +116,28 @@ func TestMemoryStore_GC(t *testing.T) {
 	}
 	assert.Equal(t, wantIndex, store.index)
 }
+
+func TestMemoryStore_Touch(t *testing.T) {
+	ctx := context.Background()
+	now := time.Now()
+	store := newMemoryStore(
+		MemoryConfig{
+			nowFunc:  func() time.Time { return now },
+			Lifetime: time.Second,
+		},
+	)
+
+	sess, err := store.Read(ctx, "1")
+	require.Nil(t, err)
+
+	now = now.Add(2 * time.Second)
+	// Touch should keep the session alive
+	err = store.Touch(ctx, sess.ID())
+	require.Nil(t, err)
+
+	err = store.GC(ctx)
+	require.Nil(t, err)
+
+	wantHeap := []*memorySession{sess.(*memorySession)}
+	assert.Equal(t, wantHeap, store.heap)
+}