Conversation
…ntication tokens with caching capabilities.
| type ( | ||
| Option[A any] = O.Option[A] | ||
|
|
||
| ReaderIOResult[R, A any] = func(context.Context, R) (A, error) |
There was a problem hiding this comment.
For functional composition you want to use a reader, i.e. a function that accepts a single argument, not two as in Context, R
Since this is an IO operation I suggest to use:
https://pkg.go.dev/github.com/IBM/fp-go/v2@v2.2.12/context/readerioresult
for the actual side effect and https://pkg.go.dev/github.com/IBM/fp-go/v2@v2.2.12/context/readerreaderioresult for the addition of another context. Instead of the pretty verbose ReaderReaderIOResult you can also use Effect (https://pkg.go.dev/github.com/IBM/fp-go/v2@v2.2.12/effect) which is an alias with some convenience methods
|
|
||
| HTTPClient *http.Client | ||
|
|
||
| Cache *TokenCache |
There was a problem hiding this comment.
The type of the cache should be https://pkg.go.dev/github.com/IBM/fp-go/v2@v2.2.12/ioref actually IORef[CachedToken]. You do not need the Cache data type.
|
|
||
| Cache *TokenCache | ||
|
|
||
| Now func() time.Time |
There was a problem hiding this comment.
Although equivalent, you might want to write
Now IO[time.Time]to emphasize that this is a side effect
| type Env struct { | ||
| AuthURL string | ||
|
|
||
| APIKey string |
There was a problem hiding this comment.
Modelling this as a string in the environment makes sense if you assume that the APIKey cannot change over the runtime of the application (since the dependencies are conceptually static).
But considering a long running application, it might happen that the APIKey gets updated (in addition to the token that gets updated anyway on a regular basis).
Maybe it's worth considering to model the key in the dependencies as Effect[string]
| type authResponse struct { | ||
| AccessToken string `json:"access_token"` | ||
|
|
||
| ExpiresIn int `json:"expires_in"` |
There was a problem hiding this comment.
might be worth adding a comment that this is in seconds.
| ) | ||
|
|
||
| // Cache is a generic thread-safe cache that stores an optional value using IORef. | ||
| type Cache[T any] struct { |
There was a problem hiding this comment.
You do not need to wrap IORef into the cache, you can use it directly and define Cache just as a type alias.
I think the content of IORef should be Result[T] because it's possible that the resolution of the token resulted in an error
| func MakeCache[T any]() IO.IO[*Cache[T]] { | ||
| return F.Pipe1( | ||
| IORef.MakeIORef(O.None[T]()), | ||
| IO.Map(func(ref IORef.IORef[Option[T]]) *Cache[T] { |
There was a problem hiding this comment.
This extra indirection is not needed
| } | ||
|
|
||
| // Set stores a new value in the cache, replacing any existing value. | ||
| func (c *Cache[T]) Set(value T) IO.IO[T] { |
There was a problem hiding this comment.
This is where a race condition can occur, because when using Set in the service, you first read, validate, refresh and then set. But all of these operations need to be coordinated across callers.
You want to use IORef.ModifyIOK instead.
| } | ||
|
|
||
| // Update applies a transformation function to the cached value. | ||
| func (c *Cache[T]) Update(f func(T) T) IO.IO[Option[T]] { |
There was a problem hiding this comment.
From a signature perspective you could write f Endomorphism[T] just for clarity (or leave it as is).
An update with an endomorphism makes sense if the update operation is a pure function. But in this case it's an effectful function, so you want to use ModifyIOK
| } | ||
|
|
||
| // IsExpired checks if the cached token is expired based on the current time. | ||
| func (c *TokenCache) IsExpired(currentTime time.Time) IO.IO[bool] { |
There was a problem hiding this comment.
- instead of using
IO.IO[bool]as a response value consider usingIO[Option[CachedToken]]. Reason: what would you do with afalsereturn value? In this case the token is not expired, so you want to return the token from the cache. This needs another access to the cache in addition to the one that validated the token. This's not necessary if you return the unexpired token directly (and it avoids a race condition) (pattern: "Boolean Blindness) - is the signature
IsExpired(currentTime time.Time) IO.IO[bool]really intended? The resultingIOis an operation that is executed at some point in the future, there is no correlation that it is executed in temporal proximity of thecurrentTime. This might make sense but from the termIsExpiredI would expect that the IO operation tests whether or not the operation is expired at the point in time when it is invoked. In this case a better signature would beIsExpired(currentTime IO[time.Time]) IO[bool](actuallyIsExpired(currentTime IO[time.Time]) IO[Option[CachedToken]]. This signature is basicallyIO.Kleisli[time.Time, CachedToken]
2 participants
No description provided.