search

nickschot / lux-redis-cache

[WIP] Middleware implementation of a redis based cache for Lux
MIT License
8 stars 1 forks source link

add timeBased expiration strategy #13

Closed jamemackson closed 7 years ago

jamemackson commented 7 years ago

very simple cache strategy purely based on redis expiration time. Implementor can manually remove cache items from redis outside of the scope of lux-redis-cache if desired.

nickschot commented 7 years ago

I've been thinking about whether this is necessary or not. There currently is an expiresIn option which is -1 by default which does exactly the same thing (if you disable the caching engines). What you lose by moving it to a separate caching strategy is the use case where you'd want to use both one of the other engines combined with an expiresIn time (which could be applicable to any caching engine).

If we move it to a separate basic caching engine I think we should allow for engine configuration options (where any current engine would also have an expiresIn option).

Not really sure what's the best option yet.

jamemackson commented 7 years ago

What you lose by moving it to a separate caching strategy is the use case where you'd want to use both one of the other engines combined with an expiresIn time (which could be applicable to any caching engine).

I'm not seeing how adding this caching engine would prevent someone from using a different engine and specifying an expiration time. Can you elaborate on your view here?

One difference with this strategy are that this engine uses the url path in the cache key so that it's easy for a human to reason about, and while I haven't yet provided a mechanism to expire keys, using the url path leaves the door open to a subsequent update that could expire keys based on JSON-API query convention so, for example, if I update category 1518 I could use a regex search and find patterns of /categories/1518 or /categories?filter[id]=1518... (obviously this doesn't cover all the use cases here for expiration - I'm still thinking thru the details of this but am planning on this as a future enhancement here)

timebasedcachetreeexample

This cache strategy also does not suffer from #11 while running in a clustered environment, so that's a plus for my project.

Also, for my project, I'll likely have caching disabled entirely for our admin application where edits actually take place and will utilize the cache for our public facing e-commerce site (coming in our next phase of things) where simply caching requests for a static period of time is just fine or we can expire specific models with a broad brush if we really need to.

I guess, in the end, with the architecture of this addon that you've so nicely put in place, I don't see any harm in adding additional cache engines (pending that there's not something I'm missing about this per your above comment about losing functionality...).

nickschot commented 7 years ago

Alright, good to go!