From 0d4b064541b865c70789516e63b73a76a338b8fd Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 15 Sep 2017 18:02:29 +0200 Subject: [PATCH 1/4] switched to self links as keys in the object cache --- .../builders/remote-data-build.service.ts | 10 +-- .../core/cache/object-cache.reducer.spec.ts | 46 ++++++------- src/app/core/cache/object-cache.reducer.ts | 12 ++-- .../core/cache/object-cache.service.spec.ts | 57 ++++++++-------- src/app/core/cache/object-cache.service.ts | 68 +++++++++---------- src/app/core/cache/response-cache.models.ts | 2 +- src/app/core/core.effects.ts | 4 +- src/app/core/core.reducers.ts | 6 +- src/app/core/data/data.service.ts | 12 +++- src/app/core/data/request.actions.ts | 4 +- src/app/core/data/request.effects.ts | 6 +- src/app/core/data/request.models.ts | 6 +- src/app/core/data/request.reducer.ts | 2 +- src/app/core/data/request.service.ts | 6 +- src/app/core/index/href-index.actions.ts | 60 ---------------- src/app/core/index/href-index.reducer.ts | 46 ------------- src/app/core/index/uuid-index.actions.ts | 60 ++++++++++++++++ ...index.effects.ts => uuid-index.effects.ts} | 14 ++-- src/app/core/index/uuid-index.reducer.ts | 46 +++++++++++++ 19 files changed, 236 insertions(+), 231 deletions(-) delete mode 100644 src/app/core/index/href-index.actions.ts delete mode 100644 src/app/core/index/href-index.reducer.ts create mode 100644 src/app/core/index/uuid-index.actions.ts rename src/app/core/index/{href-index.effects.ts => uuid-index.effects.ts} (66%) create mode 100644 src/app/core/index/uuid-index.reducer.ts diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index d01959d3dd..727eef41d4 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -70,10 +70,10 @@ export class RemoteDataBuildService { this.objectCache.getBySelfLink(href, normalizedType).startWith(undefined), responseCacheObs .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceUUIDs) - .flatMap((resourceUUIDs: string[]) => { - if (isNotEmpty(resourceUUIDs)) { - return this.objectCache.get(resourceUUIDs[0], normalizedType); + .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) + .flatMap((resourceSelfLinks: string[]) => { + if (isNotEmpty(resourceSelfLinks)) { + return this.objectCache.getBySelfLink(resourceSelfLinks[0], normalizedType); } else { return Observable.of(undefined); } @@ -137,7 +137,7 @@ export class RemoteDataBuildService { const payload = responseCacheObs .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceUUIDs) + .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) .flatMap((resourceUUIDs: string[]) => { return this.objectCache.getList(resourceUUIDs, normalizedType) .map((normList: TNormalized[]) => { diff --git a/src/app/core/cache/object-cache.reducer.spec.ts b/src/app/core/cache/object-cache.reducer.spec.ts index d02e1faab7..2c059c4dd3 100644 --- a/src/app/core/cache/object-cache.reducer.spec.ts +++ b/src/app/core/cache/object-cache.reducer.spec.ts @@ -16,26 +16,26 @@ class NullAction extends RemoveFromObjectCacheAction { } describe('objectCacheReducer', () => { - const uuid1 = '1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; - const uuid2 = '28b04544-1766-4e82-9728-c4e93544ecd3'; + const selfLink1 = 'https://localhost:8080/api/core/items/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; + const selfLink2 = 'https://localhost:8080/api/core/items/28b04544-1766-4e82-9728-c4e93544ecd3'; const testState = { - [uuid1]: { + [selfLink1]: { data: { - uuid: uuid1, + self: selfLink1, foo: 'bar' }, timeAdded: new Date().getTime(), msToLive: 900000, - requestHref: 'https://rest.api/endpoint/uuid1' + requestHref: selfLink1 }, - [uuid2]: { + [selfLink2]: { data: { - uuid: uuid2, + self: selfLink2, foo: 'baz' }, timeAdded: new Date().getTime(), msToLive: 900000, - requestHref: 'https://rest.api/endpoint/uuid2' + requestHref: selfLink2 } }; deepFreeze(testState); @@ -56,38 +56,38 @@ describe('objectCacheReducer', () => { it('should add the payload to the cache in response to an ADD action', () => { const state = Object.create(null); - const objectToCache = { uuid: uuid1 }; + const objectToCache = { self: selfLink1 }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/uuid1'; + const requestHref = 'https://rest.api/endpoint/selfLink1'; const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); const newState = objectCacheReducer(state, action); - expect(newState[uuid1].data).toEqual(objectToCache); - expect(newState[uuid1].timeAdded).toEqual(timeAdded); - expect(newState[uuid1].msToLive).toEqual(msToLive); + expect(newState[selfLink1].data).toEqual(objectToCache); + expect(newState[selfLink1].timeAdded).toEqual(timeAdded); + expect(newState[selfLink1].msToLive).toEqual(msToLive); }); it('should overwrite an object in the cache in response to an ADD action if it already exists', () => { - const objectToCache = { uuid: uuid1, foo: 'baz', somethingElse: true }; + const objectToCache = { self: selfLink1, foo: 'baz', somethingElse: true }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/uuid1'; + const requestHref = 'https://rest.api/endpoint/selfLink1'; const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); const newState = objectCacheReducer(testState, action); /* tslint:disable:no-string-literal */ - expect(newState[uuid1].data['foo']).toBe('baz'); - expect(newState[uuid1].data['somethingElse']).toBe(true); + expect(newState[selfLink1].data['foo']).toBe('baz'); + expect(newState[selfLink1].data['somethingElse']).toBe(true); /* tslint:enable:no-string-literal */ }); it('should perform the ADD action without affecting the previous state', () => { const state = Object.create(null); - const objectToCache = { uuid: uuid1 }; + const objectToCache = { self: selfLink1 }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/uuid1'; + const requestHref = 'https://rest.api/endpoint/selfLink1'; const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); deepFreeze(state); @@ -95,11 +95,11 @@ describe('objectCacheReducer', () => { }); it('should remove the specified object from the cache in response to the REMOVE action', () => { - const action = new RemoveFromObjectCacheAction(uuid1); + const action = new RemoveFromObjectCacheAction(selfLink1); const newState = objectCacheReducer(testState, action); - expect(testState[uuid1]).not.toBeUndefined(); - expect(newState[uuid1]).toBeUndefined(); + expect(testState[selfLink1]).not.toBeUndefined(); + expect(newState[selfLink1]).toBeUndefined(); }); it("shouldn't do anything in response to the REMOVE action for an object that isn't cached", () => { @@ -112,7 +112,7 @@ describe('objectCacheReducer', () => { }); it('should perform the REMOVE action without affecting the previous state', () => { - const action = new RemoveFromObjectCacheAction(uuid1); + const action = new RemoveFromObjectCacheAction(selfLink1); // testState has already been frozen above objectCacheReducer(testState, action); }); diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index 494db4c34d..3af7209b24 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -8,11 +8,11 @@ import { CacheEntry } from './cache-entry'; /** * An interface to represent objects that can be cached * - * A cacheable object should have a uuid + * A cacheable object should have a self link */ export interface CacheableObject { - uuid: string; - self?: string; + uuid?: string; + self: string; } /** @@ -28,11 +28,11 @@ export class ObjectCacheEntry implements CacheEntry { /** * The ObjectCache State * - * Consists of a map with UUIDs as keys, + * Consists of a map with self links as keys, * and ObjectCacheEntries as values */ export interface ObjectCacheState { - [uuid: string]: ObjectCacheEntry + [href: string]: ObjectCacheEntry } // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) @@ -81,7 +81,7 @@ export function objectCacheReducer(state = initialState, action: ObjectCacheActi */ function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheAction): ObjectCacheState { return Object.assign({}, state, { - [action.payload.objectToCache.uuid]: { + [action.payload.objectToCache.self]: { data: action.payload.objectToCache, timeAdded: action.payload.timeAdded, msToLive: action.payload.msToLive, diff --git a/src/app/core/cache/object-cache.service.spec.ts b/src/app/core/cache/object-cache.service.spec.ts index 54b49d3cf6..2cf7eebd0a 100644 --- a/src/app/core/cache/object-cache.service.spec.ts +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -8,12 +8,12 @@ import { CoreState } from '../core.reducers'; class TestClass implements CacheableObject { constructor( - public uuid: string, + public self: string, public foo: string ) { } test(): string { - return this.foo + this.uuid; + return this.foo + this.self; } } @@ -21,12 +21,11 @@ describe('ObjectCacheService', () => { let service: ObjectCacheService; let store: Store; - const uuid = '1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; - const requestHref = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; + const selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; const timestamp = new Date().getTime(); const msToLive = 900000; const objectToCache = { - uuid: uuid, + self: selfLink, foo: 'bar' }; const cacheEntry = { @@ -48,73 +47,73 @@ describe('ObjectCacheService', () => { 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, requestHref); - expect(store.dispatch).toHaveBeenCalledWith(new AddToObjectCacheAction(objectToCache, timestamp, msToLive, requestHref)); + service.add(objectToCache, msToLive, selfLink); + expect(store.dispatch).toHaveBeenCalledWith(new AddToObjectCacheAction(objectToCache, timestamp, msToLive, selfLink)); }); }); 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)); + it('should dispatch a REMOVE action with the self link of the object to remove', () => { + service.remove(selfLink); + expect(store.dispatch).toHaveBeenCalledWith(new RemoveFromObjectCacheAction(selfLink)); }); }); - describe('get', () => { - it('should return an observable of the cached object with the specified UUID and type', () => { + describe('getBySelfLink', () => { + it('should return an observable of the cached object with the specified self link 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); + service.getBySelfLink(selfLink, TestClass).take(1).subscribe((o) => testObj = o); + expect(testObj.self).toBe(selfLink); expect(testObj.foo).toBe('bar'); // this only works if testObj is an instance of TestClass - expect(testObj.test()).toBe('bar' + uuid); + expect(testObj.test()).toBe('bar' + selfLink); }); it('should not return a cached object that has exceeded its time to live', () => { spyOn(store, 'select').and.returnValue(Observable.of(invalidCacheEntry)); let getObsHasFired = false; - const subscription = service.get(uuid, TestClass).subscribe((o) => getObsHasFired = true); + const subscription = service.getBySelfLink(selfLink, TestClass).subscribe((o) => getObsHasFired = true); expect(getObsHasFired).toBe(false); subscription.unsubscribe(); }); }); 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'))); + it('should return an observable of the array of cached objects with the specified self link and type', () => { + spyOn(service, 'getBySelfLink').and.returnValue(Observable.of(new TestClass(selfLink, 'bar'))); let testObjs: any[]; - service.getList([uuid, uuid], TestClass).take(1).subscribe((arr) => testObjs = arr); - expect(testObjs[0].uuid).toBe(uuid); + service.getList([selfLink, selfLink], TestClass).take(1).subscribe((arr) => testObjs = arr); + expect(testObjs[0].self).toBe(selfLink); expect(testObjs[0].foo).toBe('bar'); - expect(testObjs[0].test()).toBe('bar' + uuid); - expect(testObjs[1].uuid).toBe(uuid); + expect(testObjs[0].test()).toBe('bar' + selfLink); + expect(testObjs[1].self).toBe(selfLink); expect(testObjs[1].foo).toBe('bar'); - expect(testObjs[1].test()).toBe('bar' + uuid); + expect(testObjs[1].test()).toBe('bar' + selfLink); }); }); describe('has', () => { - it('should return true if the object with the supplied UUID is cached and still valid', () => { + it('should return true if the object with the supplied self link is cached and still valid', () => { spyOn(store, 'select').and.returnValue(Observable.of(cacheEntry)); - expect(service.has(uuid)).toBe(true); + expect(service.hasBySelfLink(selfLink)).toBe(true); }); - it("should return false if the object with the supplied UUID isn't cached", () => { + it("should return false if the object with the supplied self link isn't cached", () => { spyOn(store, 'select').and.returnValue(Observable.of(undefined)); - expect(service.has(uuid)).toBe(false); + expect(service.hasBySelfLink(selfLink)).toBe(false); }); - it('should return false if the object with the supplied UUID is cached but has exceeded its time to live', () => { + it('should return false if the object with the supplied self link is cached but has exceeded its time to live', () => { spyOn(store, 'select').and.returnValue(Observable.of(invalidCacheEntry)); - expect(service.has(uuid)).toBe(false); + expect(service.hasBySelfLink(selfLink)).toBe(false); }); }); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 0755b268cf..31eb2d0b6a 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -10,12 +10,12 @@ import { GenericConstructor } from '../shared/generic-constructor'; import { CoreState } from '../core.reducers'; import { keySelector } from '../shared/selectors'; -function objectFromUuidSelector(uuid: string): MemoizedSelector { - return keySelector('data/object', uuid); +function selfLinkFromUuidSelector(uuid: string): MemoizedSelector { + return keySelector('index/uuid', uuid); } -function uuidFromHrefSelector(href: string): MemoizedSelector { - return keySelector('index/href', href); +function entryFromSelfLinkSelector(selfLink: string): MemoizedSelector { + return keySelector('data/object', selfLink); } /** @@ -35,7 +35,7 @@ export class ObjectCacheService { * @param msToLive * The number of milliseconds it should be cached for * @param requestHref - * The href of the request that resulted in this object + * The selfLink of the request that resulted in this object * This isn't necessarily the same as the object's self * link, it could have been part of a list for example */ @@ -69,55 +69,55 @@ export class ObjectCacheService { * @return Observable * An observable of the requested object */ - get(uuid: string, type: GenericConstructor): Observable { - return this.getEntry(uuid) + getByUUID(uuid: string, type: GenericConstructor): Observable { + return this.store.select(selfLinkFromUuidSelector(uuid)) + .flatMap((selfLink: string) => this.getBySelfLink(selfLink, type)) + } + + getBySelfLink(selfLink: string, type: GenericConstructor): Observable { + return this.getEntry(selfLink) .map((entry: ObjectCacheEntry) => Object.assign(new type(), entry.data) as T); } - getBySelfLink(href: string, type: GenericConstructor): Observable { - return this.store.select(uuidFromHrefSelector(href)) - .flatMap((uuid: string) => this.get(uuid, type)) - } - - private getEntry(uuid: string): Observable { - return this.store.select(objectFromUuidSelector(uuid)) + private getEntry(selfLink: string): Observable { + return this.store.select(entryFromSelfLinkSelector(selfLink)) .filter((entry) => this.isValid(entry)) .distinctUntilChanged(); } - getRequestHref(uuid: string): Observable { - return this.getEntry(uuid) + getRequestHrefBySelfLink(selfLink: string): Observable { + return this.getEntry(selfLink) .map((entry: ObjectCacheEntry) => entry.requestHref) .distinctUntilChanged(); } - getRequestHrefBySelfLink(self: string): Observable { - return this.store.select(uuidFromHrefSelector(self)) - .flatMap((uuid: string) => this.getRequestHref(uuid)); + getRequestHrefByUUID(uuid: string): Observable { + return this.store.select(selfLinkFromUuidSelector(uuid)) + .flatMap((selfLink: string) => this.getRequestHrefBySelfLink(selfLink)); } /** * Get an observable for an array of objects of the same type - * with the specified UUIDs + * with the specified self links * * 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' + * 'http://localhost:8080/api/core/collections/c96588c6-72d3-425d-9d47-fa896255a695', + * 'http://localhost:8080/api/core/collections/cff860da-cf5f-4fda-b8c9-afb7ec0b2d9e' * ], Collection) * - * @param uuids - * An array of UUIDs of the objects to get + * @param selfLinks + * An array of self links of the objects to get * @param type * The type of the objects to get * @return Observable> */ - getList(uuids: string[], type: GenericConstructor): Observable { + getList(selfLinks: string[], type: GenericConstructor): Observable { return Observable.combineLatest( - uuids.map((id: string) => this.get(id, type)) + selfLinks.map((selfLink: string) => this.getBySelfLink(selfLink, type)) ); } @@ -130,12 +130,12 @@ export class ObjectCacheService { * true if the object with the specified UUID is cached, * false otherwise */ - has(uuid: string): boolean { + hasByUUID(uuid: string): boolean { let result: boolean; - this.store.select(objectFromUuidSelector(uuid)) + this.store.select(selfLinkFromUuidSelector(uuid)) .take(1) - .subscribe((entry) => result = this.isValid(entry)); + .subscribe((selfLink: string) => result = this.hasBySelfLink(selfLink)); return result; } @@ -143,18 +143,18 @@ export class ObjectCacheService { /** * Check whether the object with the specified self link is cached * - * @param href + * @param selfLink * The self link of the object to check * @return boolean * true if the object with the specified self link is cached, * false otherwise */ - hasBySelfLink(href: string): boolean { + hasBySelfLink(selfLink: string): boolean { let result = false; - this.store.select(uuidFromHrefSelector(href)) + this.store.select(entryFromSelfLinkSelector(selfLink)) .take(1) - .subscribe((uuid: string) => result = this.has(uuid)); + .subscribe((entry: ObjectCacheEntry) => result = this.isValid(entry)); return result; } @@ -175,7 +175,7 @@ export class ObjectCacheService { const timeOutdated = entry.timeAdded + entry.msToLive; const isOutDated = new Date().getTime() > timeOutdated; if (isOutDated) { - this.store.dispatch(new RemoveFromObjectCacheAction(entry.data.uuid)); + this.store.dispatch(new RemoveFromObjectCacheAction(entry.data.self)); } return !isOutDated; } diff --git a/src/app/core/cache/response-cache.models.ts b/src/app/core/cache/response-cache.models.ts index ef1bfb0925..a860d682bf 100644 --- a/src/app/core/cache/response-cache.models.ts +++ b/src/app/core/cache/response-cache.models.ts @@ -11,7 +11,7 @@ export class Response { export class SuccessResponse extends Response { constructor( - public resourceUUIDs: string[], + public resourceSelfLinks: string[], public statusCode: string, public pageInfo?: PageInfo ) { diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index 51a2cb7e06..593b17e0f5 100644 --- a/src/app/core/core.effects.ts +++ b/src/app/core/core.effects.ts @@ -1,12 +1,12 @@ import { ObjectCacheEffects } from './data/object-cache.effects'; import { RequestCacheEffects } from './data/request-cache.effects'; -import { HrefIndexEffects } from './index/href-index.effects'; +import { UUIDIndexEffects } from './index/uuid-index.effects'; import { RequestEffects } from './data/request.effects'; export const coreEffects = [ RequestCacheEffects, RequestEffects, ObjectCacheEffects, - HrefIndexEffects, + UUIDIndexEffects, ]; diff --git a/src/app/core/core.reducers.ts b/src/app/core/core.reducers.ts index 29959f7b43..493c9e96d9 100644 --- a/src/app/core/core.reducers.ts +++ b/src/app/core/core.reducers.ts @@ -2,21 +2,21 @@ import { ActionReducerMap, createFeatureSelector } from '@ngrx/store'; import { responseCacheReducer, ResponseCacheState } from './cache/response-cache.reducer'; import { objectCacheReducer, ObjectCacheState } from './cache/object-cache.reducer'; -import { hrefIndexReducer, HrefIndexState } from './index/href-index.reducer'; +import { uuidIndexReducer, UUIDIndexState } from './index/uuid-index.reducer'; import { requestReducer, RequestState } from './data/request.reducer'; export interface CoreState { 'data/object': ObjectCacheState, 'data/response': ResponseCacheState, 'data/request': RequestState, - 'index/href': HrefIndexState + 'index/uuid': UUIDIndexState } export const coreReducers: ActionReducerMap = { 'data/object': objectCacheReducer, 'data/response': responseCacheReducer, 'data/request': requestReducer, - 'index/href': hrefIndexReducer + 'index/uuid': uuidIndexReducer }; export const coreSelector = createFeatureSelector('core'); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 56aac053b0..3d7a724709 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -5,14 +5,13 @@ import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { RemoteData } from './remote-data'; import { FindAllOptions, FindAllRequest, FindByIDRequest, Request } from './request.models'; import { Store } from '@ngrx/store'; -import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; import { CoreState } from '../core.reducers'; import { RequestService } from './request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { GenericConstructor } from '../shared/generic-constructor'; -import { Inject } from '@angular/core'; -import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; +import { GlobalConfig } from '../../../config'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; +import { Observable } from 'rxjs/Observable'; export abstract class DataService { protected abstract objectCache: ObjectCacheService; @@ -30,6 +29,13 @@ export abstract class DataService } + private getEndpoint(linkName: string): Observable { + const apiUrl = new RESTURLCombiner(this.EnvConfig, '/').toString(); + this.requestService.configure(new Request(apiUrl)); + // TODO fetch from store + return Observable.of(undefined); + } + protected getFindAllHref(options: FindAllOptions = {}): string { let result; const args = []; diff --git a/src/app/core/data/request.actions.ts b/src/app/core/data/request.actions.ts index 9747294c2d..b17046005d 100644 --- a/src/app/core/data/request.actions.ts +++ b/src/app/core/data/request.actions.ts @@ -15,10 +15,10 @@ export const RequestActionTypes = { /* tslint:disable:max-classes-per-file */ export class RequestConfigureAction implements Action { type = RequestActionTypes.CONFIGURE; - payload: Request; + payload: Request; constructor( - request: Request + request: Request ) { this.payload = request; } diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index df4205bf32..c5d7eb57f0 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -57,8 +57,8 @@ export class RequestEffects { return this.restApi.get(entry.request.href) .map((data: DSpaceRESTV2Response) => { const processRequestDTO = this.process(data.payload, entry.request.href); - const uuids = flattenSingleKeyObject(processRequestDTO).map((no) => no.uuid); - return new SuccessResponse(uuids, data.statusCode, this.processPageInfo(data.payload.page)) + const selfLinks = flattenSingleKeyObject(processRequestDTO).map((no) => no.self); + return new SuccessResponse(selfLinks, data.statusCode, this.processPageInfo(data.payload.page)) }).do((response: Response) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) .map((response: Response) => new RequestCompleteAction(entry.request.href)) .catch((error: RequestError) => Observable.of(new ErrorResponse(error)) @@ -157,7 +157,7 @@ export class RequestEffects { } protected addToObjectCache(co: CacheableObject, requestHref: string): void { - if (hasNoValue(co) || hasNoValue(co.uuid)) { + if (hasNoValue(co) || hasNoValue(co.self)) { throw new Error('The server returned an invalid object'); } this.objectCache.add(co, this.EnvConfig.cache.msToLive, requestHref); diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 47888a5e61..ec29fa1c5e 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -3,13 +3,13 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c import { GenericConstructor } from '../shared/generic-constructor'; /* tslint:disable:max-classes-per-file */ -export class Request { +export class Request { constructor( public href: string, ) { } } -export class FindByIDRequest extends Request { +export class FindByIDRequest extends Request { constructor( href: string, public resourceID: string @@ -25,7 +25,7 @@ export class FindAllOptions { sort?: SortOptions; } -export class FindAllRequest extends Request { +export class FindAllRequest extends Request { constructor( href: string, public options?: FindAllOptions, diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 6b84fbb77c..af7140bbf4 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -6,7 +6,7 @@ import { import { Request } from './request.models'; export class RequestEntry { - request: Request; + request: Request; requestPending: boolean; responsePending: boolean; completed: boolean; diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index f9f296a666..7b401ca03a 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -45,7 +45,7 @@ export class RequestService { return this.store.select(entryFromHrefSelector(href)); } - configure(request: Request): void { + configure(request: Request): void { let isCached = this.objectCache.hasBySelfLink(request.href); if (!isCached && this.responseCache.has(request.href)) { @@ -54,8 +54,8 @@ export class RequestService { this.responseCache.get(request.href) .take(1) .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceUUIDs) - .map((resourceUUIDs: string[]) => resourceUUIDs.every((uuid) => this.objectCache.has(uuid))) + .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) + .map((resourceSelfLinks: string[]) => resourceSelfLinks.every((selfLink) => this.objectCache.hasBySelfLink(selfLink))) .subscribe((c) => isCached = c); } diff --git a/src/app/core/index/href-index.actions.ts b/src/app/core/index/href-index.actions.ts deleted file mode 100644 index bf854abff7..0000000000 --- a/src/app/core/index/href-index.actions.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { Action } from '@ngrx/store'; - -import { type } from '../../shared/ngrx/type'; - -/** - * The list of HrefIndexAction type definitions - */ -export const HrefIndexActionTypes = { - ADD: type('dspace/core/index/href/ADD'), - REMOVE_UUID: type('dspace/core/index/href/REMOVE_UUID') -}; - -/* tslint:disable:max-classes-per-file */ -/** - * An ngrx action to add an href to the index - */ -export class AddToHrefIndexAction implements Action { - type = HrefIndexActionTypes.ADD; - payload: { - href: string; - uuid: string; - }; - - /** - * Create a new AddToHrefIndexAction - * - * @param href - * the href to add - * @param uuid - * the uuid of the resource the href links to - */ - constructor(href: string, uuid: string) { - this.payload = { href, uuid }; - } -} - -/** - * An ngrx action to remove an href from the index - */ -export class RemoveUUIDFromHrefIndexAction implements Action { - type = HrefIndexActionTypes.REMOVE_UUID; - payload: string; - - /** - * Create a new RemoveUUIDFromHrefIndexAction - * - * @param uuid - * the uuid to remove all hrefs for - */ - constructor(uuid: string) { - this.payload = uuid; - } - -} -/* tslint:enable:max-classes-per-file */ - -/** - * A type to encompass all HrefIndexActions - */ -export type HrefIndexAction = AddToHrefIndexAction | RemoveUUIDFromHrefIndexAction; diff --git a/src/app/core/index/href-index.reducer.ts b/src/app/core/index/href-index.reducer.ts deleted file mode 100644 index 160b04f5b3..0000000000 --- a/src/app/core/index/href-index.reducer.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { - HrefIndexAction, - HrefIndexActionTypes, - AddToHrefIndexAction, - RemoveUUIDFromHrefIndexAction -} from './href-index.actions'; - -export interface HrefIndexState { - [href: string]: string -} - -// Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) -const initialState: HrefIndexState = Object.create(null); - -export function hrefIndexReducer(state = initialState, action: HrefIndexAction): HrefIndexState { - switch (action.type) { - - case HrefIndexActionTypes.ADD: { - return addToHrefIndex(state, action as AddToHrefIndexAction); - } - - case HrefIndexActionTypes.REMOVE_UUID: { - return removeUUIDFromHrefIndex(state, action as RemoveUUIDFromHrefIndexAction) - } - - default: { - return state; - } - } -} - -function addToHrefIndex(state: HrefIndexState, action: AddToHrefIndexAction): HrefIndexState { - return Object.assign({}, state, { - [action.payload.href]: action.payload.uuid - }); -} - -function removeUUIDFromHrefIndex(state: HrefIndexState, action: RemoveUUIDFromHrefIndexAction): HrefIndexState { - const newState = Object.create(null); - for (const href in state) { - if (state[href] !== action.payload) { - newState[href] = state[href]; - } - } - return newState; -} diff --git a/src/app/core/index/uuid-index.actions.ts b/src/app/core/index/uuid-index.actions.ts new file mode 100644 index 0000000000..0bea11204c --- /dev/null +++ b/src/app/core/index/uuid-index.actions.ts @@ -0,0 +1,60 @@ +import { Action } from '@ngrx/store'; + +import { type } from '../../shared/ngrx/type'; + +/** + * The list of HrefIndexAction type definitions + */ +export const UUIDIndexActionTypes = { + ADD: type('dspace/core/index/uuid/ADD'), + REMOVE_HREF: type('dspace/core/index/uuid/REMOVE_HREF') +}; + +/* tslint:disable:max-classes-per-file */ +/** + * An ngrx action to add an href to the index + */ +export class AddToUUIDIndexAction implements Action { + type = UUIDIndexActionTypes.ADD; + payload: { + href: string; + uuid: string; + }; + + /** + * Create a new AddToUUIDIndexAction + * + * @param uuid + * the uuid to add + * @param href + * the self link of the resource the uuid belongs to + */ + constructor(uuid: string, href: string) { + this.payload = { href, uuid }; + } +} + +/** + * An ngrx action to remove an href from the index + */ +export class RemoveHrefFromUUIDIndexAction implements Action { + type = UUIDIndexActionTypes.REMOVE_HREF; + payload: string; + + /** + * Create a new RemoveHrefFromUUIDIndexAction + * + * @param href + * the href to remove the UUID for + */ + constructor(href: string) { + this.payload = href; + } + +} +/* tslint:enable:max-classes-per-file */ + +/** + * A type to encompass all HrefIndexActions + */ +export type UUIDIndexAction = AddToUUIDIndexAction | RemoveHrefFromUUIDIndexAction; diff --git a/src/app/core/index/href-index.effects.ts b/src/app/core/index/uuid-index.effects.ts similarity index 66% rename from src/app/core/index/href-index.effects.ts rename to src/app/core/index/uuid-index.effects.ts index f7a4ad5d91..2f5900ed04 100644 --- a/src/app/core/index/href-index.effects.ts +++ b/src/app/core/index/uuid-index.effects.ts @@ -5,26 +5,26 @@ import { ObjectCacheActionTypes, AddToObjectCacheAction, RemoveFromObjectCacheAction } from '../cache/object-cache.actions'; -import { AddToHrefIndexAction, RemoveUUIDFromHrefIndexAction } from './href-index.actions'; +import { AddToUUIDIndexAction, RemoveHrefFromUUIDIndexAction } from './uuid-index.actions'; import { hasValue } from '../../shared/empty.util'; @Injectable() -export class HrefIndexEffects { +export class UUIDIndexEffects { @Effect() add$ = this.actions$ .ofType(ObjectCacheActionTypes.ADD) - .filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.self)) + .filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)) .map((action: AddToObjectCacheAction) => { - return new AddToHrefIndexAction( - action.payload.objectToCache.self, - action.payload.objectToCache.uuid + return new AddToUUIDIndexAction( + action.payload.objectToCache.uuid, + action.payload.objectToCache.self ); }); @Effect() remove$ = this.actions$ .ofType(ObjectCacheActionTypes.REMOVE) .map((action: RemoveFromObjectCacheAction) => { - return new RemoveUUIDFromHrefIndexAction(action.payload); + return new RemoveHrefFromUUIDIndexAction(action.payload); }); constructor(private actions$: Actions) { diff --git a/src/app/core/index/uuid-index.reducer.ts b/src/app/core/index/uuid-index.reducer.ts new file mode 100644 index 0000000000..191dd8f463 --- /dev/null +++ b/src/app/core/index/uuid-index.reducer.ts @@ -0,0 +1,46 @@ +import { + UUIDIndexAction, + UUIDIndexActionTypes, + AddToUUIDIndexAction, + RemoveHrefFromUUIDIndexAction +} from './uuid-index.actions'; + +export interface UUIDIndexState { + [uuid: string]: string +} + +// Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) +const initialState: UUIDIndexState = Object.create(null); + +export function uuidIndexReducer(state = initialState, action: UUIDIndexAction): UUIDIndexState { + switch (action.type) { + + case UUIDIndexActionTypes.ADD: { + return addToUUIDIndex(state, action as AddToUUIDIndexAction); + } + + case UUIDIndexActionTypes.REMOVE_HREF: { + return removeHrefFromUUIDIndex(state, action as RemoveHrefFromUUIDIndexAction) + } + + default: { + return state; + } + } +} + +function addToUUIDIndex(state: UUIDIndexState, action: AddToUUIDIndexAction): UUIDIndexState { + return Object.assign({}, state, { + [action.payload.uuid]: action.payload.href + }); +} + +function removeHrefFromUUIDIndex(state: UUIDIndexState, action: RemoveHrefFromUUIDIndexAction): UUIDIndexState { + const newState = Object.create(null); + for (const uuid in state) { + if (state[uuid] !== action.payload) { + newState[uuid] = state[uuid]; + } + } + return newState; +} From 712c366756e46fb077cffe3288f486c59fd26324 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 19 Sep 2017 18:21:27 +0200 Subject: [PATCH 2/4] fetch resource endpoints from HAL response instead of hardcoding them --- .../builders/remote-data-build.service.ts | 51 +++--- src/app/core/cache/response-cache.actions.ts | 6 +- src/app/core/cache/response-cache.models.ts | 15 +- src/app/core/cache/response-cache.reducer.ts | 4 +- src/app/core/cache/response-cache.service.ts | 4 +- src/app/core/core.module.ts | 6 +- src/app/core/data/collection-data.service.ts | 4 +- src/app/core/data/community-data.service.ts | 4 +- src/app/core/data/data.service.ts | 65 ++++--- .../core/data/dso-response-parsing.service.ts | 149 ++++++++++++++++ src/app/core/data/item-data.service.ts | 4 +- src/app/core/data/parsing.service.ts | 7 + src/app/core/data/remote-data.ts | 2 +- src/app/core/data/request.actions.ts | 7 +- src/app/core/data/request.effects.ts | 160 ++---------------- src/app/core/data/request.models.ts | 27 ++- src/app/core/data/request.reducer.ts | 5 +- src/app/core/data/request.service.ts | 35 ++-- .../data/root-response-parsing.service.ts | 37 ++++ src/app/search/search.service.ts | 2 +- 20 files changed, 359 insertions(+), 235 deletions(-) create mode 100644 src/app/core/data/dso-response-parsing.service.ts create mode 100644 src/app/core/data/parsing.service.ts create mode 100644 src/app/core/data/root-response-parsing.service.ts diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index 727eef41d4..1089eff555 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -8,12 +8,12 @@ import { ResponseCacheService } from '../response-cache.service'; import { RequestEntry } from '../../data/request.reducer'; import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { ResponseCacheEntry } from '../response-cache.reducer'; -import { ErrorResponse, SuccessResponse } from '../response-cache.models'; +import { ErrorResponse, DSOSuccessResponse } from '../response-cache.models'; import { RemoteData } from '../../data/remote-data'; import { GenericConstructor } from '../../shared/generic-constructor'; import { getMapsTo, getRelationMetadata, getRelationships } from './build-decorators'; import { NormalizedObjectFactory } from '../models/normalized-object-factory'; -import { Request } from '../../data/request.models'; +import { RestRequest } from '../../data/request.models'; @Injectable() export class RemoteDataBuildService { @@ -25,19 +25,26 @@ export class RemoteDataBuildService { } buildSingle( - href: string, + hrefObs: string | Observable, normalizedType: GenericConstructor ): RemoteData { - const requestHrefObs = this.objectCache.getRequestHrefBySelfLink(href); + if (typeof hrefObs === 'string') { + hrefObs = Observable.of(hrefObs); + } + + const requestHrefObs = hrefObs.flatMap((href: string) => + this.objectCache.getRequestHrefBySelfLink(href)); const requestObs = Observable.race( - this.requestService.get(href).filter((entry) => hasValue(entry)), + hrefObs.flatMap((href: string) => this.requestService.get(href)) + .filter((entry) => hasValue(entry)), requestHrefObs.flatMap((requestHref) => this.requestService.get(requestHref)).filter((entry) => hasValue(entry)) ); const responseCacheObs = Observable.race( - this.responseCache.get(href).filter((entry) => hasValue(entry)), + hrefObs.flatMap((href: string) => this.responseCache.get(href)) + .filter((entry) => hasValue(entry)), requestHrefObs.flatMap((requestHref) => this.responseCache.get(requestHref)).filter((entry) => hasValue(entry)) ); @@ -60,17 +67,18 @@ export class RemoteDataBuildService { /* tslint:disable:no-string-literal */ const pageInfo = responseCacheObs .filter((entry: ResponseCacheEntry) => hasValue(entry.response) && hasValue(entry.response['pageInfo'])) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).pageInfo) + .map((entry: ResponseCacheEntry) => (entry.response as DSOSuccessResponse).pageInfo) .distinctUntilChanged(); /* tslint:enable:no-string-literal */ // always use self link if that is cached, only if it isn't, get it via the response. const payload = Observable.combineLatest( - this.objectCache.getBySelfLink(href, normalizedType).startWith(undefined), + hrefObs.flatMap((href: string) => this.objectCache.getBySelfLink(href, normalizedType)) + .startWith(undefined), responseCacheObs .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) + .map((entry: ResponseCacheEntry) => (entry.response as DSOSuccessResponse).resourceSelfLinks) .flatMap((resourceSelfLinks: string[]) => { if (isNotEmpty(resourceSelfLinks)) { return this.objectCache.getBySelfLink(resourceSelfLinks[0], normalizedType); @@ -93,7 +101,7 @@ export class RemoteDataBuildService { }).distinctUntilChanged(); return new RemoteData( - href, + hrefObs, requestPending, responsePending, isSuccessFul, @@ -105,12 +113,17 @@ export class RemoteDataBuildService { } buildList( - href: string, + hrefObs: string | Observable, normalizedType: GenericConstructor ): RemoteData { - const requestObs = this.requestService.get(href) + if (typeof hrefObs === 'string') { + hrefObs = Observable.of(hrefObs); + } + + const requestObs = hrefObs.flatMap((href: string) => this.requestService.get(href)) + .filter((entry) => hasValue(entry)); + const responseCacheObs = hrefObs.flatMap((href: string) => this.responseCache.get(href)) .filter((entry) => hasValue(entry)); - const responseCacheObs = this.responseCache.get(href).filter((entry) => hasValue(entry)); const requestPending = requestObs.map((entry: RequestEntry) => entry.requestPending).distinctUntilChanged(); @@ -131,13 +144,13 @@ export class RemoteDataBuildService { /* tslint:disable:no-string-literal */ const pageInfo = responseCacheObs .filter((entry: ResponseCacheEntry) => hasValue(entry.response) && hasValue(entry.response['pageInfo'])) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).pageInfo) + .map((entry: ResponseCacheEntry) => (entry.response as DSOSuccessResponse).pageInfo) .distinctUntilChanged(); /* tslint:enable:no-string-literal */ const payload = responseCacheObs .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) + .map((entry: ResponseCacheEntry) => (entry.response as DSOSuccessResponse).resourceSelfLinks) .flatMap((resourceUUIDs: string[]) => { return this.objectCache.getList(resourceUUIDs, normalizedType) .map((normList: TNormalized[]) => { @@ -149,7 +162,7 @@ export class RemoteDataBuildService { .distinctUntilChanged(); return new RemoteData( - href, + hrefObs, requestPending, responsePending, isSuccessFul, @@ -174,7 +187,7 @@ export class RemoteDataBuildService { // are dispatched, but sometimes don't arrive. I'm unsure why atm. setTimeout(() => { normalized[relationship].forEach((href: string) => { - this.requestService.configure(new Request(href)) + this.requestService.configure(new RestRequest(href)) }); }, 0); @@ -192,7 +205,7 @@ export class RemoteDataBuildService { // without the setTimeout, the actions inside requestService.configure // are dispatched, but sometimes don't arrive. I'm unsure why atm. setTimeout(() => { - this.requestService.configure(new Request(normalized[relationship])); + this.requestService.configure(new RestRequest(normalized[relationship])); }, 0); // The rest API can return a single URL to represent a list of resources (e.g. /items/:id/bitstreams) @@ -259,7 +272,7 @@ export class RemoteDataBuildService { // This is an aggregated object, it doesn't necessarily correspond // to a single REST endpoint, so instead of a self link, use the // current time in ms for a somewhat unique id - `${new Date().getTime()}`, + Observable.of(`${new Date().getTime()}`), requestPending, responsePending, isSuccessFul, diff --git a/src/app/core/cache/response-cache.actions.ts b/src/app/core/cache/response-cache.actions.ts index 426996b4ac..0389067690 100644 --- a/src/app/core/cache/response-cache.actions.ts +++ b/src/app/core/cache/response-cache.actions.ts @@ -1,7 +1,7 @@ import { Action } from '@ngrx/store'; import { type } from '../../shared/ngrx/type'; -import { Response } from './response-cache.models'; +import { RestResponse } from './response-cache.models'; /** * The list of ResponseCacheAction type definitions @@ -17,12 +17,12 @@ export class ResponseCacheAddAction implements Action { type = ResponseCacheActionTypes.ADD; payload: { key: string, - response: Response + response: RestResponse timeAdded: number; msToLive: number; }; - constructor(key: string, response: Response, timeAdded: number, msToLive: number) { + constructor(key: string, response: RestResponse, timeAdded: number, msToLive: number) { this.payload = { key, response, timeAdded, msToLive }; } } diff --git a/src/app/core/cache/response-cache.models.ts b/src/app/core/cache/response-cache.models.ts index a860d682bf..d70d4822bb 100644 --- a/src/app/core/cache/response-cache.models.ts +++ b/src/app/core/cache/response-cache.models.ts @@ -2,14 +2,14 @@ import { RequestError } from '../data/request.models'; import { PageInfo } from '../shared/page-info.model'; /* tslint:disable:max-classes-per-file */ -export class Response { +export class RestResponse { constructor( public isSuccessful: boolean, public statusCode: string ) { } } -export class SuccessResponse extends Response { +export class DSOSuccessResponse extends RestResponse { constructor( public resourceSelfLinks: string[], public statusCode: string, @@ -19,7 +19,16 @@ export class SuccessResponse extends Response { } } -export class ErrorResponse extends Response { +export class RootSuccessResponse extends RestResponse { + constructor( + public endpointMap: { [linkName: string]: string }, + public statusCode: string, + ) { + super(true, statusCode); + } +} + +export class ErrorResponse extends RestResponse { errorMessage: string; constructor(error: RequestError) { diff --git a/src/app/core/cache/response-cache.reducer.ts b/src/app/core/cache/response-cache.reducer.ts index b6a9d903b4..73c680c1f5 100644 --- a/src/app/core/cache/response-cache.reducer.ts +++ b/src/app/core/cache/response-cache.reducer.ts @@ -5,14 +5,14 @@ import { } from './response-cache.actions'; import { CacheEntry } from './cache-entry'; import { hasValue } from '../../shared/empty.util'; -import { Response } from './response-cache.models'; +import { RestResponse } from './response-cache.models'; /** * An entry in the ResponseCache */ export class ResponseCacheEntry implements CacheEntry { key: string; - response: Response; + response: RestResponse; timeAdded: number; msToLive: number; } diff --git a/src/app/core/cache/response-cache.service.ts b/src/app/core/cache/response-cache.service.ts index ed1291e67a..eac76c519e 100644 --- a/src/app/core/cache/response-cache.service.ts +++ b/src/app/core/cache/response-cache.service.ts @@ -6,7 +6,7 @@ import { Observable } from 'rxjs/Observable'; import { ResponseCacheEntry } from './response-cache.reducer'; import { hasNoValue } from '../../shared/empty.util'; import { ResponseCacheRemoveAction, ResponseCacheAddAction } from './response-cache.actions'; -import { Response } from './response-cache.models'; +import { RestResponse } from './response-cache.models'; import { CoreState } from '../core.reducers'; import { keySelector } from '../shared/selectors'; @@ -23,7 +23,7 @@ export class ResponseCacheService { private store: Store ) { } - add(key: string, response: Response, msToLive: number): Observable { + add(key: string, response: RestResponse, msToLive: number): Observable { if (!this.has(key)) { // this.store.dispatch(new ResponseCacheFindAllAction(key, service, scopeID, paginationOptions, sortOptions)); this.store.dispatch(new ResponseCacheAddAction(key, response, new Date().getTime(), msToLive)); diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 2f98eecb0c..a65be35d5b 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -18,6 +18,8 @@ import { coreEffects } from './core.effects'; import { EffectsModule } from '@ngrx/effects'; import { StoreModule } from '@ngrx/store'; import { coreReducers } from './core.reducers'; +import { DSOResponseParsingService } from './data/dso-response-parsing.service'; +import { RootResponseParsingService } from './data/root-response-parsing.service'; const IMPORTS = [ CommonModule, @@ -43,7 +45,9 @@ const PROVIDERS = [ PaginationComponentOptions, ResponseCacheService, RequestService, - RemoteDataBuildService + RemoteDataBuildService, + DSOResponseParsingService, + RootResponseParsingService ]; @NgModule({ diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 8e69827184..ec765c3cb1 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -3,7 +3,6 @@ import { Store } from '@ngrx/store'; import { DataService } from './data.service'; import { Collection } from '../shared/collection.model'; -import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { NormalizedCollection } from '../cache/models/normalized-collection.model'; import { CoreState } from '../core.reducers'; @@ -13,11 +12,10 @@ import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; @Injectable() export class CollectionDataService extends DataService { - protected resourceEndpoint = '/core/collections'; + protected linkName = 'collections'; protected browseEndpoint = '/discover/browses/dateissued/collections'; constructor( - protected objectCache: ObjectCacheService, protected responseCache: ResponseCacheService, protected requestService: RequestService, protected rdbService: RemoteDataBuildService, diff --git a/src/app/core/data/community-data.service.ts b/src/app/core/data/community-data.service.ts index d749171e1f..532bce5ee6 100644 --- a/src/app/core/data/community-data.service.ts +++ b/src/app/core/data/community-data.service.ts @@ -4,7 +4,6 @@ import { Store } from '@ngrx/store'; import { DataService } from './data.service'; import { Community } from '../shared/community.model'; -import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { NormalizedCommunity } from '../cache/models/normalized-community.model'; import { CoreState } from '../core.reducers'; @@ -14,11 +13,10 @@ import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; @Injectable() export class CommunityDataService extends DataService { - protected resourceEndpoint = '/core/communities'; + protected linkName = 'communities'; protected browseEndpoint = '/discover/browses/dateissued/communities'; constructor( - protected objectCache: ObjectCacheService, protected responseCache: ResponseCacheService, protected requestService: RequestService, protected rdbService: RemoteDataBuildService, diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 3d7a724709..abdd364667 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -1,9 +1,11 @@ -import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CacheableObject } from '../cache/object-cache.reducer'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { RemoteData } from './remote-data'; -import { FindAllOptions, FindAllRequest, FindByIDRequest, Request } from './request.models'; +import { + FindAllOptions, FindAllRequest, FindByIDRequest, RestRequest, + RootEndpointRequest +} from './request.models'; import { Store } from '@ngrx/store'; import { CoreState } from '../core.reducers'; import { RequestService } from './request.service'; @@ -12,14 +14,15 @@ import { GenericConstructor } from '../shared/generic-constructor'; import { GlobalConfig } from '../../../config'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { Observable } from 'rxjs/Observable'; +import { ResponseCacheEntry } from '../cache/response-cache.reducer'; +import { RootSuccessResponse } from '../cache/response-cache.models'; export abstract class DataService { - protected abstract objectCache: ObjectCacheService; protected abstract responseCache: ResponseCacheService; protected abstract requestService: RequestService; protected abstract rdbService: RemoteDataBuildService; protected abstract store: Store; - protected abstract resourceEndpoint: string; + protected abstract linkName: string; protected abstract browseEndpoint: string; constructor( @@ -30,21 +33,23 @@ export abstract class DataService } private getEndpoint(linkName: string): Observable { - const apiUrl = new RESTURLCombiner(this.EnvConfig, '/').toString(); - this.requestService.configure(new Request(apiUrl)); - // TODO fetch from store - return Observable.of(undefined); + const request = new RootEndpointRequest(this.EnvConfig); + this.requestService.configure(request); + return this.responseCache.get(request.href) + .map((entry: ResponseCacheEntry) => entry.response) + .filter((response: RootSuccessResponse) => isNotEmpty(response) && isNotEmpty(response.endpointMap)) + .map((response: RootSuccessResponse) => response.endpointMap[linkName]) } - protected getFindAllHref(options: FindAllOptions = {}): string { + protected getFindAllHref(endpoint, options: FindAllOptions = {}): string { let result; const args = []; if (hasValue(options.scopeID)) { - result = this.browseEndpoint; + result = new RESTURLCombiner(this.EnvConfig, this.browseEndpoint).toString(); args.push(`scope=${options.scopeID}`); } else { - result = this.resourceEndpoint; + result = endpoint; } if (hasValue(options.currentPage) && typeof options.currentPage === 'number') { @@ -67,31 +72,41 @@ export abstract class DataService if (isNotEmpty(args)) { result = `${result}?${args.join('&')}`; } - return new RESTURLCombiner(this.EnvConfig, result).toString(); + return result; } findAll(options: FindAllOptions = {}): RemoteData { - const href = this.getFindAllHref(options); - const request = new FindAllRequest(href, options); - this.requestService.configure(request); - return this.rdbService.buildList(href, this.normalizedResourceType); - // return this.rdbService.buildList(href); + const hrefObs = this.getEndpoint(this.linkName) + .map((endpoint: string) => this.getFindAllHref(endpoint, options)); + + hrefObs + .subscribe((href: string) => { + const request = new FindAllRequest(href, options); + this.requestService.configure(request); + }); + + return this.rdbService.buildList(hrefObs, this.normalizedResourceType); } - protected getFindByIDHref(resourceID): string { - return new RESTURLCombiner(this.EnvConfig, `${this.resourceEndpoint}/${resourceID}`).toString(); + protected getFindByIDHref(endpoint, resourceID): string { + return `${endpoint}/${resourceID}`; } findById(id: string): RemoteData { - const href = this.getFindByIDHref(id); - const request = new FindByIDRequest(href, id); - this.requestService.configure(request); - return this.rdbService.buildSingle(href, this.normalizedResourceType); - // return this.rdbService.buildSingle(href); + const hrefObs = this.getEndpoint(this.linkName) + .map((endpoint: string) => this.getFindByIDHref(endpoint, id)); + + hrefObs + .subscribe((href: string) => { + const request = new FindByIDRequest(href, id); + this.requestService.configure(request); + }); + + return this.rdbService.buildSingle(hrefObs, this.normalizedResourceType); } findByHref(href: string): RemoteData { - this.requestService.configure(new Request(href)); + this.requestService.configure(new RestRequest(href)); return this.rdbService.buildSingle(href, this.normalizedResourceType); // return this.rdbService.buildSingle(href)); } diff --git a/src/app/core/data/dso-response-parsing.service.ts b/src/app/core/data/dso-response-parsing.service.ts new file mode 100644 index 0000000000..b7929498f1 --- /dev/null +++ b/src/app/core/data/dso-response-parsing.service.ts @@ -0,0 +1,149 @@ +import { ObjectCacheService } from '../cache/object-cache.service'; +import { Inject, Injectable } from '@angular/core'; +import { GlobalConfig } from '../../../config/global-config.interface'; +import { GLOBAL_CONFIG } from '../../../config'; +import { NormalizedObject } from '../cache/models/normalized-object.model'; +import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util'; +import { ResourceType } from '../shared/resource-type'; +import { NormalizedObjectFactory } from '../cache/models/normalized-object-factory'; +import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; +import { CacheableObject } from '../cache/object-cache.reducer'; +import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; +import { RestResponse, DSOSuccessResponse } from '../cache/response-cache.models'; +import { RestRequest } from './request.models'; +import { PageInfo } from '../shared/page-info.model'; +import { ResponseParsingService } from './parsing.service'; + +function isObjectLevel(halObj: any) { + return isNotEmpty(halObj._links) && hasValue(halObj._links.self); +} + +function isPaginatedResponse(halObj: any) { + return isNotEmpty(halObj.page) && hasValue(halObj._embedded); +} + +function flattenSingleKeyObject(obj: any): any { + const keys = Object.keys(obj); + if (keys.length !== 1) { + throw new Error(`Expected an object with a single key, got: ${JSON.stringify(obj)}`); + } + return obj[keys[0]]; +} + +/* tslint:disable:max-classes-per-file */ +class ProcessRequestDTO { + [key: string]: NormalizedObject[] +} + +@Injectable() +export class DSOResponseParsingService implements ResponseParsingService { + constructor( + @Inject(GLOBAL_CONFIG) private EnvConfig: GlobalConfig, + private objectCache: ObjectCacheService, + ) { + } + + parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { + const processRequestDTO = this.process(data.payload, request.href); + const selfLinks = flattenSingleKeyObject(processRequestDTO).map((no) => no.self); + return new DSOSuccessResponse(selfLinks, data.statusCode, this.processPageInfo(data.payload.page)) + } + + protected process(data: any, requestHref: string): ProcessRequestDTO { + + if (isNotEmpty(data)) { + if (isPaginatedResponse(data)) { + return this.process(data._embedded, requestHref); + } else if (isObjectLevel(data)) { + return { topLevel: this.deserializeAndCache(data, requestHref) }; + } else { + const result = new ProcessRequestDTO(); + if (Array.isArray(data)) { + result.topLevel = []; + data.forEach((datum) => { + if (isPaginatedResponse(datum)) { + const obj = this.process(datum, requestHref); + result.topLevel = [...result.topLevel, ...flattenSingleKeyObject(obj)]; + } else { + result.topLevel = [...result.topLevel, ...this.deserializeAndCache(datum, requestHref)]; + } + }); + } else { + Object.keys(data) + .filter((property) => data.hasOwnProperty(property)) + .filter((property) => hasValue(data[property])) + .forEach((property) => { + if (isPaginatedResponse(data[property])) { + const obj = this.process(data[property], requestHref); + result[property] = flattenSingleKeyObject(obj); + } else { + result[property] = this.deserializeAndCache(data[property], requestHref); + } + }); + } + return result; + } + } + } + + protected deserializeAndCache(obj, requestHref: string): NormalizedObject[] { + if (Array.isArray(obj)) { + let result = []; + obj.forEach((o) => result = [...result, ...this.deserializeAndCache(o, requestHref)]) + return result; + } + + const type: ResourceType = obj.type; + if (hasValue(type)) { + const normObjConstructor = NormalizedObjectFactory.getConstructor(type); + + if (hasValue(normObjConstructor)) { + const serializer = new DSpaceRESTv2Serializer(normObjConstructor); + + let processed; + if (isNotEmpty(obj._embedded)) { + processed = this.process(obj._embedded, requestHref); + } + const normalizedObj = serializer.deserialize(obj); + + if (isNotEmpty(processed)) { + const linksOnly = {}; + Object.keys(processed).forEach((key) => { + linksOnly[key] = processed[key].map((no: NormalizedObject) => no.self); + }); + Object.assign(normalizedObj, linksOnly); + } + + this.addToObjectCache(normalizedObj, requestHref); + return [normalizedObj]; + + } else { + // TODO: move check to Validator? + // throw new Error(`The server returned an object with an unknown a known type: ${type}`); + return []; + } + + } else { + // TODO: move check to Validator + // throw new Error(`The server returned an object without a type: ${JSON.stringify(obj)}`); + return []; + } + } + + protected addToObjectCache(co: CacheableObject, requestHref: string): void { + if (hasNoValue(co) || hasNoValue(co.self)) { + throw new Error('The server returned an invalid object'); + } + this.objectCache.add(co, this.EnvConfig.cache.msToLive, requestHref); + } + + protected processPageInfo(pageObj: any): PageInfo { + if (isNotEmpty(pageObj)) { + return new DSpaceRESTv2Serializer(PageInfo).deserialize(pageObj); + } else { + return undefined; + } + } + +} +/* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/data/item-data.service.ts b/src/app/core/data/item-data.service.ts index f362e7538a..d155910b4e 100644 --- a/src/app/core/data/item-data.service.ts +++ b/src/app/core/data/item-data.service.ts @@ -4,7 +4,6 @@ import { Store } from '@ngrx/store'; import { DataService } from './data.service'; import { Item } from '../shared/item.model'; -import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; import { NormalizedItem } from '../cache/models/normalized-item.model'; @@ -14,11 +13,10 @@ import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; @Injectable() export class ItemDataService extends DataService { - protected resourceEndpoint = '/core/items'; + protected linkName = 'items'; protected browseEndpoint = '/discover/browses/dateissued/items'; constructor( - protected objectCache: ObjectCacheService, protected responseCache: ResponseCacheService, protected requestService: RequestService, protected rdbService: RemoteDataBuildService, diff --git a/src/app/core/data/parsing.service.ts b/src/app/core/data/parsing.service.ts new file mode 100644 index 0000000000..a137b99079 --- /dev/null +++ b/src/app/core/data/parsing.service.ts @@ -0,0 +1,7 @@ +import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; +import { RestRequest } from './request.models'; +import { RestResponse } from '../cache/response-cache.models'; + +export interface ResponseParsingService { + parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse; +} diff --git a/src/app/core/data/remote-data.ts b/src/app/core/data/remote-data.ts index b5c53b19e4..b9f58a5567 100644 --- a/src/app/core/data/remote-data.ts +++ b/src/app/core/data/remote-data.ts @@ -14,7 +14,7 @@ export enum RemoteDataState { */ export class RemoteData { constructor( - public self: string, + public self: Observable, private requestPending: Observable, private responsePending: Observable, private isSuccessFul: Observable, diff --git a/src/app/core/data/request.actions.ts b/src/app/core/data/request.actions.ts index b17046005d..31f0dc5996 100644 --- a/src/app/core/data/request.actions.ts +++ b/src/app/core/data/request.actions.ts @@ -1,7 +1,6 @@ import { Action } from '@ngrx/store'; import { type } from '../../shared/ngrx/type'; -import { CacheableObject } from '../cache/object-cache.reducer'; -import { Request } from './request.models'; +import { RestRequest } from './request.models'; /** * The list of RequestAction type definitions @@ -15,10 +14,10 @@ export const RequestActionTypes = { /* tslint:disable:max-classes-per-file */ export class RequestConfigureAction implements Action { type = RequestActionTypes.CONFIGURE; - payload: Request; + payload: RestRequest; constructor( - request: Request + request: RestRequest ) { this.payload = request; } diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index c5d7eb57f0..84f19679b1 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -1,48 +1,18 @@ -import { Injectable, Inject } from '@angular/core'; +import { Inject, Injectable, Injector } from '@angular/core'; import { Actions, Effect } from '@ngrx/effects'; - // tslint:disable-next-line:import-blacklist import { Observable } from 'rxjs'; -import { DSpaceRESTv2Service } from '../dspace-rest-v2/dspace-rest-v2.service'; -import { ObjectCacheService } from '../cache/object-cache.service'; -import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; -import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; -import { CacheableObject } from '../cache/object-cache.reducer'; -import { Response, SuccessResponse, ErrorResponse } from '../cache/response-cache.models'; -import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; -import { RequestEntry } from './request.reducer'; -import { RequestActionTypes, RequestExecuteAction, RequestCompleteAction } from './request.actions'; +import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; +import { ErrorResponse, RestResponse } from '../cache/response-cache.models'; import { ResponseCacheService } from '../cache/response-cache.service'; -import { RequestService } from './request.service'; -import { NormalizedObjectFactory } from '../cache/models/normalized-object-factory'; -import { ResourceType } from '../shared/resource-type'; +import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; + +import { DSpaceRESTv2Service } from '../dspace-rest-v2/dspace-rest-v2.service'; +import { RequestActionTypes, RequestCompleteAction, RequestExecuteAction } from './request.actions'; import { RequestError } from './request.models'; -import { PageInfo } from '../shared/page-info.model'; -import { NormalizedObject } from '../cache/models/normalized-object.model'; - -import { GlobalConfig, GLOBAL_CONFIG } from '../../../config'; - -function isObjectLevel(halObj: any) { - return isNotEmpty(halObj._links) && hasValue(halObj._links.self); -} - -function isPaginatedResponse(halObj: any) { - return isNotEmpty(halObj.page) && hasValue(halObj._embedded); -} - -function flattenSingleKeyObject(obj: any): any { - const keys = Object.keys(obj); - if (keys.length !== 1) { - throw new Error(`Expected an object with a single key, got: ${JSON.stringify(obj)}`); - } - return obj[keys[0]]; -} - -/* tslint:disable:max-classes-per-file */ -class ProcessRequestDTO { - [key: string]: NormalizedObject[] -} +import { RequestEntry } from './request.reducer'; +import { RequestService } from './request.service'; @Injectable() export class RequestEffects { @@ -55,121 +25,23 @@ export class RequestEffects { }) .flatMap((entry: RequestEntry) => { return this.restApi.get(entry.request.href) - .map((data: DSpaceRESTV2Response) => { - const processRequestDTO = this.process(data.payload, entry.request.href); - const selfLinks = flattenSingleKeyObject(processRequestDTO).map((no) => no.self); - return new SuccessResponse(selfLinks, data.statusCode, this.processPageInfo(data.payload.page)) - }).do((response: Response) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: Response) => new RequestCompleteAction(entry.request.href)) + .map((data: DSpaceRESTV2Response) => + this.injector.get(entry.request.getResponseParser()).parse(entry.request, data)) + .do((response: RestResponse) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) + .map((response: RestResponse) => new RequestCompleteAction(entry.request.href)) .catch((error: RequestError) => Observable.of(new ErrorResponse(error)) - .do((response: Response) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: Response) => new RequestCompleteAction(entry.request.href))); + .do((response: RestResponse) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) + .map((response: RestResponse) => new RequestCompleteAction(entry.request.href))); }); constructor( @Inject(GLOBAL_CONFIG) private EnvConfig: GlobalConfig, private actions$: Actions, private restApi: DSpaceRESTv2Service, - private objectCache: ObjectCacheService, + private injector: Injector, private responseCache: ResponseCacheService, protected requestService: RequestService ) { } - protected process(data: any, requestHref: string): ProcessRequestDTO { - - if (isNotEmpty(data)) { - if (isPaginatedResponse(data)) { - return this.process(data._embedded, requestHref); - } else if (isObjectLevel(data)) { - return { topLevel: this.deserializeAndCache(data, requestHref) }; - } else { - const result = new ProcessRequestDTO(); - if (Array.isArray(data)) { - result.topLevel = []; - data.forEach((datum) => { - if (isPaginatedResponse(datum)) { - const obj = this.process(datum, requestHref); - result.topLevel = [...result.topLevel, ...flattenSingleKeyObject(obj)]; - } else { - result.topLevel = [...result.topLevel, ...this.deserializeAndCache(datum, requestHref)]; - } - }); - } else { - Object.keys(data) - .filter((property) => data.hasOwnProperty(property)) - .filter((property) => hasValue(data[property])) - .forEach((property) => { - if (isPaginatedResponse(data[property])) { - const obj = this.process(data[property], requestHref); - result[property] = flattenSingleKeyObject(obj); - } else { - result[property] = this.deserializeAndCache(data[property], requestHref); - } - }); - } - return result; - } - } - } - - protected deserializeAndCache(obj, requestHref: string): NormalizedObject[] { - if (Array.isArray(obj)) { - let result = []; - obj.forEach((o) => result = [...result, ...this.deserializeAndCache(o, requestHref)]) - return result; - } - - const type: ResourceType = obj.type; - if (hasValue(type)) { - const normObjConstructor = NormalizedObjectFactory.getConstructor(type); - - if (hasValue(normObjConstructor)) { - const serializer = new DSpaceRESTv2Serializer(normObjConstructor); - - let processed; - if (isNotEmpty(obj._embedded)) { - processed = this.process(obj._embedded, requestHref); - } - const normalizedObj = serializer.deserialize(obj); - - if (isNotEmpty(processed)) { - const linksOnly = {}; - Object.keys(processed).forEach((key) => { - linksOnly[key] = processed[key].map((no: NormalizedObject) => no.self); - }); - Object.assign(normalizedObj, linksOnly); - } - - this.addToObjectCache(normalizedObj, requestHref); - return [normalizedObj]; - - } else { - // TODO: move check to Validator? - // throw new Error(`The server returned an object with an unknown a known type: ${type}`); - return []; - } - - } else { - // TODO: move check to Validator - // throw new Error(`The server returned an object without a type: ${JSON.stringify(obj)}`); - return []; - } - } - - protected addToObjectCache(co: CacheableObject, requestHref: string): void { - if (hasNoValue(co) || hasNoValue(co.self)) { - throw new Error('The server returned an invalid object'); - } - this.objectCache.add(co, this.EnvConfig.cache.msToLive, requestHref); - } - - protected processPageInfo(pageObj: any): PageInfo { - if (isNotEmpty(pageObj)) { - return new DSpaceRESTv2Serializer(PageInfo).deserialize(pageObj); - } else { - return undefined; - } - } - } /* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index ec29fa1c5e..8c415e71ef 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -1,15 +1,23 @@ import { SortOptions } from '../cache/models/sort-options.model'; -import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { GenericConstructor } from '../shared/generic-constructor'; +import { GlobalConfig } from '../../../config/global-config.interface'; +import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; +import { DSOResponseParsingService } from './dso-response-parsing.service'; +import { ResponseParsingService } from './parsing.service'; +import { RootResponseParsingService } from './root-response-parsing.service'; /* tslint:disable:max-classes-per-file */ -export class Request { +export class RestRequest { constructor( public href: string, ) { } + + getResponseParser(): GenericConstructor { + return DSOResponseParsingService; + } } -export class FindByIDRequest extends Request { +export class FindByIDRequest extends RestRequest { constructor( href: string, public resourceID: string @@ -25,7 +33,7 @@ export class FindAllOptions { sort?: SortOptions; } -export class FindAllRequest extends Request { +export class FindAllRequest extends RestRequest { constructor( href: string, public options?: FindAllOptions, @@ -34,6 +42,17 @@ export class FindAllRequest extends Request { } } +export class RootEndpointRequest extends RestRequest { + constructor(EnvConfig: GlobalConfig) { + const href = new RESTURLCombiner(EnvConfig, '/').toString(); + super(href); + } + + getResponseParser(): GenericConstructor { + return RootResponseParsingService; + } +} + export class RequestError extends Error { statusText: string; } diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index af7140bbf4..628725f745 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -1,12 +1,11 @@ -import { CacheableObject } from '../cache/object-cache.reducer'; import { RequestActionTypes, RequestAction, RequestConfigureAction, RequestExecuteAction, RequestCompleteAction } from './request.actions'; -import { Request } from './request.models'; +import { RestRequest } from './request.models'; export class RequestEntry { - request: Request; + request: RestRequest; requestPending: boolean; responsePending: boolean; completed: boolean; diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 7b401ca03a..e6b4f816f1 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -3,18 +3,18 @@ import { Injectable } from '@angular/core'; import { MemoizedSelector, Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; - -import { RequestEntry } from './request.reducer'; -import { Request } from './request.models'; import { hasValue } from '../../shared/empty.util'; -import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; -import { ResponseCacheService } from '../cache/response-cache.service'; -import { ObjectCacheService } from '../cache/object-cache.service'; import { CacheableObject } from '../cache/object-cache.reducer'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { DSOSuccessResponse } from '../cache/response-cache.models'; import { ResponseCacheEntry } from '../cache/response-cache.reducer'; -import { SuccessResponse } from '../cache/response-cache.models'; +import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; import { keySelector } from '../shared/selectors'; +import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; +import { RestRequest } from './request.models'; + +import { RequestEntry } from './request.reducer'; function entryFromHrefSelector(href: string): MemoizedSelector { return keySelector('data/request', href); @@ -45,18 +45,25 @@ export class RequestService { return this.store.select(entryFromHrefSelector(href)); } - configure(request: Request): void { + configure(request: RestRequest): void { let isCached = this.objectCache.hasBySelfLink(request.href); if (!isCached && this.responseCache.has(request.href)) { - // if it isn't cached it may be a list endpoint, if so verify - // every object included in the response is still cached - this.responseCache.get(request.href) + const [dsoSuccessResponse, otherSuccessResponse] = this.responseCache.get(request.href) .take(1) .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) - .map((entry: ResponseCacheEntry) => (entry.response as SuccessResponse).resourceSelfLinks) - .map((resourceSelfLinks: string[]) => resourceSelfLinks.every((selfLink) => this.objectCache.hasBySelfLink(selfLink))) - .subscribe((c) => isCached = c); + .map((entry: ResponseCacheEntry) => entry.response) + .share() + .partition((response: DSOSuccessResponse) => hasValue(response.resourceSelfLinks)); + + Observable.merge( + otherSuccessResponse.map(() => true), + dsoSuccessResponse // a DSOSuccessResponse should only be considered cached if all its resources are cached + .map((response: DSOSuccessResponse) => response.resourceSelfLinks) + .map((resourceSelfLinks: string[]) => resourceSelfLinks + .every((selfLink) => this.objectCache.hasBySelfLink(selfLink)) + ) + ).subscribe((c) => isCached = c); } const isPending = this.isPending(request.href); diff --git a/src/app/core/data/root-response-parsing.service.ts b/src/app/core/data/root-response-parsing.service.ts new file mode 100644 index 0000000000..a4841f69fb --- /dev/null +++ b/src/app/core/data/root-response-parsing.service.ts @@ -0,0 +1,37 @@ +import { Inject, Injectable } from '@angular/core'; +import { GLOBAL_CONFIG } from '../../../config'; +import { GlobalConfig } from '../../../config/global-config.interface'; +import { ErrorResponse, RestResponse, RootSuccessResponse } from '../cache/response-cache.models'; +import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; +import { ResponseParsingService } from './parsing.service'; +import { RestRequest } from './request.models'; +import { isNotEmpty } from '../../shared/empty.util'; +import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; + +@Injectable() +export class RootResponseParsingService implements ResponseParsingService { + constructor( + @Inject(GLOBAL_CONFIG) private EnvConfig: GlobalConfig, + ) { + } + + parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { + if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links)) { + const links = data.payload._links; + for (const link of Object.keys(links)) { + let href = links[link].href; + // TODO temporary workaround as these endpoint paths are relative, but should be absolute + href = new RESTURLCombiner(this.EnvConfig, href.substring(this.EnvConfig.rest.nameSpace.length)).toString(); + links[link] = href; + } + return new RootSuccessResponse(links, data.statusCode); + } else { + return new ErrorResponse( + Object.assign( + new Error('Unexpected response from root endpoint'), + { statusText: data.statusCode } + ) + ); + } + } +} diff --git a/src/app/search/search.service.ts b/src/app/search/search.service.ts index 5562056ad2..9f43b0b4a6 100644 --- a/src/app/search/search.service.ts +++ b/src/app/search/search.service.ts @@ -78,7 +78,7 @@ export class SearchService { }); return new RemoteData( - self, + Observable.of(self), requestPending, responsePending, isSuccessFul, From 6348057b467af30fc0da97275484515572054b48 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 20 Sep 2017 14:52:53 +0200 Subject: [PATCH 3/4] added isEnabledOnRestApi() method to dataservices --- src/app/core/cache/response-cache.models.ts | 6 ++++- src/app/core/data/data.service.ts | 26 +++++++++++++++++---- src/app/core/shared/item.model.spec.ts | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/app/core/cache/response-cache.models.ts b/src/app/core/cache/response-cache.models.ts index d70d4822bb..8444a86490 100644 --- a/src/app/core/cache/response-cache.models.ts +++ b/src/app/core/cache/response-cache.models.ts @@ -19,9 +19,13 @@ export class DSOSuccessResponse extends RestResponse { } } +export class EndpointMap { + [linkName: string]: string +} + export class RootSuccessResponse extends RestResponse { constructor( - public endpointMap: { [linkName: string]: string }, + public endpointMap: EndpointMap, public statusCode: string, ) { super(true, statusCode); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index abdd364667..e48e7a8bb8 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -15,7 +15,7 @@ import { GlobalConfig } from '../../../config'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { Observable } from 'rxjs/Observable'; import { ResponseCacheEntry } from '../cache/response-cache.reducer'; -import { RootSuccessResponse } from '../cache/response-cache.models'; +import { EndpointMap, RootSuccessResponse } from '../cache/response-cache.models'; export abstract class DataService { protected abstract responseCache: ResponseCacheService; @@ -32,13 +32,29 @@ export abstract class DataService } - private getEndpoint(linkName: string): Observable { + private getEndpointMap(): Observable { const request = new RootEndpointRequest(this.EnvConfig); this.requestService.configure(request); return this.responseCache.get(request.href) .map((entry: ResponseCacheEntry) => entry.response) .filter((response: RootSuccessResponse) => isNotEmpty(response) && isNotEmpty(response.endpointMap)) - .map((response: RootSuccessResponse) => response.endpointMap[linkName]) + .map((response: RootSuccessResponse) => response.endpointMap) + .distinctUntilChanged(); + } + + public getEndpoint(): Observable { + const request = new RootEndpointRequest(this.EnvConfig); + this.requestService.configure(request); + return this.getEndpointMap() + .map((map: EndpointMap) => map[this.linkName]) + .distinctUntilChanged(); + } + + public isEnabledOnRestApi(): Observable { + return this.getEndpointMap() + .map((map: EndpointMap) => isNotEmpty(map[this.linkName])) + .startWith(undefined) + .distinctUntilChanged(); } protected getFindAllHref(endpoint, options: FindAllOptions = {}): string { @@ -76,7 +92,7 @@ export abstract class DataService } findAll(options: FindAllOptions = {}): RemoteData { - const hrefObs = this.getEndpoint(this.linkName) + const hrefObs = this.getEndpoint() .map((endpoint: string) => this.getFindAllHref(endpoint, options)); hrefObs @@ -93,7 +109,7 @@ export abstract class DataService } findById(id: string): RemoteData { - const hrefObs = this.getEndpoint(this.linkName) + const hrefObs = this.getEndpoint() .map((endpoint: string) => this.getFindByIDHref(endpoint, id)); hrefObs diff --git a/src/app/core/shared/item.model.spec.ts b/src/app/core/shared/item.model.spec.ts index 323cc8ef3f..0c89828c9e 100644 --- a/src/app/core/shared/item.model.spec.ts +++ b/src/app/core/shared/item.model.spec.ts @@ -103,7 +103,7 @@ describe('Item', () => { }); function createRemoteDataObject(object: any) { - const self = ''; + const self = Observable.of(''); const requestPending = Observable.of(false); const responsePending = Observable.of(false); const isSuccessful = Observable.of(true); From bf172461640a41f37051ea179ca5bdb272b900a0 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 21 Sep 2017 16:52:23 +0200 Subject: [PATCH 4/4] ensured the workaround doesn't break things when the issue gets fixed --- src/app/core/data/root-response-parsing.service.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/core/data/root-response-parsing.service.ts b/src/app/core/data/root-response-parsing.service.ts index a4841f69fb..016a501685 100644 --- a/src/app/core/data/root-response-parsing.service.ts +++ b/src/app/core/data/root-response-parsing.service.ts @@ -21,7 +21,9 @@ export class RootResponseParsingService implements ResponseParsingService { for (const link of Object.keys(links)) { let href = links[link].href; // TODO temporary workaround as these endpoint paths are relative, but should be absolute - href = new RESTURLCombiner(this.EnvConfig, href.substring(this.EnvConfig.rest.nameSpace.length)).toString(); + if (isNotEmpty(href) && !href.startsWith('http')) { + href = new RESTURLCombiner(this.EnvConfig, href.substring(this.EnvConfig.rest.nameSpace.length)).toString(); + } links[link] = href; } return new RootSuccessResponse(links, data.statusCode);