Skip to content

Add session helper functions for value manipulation - #45

Merged
paganotoni merged 1 commit into
mainfrom
feature-session-helpers
Jan 11, 2026
Merged

Add session helper functions for value manipulation#45
paganotoni merged 1 commit into
mainfrom
feature-session-helpers

Conversation

@paganotoni

Copy link
Copy Markdown
Contributor

Summary

This PR adds convenient helper functions to simplify session value operations in the server/session package:

  • session.Get(r, key) - retrieve a value from the session
  • session.Set(r, w, key, value) - store a value in the session
  • session.Delete(r, w, key) - remove a value from the session
  • session.Clear(r, w) - remove all values from the session
  • session.Save(r, w) - explicitly persist the session

Motivation

Previously, users had to:

  1. Call session.FromCtx(r.Context()) to get the session
  2. Directly manipulate the Values map

The new helpers provide a cleaner API and safer error handling - they return ErrSessionNotFound instead of panicking when the session is not in context.

Usage

// Set a value
err := session.Set(r, w, "user_id", "1234")

// Get a value
val, exists, err := session.Get(r, "user_id")

// Delete a value
err := session.Delete(r, w, "user_id")

// Clear all values
err := session.Clear(r, w)

// Explicitly save (useful for WebSocket/SSE scenarios)
err := session.Save(r, w)

Testing

  • Added comprehensive tests in values_test.go
  • All tests pass with 100% coverage
  • Verified with go vet and gofmt

Add convenient helper functions to simplify session value operations:
- Get(r, key) - retrieve a value from the session
- Set(r, w, key, value) - store a value in the session
- Delete(r, w, key) - remove a value from the session
- Clear(r, w) - remove all values from the session
- Save(r, w) - explicitly persist the session

All functions return ErrSessionNotFound if the session is not in context,
providing safer error handling compared to FromCtx which panics.
@paganotoni
paganotoni requested a review from Copilot January 11, 2026 14:29
@paganotoni
paganotoni merged commit 2c1572d into main Jan 11, 2026
5 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds convenient helper functions to the server/session package to simplify session value manipulation. The new functions provide a cleaner API with explicit error handling instead of requiring users to call session.FromCtx and directly manipulate the Values map.

Changes:

  • Added five new helper functions: Get, Set, Delete, Clear, and Save for session value operations
  • Introduced ErrSessionNotFound error for safer error handling when session is not in context
  • Comprehensive test suite with 100% coverage for the new functionality

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.

File Description
server/session/values.go Implements the new session helper functions with error handling and context validation
server/session/values_test.go Comprehensive test suite covering all new functions including error cases and persistence scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

})

s.HandleFunc("GET /set/{$}", func(w http.ResponseWriter, r *http.Request) {
session.Set(r, w, "to_delete", "value")

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value from session.Set is not checked for errors. While this call is unlikely to fail in this test context, it's a best practice to check all error returns in tests for consistency and to catch unexpected issues.

Copilot uses AI. Check for mistakes.
})

