Services inheriting from common base need different caches.

[jonstorer]

I really like Cacheable. And I really want it to work for my needs! However, it's currently not working. Here's my scenario:

I have a base service called Repository. All services inherit from it. When I fetch data from each child service, they share the same cache & the service that calls 2nd, gets the first service's data.

The calls in the follow code block are nested for force an order for this example. Typically, there's a race condition and whoever resolves first wins. The response in the 2nd call should be Widget[], however, it comes back as Part[] because of the cache.

this.partsService.find().subscribe((parts) => {
  this.parts = parts;

  this.widgetsService.find().subscribe((widgets) => {
    this.widgets = widgets;
  });
});

widgets.service.ts

import { RepositoryService } from './repository.service';
import { Widget } from '../models';

export class WidgetService extends RepositoryService<Widget> {}

parts.service.ts

import { RepositoryService } from './repository.service';
import { Part } from '../models';

export class PartService extends RepositoryService<Part> {}

repository.service.ts

import { HttpClient, HttpParams } from '@angular/common/http';
import { Observable, Subject } from 'rxjs';
import { Cacheable, CacheBuster } from 'ngx-cacheable';
import { Resource } from '../models/resource.model';
import * as i from 'inflection';

const cacheBuster$ = new Subject<void>();

export class RepositoryService<T extends Resource> {
  private base:string = '/api';
  private collection:string;

  constructor(private http: HttpClient) {
    this.collection = i.underscore(i.pluralize(
      this.constructor.name.replace('Service', '')
    ));
  }

  @Cacheable({
    cacheBusterObserver: cacheBuster$
  })
  public find(query: object = {}): Observable<T[]> {
    const options = { params: new HttpParams() };

    for (const [key, value] of Object.entries(query)) {
      options.params.set(key, value);
    }

    return this.http.get<T[]>(this.url(), options);
  }

  @Cacheable({
    cacheBusterObserver: cacheBuster$
  })
  public findById(id: string): Observable<T> {
    return this.http.get<T>(this.url(id));
  }

  @CacheBuster({
    cacheBusterNotifier: cacheBuster$
  })
  public save(item: T): Observable<T> {
    return item.id ? this.update(item) : this.create(item);
  }

  private create(item: T): Observable<T> {
    return this.http.post<T>(this.url(), item);
  }

  private update(item: T): Observable<T> {
    const { id, ...payload } = <T>item;
    return this.http.patch<T>(this.url(id), payload);
  }

  private url(id?: string) {
    return [this.base, this.collection, id].filter(x => !!x).join('/');
  }
}

[jonstorer]

is there a way to make const cacheBuster$ = new Subject<void>(); specific to each instance of each child service?

[RobARichardson]

I ran into this exact same situation about a week ago. A solution to this would make this library even more amazing.

[angelnikolov]

Hey, thanks for your question. Will take a look after work :)

[angelnikolov]

@jonstorer the issue you are experiencing is due to the fact that the decorators are executed immediately, once your class is parsed. Therefore, you will really have only one cache storage, which will be for your Repository class. In short, each method is cached by combining the class constructor name + the property key (method name), unless you pass cacheKey as a custom config. Obviously none of those options work for you, so there are two solutions to your problem.

Option 1

Use a decorator mixin (I have used that in a project of mine and it works well)

import {PCacheable, PCacheBuster} from 'ngx-cacheable';
import {Subject} from 'rxjs';
import {Duration} from '@app/enums/duration.enum';
/**
 * A base mixin decorator which will cache to our default service methods like getById, query and etc
 */
