angelnikolov / ts-cacheable

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

Possibility to cache per instance #102

Closed KG3RK3N closed 3 years ago

KG3RK3N commented 3 years ago

We have the problem, that we can't cache per instance. That necessary for us because we have a service instance per entity for odata.

The problem is, that currently the cache for a method is per class, not instance. That makes sense of the most implementations, but a possibility to support per instance or a workaround was nice.

I have a simple application created that shows the problem, the result is mostly wrong after first get. You can see it on the console log. https://stackblitz.com/edit/angular-ivy-kuo2jx

An idea was that I can set a method for cacheKey that has a param with the current instance and I can return a string that generated by the instance.

angelnikolov commented 3 years ago

Hey @KG3RK3N! Thanks for the detailed explanation. I believe you've taken the right path to solve the issue. Unfortunately, I can't think of another way to do it, since this is how the cache works in general. It decorates the class methods before the class is instantiated.

KG3RK3N commented 3 years ago

@angelnikolov Thanks for the fast response! I will see that I can find a good solution for this scenario and hopefully we hear us in the PR again 😄

angelnikolov commented 3 years ago

Btw, you could also try something like this (in case you are subclassing a common class for your different services)

import {Cacheable} from 'ngx-cacheable';
import {Observable, of, Subject} from 'rxjs';
/*tslint:disable*/
/**
 * A base mixin decorator which will cache to our default service methods like 
 * @param serviceName we need to provide the serviceName to the decorator,
 * so it can use that to construct the cacheKey.
 * @param cacheDuration the default duration that will be used for caching the items
 * @param cacheBusterSubject the cache subject you have defined in the child class.
 * Useful when you want to get custom method's cache busted (one defined in the child class)
 * from generic cache busters (ones defined here, in the BaseApiCache mixin)
 */
export function BaseClass(
  serviceName: string,
  cacheBusterSubject: Subject<any> = new Subject(),
): Function {
  /**
   * A mixin function which will take all properties of BaseClass and stamp them
   * on the provided constructor (i.e all services which require the basic get/set/update functionality)
   */
  return function(derivedCtor: Function): void {
    class BaseClass {
      cacheBuster = cacheBusterSubject;
      @Cacheable({
        cacheKey: serviceName + '#get',
        cacheBusterObserver: cacheBusterSubject.asObservable()
      })
      public get(
      ): Observable<any> {
        return of([]);
      }
    }
    const fieldCollector = {};
    (BaseClass as Function).constructor.apply(fieldCollector);
    Object.getOwnPropertyNames(fieldCollector).forEach((name) => {
      derivedCtor.prototype[name] = fieldCollector[name];
    });

    Object.getOwnPropertyNames(BaseClass.prototype).forEach((name) => {
      if (name !== 'constructor') {
        derivedCtor.prototype[name] = BaseClass.prototype[name];
      }
    });
    derivedCtor.prototype['cacheBuster'] = cacheBusterSubject;
  };
}

And then use it like


@BaseClass('concreteService')
class ConcreteService {}

This would create new cache storage per service, but not per instance as you wanted, but if your case is that you have multiple different services, subclassing the Odata service, it could work. I mean, you'd have one service instance for the class PersonODataService extends ODataService and one service instance for the class CountryODataService extends ODataService. But if you have multiple instances of the PersonODataService itself, it won't work :(

KG3RK3N commented 3 years ago

Thanks for the example. Currently my workaround for extended classes is the sample below:


class BaseService {
  getAll(): Observable<any[]> {
    return of([]);
  }

  get(id: string): Observable<any> {
    return of({});
  }
}

class PersonService extends BaseService {
  @Cacheable()
  get(id: string): Observable<any> {
    return super.get(id);
  }
}

// simple endpoint access: /odata/Person

But that don't resolve my problem, because sometimes its necessary that I need access to a sub collection endpoint. For example:


class PersonAclService extends BaseService {
  constructor(private parentId: string) {}

  // getAll => that should be cached
  // get
}

class PersonService extends BaseService {
  private aclServices = {};

  getAcls(personId: string): PersonAclService {
     if (!this.aclServices.hasOwnProperty(personId)) {
        this.aclServices[personId] = new PersonAclService(personId);
     }
     return this.aclServices[personId];
  }
}

// sub collection endpoint: /odata/Person(xyz)/Acls

And that is currently not possible because each PersonAclService instance have the same cache.

angelnikolov commented 3 years ago

@KG3RK3N There are ways to accomplish that (explained here for example). However that will require a large change to the decorators which I don't think we can make right now since it might affect other users. Could maybe including the execution context in all calls of the storage strategy help you? That way you could create your own storage strategy where you can check which instance exactly the method has been called in and based on that figure out where to store? (i.e different cache storage for different instances of the same class). Think of it like:

export abstract class IStorageStrategy {
  abstract getAll(cacheKey: string, ctx): Array<ICachePair<any>>;
  abstract add(entity: ICachePair<any>, cacheKey: string, ctx): void;
  abstract updateAtIndex(index: number, entity: ICachePair<any>, cacheKey: string, ctx): void;
  abstract update?(index: number, entity: ICachePair<any>, cacheKey: string, ctx): void;
  abstract removeAtIndex(index: number, cacheKey: string, ctx): void;
  abstract remove?(index: number, entity: ICachePair<any>, cacheKey: string, ctx): void;
  abstract removeAll(cacheKey: string, ctx): void;
  abstract addMany(entities: ICachePair<any>[], cacheKey: string, ctx): void;
}

However, in that case you'd still need some unique instance id, which brings us back at your initial solution.

KG3RK3N commented 3 years ago

@angelnikolov Greate idea, that should resolve my issue if the class instance available in the storage strategy methods.

KG3RK3N commented 3 years ago

@angelnikolov Thanks very much.

With the follow code sample for the custom strategy I have fixed the problem with the new context accessibility:

export class MyStorageStrategy implements IStorageStrategy {
  private cachePairs: {
    [key: string]: Array<ICachePair<any>>
  } = {};

  getAll(cacheKey: string, ctx?: any): ICachePair<any>[] {
    this.checkAndInitPair(ctx);
    return this.cachePairs[ctx.parentId];
  }
  add(entity: ICachePair<any>, cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    this.cachePairs[ctx.parentId].push(entity)
  }
  updateAtIndex(index: number, entity: ICachePair<any>, cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    const updatee = this.cachePairs[ctx.parentId][index];
    Object.assign(updatee, entity);
  }
  update?(index: number, entity: ICachePair<any>, cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    const updatee = this.cachePairs[ctx.parentId][index];
    Object.assign(updatee, entity);
  }
  removeAtIndex(index: number, cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    this.cachePairs[ctx.parentId].splice(index, 1);
  }
  remove?(index: number, entity: ICachePair<any>, cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    this.cachePairs[ctx.parentId].splice(index, 1);
  }
  removeAll(cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    this.cachePairs[ctx.parentId].length = 0;
  }
  addMany(entities: ICachePair<any>[], cacheKey: string, ctx?: any): void {
    this.checkAndInitPair(ctx);
    this.cachePairs[ctx.parentId] = entities;
  }

  private checkAndInitPair(ctx: any) {
    if (!this.cachePairs[ctx.parentId]) {
      this.cachePairs[ctx.parentId] = [];
    }
  }

}