- 
                Notifications
    You must be signed in to change notification settings 
- Fork 37
ResponseCache updates to support varyby rules #14
Conversation
|  | ||
| public void Set(string key, IResponseCacheEntry entry) | ||
| { | ||
| _cache.Set(key, DefaultResponseCacheSerializer.Serialize(entry)); | 
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.
null checks needed to be added everywhere
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.
And hardening. None of these operations should throw.
| Public = true, | ||
| MaxAge = TimeSpan.FromSeconds(10) | ||
| }; | ||
| context.Response.GetTypedHeaders().AppendList("Vary", new string[] { "Accept-Encoding", "Non-Existent" }); | 
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.
???
context.Response.Headers["Vary"] = new string[] { "Accept-Encoding", "Non-Existent" };
fc22a93    to
    b2812fb      
    Compare
  
    | 🆙📅 | 
| { | ||
| public int StatusCode { get; set; } | ||
|  | ||
| internal IHeaderDictionary Headers { get; set; } = new HeaderDictionary(); | 
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.
Why are these internal and status code public?
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.
No particular reason, could make everything internal until there's a reason to make them public. I don't see why these would be used by anyone else other than response caching middleware.
03d104f    to
    03e3284      
    Compare
  
    | using System; | ||
| using Microsoft.Extensions.Caching.Distributed; | ||
|  | ||
| namespace Microsoft.AspNetCore.ResponseCaching | 
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.
Internal?
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'll move all the storage related types (other than the interface IResponseCache) to the internal namespace and make them internal types since they shouldn't be needed outside of response caching.
| 
 | 
25f732a    to
    a40cc88      
    Compare
  
     after one comment
 after one comment
Updating the design of ResponseCache/ResponseCacheEntries to support varyby rules. The design of how to configure these rules and what elements to vary by have not been finalized yet.
cc @Tratcher @davidfowl