s.HandleFunc("GET /delete/{$}", func(w http.ResponseWriter, r *http.Request) {
session.Delete(r, w, "to_delete")

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value from session.Delete is not checked for errors. While this call is unlikely to fail in this test context, it's a best practice to check all error returns in tests for consistency and to catch unexpected issues.

Suggested change
session.Delete(r, w, "to_delete")
if err := session.Delete(r, w, "to_delete"); err != nil {
t.Errorf("Delete failed: %v", err)
}

Copilot uses AI. Check for mistakes.
Comment on lines +449 to +450
_, exists1, _ := session.Get(r, "key1")
_, exists2, _ := session.Get(r, "key2")

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error returns from session.Get calls are ignored using the blank identifier. While this may be acceptable if the test is only checking value existence, it's better practice to check the errors to catch unexpected issues. Consider adding error checking for consistency with other tests.

Suggested change
_, exists1, _ := session.Get(r, "key1")
_, exists2, _ := session.Get(r, "key2")
_, exists1, err := session.Get(r, "key1")
if err != nil {
t.Errorf("Get failed for key1: %v", err)
}
_, exists2, err := session.Get(r, "key2")
if err != nil {
t.Errorf("Get failed for key2: %v", err)
}

Copilot uses AI. Check for mistakes.
Comment thread server/session/values.go
Comment on lines +38 to +39
// Delete removes a value from the session by key.
// Returns an error if the session is not in the request context.

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should clarify that sessions are automatically saved when the response is written via the saver middleware, and that this function only modifies the session in memory. Add a note similar to the one in the Save function documentation to explain when/how the session is persisted.

Copilot uses AI. Check for mistakes.
Comment on lines +241 to +243
session.Set(r, w, "key1", "value1")
session.Set(r, w, "key2", "value2")
session.Set(r, w, "key3", "value3")

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return values from session.Set calls are not checked for errors. While these calls are unlikely to fail in this test context, it's a best practice to check all error returns in tests for consistency and to catch unexpected issues. Consider adding error checks similar to those used elsewhere in the test file.

Suggested change
session.Set(r, w, "key1", "value1")
session.Set(r, w, "key2", "value2")
session.Set(r, w, "key3", "value3")
if err := session.Set(r, w, "key1", "value1"); err != nil {
t.Fatalf("Set failed: %v", err)
}
if err := session.Set(r, w, "key2", "value2"); err != nil {
t.Fatalf("Set failed: %v", err)
}
if err := session.Set(r, w, "key3", "value3"); err != nil {
t.Fatalf("Set failed: %v", err)
}

Copilot uses AI. Check for mistakes.
Comment thread server/session/values.go

// Delete removes a value from the session by key.
// Returns an error if the session is not in the request context.
func Delete(r *http.Request, w http.ResponseWriter, key string) error {

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The w http.ResponseWriter parameter is accepted but never used in this function. While it may be included for API consistency, this could confuse users. Consider either removing this parameter or documenting clearly in the function comment that the session will be automatically saved when the response is written, and this parameter is reserved for future use or API consistency.

Copilot uses AI. Check for mistakes.
Comment thread server/session/values.go
Comment on lines +26 to +27
// Set stores a value in the session.
// Returns an error if the session is not in the request context.

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should clarify that sessions are automatically saved when the response is written via the saver middleware, and that this function only modifies the session in memory. Users might expect that calling Set immediately persists the session, but it doesn't - it only modifies the session object which is later saved automatically.

Copilot uses AI. Check for mistakes.
Comment thread server/session/values.go
Comment on lines +50 to +51
// Clear removes all values from the session.
// Returns an error if the session is not in the request context.

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should clarify that sessions are automatically saved when the response is written via the saver middleware, and that this function only modifies the session in memory. Add a note similar to the one in the Save function documentation to explain when/how the session is persisted.

Copilot uses AI. Check for mistakes.
Comment on lines +302 to +305
session.Set(r, w, "key", "value")

err := session.Save(r, w)
if err != nil {

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value from session.Set is not checked for errors. While this call is unlikely to fail in this test context, it's a best practice to check all error returns in tests for consistency and to catch unexpected issues.

Suggested change
session.Set(r, w, "key", "value")
err := session.Save(r, w)
if err != nil {
if err := session.Set(r, w, "key", "value"); err != nil {
t.Errorf("Set failed: %v", err)
}
if err := session.Save(r, w); err != nil {

Copilot uses AI. Check for mistakes.
})

s.HandleFunc("GET /set/{$}", func(w http.ResponseWriter, r *http.Request) {
session.Set(r, w, "persistent", "data")

Copilot AI Jan 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value from session.Set is not checked for errors. While this call is unlikely to fail in this test context, it's a best practice to check all error returns in tests for consistency and to catch unexpected issues.

Suggested change
session.Set(r, w, "persistent", "data")
if err := session.Set(r, w, "persistent", "data"); err != nil {
t.Fatalf("Set failed: %v", err)
}

Copilot uses AI. Check for mistakes.
@tatang26
tatang26 deleted the feature-session-helpers branch April 17, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants