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 a986754b0d..9ed43c242b 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -88,18 +88,18 @@ export class RemoteDataBuildService { (href: string, reqEntry: RequestEntry, resEntry: ResponseCacheEntry, payload: T) => { const requestPending = hasValue(reqEntry.requestPending) ? reqEntry.requestPending : true; const responsePending = hasValue(reqEntry.responsePending) ? reqEntry.responsePending : false; - let isSuccessFul: boolean; + let isSuccessful: boolean; let error: RemoteDataError; if (hasValue(resEntry) && hasValue(resEntry.response)) { - isSuccessFul = resEntry.response.isSuccessful; - const errorMessage = isSuccessFul === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; + isSuccessful = resEntry.response.isSuccessful; + const errorMessage = isSuccessful === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; error = new RemoteDataError(resEntry.response.statusCode, errorMessage); } return new RemoteData( requestPending, responsePending, - isSuccessFul, + isSuccessful, error, payload ); @@ -212,7 +212,7 @@ export class RemoteDataBuildService { .map((d: RemoteData) => d.isResponsePending) .every((b: boolean) => b === true); - const isSuccessFul: boolean = arr + const isSuccessful: boolean = arr .map((d: RemoteData) => d.hasSucceeded) .every((b: boolean) => b === true); @@ -241,7 +241,7 @@ export class RemoteDataBuildService { return new RemoteData( requestPending, responsePending, - isSuccessFul, + isSuccessful, error, payload ); diff --git a/src/app/core/data/remote-data.ts b/src/app/core/data/remote-data.ts index 41953260ac..2aa3227d12 100644 --- a/src/app/core/data/remote-data.ts +++ b/src/app/core/data/remote-data.ts @@ -15,16 +15,16 @@ export class RemoteData { constructor( private requestPending: boolean, private responsePending: boolean, - private isSuccessFul: boolean, + private isSuccessful: boolean, public error: RemoteDataError, public payload: T ) { } get state(): RemoteDataState { - if (this.isSuccessFul === true && hasValue(this.payload)) { + if (this.isSuccessful === true && hasValue(this.payload)) { return RemoteDataState.Success - } else if (this.isSuccessFul === false) { + } else if (this.isSuccessful === false) { return RemoteDataState.Failed } else if (this.requestPending === true) { return RemoteDataState.RequestPending diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index afc4209d67..c4e5eb136e 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -8,18 +8,21 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; import { UUIDService } from '../shared/uuid.service'; +import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; import { - DeleteRequest, GetRequest, + DeleteRequest, + GetRequest, HeadRequest, OptionsRequest, PatchRequest, PostRequest, - PutRequest, RestRequest + PutRequest } from './request.models'; import { RequestService } from './request.service'; describe('RequestService', () => { let service: RequestService; + let serviceAsAny: any; let objectCache: ObjectCacheService; let responseCache: ResponseCacheService; let uuidService: UUIDService; @@ -27,15 +30,23 @@ describe('RequestService', () => { const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'; const testHref = 'https://rest.api/endpoint/selfLink'; + const testGetRequest = new GetRequest(testUUID, testHref); + const testPostRequest = new PostRequest(testUUID, testHref); + const testPutRequest = new PutRequest(testUUID, testHref); + const testDeleteRequest = new DeleteRequest(testUUID, testHref); + const testOptionsRequest = new OptionsRequest(testUUID, testHref); + const testHeadRequest = new HeadRequest(testUUID, testHref); + const testPatchRequest = new PatchRequest(testUUID, testHref); - function initMockResponseCacheService() { + const defaultSelectResult = Observable.of(undefined); + const defaultHasResponse = false; + const defaultGetResponse = Observable.of(undefined); + const defaultHasObjectCache = false; + + function initMockResponseCacheService(hasResponse, getResponse) { return jasmine.createSpyObj('responseCache', { - has: true, - get: cold('b-', { - b: { - response: {} - } - }) + has: hasResponse, + get: getResponse }); } @@ -48,17 +59,22 @@ describe('RequestService', () => { ); } - function initServices(selectResult: any) { + function initServices(selectResult = defaultSelectResult, hasResponse = defaultHasResponse, getResponse = defaultGetResponse, hasObjectCache: boolean | boolean[] = defaultHasObjectCache) { + if (!Array.isArray(hasObjectCache)) { + hasObjectCache = [hasObjectCache]; + } objectCache = initMockObjectCacheService(); - responseCache = initMockResponseCacheService(); + (objectCache.hasBySelfLink as any).and.returnValues(...hasObjectCache); + responseCache = initMockResponseCacheService(hasResponse, getResponse); uuidService = initMockUUIDService(); store = initMockStore(selectResult); service = initTestService(); + serviceAsAny = service as any; } describe('generateRequestId', () => { beforeEach(() => { - initServices(Observable.of(undefined)); + initServices(); }); it('should generate a new request ID', () => { @@ -70,15 +86,13 @@ describe('RequestService', () => { }); describe('isPending', () => { - let testRequest: GetRequest; describe('before the request is configured', () => { beforeEach(() => { - initServices(Observable.of(undefined)); - testRequest = new GetRequest(testUUID, testHref) + initServices(); }); it('should return false', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = false; expect(result).toBe(expected); @@ -87,12 +101,12 @@ describe('RequestService', () => { describe('when the request has been configured but hasn\'t reached the store yet', () => { beforeEach(() => { - initServices(Observable.of(undefined)); + initServices(); }); it('should return true', () => { - (service as any).requestsOnTheirWayToTheStore = [testHref]; - const result = service.isPending(testRequest); + serviceAsAny.requestsOnTheirWayToTheStore = [testHref]; + const result = service.isPending(testGetRequest); const expected = true; expect(result).toBe(expected); @@ -107,7 +121,7 @@ describe('RequestService', () => { }); it('should return true', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = true; expect(result).toBe(expected); @@ -122,150 +136,309 @@ describe('RequestService', () => { }); it('should return false', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = false; expect(result).toBe(expected); }); }); - describe('getByUUID', () => { - describe('if the request with the specified UUID exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { - a: { - completed: true - } - })); + }); - const result = service.getByUUID(testUUID); - const expected = cold('b', { - b: { - completed: true - } - }); + describe('getByUUID', () => { + describe('if the request with the specified UUID exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: { + completed: true + } + })); - expect(result).toBeObservable(expected); + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: { + completed: true + } }); - }); - describe('if the request with the specified UUID doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { - a: undefined - })); - - const result = service.getByUUID(testUUID); - const expected = cold('b', { - b: undefined - }); - - expect(result).toBeObservable(expected); - }); - }); - - }); - - describe('getByHref', () => { - describe('if the request with the specified href exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { - a: testUUID - })); - spyOn(service, 'getByUUID').and.returnValue(cold('b', { - b: { - completed: true - } - })); - - const result = service.getByHref(testHref); - const expected = cold('c', { - c: { - completed: true - } - }); - - expect(result).toBeObservable(expected); - }); - }); - - describe('if the request with the specified href doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { - a: undefined - })); - spyOn(service, 'getByUUID').and.returnValue(cold('b', { - b: undefined - })); - - const result = service.getByHref(testHref); - const expected = cold('c', { - c: undefined - }); - - expect(result).toBeObservable(expected); - }); + expect(result).toBeObservable(expected); }); }); - describe('configure', () => { - describe('if the request is a GET request', () => { - describe('and it isn\'t already cached', () => { - it('should dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - spyOn((service as any), 'isCachedOrPending').and.returnValue(false); + describe('if the request with the specified UUID doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); - const request = new GetRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - }); + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: undefined }); - describe('and it is already cached or pending', () => { - it('shouldn\'t dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - spyOn((service as any), 'isCachedOrPending').and.returnValue(true); - const request = new GetRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).not.toHaveBeenCalled(); - }); - }); - }); - - describe('if the request isn\'t a GET request', () => { - it('should dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - - let request = new PostRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new PutRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new DeleteRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new OptionsRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new HeadRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new PatchRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - }); + expect(result).toBeObservable(expected); }); }); }); + describe('getByHref', () => { + describe('if the request with the specified href exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: testUUID + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: { + completed: true + } + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified href doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: undefined + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + }); + + describe('configure', () => { + describe('if the request is a GET request', () => { + describe('and it isn\'t already cached', () => { + it('should dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(false); + + service.configure(testGetRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testGetRequest); + }); + }); + describe('and it is already cached or pending', () => { + it('shouldn\'t dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(true); + + service.configure(testGetRequest); + expect(serviceAsAny.dispatchRequest).not.toHaveBeenCalled(); + }); + }); + }); + + describe('if the request isn\'t a GET request', () => { + it('should dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + + service.configure(testPostRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPostRequest); + + service.configure(testPutRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPutRequest); + + service.configure(testDeleteRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testDeleteRequest); + + service.configure(testOptionsRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testOptionsRequest); + + service.configure(testHeadRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testHeadRequest); + + service.configure(testPatchRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPatchRequest); + }); + }); + }); + + describe('isCachedOrPending', () => { + describe('when the request is cached', () => { + describe('in the ObjectCache', () => { + it('should return true', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true + } + } + ), true); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + describe('in the responseCache', () => { + describe('and it\'s a DSOSuccessResponse', () => { + it('should return true if all top level links in the response are cached in the object cache', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true, + resourceSelfLinks: [ + 'https://rest.api/endpoint/selfLink1', + 'https://rest.api/endpoint/selfLink2' + ] + } + } + ), [false, true, true]); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + it('should return false if not all top level links in the response are cached in the object cache', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true, + resourceSelfLinks: [ + 'https://rest.api/endpoint/selfLink1', + 'https://rest.api/endpoint/selfLink2' + ] + } + } + ), [false, true, false]); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = false; + + expect(result).toEqual(expected); + }); + }); + describe('and it isn\'t a DSOSuccessResponse', () => { + it('should return true', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true + } + } + ), false); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + }); + }); + + describe('when the request is pending', () => { + it('should return true', () => { + initServices(); + spyOn(service, 'isPending').and.returnValue(true); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + + describe('when the request is neither cached nor pending', () => { + it('should return false', () => { + initServices(); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = false; + + expect(result).toEqual(expected); + }); + }); + }); + + describe('dispatchRequest', () => { + beforeEach(() => { + initServices(); + }); + it('should dispatch a RequestConfigureAction', () => { + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(store.dispatch).toHaveBeenCalledWith(new RequestConfigureAction(request)); + }); + it('should dispatch a RequestExecuteAction', () => { + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(store.dispatch).toHaveBeenCalledWith(new RequestExecuteAction(request.uuid)); + }); + describe('when it\'s a GET request', () => { + it('should track it on it\'s way to the store', () => { + spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).toHaveBeenCalledWith(request); + }); + }); + describe('when it\'s not a GET request', () => { + it('shouldn\'t track it', () => { + spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); + + serviceAsAny.dispatchRequest(testPostRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testPutRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testDeleteRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testOptionsRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testHeadRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testPatchRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + }); + }); + }); + + describe('trackRequestsOnTheirWayToTheStore', () => { + let request: GetRequest; + + beforeEach(() => { + initServices(); + request = testGetRequest; + }); + + describe('when the method is called with a new request', () => { + it('should start tracking the request', () => { + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy(); + serviceAsAny.trackRequestsOnTheirWayToTheStore(request); + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeTruthy(); + }); + }); + + describe('when the request is added to the store', () => { + it('should stop tracking the request', () => { + (store.select as any).and.returnValue(Observable.of({ request })); + serviceAsAny.trackRequestsOnTheirWayToTheStore(request); + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy(); + }); + }); + }); }); diff --git a/src/app/shared/mocks/mock-item.ts b/src/app/shared/mocks/mock-item.ts index 6b34a31888..81c1fcf26c 100644 --- a/src/app/shared/mocks/mock-item.ts +++ b/src/app/shared/mocks/mock-item.ts @@ -13,7 +13,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'dspace-angular://aggregated/object/1507836003548', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -25,7 +25,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/10', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -60,7 +60,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/4', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -191,7 +191,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/collections/1c11f3f1-ba1f-4f36-908a-3f1ea9a557eb', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, diff --git a/src/app/shared/mocks/mock-object-cache.service.ts b/src/app/shared/mocks/mock-object-cache.service.ts index 80f7b102fa..4fd311465e 100644 --- a/src/app/shared/mocks/mock-object-cache.service.ts +++ b/src/app/shared/mocks/mock-object-cache.service.ts @@ -1,7 +1,16 @@ import { ObjectCacheService } from '../../core/cache/object-cache.service'; export function initMockObjectCacheService(): ObjectCacheService { - return jasmine.createSpyObj('objectCacheService', { - hasBySelfLink: true, - }); + return jasmine.createSpyObj('objectCacheService', [ + 'add', + 'remove', + 'getByUUID', + 'getBySelfLink', + 'getRequestHrefBySelfLink', + 'getRequestHrefByUUID', + 'getList', + 'hasByUUID', + 'hasBySelfLink' + ]); + }