-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Type safe stashing of context values #546
Conversation
Introducing a Stasher which is a convenience helper for interacting with typed values for a single stash key. The stasher itself is generic and handles casting retrieved values to the desired type. There are three different ways to handle the value not existing or being of the wrong type: - RetrieveOrDie panics if a value of the desired type is not found - RetrieveOrError errors if a value of the desired type is not found - RetrieveOrEmpty falls back to the empty value for the type Signed-off-by: Scott Andrews <scott@andrews.me>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #546 +/- ##
==========================================
+ Coverage 60.34% 60.52% +0.17%
==========================================
Files 32 32
Lines 2867 2880 +13
==========================================
+ Hits 1730 1743 +13
Misses 1043 1043
Partials 94 94 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
// Store saves the value in the stash under the key | ||
Store(ctx context.Context, value T) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[thought]: Store is a good name. However, intuitively I was expecting it to be Stash. That's in the spirit of Stringer, Writer, Reader etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, the first draft used Stash
here. I changed it because it seemed to push too much emphasis on stashing while retrieving is just as (or more) important. Stringer, Writer and Reader have a directed purpose, a Stasher is about a round trip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me @scothis! ty
|
||
func TestStasher(t *testing.T) { | ||
ctx := WithStash(context.Background()) | ||
stasher := NewStasher[string]("my-key") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit]: I think testing against a stashed value of type string
is sufficient. Then again maybe the test becomes more interesting when using a non-primitive type like Example
. In particular I am thinking about RetrieveOrEmpty
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All types in go have a default empty value. There's a use of a vanilla struct here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it. ty
Co-authored-by: Max Brauer <mamachanko@users.noreply.github.com> Signed-off-by: Scott Andrews <scott@andrews.me>
Introducing a Stasher which is a convenience helper for interacting with typed values for a single stash key. The stasher itself is generic and handles casting retrieved values to the desired type.
There are three different ways to handle the value not existing or being of the wrong type: