Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the SetIfAbsent method across all cache implementations in the codebase. The method sets a key-value pair only if the key doesn't already exist in the cache, returning the existing value (if present) and a boolean indicating whether the insertion was performed.
- Adds
SetIfAbsentto theGecheinterface with proper documentation - Implements
SetIfAbsentacross all cache types (MapCache, MapTTLCache, RingBuffer, KVCache, KV) and wrapper types (Sharded, Updater, Locker) - Adds comprehensive test coverage including unit tests and panic tests for transaction handling
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| doc.go | Adds SetIfAbsent method to the Geche interface with documentation |
| map.go | Implements SetIfAbsent for MapCache using write lock and conditional insertion |
| map_ttl.go | Implements SetIfAbsent for MapTTLCache with TTL support |
| ring.go | Implements SetIfAbsent for RingBuffer, refactors Set method to extract internal set helper |
| kv_cache.go | Implements SetIfAbsent for KVCache using existing get and insert helpers |
| kv.go | Implements SetIfAbsent for KV wrapper using underlying cache |
| shard.go | Implements SetIfAbsent for Sharded cache wrapper with proper shard routing |
| updater.go | Implements SetIfAbsent for Updater wrapper by delegating to underlying cache |
| locker.go | Implements SetIfAbsent for transaction Tx with panic guards for unlocked/read-only access |
| common_test.go | Adds two generic test functions for SetIfAbsent behavior |
| kv_test.go | Adds test for SetIfAbsent with multiple scenarios and updates MockErrCache |
| kv_cache_test.go | Adds test for KVCache SetIfAbsent functionality |
| locker_test.go | Adds panic tests for SetIfAbsent on read-locked and unlocked transactions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| c.data[key] = value | ||
| return old, true |
There was a problem hiding this comment.
should we use value instead bang Arthur?
| return old, true | |
| return value, true |
There was a problem hiding this comment.
Comment in the inteface says:
// SetIfAbsent sets the kv only if the key didn't exist yet, and returns the existing value (if any) and whether the insertion was performed
We insert only when previous value did not exist, so old would be zero value in this case, which is consistent with other implementations.
There was a problem hiding this comment.
aaa isee, got it thanks bang Serger for the explanation
3 participants
No description provided.