Rename DataStorage.Store/StoreIfAbsent to Upsert/Create#4476
Conversation
There was a problem hiding this comment.
Pull request overview
This PR renames the DataStorage write operations to better communicate semantics at call sites: Upsert for unconditional overwrite and Create for atomic set-if-absent.
Changes:
- Renamed
DataStorage.Store→UpsertandDataStorage.StoreIfAbsent→Create. - Updated
LocalSessionDataStorageandRedisSessionDataStorageimplementations to match the new interface. - Updated DataStorage contract tests to use the new method names.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pkg/transport/session/session_data_storage.go | Renames DataStorage interface methods and updates the contract wording. |
| pkg/transport/session/session_data_storage_local.go | Renames Local implementation methods to Upsert/Create. |
| pkg/transport/session/session_data_storage_redis.go | Renames Redis implementation methods to Upsert/Create and updates docs. |
| pkg/transport/session/session_data_storage_test.go | Updates contract tests to use Upsert/Create. |
Comments suppressed due to low confidence (1)
pkg/transport/session/session_data_storage.go:29
- The PR description says “Fixes #4220”, but issue #4220 is about adding operator unit tests for horizontal scaling behaviors; this change is a session DataStorage rename. Please verify the linked issue is correct so the merge doesn’t auto-close an unrelated issue.
// - Upsert creates or overwrites the metadata for id, refreshing the TTL.
// - Load retrieves metadata and refreshes the TTL (sliding-window expiry).
// Returns ErrSessionNotFound if the session does not exist.
// - Delete removes the session. It is not an error if the session is absent.
// - Close releases any resources held by the backend (connections, goroutines).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4476 +/- ##
=======================================
Coverage 69.67% 69.67%
=======================================
Files 494 494
Lines 50434 50422 -12
=======================================
- Hits 35139 35132 -7
+ Misses 12600 12598 -2
+ Partials 2695 2692 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Clarifies the semantic distinction between the two write operations: Upsert (unconditional overwrite) vs Create (atomic set-if-absent), making the difference visible at the call site without reading docs. Also remove Exists from the interface
Summary
Clarifies the semantic distinction between the two write operations: Upsert (unconditional overwrite) vs Create (atomic set-if-absent), making the difference visible at the call site without reading docs.
Also remove Exists from the interface.
Fixes #4220
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers