ObjectCacheService docs & tests

src/app/core/cache/object-cache.service.spec.ts

+import { ObjectCacheState, CacheableObject } from "./object-cache.reducer";
+import { Store } from "@ngrx/store";
+import { ObjectCacheService } from "./object-cache.service";
+import { AddToObjectCacheAction, RemoveFromObjectCacheAction } from "./object-cache.actions";
+import { Observable } from "rxjs";
+
+class TestClass implements CacheableObject {
+  constructor(
+    public uuid: string,
+    public foo: string
+  ) {}
+
+  test(): string {
+    return this.foo + this.uuid;
+  }
+}
+
+describe("ObjectCacheService", () => {
+  let service: ObjectCacheService;
+  let store: Store<ObjectCacheState>;
+
+  const uuid = '1698f1d3-be98-4c51-9fd8-6bfedcbd59b7';
+  const timestamp = new Date().getTime();
+  const msToLive = 900000;
+  const objectToCache = {
+    uuid: uuid,
+    foo: 'bar'
+  };
+  const cacheEntry = {
+    data: objectToCache,
+    timeAdded: timestamp,
+    msToLive: msToLive
+  };
+  const invalidCacheEntry = Object.assign({}, cacheEntry, { msToLive: -1 });
+
+  beforeEach(() => {
+    store = new Store<ObjectCacheState>(undefined, undefined, undefined);
+    spyOn(store, 'dispatch');
+    service = new ObjectCacheService(store);
+
+    spyOn(window, 'Date').and.returnValue({ getTime: () => timestamp });
+  });
+
+  describe("add", () => {
+    it("should dispatch an ADD action with the object to add, the time to live, and the current timestamp", () => {
+
+      service.add(objectToCache, msToLive);
+      expect(store.dispatch).toHaveBeenCalledWith(new AddToObjectCacheAction(objectToCache, timestamp, msToLive));
+    });
+  });
+
+  describe("remove", () => {
+    it("should dispatch a REMOVE action with the UUID of the object to remove", () => {
+      service.remove(uuid);
+      expect(store.dispatch).toHaveBeenCalledWith(new RemoveFromObjectCacheAction(uuid));
+    });
+  });
+
+  describe("get", () => {
+    it("should return an observable of the cached object with the specified UUID and type", () => {
+      spyOn(store, 'select').and.returnValue(Observable.of(cacheEntry));
+
+      let testObj: any;
+      //due to the implementation of spyOn above, this subscribe will be synchronous
+      service.get(uuid, TestClass).take(1).subscribe(o => testObj = o);
+      expect(testObj.uuid).toBe(uuid);
+      expect(testObj.foo).toBe("bar");
+      // this only works if testObj is an instance of TestClass
+      expect(testObj.test()).toBe("bar" + uuid);
+    });
+
+    it("should not return a cached object that has exceeded its time to live", () => {
+      spyOn(store, 'select').and.returnValue(Observable.of(invalidCacheEntry));
+
+      let testObj: any;
+      service.get(uuid, TestClass).take(1).subscribe(o => testObj = o);
+      expect(testObj).toBeUndefined();
+    });
+  });
+
+  describe("getList", () => {
+    it("should return an observable of the array of cached objects with the specified UUID and type", () => {
+      spyOn(service, 'get').and.returnValue(Observable.of(new TestClass(uuid, "bar")));
+
+      let testObjs: Array<any>;
+      service.getList([uuid, uuid], TestClass).take(1).subscribe(arr => testObjs = arr);
+      expect(testObjs[0].uuid).toBe(uuid);
+      expect(testObjs[0].foo).toBe("bar");
+      expect(testObjs[0].test()).toBe("bar" + uuid);
+      expect(testObjs[1].uuid).toBe(uuid);
+      expect(testObjs[1].foo).toBe("bar");
+      expect(testObjs[1].test()).toBe("bar" + uuid);
+    });
+  });
+
+  describe("has", () => {
+    it("should return true if the object with the supplied UUID is cached and still valid", () => {
+      spyOn(store, 'select').and.returnValue(Observable.of(cacheEntry));
+      expect(service.has(uuid)).toBe(true);
+    });
+
+    it("should return false if the object with the supplied UUID isn't cached", () => {
+      spyOn(store, 'select').and.returnValue(Observable.of(undefined));
+      expect(service.has(uuid)).toBe(false);
+    });
+
+    it("should return false if the object with the supplied UUID is cached but has exceeded its time to live", () => {
+      spyOn(store, 'select').and.returnValue(Observable.of(invalidCacheEntry));
+      expect(service.has(uuid)).toBe(false);
+    });
+  });
+});

src/app/core/cache/object-cache.service.ts

@@ -6,33 +6,93 @@ import { Observable } from "rxjs";
 import { hasNoValue } from "../../shared/empty.util";
 import { GenericConstructor } from "../shared/generic-constructor";
 
