-
Notifications
You must be signed in to change notification settings - Fork 1
refactor app context to use composition #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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| package app | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/google/uuid" | ||
| ) | ||
|
|
||
| type ContextKey string | ||
|
|
||
| const ( | ||
| TraceIDKey ContextKey = "trace_id" | ||
Talento90 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| UserIDKey ContextKey = "user_id" | ||
| TenantIDKey ContextKey = "tenant_id" | ||
| ) | ||
|
|
||
| // Context carries the context of the current execution. | ||
| type Context struct { | ||
| // original context | ||
| context.Context | ||
| } | ||
|
|
||
| // UserID returns the user id | ||
| func (sc *Context) UserID() (string, bool) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsure why you return the bool about the presence, you are not doing it for TraceID
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea is that |
||
| userID := sc.Context.Value(UserIDKey) | ||
| id, ok := userID.(string) | ||
| return id, ok | ||
| } | ||
|
|
||
| // SetUserID sets the user id | ||
| func (sc *Context) SetUserID(userID string) *Context { | ||
| sc.Context = context.WithValue(sc.Context, UserIDKey, userID) | ||
|
|
||
| return sc | ||
| } | ||
|
|
||
| // TenantID returns the tenant id | ||
| func (sc *Context) TenantID() (string, bool) { | ||
| tenantIDKey := sc.Context.Value(TenantIDKey) | ||
| id, ok := tenantIDKey.(string) | ||
| return id, ok | ||
| } | ||
|
|
||
| // SetTenantID sets the user id | ||
| func (sc *Context) SetTenantID(tenantID string) *Context { | ||
| sc.Context = context.WithValue(sc.Context, TenantIDKey, tenantID) | ||
|
|
||
| return sc | ||
| } | ||
|
|
||
| // SetTraceID sets the trace id | ||
| func (sc *Context) SetTraceID(traceID string) *Context { | ||
| sc.Context = context.WithValue(sc.Context, TraceIDKey, traceID) | ||
|
|
||
| return sc | ||
| } | ||
|
|
||
| // TraceID returns the trace identifier for the current flow | ||
| func (sc *Context) TraceID() string { | ||
| traceID := sc.Context.Value(TraceIDKey) | ||
| id, ok := traceID.(string) | ||
|
|
||
| if !ok { | ||
| return "" | ||
| } | ||
|
|
||
| return id | ||
| } | ||
|
|
||
| // NewContext returns a new Context from a context.Context | ||
Talento90 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| func NewContext(ctx context.Context) Context { | ||
| appCtx := Context{Context: ctx} | ||
|
|
||
| if _, ok := ctx.Value(TraceIDKey).(string); !ok { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have called TraceID, and define one if the if was == "", my problem is that you somehow define two places where you check if you have a context. This comment is made on your current code. I think you could also consider to do not play/check/set with traceid in app.NewContext, but to define one when none is present in GetTraceID. It would also fix an issue that someone with your current GetTraceID implementation. Someone could define a context like this ctx : = app.Context{Context: context.Background()} // possible as app.Context cannot be nothing but Exported
traceID := ctx.TraceID() // this would be empty
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you never call the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, but the points I raised are still valid, no?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I added in the constructor and getter but it does not feel right 🤔 |
||
| appCtx.SetTraceID(uuid.NewString()) | ||
| } | ||
|
|
||
| return appCtx | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.