diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index 94b050855a..864c583dc2 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -1,6 +1,6 @@ import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; -import { cold, getTestScheduler, hot } from 'jasmine-marbles'; +import { cold } from 'jasmine-marbles'; import { Observable, of as observableOf } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { getMockRequestService } from '../../shared/mocks/request.service.mock'; @@ -13,16 +13,15 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { ComColDataService } from './comcol-data.service'; import { CommunityDataService } from './community-data.service'; import { DSOChangeAnalyzer } from './dso-change-analyzer.service'; -import { FindListOptions, GetRequest } from './request.models'; -import { RequestEntry } from './request.reducer'; +import { FindListOptions } from './request.models'; import { RequestService } from './request.service'; import { createFailedRemoteDataObject$, - createNoContentRemoteDataObject$, - createSuccessfulRemoteDataObject$ + createSuccessfulRemoteDataObject$, + createFailedRemoteDataObject, + createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; import { BitstreamDataService } from './bitstream-data.service'; -import { take } from 'rxjs/operators'; const LINK_NAME = 'test'; @@ -50,8 +49,8 @@ class TestService extends ComColDataService { } } +// tslint:disable:no-shadowed-variable describe('ComColDataService', () => { - let scheduler: TestScheduler; let service: TestService; let requestService: RequestService; let cds: CommunityDataService; @@ -59,6 +58,8 @@ describe('ComColDataService', () => { let halService: any = {}; let bitstreamDataService: BitstreamDataService; let rdbService: RemoteDataBuildService; + let testScheduler: TestScheduler; + let topEndpoint: string; const store = {} as Store; const notificationsService = {} as NotificationsService; @@ -69,17 +70,9 @@ describe('ComColDataService', () => { const options = Object.assign(new FindListOptions(), { scopeID: scopeID }); - const getRequestEntry$ = (successful: boolean) => { - return observableOf({ - response: { isSuccessful: successful } as any - } as RequestEntry); - }; - const communitiesEndpoint = 'https://rest.api/core/communities'; const communityEndpoint = `${communitiesEndpoint}/${scopeID}`; const scopedEndpoint = `${communityEndpoint}/${LINK_NAME}`; - const serviceEndpoint = `https://rest.api/core/${LINK_NAME}`; - const authHeader = 'Bearer eyJhbGciOiJIUzI1NiJ9.eyJlaWQiOiJhNjA4NmIzNC0zOTE4LTQ1YjctOGRkZC05MzI5YTcwMmEyNmEiLCJzZyI6W10sImV4cCI6MTUzNDk0MDcyNX0.RV5GAtiX6cpwBN77P_v16iG9ipeyiO7faNYSNMzq_sQ'; const mockHalService = { getEndpoint: (linkPath) => observableOf(communitiesEndpoint) @@ -98,8 +91,8 @@ describe('ComColDataService', () => { } function initMockCommunityDataService(): CommunityDataService { - return jasmine.createSpyObj('responseCache', { - getEndpoint: hot('--a-', { a: communitiesEndpoint }), + return jasmine.createSpyObj('cds', { + getEndpoint: cold('--a-', { a: communitiesEndpoint }), getIDHref: communityEndpoint }); } @@ -134,7 +127,15 @@ describe('ComColDataService', () => { ); } + const initTestScheduler = (): TestScheduler => { + return new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + }; + beforeEach(() => { + topEndpoint = 'https://rest.api/core/communities/search/top'; + testScheduler = initTestScheduler(); cds = initMockCommunityDataService(); requestService = getMockRequestService(); objectCache = initMockObjectCacheService(); @@ -145,113 +146,94 @@ describe('ComColDataService', () => { }); describe('getBrowseEndpoint', () => { - beforeEach(() => { - scheduler = getTestScheduler(); - }); - - it('should send a new FindByIDRequest for the scope Community', () => { - cds = initMockCommunityDataService(); - requestService = getMockRequestService(getRequestEntry$(true)); - objectCache = initMockObjectCacheService(); - service = initTestService(); - - const expected = new GetRequest(requestService.generateRequestId(), communityEndpoint); - - scheduler.schedule(() => service.getBrowseEndpoint(options).subscribe()); - scheduler.flush(); - - expect(requestService.send).toHaveBeenCalledWith(expected, true); + it(`should call createAndSendGetRequest with the scope Community's self link`, () => { + testScheduler.run(({ cold, flush, expectObservable }) => { + (cds.getEndpoint as jasmine.Spy).and.returnValue(cold('a', { a: communitiesEndpoint })); + (rdbService.buildSingle as jasmine.Spy).and.returnValue(cold('a', { a: createFailedRemoteDataObject() })); + spyOn(service as any, 'createAndSendGetRequest'); + service.getBrowseEndpoint(options); + flush(); + expectObservable((service as any).createAndSendGetRequest.calls.argsFor(0)[0]).toBe('(a|)', { a: communityEndpoint }); + expect((service as any).createAndSendGetRequest.calls.argsFor(0)[1]).toBeTrue(); + }); }); describe('if the scope Community can\'t be found', () => { it('should throw an error', () => { - const result = service.getBrowseEndpoint(options).pipe(take(1)); - const expected = cold('--#-', undefined, new Error(`The Community with scope ${scopeID} couldn't be retrieved`)); - - expect(result).toBeObservable(expected); - }); - }); - - describe('cache refresh', () => { - let communityWithoutParentHref; - let data; - - beforeEach(() => { - spyOn(halService, 'getEndpoint').and.returnValue(observableOf('https://rest.api/core/communities/search/top')); - }); - - describe('cache refreshed top level community', () => { - beforeEach(() => { - (rdbService.buildSingle as jasmine.Spy).and.returnValue(createNoContentRemoteDataObject$()); - data = { - dso: Object.assign(new Community(), { - metadata: [{ - key: 'dc.title', - value: 'top level community' - }] - }), - _links: { - parentCommunity: { - href: 'topLevel/parentCommunity' - } - } - }; - communityWithoutParentHref = { - dso: Object.assign(new Community(), { - metadata: [{ - key: 'dc.title', - value: 'top level community' - }] - }), - _links: {} - }; - }); - it('top level community cache refreshed', () => { - scheduler.schedule(() => (service as any).refreshCache(data)); - scheduler.flush(); - expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('https://rest.api/core/communities/search/top'); - }); - it('top level community without parent link, cache not refreshed', () => { - scheduler.schedule(() => (service as any).refreshCache(communityWithoutParentHref)); - scheduler.flush(); - expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); - }); - }); - - describe('cache refreshed child community', () => { - beforeEach(() => { - const parentCommunity = Object.assign(new Community(), { - uuid: 'a20da287-e174-466a-9926-f66as300d399', - id: 'a20da287-e174-466a-9926-f66as300d399', - metadata: [{ - key: 'dc.title', - value: 'parent community' - }], - _links: {} - }); - (rdbService.buildSingle as jasmine.Spy).and.returnValue(createSuccessfulRemoteDataObject$(parentCommunity)); - data = { - dso: Object.assign(new Community(), { - metadata: [{ - key: 'dc.title', - value: 'child community' - }] - }), - _links: { - parentCommunity: { - href: 'child/parentCommunity' - } - } - }; - }); - it('child level community cache refreshed', () => { - scheduler.schedule(() => (service as any).refreshCache(data)); - scheduler.flush(); - expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('a20da287-e174-466a-9926-f66as300d399'); + testScheduler.run(({ cold, expectObservable }) => { + // spies re-defined here to use the "cold" function from rxjs's TestScheduler + // rather than the one imported from jasmine-marbles. + // Mixing the two seems to lead to unpredictable results + (cds.getEndpoint as jasmine.Spy).and.returnValue(cold('a', { a: communitiesEndpoint })); + (rdbService.buildSingle as jasmine.Spy).and.returnValue(cold('a', { a: createFailedRemoteDataObject() })); + const expectedError = new Error(`The Community with scope ${scopeID} couldn't be retrieved`); + expectObservable(service.getBrowseEndpoint(options)).toBe('#', undefined, expectedError); }); }); }); }); + describe('cache refresh', () => { + let communityWithoutParentHref; + let communityWithParentHref; + + beforeEach(() => { + communityWithParentHref = { + _links: { + parentCommunity: { + href: 'topLevel/parentCommunity' + } + } + } as Community; + communityWithoutParentHref = { + _links: {} + } as Community; + }); + + describe('cache refreshed top level community', () => { + it(`should refresh the top level community cache when the dso has a parent link that can't be resolved`, () => { + testScheduler.run(({ flush, cold }) => { + spyOn(halService, 'getEndpoint').and.returnValue(cold('a', { a: topEndpoint })); + spyOn(service, 'findByHref').and.returnValue(cold('a', { a: createSuccessfulRemoteDataObject({}) })); + service.refreshCache(communityWithParentHref); + flush(); + expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(topEndpoint); + }); + }); + it(`shouldn't do anything when the dso doesn't have a parent link`, () => { + testScheduler.run(({ flush, cold }) => { + spyOn(halService, 'getEndpoint').and.returnValue(cold('a', { a: topEndpoint })); + spyOn(service, 'findByHref').and.returnValue(cold('a', { a: createSuccessfulRemoteDataObject({}) })); + service.refreshCache(communityWithoutParentHref); + flush(); + expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); + }); + }); + }); + + describe('cache refreshed child community', () => { + let parentCommunity: Community; + beforeEach(() => { + parentCommunity = Object.assign(new Community(), { + uuid: 'a20da287-e174-466a-9926-f66as300d399', + id: 'a20da287-e174-466a-9926-f66as300d399', + metadata: [{ + key: 'dc.title', + value: 'parent community' + }], + _links: {} + }); + }); + it('should refresh a specific cached community when the parent link can be resolved', () => { + testScheduler.run(({ flush, cold }) => { + spyOn(halService, 'getEndpoint').and.returnValue(cold('a', { a: topEndpoint })); + spyOn(service, 'findByHref').and.returnValue(cold('a', { a: createSuccessfulRemoteDataObject(parentCommunity) })); + service.refreshCache(communityWithParentHref); + flush(); + expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith('a20da287-e174-466a-9926-f66as300d399'); + }); + }); + }); + }); }); diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index a8bbfa79a0..88b15754af 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -20,6 +20,9 @@ import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub'; import { RequestParam } from '../cache/models/request-param.model'; import { getMockRemoteDataBuildService } from '../../shared/mocks/remote-data-build.service.mock'; +import { TestScheduler } from 'rxjs/testing'; +import { RemoteData } from './remote-data'; +import { RequestEntryState } from './request.reducer'; const endpoint = 'https://rest.api/core'; @@ -63,6 +66,10 @@ describe('DataService', () => { let comparator; let objectCache; let store; + let selfLink; + let linksToFollow; + let testScheduler; + let remoteDataMocks; function initTestService(): TestService { requestService = getMockRequestService(); @@ -81,6 +88,34 @@ describe('DataService', () => { } } as any; store = {} as Store; + selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; + linksToFollow = [ + followLink('a'), + followLink('b') + ]; + + testScheduler = new TestScheduler((actual, expected) => { + // asserting the two objects are equal + // e.g. using chai. + expect(actual).toEqual(expected); + }); + + const timeStamp = new Date().getTime(); + const msToLive = 15 * 60 * 1000; + const payload = { foo: 'bar' }; + const statusCodeSuccess = 200; + const statusCodeError = 404; + const errorMessage = 'not found'; + remoteDataMocks = { + RequestPending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.RequestPending, undefined, undefined, undefined), + ResponsePending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePending, undefined, undefined, undefined), + Success: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Success, undefined, payload, statusCodeSuccess), + SuccessStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.SuccessStale, undefined, payload, statusCodeSuccess), + Error: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError), + ErrorStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError), + }; + + return new TestService( requestService, rdbService, @@ -307,14 +342,12 @@ describe('DataService', () => { describe('update', () => { let operations; - let selfLink; let dso; let dso2; const name1 = 'random string'; const name2 = 'another random string'; beforeEach(() => { operations = [{ op: 'replace', path: '/0/value', value: name2 } as Operation]; - selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; dso = Object.assign(new DSpaceObject(), { _links: { self: { href: selfLink } }, @@ -340,5 +373,452 @@ describe('DataService', () => { expect(objectCache.addPatch).not.toHaveBeenCalled(); }); }); + + describe(`reRequestStaleRemoteData`, () => { + let callback: jasmine.Spy; + + beforeEach(() => { + callback = jasmine.createSpy(); + }); + + + describe(`when shouldReRequest is false`, () => { + it(`shouldn't do anything`, () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const expected = 'a-b-c-d-e-f'; + const values = { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.Success, + d: remoteDataMocks.SuccessStale, + e: remoteDataMocks.Error, + f: remoteDataMocks.ErrorStale, + }; + + expectObservable((service as any).reRequestStaleRemoteData(false, callback)(cold(expected, values))).toBe(expected, values); + // since the callback happens in a tap(), flush to ensure it has been executed + flush(); + expect(callback).not.toHaveBeenCalled(); + }); + }); + }); + + describe(`when shouldReRequest is true`, () => { + it(`should call the callback for stale RemoteData objects, but still pass the source observable unmodified`, () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const expected = 'a-b'; + const values = { + a: remoteDataMocks.SuccessStale, + b: remoteDataMocks.ErrorStale, + }; + + expectObservable((service as any).reRequestStaleRemoteData(true, callback)(cold(expected, values))).toBe(expected, values); + // since the callback happens in a tap(), flush to ensure it has been executed + flush(); + expect(callback).toHaveBeenCalledTimes(2); + }); + }); + + it(`should only call the callback for stale RemoteData objects if something is subscribed to it`, (done) => { + testScheduler.run(({ cold, expectObservable }) => { + const expected = 'a'; + const values = { + a: remoteDataMocks.SuccessStale, + }; + + const result$ = (service as any).reRequestStaleRemoteData(true, callback)(cold(expected, values)); + expectObservable(result$).toBe(expected, values); + expect(callback).not.toHaveBeenCalled(); + result$.subscribe(() => { + expect(callback).toHaveBeenCalled(); + done(); + }); + }); + }); + + it(`shouldn't do anything for RemoteData objects that aren't stale`, () => { + testScheduler.run(({ cold, expectObservable, flush }) => { + const expected = 'a-b-c-d'; + const values = { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.Success, + d: remoteDataMocks.Error, + }; + + expectObservable((service as any).reRequestStaleRemoteData(true, callback)(cold(expected, values))).toBe(expected, values); + // since the callback happens in a tap(), flush to ensure it has been executed + flush(); + expect(callback).not.toHaveBeenCalled(); + }); + }); + }); + + }); + + describe(`findByHref`, () => { + beforeEach(() => { + spyOn(service as any, 'createAndSendGetRequest').and.callFake((href$) => { href$.subscribe().unsubscribe(); }); + }); + + it(`should call buildHrefFromFindOptions with href and linksToFollow`, () => { + testScheduler.run(({ cold }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findByHref(selfLink, true, true, ...linksToFollow); + expect(service.buildHrefFromFindOptions).toHaveBeenCalledWith(selfLink, {}, [], ...linksToFollow); + }); + }); + + it(`should call createAndSendGetRequest with the result from buildHrefFromFindOptions and useCachedVersionIfAvailable`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findByHref(selfLink, true, true, ...linksToFollow); + expect((service as any).createAndSendGetRequest).toHaveBeenCalledWith(jasmine.anything(), true); + expectObservable(rdbService.buildSingle.calls.argsFor(0)[0]).toBe('(a|)', { a: 'bingo!' }); + + service.findByHref(selfLink, false, true, ...linksToFollow); + expect((service as any).createAndSendGetRequest).toHaveBeenCalledWith(jasmine.anything(), false); + expectObservable(rdbService.buildSingle.calls.argsFor(1)[0]).toBe('(a|)', { a: 'bingo!' }); + }); + }); + + it(`should call rdbService.buildSingle with the result from buildHrefFromFindOptions and linksToFollow`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findByHref(selfLink, true, true, ...linksToFollow); + expect(rdbService.buildSingle).toHaveBeenCalledWith(jasmine.anything() as any, ...linksToFollow); + expectObservable(rdbService.buildSingle.calls.argsFor(0)[0]).toBe('(a|)', { a: 'bingo!' }); + }); + }); + + it(`should return a the output from reRequestStaleRemoteData`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: 'bingo!' })); + const expected = 'a'; + const values = { + a: 'bingo!', + }; + + expectObservable(service.findByHref(selfLink, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + it(`should call reRequestStaleRemoteData with reRequestOnStale and the exact same findByHref call as a callback`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.SuccessStale })); + + service.findByHref(selfLink, true, true, ...linksToFollow); + expect((service as any).reRequestStaleRemoteData.calls.argsFor(0)[0]).toBeTrue(); + spyOn(service, 'findByHref').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); + // prove that the spy we just added hasn't been called yet + expect(service.findByHref).not.toHaveBeenCalled(); + // call the callback passed to reRequestStaleRemoteData + (service as any).reRequestStaleRemoteData.calls.argsFor(0)[1](); + // verify that findByHref _has_ been called now, with the same params as the original call + expect(service.findByHref).toHaveBeenCalledWith(jasmine.anything(), true, true, ...linksToFollow); + // ... except for selflink, which will have been turned in to an observable. + expectObservable((service.findByHref as jasmine.Spy).calls.argsFor(0)[0]).toBe('(a|)', { a: selfLink }); + }); + }); + + describe(`when useCachedVersionIfAvailable is true`, () => { + beforeEach(() => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); + }); + + it(`should emit a cached completed RemoteData immediately, and keep emitting if it gets rerequested`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = 'a-b-c-d-e'; + const values = { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findByHref(selfLink, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.SuccessStale, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findByHref(selfLink, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + }); + + describe(`when useCachedVersionIfAvailable is false`, () => { + beforeEach(() => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); + }); + + + it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.SuccessStale, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + }); + + }); + + describe(`findAllByHref`, () => { + let findListOptions; + beforeEach(() => { + findListOptions = { currentPage: 5 }; + spyOn(service as any, 'createAndSendGetRequest').and.callFake((href$) => { href$.subscribe().unsubscribe(); }); + }); + + it(`should call buildHrefFromFindOptions with href and linksToFollow`, () => { + testScheduler.run(({ cold }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow); + expect(service.buildHrefFromFindOptions).toHaveBeenCalledWith(selfLink, findListOptions, [], ...linksToFollow); + }); + }); + + it(`should call createAndSendGetRequest with the result from buildHrefFromFindOptions and useCachedVersionIfAvailable`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow); + expect((service as any).createAndSendGetRequest).toHaveBeenCalledWith(jasmine.anything(), true); + expectObservable(rdbService.buildList.calls.argsFor(0)[0]).toBe('(a|)', { a: 'bingo!' }); + + service.findAllByHref(selfLink, findListOptions, false, true, ...linksToFollow); + expect((service as any).createAndSendGetRequest).toHaveBeenCalledWith(jasmine.anything(), false); + expectObservable(rdbService.buildList.calls.argsFor(1)[0]).toBe('(a|)', { a: 'bingo!' }); + }); + }); + + it(`should call rdbService.buildList with the result from buildHrefFromFindOptions and linksToFollow`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + + service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow); + expect(rdbService.buildList).toHaveBeenCalledWith(jasmine.anything() as any, ...linksToFollow); + expectObservable(rdbService.buildList.calls.argsFor(0)[0]).toBe('(a|)', { a: 'bingo!' }); + }); + }); + + it(`should call reRequestStaleRemoteData with reRequestOnStale and the exact same findAllByHref call as a callback`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.SuccessStale })); + + service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow); + expect((service as any).reRequestStaleRemoteData.calls.argsFor(0)[0]).toBeTrue(); + spyOn(service, 'findAllByHref').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); + // prove that the spy we just added hasn't been called yet + expect(service.findAllByHref).not.toHaveBeenCalled(); + // call the callback passed to reRequestStaleRemoteData + (service as any).reRequestStaleRemoteData.calls.argsFor(0)[1](); + // verify that findAllByHref _has_ been called now, with the same params as the original call + expect(service.findAllByHref).toHaveBeenCalledWith(jasmine.anything(), findListOptions, true, true, ...linksToFollow); + // ... except for selflink, which will have been turned in to an observable. + expectObservable((service.findAllByHref as jasmine.Spy).calls.argsFor(0)[0]).toBe('(a|)', { a: selfLink }); + }); + }); + + it(`should return a the output from reRequestStaleRemoteData`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: 'bingo!' })); + const expected = 'a'; + const values = { + a: 'bingo!', + }; + + expectObservable(service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + describe(`when useCachedVersionIfAvailable is true`, () => { + beforeEach(() => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); + }); + + it(`should emit a cached completed RemoteData immediately, and keep emitting if it gets rerequested`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = 'a-b-c-d-e'; + const values = { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.SuccessStale, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findAllByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + }); + + describe(`when useCachedVersionIfAvailable is false`, () => { + beforeEach(() => { + spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); + spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); + }); + + + it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findAllByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { + a: remoteDataMocks.SuccessStale, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + })); + const expected = '--b-c-d-e'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + e: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findAllByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + + }); + }); }); /* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 814c281a37..6bad02e776 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -12,6 +12,7 @@ import { takeWhile, switchMap, tap, + skipWhile, } from 'rxjs/operators'; import { hasValue, isNotEmpty, isNotEmptyOperator } from '../../shared/empty.util'; import { NotificationOptions } from '../../shared/notifications/models/notification-options.model'; @@ -45,29 +46,6 @@ import { UpdateDataService } from './update-data.service'; import { GenericConstructor } from '../shared/generic-constructor'; import { NoContent } from '../shared/NoContent.model'; -/** - * An operator that will call the given function if the incoming RemoteData is stale and - * shouldReRequest is true - * - * @param shouldReRequest Whether or not to call the re-request function if the RemoteData is stale - * @param requestFn The function to call if the RemoteData is stale and shouldReRequest is - * true - */ -export const reRequestStaleRemoteData = (shouldReRequest: boolean, requestFn: () => Observable>) => - (source: Observable>): Observable> => { - if (shouldReRequest === true) { - return source.pipe( - tap((remoteData: RemoteData) => { - if (hasValue(remoteData) && remoteData.isStale) { - requestFn(); - } - }) - ); - } else { - return source; - } - }; - export abstract class DataService implements UpdateDataService { protected abstract requestService: RequestService; protected abstract rdbService: RemoteDataBuildService; @@ -332,6 +310,30 @@ export abstract class DataService implements UpdateDa return this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow); } + /** + * An operator that will call the given function if the incoming RemoteData is stale and + * shouldReRequest is true + * + * @param shouldReRequest Whether or not to call the re-request function if the RemoteData is stale + * @param requestFn The function to call if the RemoteData is stale and shouldReRequest is + * true + */ + protected reRequestStaleRemoteData(shouldReRequest: boolean, requestFn: () => Observable>) { + return (source: Observable>): Observable> => { + if (shouldReRequest === true) { + return source.pipe( + tap((remoteData: RemoteData) => { + if (hasValue(remoteData) && remoteData.isStale) { + requestFn(); + } + }) + ); + } else { + return source; + } + }; + } + /** * Returns an observable of {@link RemoteData} of an object, based on an href, with a list of * {@link FollowLinkConfig}, to automatically resolve {@link HALLink}s of the object @@ -358,7 +360,12 @@ export abstract class DataService implements UpdateDa this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); return this.rdbService.buildSingle(requestHref$, ...linksToFollow).pipe( - reRequestStaleRemoteData(reRequestOnStale, () => + // This skip ensures that if a stale object is present in the cache when you do a + // call it isn't immediately returned, but we wait until the remote data for the new request + // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a + // cached completed object + skipWhile((rd: RemoteData) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + this.reRequestStaleRemoteData(reRequestOnStale, () => this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)) ); } @@ -390,7 +397,12 @@ export abstract class DataService implements UpdateDa this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); return this.rdbService.buildList(requestHref$, ...linksToFollow).pipe( - reRequestStaleRemoteData(reRequestOnStale, () => + // This skip ensures that if a stale object is present in the cache when you do a + // call it isn't immediately returned, but we wait until the remote data for the new request + // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a + // cached completed object + skipWhile((rd: RemoteData>) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + this.reRequestStaleRemoteData(reRequestOnStale, () => this.findAllByHref(href$, findListOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)) ); } diff --git a/src/app/core/tasks/tasks.service.ts b/src/app/core/tasks/tasks.service.ts index f23c71e65e..7aeb522170 100644 --- a/src/app/core/tasks/tasks.service.ts +++ b/src/app/core/tasks/tasks.service.ts @@ -110,7 +110,6 @@ export abstract class TasksService extends DataServic find((href: string) => hasValue(href)), mergeMap((href) => this.findByHref(href, false, true).pipe( getAllCompletedRemoteData(), - filter((rd: RemoteData) => !rd.isSuccessStale), tap(() => this.requestService.setStaleByHrefSubstring(href))) ) );