+/**
+ * A service to interact with the object cache
+ */
 @Injectable()
 export class ObjectCacheService {
   constructor(
     private store: Store<ObjectCacheState>
   ) {}
 
+  /**
+   * Add an object to the cache
+   *
+   * @param objectToCache
+   *    The object to add
+   * @param msToLive
+   *    The number of milliseconds it should be cached for
+   */
   add(objectToCache: CacheableObject, msToLive: number): void {
     this.store.dispatch(new AddToObjectCacheAction(objectToCache, new Date().getTime(), msToLive));
   }
 
+  /**
+   * Remove the object with the supplied UUID from the cache
+   *
+   * @param uuid
+   *    The UUID of the object to be removed
+   */
   remove(uuid: string): void {
     this.store.dispatch(new RemoveFromObjectCacheAction(uuid));
   }
 
-  get<T extends CacheableObject>(uuid: string, ctor: GenericConstructor<T>): Observable<T> {
+  /**
+   * Get an observable of the object with the specified UUID
+   *
+   * The type needs to be specified as well, in order to turn
+   * the cached plain javascript object in to an instance of
+   * a class.
+   *
+   * e.g. get('c96588c6-72d3-425d-9d47-fa896255a695', Item)
+   *
+   * @param uuid
+   *    The UUID of the object to get
+   * @param type
+   *    The type of the object to get
+   * @return Observable<T>
+   */
+  get<T extends CacheableObject>(uuid: string, type: GenericConstructor<T>): Observable<T> {
     return this.store.select<ObjectCacheEntry>('core', 'cache', 'object', uuid)
       .filter(entry => this.isValid(entry))
       .distinctUntilChanged()
-      .map((entry: ObjectCacheEntry) => <T> Object.assign(new ctor(), entry.data));
+      .map((entry: ObjectCacheEntry) => <T> Object.assign(new type(), entry.data));
   }
 
-  getList<T extends CacheableObject>(uuids: Array<string>, ctor: GenericConstructor<T>): Observable<Array<T>> {
+  /**
+   * Get an observable for an array of objects of the same type
+   * with the specified UUIDs
+   *
+   * The type needs to be specified as well, in order to turn
+   * the cached plain javascript object in to an instance of
+   * a class.
+   *
+   * e.g. getList([
+   *        'c96588c6-72d3-425d-9d47-fa896255a695',
+   *        'cff860da-cf5f-4fda-b8c9-afb7ec0b2d9e'
+   *      ], Collection)
+   *
+   * @param uuids
+   *    An array of UUIDs of the objects to get
+   * @param type
+   *    The type of the objects to get
+   * @return Observable<Array<T>>
+   */
+  getList<T extends CacheableObject>(uuids: Array<string>, type: GenericConstructor<T>): Observable<Array<T>> {
     return Observable.combineLatest(
-      uuids.map((id: string) => this.get<T>(id, ctor))
+      uuids.map((id: string) => this.get<T>(id, type))
     );
   }
 
+  /**
+   * Check whether the object with the specified UUID is cached
+   *
+   * @param uuid
+   *    The UUID of the object to check
+   * @return boolean
+   *    true if the object with the specified UUID is cached,
+   *    false otherwise
+   */
   has(uuid: string): boolean {
     let result: boolean;
 
@@ -43,6 +103,15 @@ export class ObjectCacheService {
     return result;
   }
 
+  /**
+   * Check whether an ObjectCacheEntry should still be cached
+   *
+   * @param entry
+   *    the entry to check
+   * @return boolean
+   *    false if the entry is null, undefined, or its time to
+   *    live has been exceeded, true otherwise
+   */
   private isValid(entry: ObjectCacheEntry): boolean {
     if (hasNoValue(entry)) {
       return false;

src/app/core/data-services/data.effects.ts

@@ -11,6 +11,7 @@ import {
   RequestCacheErrorAction, RequestCacheFindByIDAction
 } from "../cache/request-cache.actions";
 import { DataService } from "./data.service";
+import { hasNoValue } from "../../shared/empty.util";
 
 export abstract class DataEffects<T extends CacheableObject> {
   protected abstract getFindAllEndpoint(action: RequestCacheFindAllAction): string;
@@ -34,12 +35,15 @@ export abstract class DataEffects<T extends CacheableObject> {
         .map((data: DSpaceRESTV2Response) => this.getSerializer().deserializeArray(data))
         .do((ts: T[]) => {
           ts.forEach((t) => {
+            if (hasNoValue(t) || hasNoValue(t.uuid)) {
+              throw new Error('The server returned an invalid object');
+            }
             this.objectCache.add(t, GlobalConfig.cache.msToLive);
           });
         })
         .map((ts: Array<T>) => ts.map(t => t.uuid))
         .map((ids: Array<string>) => new RequestCacheSuccessAction(action.payload.key, ids, new Date().getTime(), GlobalConfig.cache.msToLive))
-        .catch((errorMsg: string) => Observable.of(new RequestCacheErrorAction(action.payload.key, errorMsg)));
+        .catch((error: Error) => Observable.of(new RequestCacheErrorAction(action.payload.key, error.message)));
     });
 
   protected findById = this.actions$
@@ -49,10 +53,13 @@ export abstract class DataEffects<T extends CacheableObject> {
       return this.restApi.get(this.getFindByIdEndpoint(action))
         .map((data: DSpaceRESTV2Response) => this.getSerializer().deserialize(data))
         .do((t: T) => {
+          if (hasNoValue(t) || hasNoValue(t.uuid)) {
+            throw new Error('The server returned an invalid object');
+          }
           this.objectCache.add(t, GlobalConfig.cache.msToLive);
         })
         .map((t: T) => new RequestCacheSuccessAction(action.payload.key, [t.uuid], new Date().getTime(), GlobalConfig.cache.msToLive))
-        .catch((errorMsg: string) => Observable.of(new RequestCacheErrorAction(action.payload.key, errorMsg)));
+        .catch((error: Error) => Observable.of(new RequestCacheErrorAction(action.payload.key, error.message)));
     });
 
 }