Feature/cache support trough redis #430
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
| _toolset.Logger?.LogDebug("Response received from cache. Request id: '{requestId}'.", request.Id); | ||
| return cachedResult; | ||
| } | ||
|
|
||
| if (resultType == typeof(TResponse)) | ||
| return await transportNClient |
There was a problem hiding this comment.
We need to think about how to cache TResponse
| .GetHttpResponseAsync(request, resiliencePolicy, cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| await _responseCacheWorker.Put(request, result, cachingAttribute, cancellationToken); |
There was a problem hiding this comment.
You need to get and save the cache in the transport client. I'll explain why: if you save the cache here, then invalid responses will not be cached, because they will not come here.
| { | ||
| if (await _responseCacheWorker.TryGet(request, cancellationToken) is { } cachedResult) |
There was a problem hiding this comment.
Will the cache work correctly if we have two methods that create the same request, but map the response differently? For example:
[GetMethod("entity")]
public Task<Entity> Get();
[GetMethod("entity")]
public Task<IResult<Entity>> Get();|
|
||
| await _responseCacheWorker.Put(request, result, cachingAttribute, cancellationToken); | ||
| return result; | ||
| } | ||
|
|
||
| if (resultType != typeof(void)) | ||
| return await transportNClient |
There was a problem hiding this comment.
You also need to not forget about caching void methods :)
| { | ||
| if (await _responseCacheWorker.TryGet(request, cancellationToken) is { } cachedResult) |
There was a problem hiding this comment.
What if the cache becomes unavailable and throws exceptions? I think the client should continue to work without a cache. The same goes for saving the cache
| .GetHttpResponseAsync(request, dataType: resultType.GetGenericArguments().Single(), resiliencePolicy, cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| await _responseCacheWorker.Put(request, result, cachingAttribute, cancellationToken); |
There was a problem hiding this comment.
I would add logging about saving to the cache too :)
Now we can use special extension for caching responses through separate providers (Redis as Example)