search

KevinDockx / HttpCacheHeaders

ASP.NET Core middleware that adds HttpCache headers to responses (Cache-Control, Expires, ETag, Last-Modified), and implements cache expiration & validation models
MIT License
266 stars 56 forks source link

Make the StoreKey object round-trippable #121

Closed SeanFarrow closed 1 year ago

SeanFarrow commented 1 year ago

Currently, the StoreKey object is not round trippable.

This is because the ToString method only outputs the values.

We probably need to serialize the StoreKey to Json and use this string when storing a new cache entry.

The only remaining question is whether we should provide this as an extension point, so that other users can serialize the StoreKey to different formats (protobuf etc). I don't see a need for this within the current codebase, but you never know what people will want.

KevinDockx commented 1 year ago

If there's no current need for it, I'd say: no. We can add features when they're requested :)

SeanFarrow commented 1 year ago

True, my only thought is/continues to be that we only rehydrate the values within a key, I might need to do this for the redis stuff, I’m not sure yet.

The other way to look at it is whether we need to store the key names at ll within the keys store in a distributed cache.

From: Kevin Dockx @.> Sent: Monday, August 28, 2023 12:25 PM To: KevinDockx/HttpCacheHeaders @.> Cc: Sean Farrow @.>; Author @.> Subject: Re: [KevinDockx/HttpCacheHeaders] Make the StoreKey object round-trippable (Issue #121)

If there's no current need for it, I'd say: no. We can add features when they're requested :)

— Reply to this email directly, view it on GitHubhttps://github.com/KevinDockx/HttpCacheHeaders/issues/121#issuecomment-1695522663, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7UPMTZ7WAAUIOBKGY3XXR5X7ANCNFSM6AAAAAA37JUIMI. You are receiving this because you authored the thread.Message ID: @.***>

KevinDockx commented 1 year ago

If you need it for the Redis stuff I'm definitely open to consider / look into it. Also: I'd keep storing those key names as well, if only for consistency across storage approaches.

By the way: if there's something you need me to pick up to get all if this up & running, let me know and I'll try to set aside a bit of time for developing :)

SeanFarrow commented 1 year ago

I now have a branch that does this and will be raising a PR imminently.

I’ve also noticed the following issues: We don’t have any tests for the SetAsync, RemoveAsync and FindStoreKeysByKeyPartAsync methods of the InMemoryValidatorValueStore, so I’ll raise an issue for this and fix.

Also, now that we are serializing keys to Json by default, we could if we wanted use the IInmemoryCache interface rather than dictionaries to store them. I’ll add an issue for this and explore.

From: Kevin Dockx @.> Sent: Tuesday, August 29, 2023 9:17 AM To: KevinDockx/HttpCacheHeaders @.> Cc: Sean Farrow @.>; Author @.> Subject: Re: [KevinDockx/HttpCacheHeaders] Make the StoreKey object round-trippable (Issue #121)

If you need it for the Redis stuff I'm definitely open to consider / look into it. Also: I'd keep storing those key names as well, if only for consistency across storage approaches.

By the way: if there's something you need me to pick up to get all if this up & running, let me know and I'll try to set aside a bit of time for developing :)

— Reply to this email directly, view it on GitHubhttps://github.com/KevinDockx/HttpCacheHeaders/issues/121#issuecomment-1696979945, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7R2NHPOT56VK7YSAKDXXWQOHANCNFSM6AAAAAA37JUIMI. You are receiving this because you authored the thread.Message ID: @.***>

KevinDockx commented 1 year ago

Thanks Sean, just approved the PR - closing this issue.