angelnikolov / ts-cacheable

Observable/Promise Cache Decorator
https://npmjs.com/package/ts-cacheable
ISC License
340 stars 42 forks source link

Possible to cache multiple invocations of a method, varied by method's arguments? #65

Closed jaydiablo closed 4 years ago

jaydiablo commented 4 years ago

I've been testing this decorator in an application and was curious if what I'm seeing is the intended behavior or if it's something about my environment.

I've added the @PCacheable() decorator to some API methods that return a Promise.

These methods typically have some parameters to control which object(s) to retrieve from the API (like an object's id, or some filters for a list of objects). An example:

class ApiService {
  @PCacheable()
  public async getObject (id: number): Promise<any> {
    // HTTP call here
  }
}

When this method is called with an id, let's say 1, I see that any future requests with that same id are cached, so the network isn't hit, unless the method is called with a different id, in which case that response is cached. This initially seemed to be working the way I expected, but if I then call the method with id 1 (after it has been called with a different id), the response isn't cached and the network is hit.

Unless I'm mistaken, it appears that only one response for a given method is ever cached at a time, and that cache is cleared if the parameters change, so only subsequent calls with the same id will hit the cache, if any calls with a different id happen after the cache is stored, the cache is cleared entirely for that method.

Is this expected? Looking in the code a bit, it seems that the cacheKey is based on the name of the class and then the name of the method (unless provided in config) separated by a #. And the parameters are only used to determine if the cached value should be used or not (via the result of the cacheResolver).

Is there a way to store a cached response for every variation of the parameters of the method?

I was thinking that the cacheKey could be modified to do this, but we'd need a dynamic way to set that based on the method's parameters, which doesn't seem possible unless I'm missing something.

I tried using the custom cacheHasher but that still didn't seem to allow me to cache multiple calls to the same method, unless I'm doing something wrong.

I've put a stackblitz together that shows what I mean: https://angular-njkvn4.stackblitz.io/ (source: https://stackblitz.com/edit/angular-njkvn4) If you repeatedly press the "Get 1" button, you'll only see one console.log statement, and just one HTTP request, however, if you press "Get 1", "Get 2" and then "Get 1", you'll see it make the request for the 1 post twice because the call with id 2 clears the cache and stores the result of "Get 2" (which is then cleared when "Get 1" is called again).

Thanks!

angelnikolov commented 4 years ago

First of all - thanks for the well described question and the stackblitz you've put together. It sounds like what you are looking for is maxCacheCount which will allow you to specify how many invocations should be cached per method. That means that calling your method with 1 and 2 will both be cached if you specify a maxCacheCount of 2 or more. Also, from a couple of weeks ago, you can also specify that configuration globally (check out the README). There have also been talks about adding a keepAlive option to each decorator or globally which will basically make the decorators cache indefinitely, but I haven't reached it yet. If you got any other questions, please let me know. :)

jaydiablo commented 4 years ago

I see, does maxCacheCount default to 1 then?

I updated my stackblitz to bump that to 3 and it does seem like it does what I want now. Thanks!

Maybe something that could be added to the readme (the default of maxCacheCount).

angelnikolov commented 4 years ago

Yes, it actually doesn't have a default, but if it's not set it will indeed act as 1, so I will update the Readme :) Thanks!

jaydiablo commented 4 years ago

Thanks!