export function BaseApiCache(
  serviceName: string,
  cacheDuration: number = Duration.FIVE_MINUTES,
  cacheBusterSubject: Subject<any> = new Subject(),
  excludeMethods: string[] = []
): Function {
  /**
   * A mixin function which will take all properties of BaseApiCache 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 BaseApiCache extends Api {
      cacheBuster = cacheBusterSubject;
      @PCacheable({
        cacheKey: serviceName + '#getById',
        maxAge: cacheDuration,
        cacheBusterObserver: cacheBusterSubject.asObservable()
      })
      public getById<TParams, TData>(
        id: string,
        asset?: string,
        params?: TParams,
        subEntities?: string
      ): Promise<TData> {
        return super.getById(id, asset, params, subEntities);
      }

      @PCacheable({
        cacheKey: serviceName + '#query',
        maxCacheCount: 5,
        maxAge: cacheDuration,
        cacheBusterObserver: cacheBusterSubject.asObservable()
      })
      public query<TParams, TData>(params?: TParams, asset?: string, subEntities?: string): Promise<TData> {
        return super.query<TParams, TData>(params, asset, subEntities);
      }

      @PCacheable({
        cacheKey: serviceName + '#count',
        maxAge: cacheDuration,
        cacheBusterObserver: cacheBusterSubject.asObservable()
      })
      public count(
        params: any
      ): Promise<number> {
        return super.count(params);
      }

      @PCacheBuster({
        cacheBusterNotifier: cacheBusterSubject
      })
      create<TData, TResponse>(data: TData, asset?: string): Promise<TResponse> {
        return super.create<TData, TResponse>(data, asset);
      }

      @PCacheBuster({
        cacheBusterNotifier: cacheBusterSubject
      })
      public upsert<TData, TResponse>(
        id: string,
        data: TData,
        asset?: string,
        isMultipart?: boolean,
        subEntities?: string
      ): Promise<TResponse> {
        return super.upsert<TData, TResponse>(id, data, asset, isMultipart, subEntities);
      }

      @PCacheBuster({
        cacheBusterNotifier: cacheBusterSubject
      })
      public update<TData, TResponse>(id: string, data: Partial<TData>, asset?: string, subEntities?: string): Promise<TResponse> {
        return super.update<TData, TResponse>(id, data, asset, subEntities);
      }

      @PCacheBuster({
        cacheBusterNotifier: cacheBusterSubject
      })
      public remove(id: string, asset?: string, subEntities?: string): Promise<void> {
        return super.remove(id, asset, subEntities);
      }
    }
    const fieldCollector = {};
    (BaseApiCache as Function).apply(fieldCollector);
    Object.getOwnPropertyNames(fieldCollector).forEach((name) => {
      derivedCtor.prototype[name] = fieldCollector[name];
    });

    Object.getOwnPropertyNames(BaseApiCache.prototype).forEach((name) => {
      if (name !== 'constructor' && excludeMethods.indexOf(name) === -1) {
        derivedCtor.prototype[name] = BaseApiCache.prototype[name];
      }
    });
  };
}

And then use it like:

/**
 * Make API calls to Parts endpoint.
 */
@BaseApiCache('partService', 1000000)
@Injectable()
export class PartService {
}

Option 2

I can add a cache config callback option of something like: cacheKeyCallback which you can add to each Repository class method and will be called everytime, that method is called. In it, you can derive a unique cache key based on the service which the method is called from. So you can constructor a new key, which will essentially be somethign like PartService#find. What do you think about that?

[angelnikolov]

Scratch that, option 2 is not really an option since there will be no way to subscribe to cache busters.

[jonstorer]

@angelnikolov is there anyway to reach into the method and pay attention to the route being called? that could be used as the cache key.

[angelnikolov]

Well, it actually is looking into the route being called. Let's say you have this Repository service method:

  @Cacheable({
    cacheBusterObserver: cacheBuster$
  })
  public findById(id: string): Observable<T> {
    return this.http.get<T>(this.url(id));
  }

Then you call it through the WidgetService child by this.widgetService.findById('1'). This will essentially always go through the same Repository service method through the prototype chain. Therefore, there will only be 1 cache storage. For every different service call, it will remove the previous cache and store a new one. Say you first call the widgetService.findById and then call partsService.findById. The produced url will be different, so the cache for the widgetService call will be evicted by the call of the parstService's method. The way to resolve that is either to use my proposal here or increate the maxCacheCount of the base Repository class to a larger number, so all your method calls are cached.

[jonstorer]

in that example, the paths resolve to /widgets/1 and /parts/1. Does that make a difference? Based on what you're saying, the cache keys would be different.

edit: they actually resolve to full urls. https://api.example.com/widgets/1 & https://api.example.com/parts/1

[angelnikolov]

@jonstorer Yes, it would be different, but since you are using a single service (Base repository service) and a single method (findById), all your calls will go to the same cache storage and the second call's cache will override the first one's. If you add maxCacheCount: 100 you will store a hundred of responses in the cache of RepositoryService#findById. The other option is to use a mixin as described above, which will apply the decorator dynamically, to each service you want. Then you won't have this issue.

[angelnikolov]

@jonstorer Anything to add here?

[jonstorer]

@angelnikolov I think there's an opportunity to identify your cache key further down in execution. I've not spent the proper time to research this, however, you have access to oldMethod which should be the specific method on the specific subclass. Again, I've not done my research, but won't that evaluate to the correct class name and method name?

`${oldMethod.constructor.name}/${oldMethod.name}

I will investigate this more. However, it won't be until next week sometime as I'm slammed this week

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.