From dd24958f618980cdaaab1e8ff5250054944de2fd Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 5 Nov 2018 12:01:51 +0100 Subject: [PATCH] finished tests and docs --- .../auth/auth-response-parsing.service.ts | 2 +- src/app/core/auth/auth.service.ts | 1 - src/app/core/auth/server-auth.service.ts | 1 - src/app/core/browse/browse.service.ts | 9 +- .../remote-data-build.service.spec.ts | 2 +- .../builders/remote-data-build.service.ts | 24 ++-- src/app/core/cache/object-cache.actions.ts | 6 +- .../core/cache/object-cache.reducer.spec.ts | 20 ++-- src/app/core/cache/object-cache.reducer.ts | 4 +- .../core/cache/object-cache.service.spec.ts | 5 +- src/app/core/cache/object-cache.service.ts | 18 ++- .../core/cache/server-sync-buffer.actions.ts | 2 +- .../cache/server-sync-buffer.reducer.spec.ts | 10 +- .../data/base-response-parsing.service.ts | 28 ++--- .../data/config-response-parsing.service.ts | 2 +- .../core/data/dso-response-parsing.service.ts | 2 +- ...acet-value-map-response-parsing.service.ts | 2 - .../facet-value-response-parsing.service.ts | 9 +- src/app/core/data/request.effects.ts | 1 - src/app/core/data/request.service.spec.ts | 103 +++++++++++++++++- src/app/core/data/request.service.ts | 30 ++++- .../integration-response-parsing.service.ts | 2 +- src/app/core/metadata/metadata.service.ts | 6 +- .../core/shared/hal-endpoint.service.spec.ts | 66 +++++++++++ src/app/core/shared/hal-endpoint.service.ts | 6 + src/app/core/shared/operators.spec.ts | 81 +++++++++++--- src/app/core/shared/operators.ts | 9 +- src/app/shared/mocks/mock-request.service.ts | 5 +- 28 files changed, 341 insertions(+), 115 deletions(-) diff --git a/src/app/core/auth/auth-response-parsing.service.ts b/src/app/core/auth/auth-response-parsing.service.ts index 65d093de61..61559991ec 100644 --- a/src/app/core/auth/auth-response-parsing.service.ts +++ b/src/app/core/auth/auth-response-parsing.service.ts @@ -27,7 +27,7 @@ export class AuthResponseParsingService extends BaseResponseParsingService imple parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links) && (data.statusCode === '200' || data.statusCode === 'OK')) { - const response = this.process(data.payload, request.href); + const response = this.process(data.payload, request.uuid); return new AuthStatusResponse(response, data.statusCode); } else { return new AuthStatusResponse(data.payload as AuthStatus, data.statusCode); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 4c520e8f30..229c44bcfa 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -116,7 +116,6 @@ export class AuthService { options.headers = headers; return this.authRequestService.postToEndpoint('login', body, options).pipe( map((status: AuthStatus) => { - console.log('yey response'); if (status.authenticated) { return status; } else { diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 9ab2d84c20..c83410f6e3 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -41,7 +41,6 @@ export class ServerAuthService extends AuthService { // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole... const person$ = this.rdbService.buildSingle(status.eperson.toString()); - // person$.subscribe(() => console.log('test')); return person$.pipe( map((eperson) => eperson.payload) ); diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index ca56c0d267..b807a77e99 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -28,8 +28,7 @@ import { configureRequest, filterSuccessfulResponses, getBrowseDefinitionLinks, - getRemoteDataPayload, - getRequestFromSelflink + getRemoteDataPayload, getRequestFromRequestHref } from '../shared/operators'; import { URLCombiner } from '../url-combiner/url-combiner'; import { Item } from '../shared/item.model'; @@ -68,7 +67,7 @@ export class BrowseService { ); const href$ = request$.pipe(map((request: RestRequest) => request.href)); - const requestEntry$ = href$.pipe(getRequestFromSelflink(this.requestService)); + const requestEntry$ = href$.pipe(getRequestFromRequestHref(this.requestService)); const payload$ = requestEntry$.pipe( filterSuccessfulResponses(), map((response: GenericSuccessResponse) => response.payload), @@ -111,7 +110,7 @@ export class BrowseService { const href$ = request$.pipe(map((request: RestRequest) => request.href)); - const requestEntry$ = href$.pipe(getRequestFromSelflink(this.requestService)); + const requestEntry$ = href$.pipe(getRequestFromRequestHref(this.requestService)); const payload$ = requestEntry$.pipe( filterSuccessfulResponses(), @@ -166,7 +165,7 @@ export class BrowseService { const href$ = request$.pipe(map((request: RestRequest) => request.href)); - const requestEntry$ = href$.pipe(getRequestFromSelflink(this.requestService)); + const requestEntry$ = href$.pipe(getRequestFromRequestHref(this.requestService)); const payload$ = requestEntry$.pipe( filterSuccessfulResponses(), diff --git a/src/app/core/cache/builders/remote-data-build.service.spec.ts b/src/app/core/cache/builders/remote-data-build.service.spec.ts index 8aea54102c..e4444ca803 100644 --- a/src/app/core/cache/builders/remote-data-build.service.spec.ts +++ b/src/app/core/cache/builders/remote-data-build.service.spec.ts @@ -32,7 +32,7 @@ describe('RemoteDataBuildService', () => { let service: RemoteDataBuildService; beforeEach(() => { - service = new RemoteDataBuildService(undefined, undefined, undefined); + service = new RemoteDataBuildService(undefined, undefined); }); describe('when toPaginatedList is called', () => { 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 121ad1aa8c..52ec4382ae 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -5,15 +5,7 @@ import { race as observableRace } from 'rxjs'; import { Injectable } from '@angular/core'; -import { - distinctUntilChanged, - first, - flatMap, - map, - startWith, - switchMap, - takeUntil, tap -} from 'rxjs/operators'; +import { distinctUntilChanged, first, flatMap, map, startWith, switchMap } from 'rxjs/operators'; import { hasValue, hasValueOperator, isEmpty, isNotEmpty } from '../../../shared/empty.util'; import { PaginatedList } from '../../data/paginated-list'; import { RemoteData } from '../../data/remote-data'; @@ -29,7 +21,7 @@ import { getMapsTo, getRelationMetadata, getRelationships } from './build-decora import { PageInfo } from '../../shared/page-info.model'; import { filterSuccessfulResponses, - getRequestFromSelflink, + getRequestFromRequestHref, getRequestFromRequestUUID, getResourceLinksFromResponse } from '../../shared/operators'; @@ -43,16 +35,16 @@ export class RemoteDataBuildService { if (typeof href$ === 'string') { href$ = observableOf(href$); } - const requestHref$ = href$.pipe( + const requestUUID$ = href$.pipe( switchMap((href: string) => - this.objectCache.getRequestHrefBySelfLink(href)), + this.objectCache.getRequestUUIDBySelfLink(href)), ); const requestEntry$ = observableRace( - href$.pipe(getRequestFromSelflink(this.requestService)), - requestHref$.pipe(getRequestFromSelflink(this.requestService)), + href$.pipe(getRequestFromRequestHref(this.requestService)), + requestUUID$.pipe(getRequestFromRequestUUID(this.requestService)), ).pipe( - first() + first() ); // always use self link if that is cached, only if it isn't, get it via the response. @@ -121,7 +113,7 @@ export class RemoteDataBuildService { href$ = observableOf(href$); } - const requestEntry$ = href$.pipe(getRequestFromSelflink(this.requestService)); + const requestEntry$ = href$.pipe(getRequestFromRequestHref(this.requestService)); const tDomainList$ = requestEntry$.pipe( getResourceLinksFromResponse(), flatMap((resourceUUIDs: string[]) => { diff --git a/src/app/core/cache/object-cache.actions.ts b/src/app/core/cache/object-cache.actions.ts index 024a0e7061..8531677ffc 100644 --- a/src/app/core/cache/object-cache.actions.ts +++ b/src/app/core/cache/object-cache.actions.ts @@ -25,7 +25,7 @@ export class AddToObjectCacheAction implements Action { objectToCache: CacheableObject; timeAdded: number; msToLive: number; - requestHref: string; + requestUUID: string; }; /** @@ -42,8 +42,8 @@ export class AddToObjectCacheAction implements Action { * This isn't necessarily the same as the object's self * link, it could have been part of a list for example */ - constructor(objectToCache: CacheableObject, timeAdded: number, msToLive: number, requestHref: string) { - this.payload = { objectToCache, timeAdded, msToLive, requestHref }; + constructor(objectToCache: CacheableObject, timeAdded: number, msToLive: number, requestUUID: string) { + this.payload = { objectToCache, timeAdded, msToLive, requestUUID }; } } diff --git a/src/app/core/cache/object-cache.reducer.spec.ts b/src/app/core/cache/object-cache.reducer.spec.ts index 4e383a82e4..efa28d7249 100644 --- a/src/app/core/cache/object-cache.reducer.spec.ts +++ b/src/app/core/cache/object-cache.reducer.spec.ts @@ -20,6 +20,8 @@ class NullAction extends RemoveFromObjectCacheAction { } describe('objectCacheReducer', () => { + const requestUUID1 = '8646169a-a8fc-4b31-a368-384c07867eb1'; + const requestUUID2 = 'bd36820b-4bf7-4d58-bd80-b832058b7279'; 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 newName = 'new different name'; @@ -31,18 +33,18 @@ describe('objectCacheReducer', () => { }, timeAdded: new Date().getTime(), msToLive: 900000, - requestHref: selfLink1, + requestUUID: requestUUID1, patches: [], isDirty: false }, [selfLink2]: { data: { - self: selfLink2, + self: requestUUID2, foo: 'baz' }, timeAdded: new Date().getTime(), msToLive: 900000, - requestHref: selfLink2, + requestUUID: selfLink2, patches: [], isDirty: false } @@ -68,8 +70,8 @@ describe('objectCacheReducer', () => { const objectToCache = { self: selfLink1 }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/selfLink1'; - const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); + const requestUUID = requestUUID1; + const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestUUID); const newState = objectCacheReducer(state, action); expect(newState[selfLink1].data).toEqual(objectToCache); @@ -81,8 +83,8 @@ describe('objectCacheReducer', () => { const objectToCache = { self: selfLink1, foo: 'baz', somethingElse: true }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/selfLink1'; - const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); + const requestUUID = requestUUID1; + const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestUUID); const newState = objectCacheReducer(testState, action); /* tslint:disable:no-string-literal */ @@ -96,8 +98,8 @@ describe('objectCacheReducer', () => { const objectToCache = { self: selfLink1 }; const timeAdded = new Date().getTime(); const msToLive = 900000; - const requestHref = 'https://rest.api/endpoint/selfLink1'; - const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestHref); + const requestUUID = requestUUID1; + const action = new AddToObjectCacheAction(objectToCache, timeAdded, msToLive, requestUUID); deepFreeze(state); objectCacheReducer(state, action); diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index 4424bb2142..867f31e1bb 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -45,7 +45,7 @@ export class ObjectCacheEntry implements CacheEntry { data: CacheableObject; timeAdded: number; msToLive: number; - requestHref: string; + requestUUID: string; patches: Patch[] = []; isDirty: boolean; } @@ -119,7 +119,7 @@ function addToObjectCache(state: ObjectCacheState, action: AddToObjectCacheActio data: action.payload.objectToCache, timeAdded: action.payload.timeAdded, msToLive: action.payload.msToLive, - requestHref: action.payload.requestHref, + requestUUID: action.payload.requestUUID, isDirty: (hasValue(existing) ? isNotEmpty(existing.patches) : false), patches: (hasValue(existing) ? existing.patches : []) } diff --git a/src/app/core/cache/object-cache.service.spec.ts b/src/app/core/cache/object-cache.service.spec.ts index 5a1a3c5ac6..af353a38c1 100644 --- a/src/app/core/cache/object-cache.service.spec.ts +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -22,6 +22,7 @@ describe('ObjectCacheService', () => { let store: Store; const selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; + const requestUUID = '4d3a4ce8-a375-4b98-859b-39f0a014d736'; const timestamp = new Date().getTime(); const msToLive = 900000; let objectToCache = { @@ -58,8 +59,8 @@ 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, selfLink); - expect(store.dispatch).toHaveBeenCalledWith(new AddToObjectCacheAction(objectToCache, timestamp, msToLive, selfLink)); + service.add(objectToCache, msToLive, requestUUID); + expect(store.dispatch).toHaveBeenCalledWith(new AddToObjectCacheAction(objectToCache, timestamp, msToLive, requestUUID)); }); }); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 3ac644a045..44297d6f61 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -45,13 +45,11 @@ export class ObjectCacheService { * The object to add * @param msToLive * The number of milliseconds it should be cached for - * @param requestHref - * 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 + * @param requestUUID + * The UUID of the request that resulted in this object */ - add(objectToCache: CacheableObject, msToLive: number, requestHref: string): void { - this.store.dispatch(new AddToObjectCacheAction(objectToCache, new Date().getTime(), msToLive, requestHref)); + add(objectToCache: CacheableObject, msToLive: number, requestUUID: string): void { + this.store.dispatch(new AddToObjectCacheAction(objectToCache, new Date().getTime(), msToLive, requestUUID)); } /** @@ -115,16 +113,16 @@ export class ObjectCacheService { ); } - getRequestHrefBySelfLink(selfLink: string): Observable { + getRequestUUIDBySelfLink(selfLink: string): Observable { return this.getEntry(selfLink).pipe( - map((entry: ObjectCacheEntry) => entry.requestHref), + map((entry: ObjectCacheEntry) => entry.requestUUID), distinctUntilChanged()); } - getRequestHrefByUUID(uuid: string): Observable { + getRequestUUIDByObjectUUID(uuid: string): Observable { return this.store.pipe( select(selfLinkFromUuidSelector(uuid)), - mergeMap((selfLink: string) => this.getRequestHrefBySelfLink(selfLink)) + mergeMap((selfLink: string) => this.getRequestUUIDBySelfLink(selfLink)) ); } diff --git a/src/app/core/cache/server-sync-buffer.actions.ts b/src/app/core/cache/server-sync-buffer.actions.ts index 638d837bea..fd7e04ef8a 100644 --- a/src/app/core/cache/server-sync-buffer.actions.ts +++ b/src/app/core/cache/server-sync-buffer.actions.ts @@ -15,7 +15,7 @@ export const ServerSyncBufferActionTypes = { /* tslint:disable:max-classes-per-file */ /** - * An ngrx action to add a new cached object to the server's sync buffer + * An ngrx action to add a new cached object to the server sync buffer */ export class AddToSSBAction implements Action { type = ServerSyncBufferActionTypes.ADD; diff --git a/src/app/core/cache/server-sync-buffer.reducer.spec.ts b/src/app/core/cache/server-sync-buffer.reducer.spec.ts index 666144104b..8f1392c99d 100644 --- a/src/app/core/cache/server-sync-buffer.reducer.spec.ts +++ b/src/app/core/cache/server-sync-buffer.reducer.spec.ts @@ -1,13 +1,5 @@ import * as deepFreeze from 'deep-freeze'; - -import { objectCacheReducer } from './object-cache.reducer'; -import { - AddPatchObjectCacheAction, - AddToObjectCacheAction, ApplyPatchObjectCacheAction, - RemoveFromObjectCacheAction, - ResetObjectCacheTimestampsAction -} from './object-cache.actions'; -import { Operation } from '../../../../node_modules/fast-json-patch'; +import { RemoveFromObjectCacheAction } from './object-cache.actions'; import { serverSyncBufferReducer } from './server-sync-buffer.reducer'; import { RestRequestMethod } from '../data/rest-request-method'; import { AddToSSBAction, EmptySSBAction } from './server-sync-buffer.actions'; diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index ee87338899..eada156ce9 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/src/app/core/data/base-response-parsing.service.ts @@ -25,14 +25,14 @@ export abstract class BaseResponseParsingService { protected abstract objectFactory: any; protected abstract toCache: boolean; - protected process(data: any, requestHref: string): any { + protected process(data: any, requestUUID: string): any { if (isNotEmpty(data)) { if (hasNoValue(data) || (typeof data !== 'object')) { return data; } else if (isPaginatedResponse(data)) { - return this.processPaginatedList(data, requestHref); + return this.processPaginatedList(data, requestUUID); } else if (Array.isArray(data)) { - return this.processArray(data, requestHref); + return this.processArray(data, requestUUID); } else if (isObjectLevel(data)) { data = this.fixBadEPersonRestResponse(data); const object = this.deserialize(data); @@ -41,7 +41,7 @@ export abstract class BaseResponseParsingService { .keys(data._embedded) .filter((property) => data._embedded.hasOwnProperty(property)) .forEach((property) => { - const parsedObj = this.process(data._embedded[property], requestHref); + const parsedObj = this.process(data._embedded[property], requestUUID); if (isNotEmpty(parsedObj)) { if (isPaginatedResponse(data._embedded[property])) { object[property] = parsedObj; @@ -55,7 +55,7 @@ export abstract class BaseResponseParsingService { }); } - this.cache(object, requestHref); + this.cache(object, requestUUID); return object; } const result = {}; @@ -63,7 +63,7 @@ export abstract class BaseResponseParsingService { .filter((property) => data.hasOwnProperty(property)) .filter((property) => hasValue(data[property])) .forEach((property) => { - const obj = this.process(data[property], requestHref); + const obj = this.process(data[property], requestUUID); result[property] = obj; }); return result; @@ -71,7 +71,7 @@ export abstract class BaseResponseParsingService { } } - protected processPaginatedList(data: any, requestHref: string): PaginatedList { + protected processPaginatedList(data: any, requestUUID: string): PaginatedList { const pageInfo: PageInfo = this.processPageInfo(data); let list = data._embedded; @@ -79,14 +79,14 @@ export abstract class BaseResponseParsingService { if (!Array.isArray(list)) { list = this.flattenSingleKeyObject(list); } - const page: ObjectDomain[] = this.processArray(list, requestHref); + const page: ObjectDomain[] = this.processArray(list, requestUUID); return new PaginatedList(pageInfo, page); } - protected processArray(data: any, requestHref: string): ObjectDomain[] { + protected processArray(data: any, requestUUID: string): ObjectDomain[] { let array: ObjectDomain[] = []; data.forEach((datum) => { - array = [...array, this.process(datum, requestHref)]; + array = [...array, this.process(datum, requestUUID)]; } ); return array; @@ -114,17 +114,17 @@ export abstract class BaseResponseParsingService { } } - protected cache(obj, requestHref) { + protected cache(obj, requestUUID) { if (this.toCache) { - this.addToObjectCache(obj, requestHref); + this.addToObjectCache(obj, requestUUID); } } - protected addToObjectCache(co: CacheableObject, requestHref: string): void { + protected addToObjectCache(co: CacheableObject, requestUUID: 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.default, requestHref); + this.objectCache.add(co, this.EnvConfig.cache.msToLive.default, requestUUID); } processPageInfo(payload: any): PageInfo { diff --git a/src/app/core/data/config-response-parsing.service.ts b/src/app/core/data/config-response-parsing.service.ts index ddf884e02b..50303d0a09 100644 --- a/src/app/core/data/config-response-parsing.service.ts +++ b/src/app/core/data/config-response-parsing.service.ts @@ -28,7 +28,7 @@ export class ConfigResponseParsingService extends BaseResponseParsingService imp parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links) && (data.statusCode === '201' || data.statusCode === '200' || data.statusCode === 'OK')) { - const configDefinition = this.process(data.payload, request.href); + const configDefinition = this.process(data.payload, request.uuid); return new ConfigSuccessResponse(configDefinition, data.statusCode, this.processPageInfo(data.payload)); } else { return new ErrorResponse( diff --git a/src/app/core/data/dso-response-parsing.service.ts b/src/app/core/data/dso-response-parsing.service.ts index 568114be1a..1066d11a50 100644 --- a/src/app/core/data/dso-response-parsing.service.ts +++ b/src/app/core/data/dso-response-parsing.service.ts @@ -28,7 +28,7 @@ export class DSOResponseParsingService extends BaseResponseParsingService implem } parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { - const processRequestDTO = this.process(data.payload, request.href); + const processRequestDTO = this.process(data.payload, request.uuid); let objectList = processRequestDTO; if (hasNoValue(processRequestDTO)) { diff --git a/src/app/core/data/facet-value-map-response-parsing.service.ts b/src/app/core/data/facet-value-map-response-parsing.service.ts index 0fc5917847..2f580ee952 100644 --- a/src/app/core/data/facet-value-map-response-parsing.service.ts +++ b/src/app/core/data/facet-value-map-response-parsing.service.ts @@ -9,8 +9,6 @@ import { ResponseParsingService } from './parsing.service'; import { RestRequest } from './request.models'; import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; -import { PageInfo } from '../shared/page-info.model'; -import { isNotEmpty } from '../../shared/empty.util'; import { FacetValue } from '../../+search-page/search-service/facet-value.model'; import { BaseResponseParsingService } from './base-response-parsing.service'; import { ObjectCacheService } from '../cache/object-cache.service'; diff --git a/src/app/core/data/facet-value-response-parsing.service.ts b/src/app/core/data/facet-value-response-parsing.service.ts index 585172c22e..54f36a0564 100644 --- a/src/app/core/data/facet-value-response-parsing.service.ts +++ b/src/app/core/data/facet-value-response-parsing.service.ts @@ -1,16 +1,9 @@ import { Inject, Injectable } from '@angular/core'; -import { - FacetValueMap, - FacetValueMapSuccessResponse, - FacetValueSuccessResponse, - RestResponse -} from '../cache/response.models'; +import { FacetValueSuccessResponse, RestResponse } from '../cache/response.models'; import { ResponseParsingService } from './parsing.service'; import { RestRequest } from './request.models'; import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; -import { PageInfo } from '../shared/page-info.model'; -import { isNotEmpty } from '../../shared/empty.util'; import { FacetValue } from '../../+search-page/search-service/facet-value.model'; import { BaseResponseParsingService } from './base-response-parsing.service'; import { ObjectCacheService } from '../cache/object-cache.service'; diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index 537a0b69b6..5e7bec698b 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -42,7 +42,6 @@ export class RequestEffects { }), filter((entry: RequestEntry) => hasValue(entry)), map((entry: RequestEntry) => entry.request), - tap((entry: RequestEntry) => console.log(entry)), flatMap((request: RestRequest) => { let body; if (isNotEmpty(request.body)) { diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index debddb748c..90d2edfc84 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -174,7 +174,7 @@ describe('RequestService', () => { // b: undefined // }); - scheduler.expectObservable(result).toBe('b', {b: undefined}); + scheduler.expectObservable(result).toBe('b', { b: undefined }); }); }); @@ -458,4 +458,105 @@ describe('RequestService', () => { }); }); }); + + describe('isReusable', () => { + describe('when the given UUID is has no value', () => { + let reusable; + beforeEach(() => { + const uuid = undefined; + reusable = serviceAsAny.isReusable(uuid); + }); + it('return an observable emitting false', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(false)); + }) + }); + + describe('when the given UUID has a value, but no cached entry is found', () => { + let reusable; + beforeEach(() => { + spyOn(service, 'getByUUID').and.returnValue(observableOf(undefined)); + const uuid = 'a45bb291-1adb-40d9-b2fc-7ad9080607be'; + reusable = serviceAsAny.isReusable(uuid); + }); + it('return an observable emitting false', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(false)); + }) + }); + + describe('when the given UUID has a value, a cached entry is found, but it has no response', () => { + let reusable; + beforeEach(() => { + spyOn(service, 'getByUUID').and.returnValue(observableOf({ response: undefined })); + const uuid = '53c9b814-ad8b-4567-9bc1-d9bb6cfba6c8'; + reusable = serviceAsAny.isReusable(uuid); + }); + it('return an observable emitting false', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(false)); + }) + }); + + describe('when the given UUID has a value, a cached entry is found, but its response was not successful', () => { + let reusable; + beforeEach(() => { + spyOn(service, 'getByUUID').and.returnValue(observableOf({ response: { isSuccessful: false } })); + const uuid = '694c9b32-7b2e-4788-835b-ef3fc2252e6c'; + reusable = serviceAsAny.isReusable(uuid); + }); + it('return an observable emitting false', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(false)); + }) + }); + + describe('when the given UUID has a value, a cached entry is found, its response was successful, but the response is outdated', () => { + let reusable; + const now = 100000; + const timeAdded = 99899; + const msToLive = 100; + + beforeEach(() => { + spyOn(Date.prototype, 'getTime').and.returnValue(now); + spyOn(service, 'getByUUID').and.returnValue(observableOf({ + response: { + isSuccessful: true, + timeAdded: timeAdded + }, + request: { + responseMsToLive: msToLive + } + })); + const uuid = 'f9b85788-881c-4994-86b6-bae8dad024d2'; + reusable = serviceAsAny.isReusable(uuid); + }); + + it('return an observable emitting false', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(false)); + }) + }); + + describe('when the given UUID has a value, a cached entry is found, its response was successful, and the response is not outdated', () => { + let reusable; + const now = 100000; + const timeAdded = 99999; + const msToLive = 100; + + beforeEach(() => { + spyOn(Date.prototype, 'getTime').and.returnValue(now); + spyOn(service, 'getByUUID').and.returnValue(observableOf({ + response: { + isSuccessful: true, + timeAdded: timeAdded + }, + request: { + responseMsToLive: msToLive + } + })); + const uuid = 'f9b85788-881c-4994-86b6-bae8dad024d2'; + reusable = serviceAsAny.isReusable(uuid); + }); + + it('return an observable emitting true', () => { + reusable.subscribe((isReusable) => expect(isReusable).toBe(true)); + }) + }) + }) }); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 4b7a7c9b49..285ed06545 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -76,7 +76,7 @@ export class RequestService { } getByUUID(uuid: string): Observable { - return observableRace( + return observableRace( this.store.pipe(select(this.entryFromUUIDSelector(uuid))), this.store.pipe( select(this.originalUUIDFromUUIDSelector(uuid)), @@ -94,6 +94,12 @@ export class RequestService { ); } + /** + * Configure a certain request + * Used to make sure a request is in the cache + * @param {RestRequest} request The request to send out + * @param {boolean} forceBypassCache When true, a new request is always dispatched + */ // TODO to review "overrideRequest" param when https://github.com/DSpace/dspace-angular/issues/217 will be fixed configure(request: RestRequest, forceBypassCache: boolean = false): void { const isGetRequest = request.method === RestRequestMethod.GET; @@ -113,6 +119,11 @@ export class RequestService { } } + /** + * Check if a request is in the cache or if it's still pending + * @param {GetRequest} request The request to check + * @returns {boolean} True if the request is cached or still pending + */ private isCachedOrPending(request: GetRequest) { let isCached = this.objectCache.hasBySelfLink(request.href); if (isCached) { @@ -142,6 +153,10 @@ export class RequestService { return isCached || isPending; } + /** + * Configure and execute the request + * @param {RestRequest} request to dispatch + */ private dispatchRequest(request: RestRequest) { this.store.dispatch(new RequestConfigureAction(request)); this.store.dispatch(new RequestExecuteAction(request.uuid)); @@ -164,6 +179,10 @@ export class RequestService { }); } + /** + * Dispatch commit action to send all changes (for a certain method) to the server (buffer) + * @param {RestRequestMethod} method RestRequestMethod for which the changes should be committed + */ commit(method?: RestRequestMethod) { this.store.dispatch(new CommitSSBAction(method)) } @@ -171,11 +190,11 @@ export class RequestService { /** * Check whether a Response should still be cached * - * @param entry - * the entry to check + * @param uuid + * the uuid of the entry to check * @return boolean - * false if the entry is null, undefined, or its time to - * live has been exceeded, true otherwise + * false if the uuid has no value, no entry could be found, the response was nog successful or its time to + * live has exceeded, true otherwise */ private isReusable(uuid: string): Observable { if (hasNoValue(uuid)) { @@ -194,7 +213,6 @@ export class RequestService { } }) ); - return observableOf(false); } } } diff --git a/src/app/core/integration/integration-response-parsing.service.ts b/src/app/core/integration/integration-response-parsing.service.ts index 6eff7ab792..ef278c93de 100644 --- a/src/app/core/integration/integration-response-parsing.service.ts +++ b/src/app/core/integration/integration-response-parsing.service.ts @@ -32,7 +32,7 @@ export class IntegrationResponseParsingService extends BaseResponseParsingServic parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse { if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links)) { - const dataDefinition = this.process(data.payload, request.href); + const dataDefinition = this.process(data.payload, request.uuid); return new IntegrationSuccessResponse(dataDefinition, data.statusCode, this.processPageInfo(data.payload.page)); } else { return new ErrorResponse( diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 3a63be3f55..8bec058d08 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -263,13 +263,17 @@ export class MetadataService { .pipe( first((files) => isNotEmpty(files)), catchError((error) => { - console.debug(error); + console.debug(error.message); return [] })) .subscribe((bitstreams: Bitstream[]) => { for (const bitstream of bitstreams) { bitstream.format.pipe( first(), + catchError((error: Error) => { + console.debug(error.message); + return [] + }), map((rd: RemoteData) => rd.payload), filter((format: BitstreamFormat) => hasValue(format))) .subscribe((format: BitstreamFormat) => { diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index b65b3f905b..8b3011e7d7 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -15,6 +15,28 @@ describe('HALEndpointService', () => { const endpointMap = { test: 'https://rest.api/test', + foo: 'https://rest.api/foo', + bar: 'https://rest.api/bar', + endpoint: 'https://rest.api/endpoint', + link: 'https://rest.api/link', + another: 'https://rest.api/another', + }; + const start = 'http://start.com'; + const one = 'http://one.com'; + const two = 'http://two.com'; + const endpointMaps = { + [start]: { + one: one, + two: 'empty', + endpoint: 'https://rest.api/endpoint', + link: 'https://rest.api/link', + another: 'https://rest.api/another', + }, + [one]: { + one: 'empty', + two: two, + bar: 'https://rest.api/bar', + } }; const linkPath = 'test'; @@ -80,6 +102,50 @@ describe('HALEndpointService', () => { }); }); + describe('getEndpointAt', () => { + it('should throw an error when the list of hal endpoint names is empty', () => { + const endpointAtWithoutEndpointNames = () => { + (service as any).getEndpointAt('') + }; + expect(endpointAtWithoutEndpointNames).toThrow(); + }); + + it('should be at least called as many times as the length of halNames', () => { + spyOn(service as any, 'getEndpointMapAt').and.returnValue(observableOf(endpointMap)); + spyOn((service as any), 'getEndpointAt').and.callThrough(); + + (service as any).getEndpointAt('', 'endpoint').subscribe(); + + expect((service as any).getEndpointAt.calls.count()).toEqual(1); + + (service as any).getEndpointAt.calls.reset(); + + (service as any).getEndpointAt('', 'endpoint', 'another').subscribe(); + + expect((service as any).getEndpointAt.calls.count()).toBeGreaterThanOrEqual(2); + + (service as any).getEndpointAt.calls.reset(); + + (service as any).getEndpointAt('', 'endpoint', 'another', 'foo', 'bar', 'test').subscribe(); + + expect((service as any).getEndpointAt.calls.count()).toBeGreaterThanOrEqual(5); + }); + + it('should return the correct endpoint', () => { + spyOn(service as any, 'getEndpointMapAt').and.callFake((param) => { + return observableOf(endpointMaps[param]); + }); + + (service as any).getEndpointAt(start, 'one').subscribe((endpoint) => { + expect(endpoint).toEqual(one); + }); + + (service as any).getEndpointAt(start, 'one', 'two').subscribe((endpoint) => { + expect(endpoint).toEqual(two); + }); + }); + }); + describe('isEnabledOnRestApi', () => { beforeEach(() => { service = new HALEndpointService( diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index a0bf7aabd5..a93d54db64 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -47,6 +47,12 @@ export class HALEndpointService { return this.getEndpointAt(this.getRootHref(), ...linkPath.split('/')); } + /** + * Resolve the actual hal url based on a list of hal names + * @param {string} href The root url to start from + * @param {string} halNames List of hal names for which a url should be resolved + * @returns {Observable} Observable that emits the found hal url + */ private getEndpointAt(href: string, ...halNames: string[]): Observable { if (isEmpty(halNames)) { throw new Error('cant\'t fetch the URL without the HAL link names') diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 5f29de9e93..6aeec230c4 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -7,22 +7,24 @@ import { RequestService } from '../data/request.service'; import { configureRequest, filterSuccessfulResponses, - getRemoteDataPayload, - getRequestFromSelflink, - getResourceLinksFromResponse, + getRemoteDataPayload, getRequestFromRequestHref, getRequestFromRequestUUID, + getResourceLinksFromResponse, getResponseFromEntry, } from './operators'; describe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; let requestService: RequestService; - const testSelfLink = 'https://rest.api/'; + const testRequestHref = 'https://rest.api/'; + const testRequestUUID = 'https://rest.api/'; const testRCEs = { a: { response: { isSuccessful: true, resourceSelfLinks: ['a', 'b', 'c', 'd'] } }, b: { response: { isSuccessful: false, resourceSelfLinks: ['e', 'f'] } }, c: { response: { isSuccessful: undefined, resourceSelfLinks: ['g', 'h', 'i'] } }, d: { response: { isSuccessful: true, resourceSelfLinks: ['j', 'k', 'l', 'm', 'n'] } }, - e: { response: { isSuccessful: 1, resourceSelfLinks: [] } } + e: { response: { isSuccessful: 1, resourceSelfLinks: [] } }, + f: { response: undefined }, + g: undefined }; const testResponses = { @@ -37,14 +39,14 @@ describe('Core Module - RxJS Operators', () => { scheduler = getTestScheduler(); }); - describe('getRequestFromSelflink', () => { + describe('getRequestFromRequestHref', () => { it('should return the RequestEntry corresponding to the self link in the source', () => { requestService = getMockRequestService(); - const source = hot('a', { a: testSelfLink }); - const result = source.pipe(getRequestFromSelflink(requestService)); - const expected = cold('a', { a: new RequestEntry()}); + const source = hot('a', { a: testRequestHref }); + const result = source.pipe(getRequestFromRequestHref(requestService)); + const expected = cold('a', { a: new RequestEntry() }); expect(result).toBeObservable(expected) }); @@ -52,18 +54,51 @@ describe('Core Module - RxJS Operators', () => { it('should use the requestService to fetch the request by its self link', () => { requestService = getMockRequestService(); - const source = hot('a', { a: testSelfLink }); - scheduler.schedule(() => source.pipe(getRequestFromSelflink(requestService)).subscribe()); + const source = hot('a', { a: testRequestHref }); + scheduler.schedule(() => source.pipe(getRequestFromRequestHref(requestService)).subscribe()); scheduler.flush(); - expect(requestService.getByHref).toHaveBeenCalledWith(testSelfLink) + expect(requestService.getByHref).toHaveBeenCalledWith(testRequestHref) }); it('shouldn\'t return anything if there is no request matching the self link', () => { requestService = getMockRequestService(cold('a', { a: undefined })); - const source = hot('a', { a: testSelfLink }); - const result = source.pipe(getRequestFromSelflink(requestService)); + const source = hot('a', { a: testRequestUUID }); + const result = source.pipe(getRequestFromRequestHref(requestService)); + const expected = cold('-'); + + expect(result).toBeObservable(expected) + }); + }); + + describe('getRequestFromRequestUUID', () => { + + it('should return the RequestEntry corresponding to the request uuid in the source', () => { + requestService = getMockRequestService(); + + const source = hot('a', { a: testRequestUUID }); + const result = source.pipe(getRequestFromRequestUUID(requestService)); + const expected = cold('a', { a: new RequestEntry() }); + + expect(result).toBeObservable(expected) + }); + + it('should use the requestService to fetch the request by its request uuid', () => { + requestService = getMockRequestService(); + + const source = hot('a', { a: testRequestUUID }); + scheduler.schedule(() => source.pipe(getRequestFromRequestUUID(requestService)).subscribe()); + scheduler.flush(); + + expect(requestService.getByUUID).toHaveBeenCalledWith(testRequestUUID) + }); + + it('shouldn\'t return anything if there is no request matching the request uuid', () => { + requestService = getMockRequestService(cold('a', { a: undefined })); + + const source = hot('a', { a: testRequestUUID }); + const result = source.pipe(getRequestFromRequestUUID(requestService)); const expected = cold('-'); expect(result).toBeObservable(expected) @@ -96,7 +131,7 @@ describe('Core Module - RxJS Operators', () => { describe('configureRequest', () => { it('should call requestService.configure with the source request', () => { requestService = getMockRequestService(); - const testRequest = new GetRequest('6b789e31-f026-4ff8-8993-4eb3b730c841', testSelfLink); + const testRequest = new GetRequest('6b789e31-f026-4ff8-8993-4eb3b730c841', testRequestHref); const source = hot('a', { a: testRequest }); scheduler.schedule(() => source.pipe(configureRequest(requestService)).subscribe()); scheduler.flush(); @@ -117,4 +152,20 @@ describe('Core Module - RxJS Operators', () => { expect(result).toBeObservable(expected) }); }); + + describe('getResponseFromEntry', () => { + it('should return the response for all not empty request entries, when they have a value', () => { + const source = hot('abcdefg', testRCEs); + const result = source.pipe(getResponseFromEntry()); + const expected = cold('abcde--', { + a: testRCEs.a.response, + b: testRCEs.b.response, + c: testRCEs.c.response, + d: testRCEs.d.response, + e: testRCEs.e.response + }); + + expect(result).toBeObservable(expected) + }); + }); }); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index a6335ebb5d..4e6a95ff11 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -15,13 +15,20 @@ import { SearchResult } from '../../+search-page/search-result.model'; * This file contains custom RxJS operators that can be used in multiple places */ -export const getRequestFromSelflink = (requestService: RequestService) => +export const getRequestFromRequestHref = (requestService: RequestService) => (source: Observable): Observable => source.pipe( flatMap((href: string) => requestService.getByHref(href)), hasValueOperator() ); +export const getRequestFromRequestUUID = (requestService: RequestService) => + (source: Observable): Observable => + source.pipe( + flatMap((uuid: string) => requestService.getByUUID(uuid)), + hasValueOperator() + ); + export const filterSuccessfulResponses = () => (source: Observable): Observable => source.pipe( diff --git a/src/app/shared/mocks/mock-request.service.ts b/src/app/shared/mocks/mock-request.service.ts index d46100d56c..c947618ea1 100644 --- a/src/app/shared/mocks/mock-request.service.ts +++ b/src/app/shared/mocks/mock-request.service.ts @@ -2,10 +2,11 @@ import {of as observableOf, Observable } from 'rxjs'; import { RequestService } from '../../core/data/request.service'; import { RequestEntry } from '../../core/data/request.reducer'; -export function getMockRequestService(getByHref$: Observable = observableOf(new RequestEntry())): RequestService { +export function getMockRequestService(requestEntry$: Observable = observableOf(new RequestEntry())): RequestService { return jasmine.createSpyObj('requestService', { configure: false, generateRequestId: 'clients/b186e8ce-e99c-4183-bc9a-42b4821bdb78', - getByHref: getByHref$ + getByHref: requestEntry$, + getByUUID: requestEntry$ }); }