diff --git a/src/app/core/cache/object-cache.service.spec.ts b/src/app/core/cache/object-cache.service.spec.ts new file mode 100644 index 0000000000..a05d6fdbef --- /dev/null +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -0,0 +1,112 @@ +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; + + 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(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; + 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); + }); + }); +}); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index c9d3bccf83..6cabbfd91d 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/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 ) {} + /** + * 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(uuid: string, ctor: GenericConstructor): Observable { + /** + * 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 + */ + get(uuid: string, type: GenericConstructor): Observable { return this.store.select('core', 'cache', 'object', uuid) .filter(entry => this.isValid(entry)) .distinctUntilChanged() - .map((entry: ObjectCacheEntry) => Object.assign(new ctor(), entry.data)); + .map((entry: ObjectCacheEntry) => Object.assign(new type(), entry.data)); } - getList(uuids: Array, ctor: GenericConstructor): Observable> { + /** + * 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> + */ + getList(uuids: Array, type: GenericConstructor): Observable> { return Observable.combineLatest( - uuids.map((id: string) => this.get(id, ctor)) + uuids.map((id: string) => this.get(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; diff --git a/src/app/core/data-services/data.effects.ts b/src/app/core/data-services/data.effects.ts index c39a718732..a68f7f2899 100644 --- a/src/app/core/data-services/data.effects.ts +++ b/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 { protected abstract getFindAllEndpoint(action: RequestCacheFindAllAction): string; @@ -34,12 +35,15 @@ export abstract class DataEffects { .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) => ts.map(t => t.uuid)) .map((ids: Array) => 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 { 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))); }); }