diff --git a/src/app/core/data/base/base-data.service.spec.ts b/src/app/core/data/base/base-data.service.spec.ts index 098f075c10..75662a691f 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -95,6 +95,7 @@ describe('BaseDataService', () => { remoteDataMocks = { RequestPending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.RequestPending, undefined, undefined, undefined), ResponsePending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePending, undefined, undefined, undefined), + ResponsePendingStale: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePendingStale, 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), @@ -303,19 +304,21 @@ describe('BaseDataService', () => { 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, + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findByHref(selfLink, true, true, ...linksToFollow)).toBe(expected, values); @@ -354,19 +357,21 @@ describe('BaseDataService', () => { 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, + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values); @@ -487,19 +492,21 @@ describe('BaseDataService', () => { 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, + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); @@ -538,21 +545,24 @@ describe('BaseDataService', () => { 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, + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e-f-g', { + a: remoteDataMocks.ResponsePendingStale, + b: remoteDataMocks.SuccessStale, + c: remoteDataMocks.ErrorStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, })); - const expected = '--b-c-d-e'; + const expected = '------d-e-f-g'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + d: remoteDataMocks.RequestPending, + e: remoteDataMocks.ResponsePending, + f: remoteDataMocks.Success, + g: remoteDataMocks.SuccessStale, }; + expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); }); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index edd6d9e2a4..c7cd5b0a70 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -273,7 +273,7 @@ export class BaseDataService implements HALDataServic // 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), + skipWhile((rd: RemoteData) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); @@ -307,7 +307,7 @@ export class BaseDataService implements HALDataServic // 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), + skipWhile((rd: RemoteData>) => rd.isStale || (!useCachedVersionIfAvailable && rd.hasCompleted)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findListByHref(href$, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); diff --git a/src/app/core/data/request-entry-state.model.spec.ts b/src/app/core/data/request-entry-state.model.spec.ts new file mode 100644 index 0000000000..7daa655566 --- /dev/null +++ b/src/app/core/data/request-entry-state.model.spec.ts @@ -0,0 +1,186 @@ +import { + isRequestPending, + isError, + isSuccess, + isErrorStale, + isSuccessStale, + isResponsePending, + isResponsePendingStale, + isLoading, + isStale, + hasFailed, + hasSucceeded, + hasCompleted, + RequestEntryState +} from './request-entry-state.model'; + +describe(`isRequestPending`, () => { + it(`should only return true if the given state is RequestPending`, () => { + expect(isRequestPending(RequestEntryState.RequestPending)).toBeTrue(); + + expect(isRequestPending(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isRequestPending(RequestEntryState.Error)).toBeFalse(); + expect(isRequestPending(RequestEntryState.Success)).toBeFalse(); + expect(isRequestPending(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isRequestPending(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isRequestPending(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isError`, () => { + it(`should only return true if the given state is Error`, () => { + expect(isError(RequestEntryState.Error)).toBeTrue(); + + expect(isError(RequestEntryState.RequestPending)).toBeFalse(); + expect(isError(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isError(RequestEntryState.Success)).toBeFalse(); + expect(isError(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isError(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isError(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isSuccess`, () => { + it(`should only return true if the given state is Success`, () => { + expect(isSuccess(RequestEntryState.Success)).toBeTrue(); + + expect(isSuccess(RequestEntryState.RequestPending)).toBeFalse(); + expect(isSuccess(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isSuccess(RequestEntryState.Error)).toBeFalse(); + expect(isSuccess(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isSuccess(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isSuccess(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isErrorStale`, () => { + it(`should only return true if the given state is ErrorStale`, () => { + expect(isErrorStale(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(isErrorStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isErrorStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isErrorStale(RequestEntryState.Error)).toBeFalse(); + expect(isErrorStale(RequestEntryState.Success)).toBeFalse(); + expect(isErrorStale(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isErrorStale(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isSuccessStale`, () => { + it(`should only return true if the given state is SuccessStale`, () => { + expect(isSuccessStale(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(isSuccessStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.Error)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.Success)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isSuccessStale(RequestEntryState.ErrorStale)).toBeFalse(); + }); +}); + +describe(`isResponsePending`, () => { + it(`should only return true if the given state is ResponsePending`, () => { + expect(isResponsePending(RequestEntryState.ResponsePending)).toBeTrue(); + + expect(isResponsePending(RequestEntryState.RequestPending)).toBeFalse(); + expect(isResponsePending(RequestEntryState.Error)).toBeFalse(); + expect(isResponsePending(RequestEntryState.Success)).toBeFalse(); + expect(isResponsePending(RequestEntryState.ResponsePendingStale)).toBeFalse(); + expect(isResponsePending(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isResponsePending(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isResponsePendingStale`, () => { + it(`should only return true if the given state is requestPending`, () => { + expect(isResponsePendingStale(RequestEntryState.ResponsePendingStale)).toBeTrue(); + + expect(isResponsePendingStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.Error)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.Success)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isResponsePendingStale(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`isLoading`, () => { + it(`should only return true if the given state is RequestPending, ResponsePending or ResponsePendingStale`, () => { + expect(isLoading(RequestEntryState.RequestPending)).toBeTrue(); + expect(isLoading(RequestEntryState.ResponsePending)).toBeTrue(); + expect(isLoading(RequestEntryState.ResponsePendingStale)).toBeTrue(); + + expect(isLoading(RequestEntryState.Error)).toBeFalse(); + expect(isLoading(RequestEntryState.Success)).toBeFalse(); + expect(isLoading(RequestEntryState.ErrorStale)).toBeFalse(); + expect(isLoading(RequestEntryState.SuccessStale)).toBeFalse(); + }); +}); + +describe(`hasFailed`, () => { + describe(`when the state is loading`, () => { + it(`should return undefined`, () => { + expect(hasFailed(RequestEntryState.RequestPending)).toBeUndefined(); + expect(hasFailed(RequestEntryState.ResponsePending)).toBeUndefined(); + expect(hasFailed(RequestEntryState.ResponsePendingStale)).toBeUndefined(); + }); + }); + + describe(`when the state has completed`, () => { + it(`should only return true if the given state is Error or ErrorStale`, () => { + expect(hasFailed(RequestEntryState.Error)).toBeTrue(); + expect(hasFailed(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(hasFailed(RequestEntryState.Success)).toBeFalse(); + expect(hasFailed(RequestEntryState.SuccessStale)).toBeFalse(); + }); + }); +}); + +describe(`hasSucceeded`, () => { + describe(`when the state is loading`, () => { + it(`should return undefined`, () => { + expect(hasSucceeded(RequestEntryState.RequestPending)).toBeUndefined(); + expect(hasSucceeded(RequestEntryState.ResponsePending)).toBeUndefined(); + expect(hasSucceeded(RequestEntryState.ResponsePendingStale)).toBeUndefined(); + }); + }); + + describe(`when the state has completed`, () => { + it(`should only return true if the given state is Error or ErrorStale`, () => { + expect(hasSucceeded(RequestEntryState.Success)).toBeTrue(); + expect(hasSucceeded(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(hasSucceeded(RequestEntryState.Error)).toBeFalse(); + expect(hasSucceeded(RequestEntryState.ErrorStale)).toBeFalse(); + }); + }); +}); + + +describe(`hasCompleted`, () => { + it(`should only return true if the given state is Error, Success, ErrorStale or SuccessStale`, () => { + expect(hasCompleted(RequestEntryState.Error)).toBeTrue(); + expect(hasCompleted(RequestEntryState.Success)).toBeTrue(); + expect(hasCompleted(RequestEntryState.ErrorStale)).toBeTrue(); + expect(hasCompleted(RequestEntryState.SuccessStale)).toBeTrue(); + + expect(hasCompleted(RequestEntryState.RequestPending)).toBeFalse(); + expect(hasCompleted(RequestEntryState.ResponsePending)).toBeFalse(); + expect(hasCompleted(RequestEntryState.ResponsePendingStale)).toBeFalse(); + }); +}); + +describe(`isStale`, () => { + it(`should only return true if the given state is ResponsePendingStale, SuccessStale or ErrorStale`, () => { + expect(isStale(RequestEntryState.ResponsePendingStale)).toBeTrue(); + expect(isStale(RequestEntryState.SuccessStale)).toBeTrue(); + expect(isStale(RequestEntryState.ErrorStale)).toBeTrue(); + + expect(isStale(RequestEntryState.RequestPending)).toBeFalse(); + expect(isStale(RequestEntryState.ResponsePending)).toBeFalse(); + expect(isStale(RequestEntryState.Error)).toBeFalse(); + expect(isStale(RequestEntryState.Success)).toBeFalse(); + }); +}); diff --git a/src/app/core/data/request-entry-state.model.ts b/src/app/core/data/request-entry-state.model.ts index a813b6e743..3aeace39d2 100644 --- a/src/app/core/data/request-entry-state.model.ts +++ b/src/app/core/data/request-entry-state.model.ts @@ -3,8 +3,9 @@ export enum RequestEntryState { ResponsePending = 'ResponsePending', Error = 'Error', Success = 'Success', + ResponsePendingStale = 'ResponsePendingStale', ErrorStale = 'ErrorStale', - SuccessStale = 'SuccessStale' + SuccessStale = 'SuccessStale', } /** @@ -42,12 +43,21 @@ export const isSuccessStale = (state: RequestEntryState) => */ export const isResponsePending = (state: RequestEntryState) => state === RequestEntryState.ResponsePending; + /** - * Returns true if the given state is RequestPending or ResponsePending, - * false otherwise + * Returns true if the given state is ResponsePendingStale, false otherwise + */ +export const isResponsePendingStale = (state: RequestEntryState) => + state === RequestEntryState.ResponsePendingStale; + +/** + * Returns true if the given state is RequestPending, RequestPendingStale, ResponsePending, or + * ResponsePendingStale, false otherwise */ export const isLoading = (state: RequestEntryState) => - isRequestPending(state) || isResponsePending(state); + isRequestPending(state) || + isResponsePending(state) || + isResponsePendingStale(state); /** * If isLoading is true for the given state, this method returns undefined, we can't know yet. @@ -82,7 +92,10 @@ export const hasCompleted = (state: RequestEntryState) => !isLoading(state); /** - * Returns true if the given state is SuccessStale or ErrorStale, false otherwise + * Returns true if the given state is isRequestPendingStale, isResponsePendingStale, SuccessStale or + * ErrorStale, false otherwise */ export const isStale = (state: RequestEntryState) => - isSuccessStale(state) || isErrorStale(state); + isResponsePendingStale(state) || + isSuccessStale(state) || + isErrorStale(state); diff --git a/src/app/core/data/request.reducer.spec.ts b/src/app/core/data/request.reducer.spec.ts index 05f074a96a..86b9c4cd5d 100644 --- a/src/app/core/data/request.reducer.spec.ts +++ b/src/app/core/data/request.reducer.spec.ts @@ -48,9 +48,16 @@ describe('requestReducer', () => { lastUpdated: 0 } }; + const testResponsePendingState = { + [id1]: { + state: RequestEntryState.ResponsePending, + lastUpdated: 0 + } + }; deepFreeze(testInitState); deepFreeze(testSuccessState); deepFreeze(testErrorState); + deepFreeze(testResponsePendingState); it('should return the current state when no valid actions have been made', () => { const action = new NullAction(); @@ -91,29 +98,94 @@ describe('requestReducer', () => { expect(newState[id1].response).toEqual(undefined); }); - it('should set state to Success for the given RestRequest in the state, in response to a SUCCESS action', () => { - const state = testInitState; + describe(`in response to a SUCCESS action`, () => { + let startState; + describe(`when the entry isn't stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePending + }) + }); + deepFreeze(startState); + }); + it('should set state to Success for the given RestRequest in the state', () => { + const action = new RequestSuccessAction(id1, 200); + const newState = requestReducer(startState, action); - const action = new RequestSuccessAction(id1, 200); - const newState = requestReducer(state, action); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.Success); + expect(newState[id1].response.statusCode).toEqual(200); + }); + }); + + describe(`when the entry is stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(startState); + }); + it('should set state to SuccessStale for the given RestRequest in the state', () => { + const action = new RequestSuccessAction(id1, 200); + const newState = requestReducer(startState, action); + + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); + expect(newState[id1].response.statusCode).toEqual(200); + }); + }); - expect(newState[id1].request.uuid).toEqual(id1); - expect(newState[id1].request.href).toEqual(link1); - expect(newState[id1].state).toEqual(RequestEntryState.Success); - expect(newState[id1].response.statusCode).toEqual(200); }); - it('should set state to Error for the given RestRequest in the state, in response to an ERROR action', () => { - const state = testInitState; + describe(`in response to an ERROR action`, () => { + let startState; + describe(`when the entry isn't stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePending + }) + }); + deepFreeze(startState); + }); + it('should set state to Error for the given RestRequest in the state', () => { + const action = new RequestErrorAction(id1, 404, 'Not Found'); + const newState = requestReducer(startState, action); - const action = new RequestErrorAction(id1, 404, 'Not Found'); - const newState = requestReducer(state, action); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.Error); + expect(newState[id1].response.statusCode).toEqual(404); + expect(newState[id1].response.errorMessage).toEqual('Not Found'); + }); + }); - expect(newState[id1].request.uuid).toEqual(id1); - expect(newState[id1].request.href).toEqual(link1); - expect(newState[id1].state).toEqual(RequestEntryState.Error); - expect(newState[id1].response.statusCode).toEqual(404); - expect(newState[id1].response.errorMessage).toEqual('Not Found'); + describe(`when the entry is stale`, () => { + beforeEach(() => { + startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(startState); + }); + it('should set state to ErrorStale for the given RestRequest in the state', () => { + const action = new RequestErrorAction(id1, 404, 'Not Found'); + const newState = requestReducer(startState, action); + + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); + expect(newState[id1].response.statusCode).toEqual(404); + expect(newState[id1].response.errorMessage).toEqual('Not Found'); + }); + + }); }); it('should update the response\'s timeCompleted for the given RestRequest in the state, in response to a RESET_TIMESTAMPS action', () => { @@ -145,28 +217,112 @@ describe('requestReducer', () => { expect(newState[id1]).toBeNull(); }); - describe(`for an entry with state: Success`, () => { - it(`should set the state to SuccessStale, in response to a STALE action`, () => { - const state = testSuccessState; + describe(`in response to a STALE action`, () => { + describe(`when the entry has been removed`, () => { + it(`shouldn't do anything`, () => { + const startState = { + [id1]: null + }; + deepFreeze(startState); - const action = new RequestStaleAction(id1); - const newState = requestReducer(state, action); + const action = new RequestStaleAction(id1); + const newState = requestReducer(startState, action); - expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); - expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + expect(newState[id1]).toBeNull(); + }); + }); + + describe(`for stale entries`, () => { + it(`shouldn't do anything`, () => { + const rpsStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ResponsePendingStale + }) + }); + deepFreeze(rpsStartState); + + const action = new RequestStaleAction(id1); + let newState = requestReducer(rpsStartState, action); + + expect(newState[id1].state).toEqual(rpsStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(rpsStartState[id1].lastUpdated); + + const ssStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.SuccessStale + }) + }); + + newState = requestReducer(ssStartState, action); + + expect(newState[id1].state).toEqual(ssStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(ssStartState[id1].lastUpdated); + + const esStartState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.ErrorStale + }) + }); + + newState = requestReducer(esStartState, action); + + expect(newState[id1].state).toEqual(esStartState[id1].state); + expect(newState[id1].lastUpdated).toBe(esStartState[id1].lastUpdated); + + }); + }); + + describe(`for and entry with state: RequestPending`, () => { + it(`shouldn't do anything`, () => { + const startState = Object.assign({}, testInitState, { + [id1]: Object.assign({}, testInitState[id1], { + state: RequestEntryState.RequestPending + }) + }); + + const action = new RequestStaleAction(id1); + const newState = requestReducer(startState, action); + + expect(newState[id1].state).toEqual(startState[id1].state); + expect(newState[id1].lastUpdated).toBe(startState[id1].lastUpdated); + + }); + }); + + describe(`for an entry with state: ResponsePending`, () => { + it(`should set the state to ResponsePendingStale`, () => { + const state = testResponsePendingState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.ResponsePendingStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); + }); + + describe(`for an entry with state: Success`, () => { + it(`should set the state to SuccessStale`, () => { + const state = testSuccessState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.SuccessStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); + }); + + describe(`for an entry with state: Error`, () => { + it(`should set the state to ErrorStale`, () => { + const state = testErrorState; + + const action = new RequestStaleAction(id1); + const newState = requestReducer(state, action); + + expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); + expect(newState[id1].lastUpdated).toBe(action.lastUpdated); + }); }); }); - - describe(`for an entry with state: Error`, () => { - it(`should set the state to ErrorStale, in response to a STALE action`, () => { - const state = testErrorState; - - const action = new RequestStaleAction(id1); - const newState = requestReducer(state, action); - - expect(newState[id1].state).toEqual(RequestEntryState.ErrorStale); - expect(newState[id1].lastUpdated).toBe(action.lastUpdated); - }); - }); - }); diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 9bf17faf8d..9cf4fee0e2 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -11,7 +11,13 @@ import { ResetResponseTimestampsAction } from './request.actions'; import { isNull } from '../../shared/empty.util'; -import { hasSucceeded, isStale, RequestEntryState } from './request-entry-state.model'; +import { + hasSucceeded, + isStale, + RequestEntryState, + isRequestPending, + isResponsePending +} from './request-entry-state.model'; import { RequestState } from './request-state.model'; // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) @@ -91,14 +97,17 @@ function executeRequest(storeState: RequestState, action: RequestExecuteAction): * the new storeState, with the response added to the request */ function completeSuccessRequest(storeState: RequestState, action: RequestSuccessAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry)) { // after a request has been removed it's possible pending changes still come in. // Don't store them return storeState; } else { return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, storeState[action.payload.uuid], { - state: RequestEntryState.Success, + [action.payload.uuid]: Object.assign({}, prevEntry, { + // If a response comes in for a request that's already stale, still store it otherwise + // components that are waiting for it might freeze + state: isStale(prevEntry.state) ? RequestEntryState.SuccessStale : RequestEntryState.Success, response: { timeCompleted: action.payload.timeCompleted, lastUpdated: action.payload.timeCompleted, @@ -124,14 +133,17 @@ function completeSuccessRequest(storeState: RequestState, action: RequestSuccess * the new storeState, with the response added to the request */ function completeFailedRequest(storeState: RequestState, action: RequestErrorAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry)) { // after a request has been removed it's possible pending changes still come in. // Don't store them return storeState; } else { return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, storeState[action.payload.uuid], { - state: RequestEntryState.Error, + [action.payload.uuid]: Object.assign({}, prevEntry, { + // If a response comes in for a request that's already stale, still store it otherwise + // components that are waiting for it might freeze + state: isStale(prevEntry.state) ? RequestEntryState.ErrorStale : RequestEntryState.Error, response: { timeCompleted: action.payload.timeCompleted, lastUpdated: action.payload.timeCompleted, @@ -155,22 +167,27 @@ function completeFailedRequest(storeState: RequestState, action: RequestErrorAct * the new storeState, set to stale */ function expireRequest(storeState: RequestState, action: RequestStaleAction): RequestState { - if (isNull(storeState[action.payload.uuid])) { - // after a request has been removed it's possible pending changes still come in. - // Don't store them + const prevEntry = storeState[action.payload.uuid]; + if (isNull(prevEntry) || isStale(prevEntry.state) || isRequestPending(prevEntry.state)) { + // No need to do anything if the entry doesn't exist, is already stale, or if the request is + // still pending, because that means it still needs to be sent to the server. Any response + // is guaranteed to have been generated after the request was set to stale. return storeState; } else { - const prevEntry = storeState[action.payload.uuid]; - if (isStale(prevEntry.state)) { - return storeState; + let nextRequestEntryState: RequestEntryState; + if (isResponsePending(prevEntry.state)) { + nextRequestEntryState = RequestEntryState.ResponsePendingStale; + } else if (hasSucceeded(prevEntry.state)) { + nextRequestEntryState = RequestEntryState.SuccessStale; } else { - return Object.assign({}, storeState, { - [action.payload.uuid]: Object.assign({}, prevEntry, { - state: hasSucceeded(prevEntry.state) ? RequestEntryState.SuccessStale : RequestEntryState.ErrorStale, - lastUpdated: action.lastUpdated - }) - }); + nextRequestEntryState = RequestEntryState.ErrorStale; } + return Object.assign({}, storeState, { + [action.payload.uuid]: Object.assign({}, prevEntry, { + state: nextRequestEntryState, + lastUpdated: action.lastUpdated + }) + }); } } diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index 108a588881..12208d7e40 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -638,4 +638,41 @@ describe('RequestService', () => { expect(done$).toBeObservable(cold('-----(t|)', { t: true })); })); }); + + describe('setStaleByHref', () => { + const uuid = 'c574a42c-4818-47ac-bbe1-6c3cd622c81f'; + const href = 'https://rest.api/some/object'; + const freshRE: any = { + request: { uuid, href }, + state: RequestEntryState.Success + }; + const staleRE: any = { + request: { uuid, href }, + state: RequestEntryState.SuccessStale + }; + + it(`should call getByHref to retrieve the RequestEntry matching the href`, () => { + spyOn(service, 'getByHref').and.returnValue(observableOf(staleRE)); + service.setStaleByHref(href); + expect(service.getByHref).toHaveBeenCalledWith(href); + }); + + it(`should dispatch a RequestStaleAction for the RequestEntry returned by getByHref`, (done: DoneFn) => { + spyOn(service, 'getByHref').and.returnValue(observableOf(staleRE)); + spyOn(store, 'dispatch'); + service.setStaleByHref(href).subscribe(() => { + expect(store.dispatch).toHaveBeenCalledWith(new RequestStaleAction(uuid)); + done(); + }); + }); + + it(`should emit true when the request in the store is stale`, () => { + spyOn(service, 'getByHref').and.returnValue(cold('a-b', { + a: freshRE, + b: staleRE + })); + const result$ = service.setStaleByHref(href); + expect(result$).toBeObservable(cold('--(c|)', { c: true })); + }); + }); }); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 1f6680203e..c908c696db 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -16,7 +16,7 @@ import { RequestExecuteAction, RequestStaleAction } from './request.actions'; -import { GetRequest} from './request.models'; +import { GetRequest } from './request.models'; import { CommitSSBAction } from '../cache/server-sync-buffer.actions'; import { RestRequestMethod } from './rest-request-method'; import { coreSelector } from '../core.selectors'; @@ -164,7 +164,7 @@ export class RequestService { this.getByHref(request.href).pipe( take(1)) .subscribe((re: RequestEntry) => { - isPending = (hasValue(re) && isLoading(re.state)); + isPending = (hasValue(re) && isLoading(re.state) && !isStale(re.state)); }); return isPending; } @@ -331,7 +331,29 @@ export class RequestService { map((request: RequestEntry) => isStale(request.state)), filter((stale: boolean) => stale), take(1), - ); + ); + } + + /** + * Mark a request as stale + * @param href the href of the request + * @return an Observable that will emit true once the Request becomes stale + */ + setStaleByHref(href: string): Observable { + const requestEntry$ = this.getByHref(href); + + requestEntry$.pipe( + map((re: RequestEntry) => re.request.uuid), + take(1), + ).subscribe((uuid: string) => { + this.store.dispatch(new RequestStaleAction(uuid)); + }); + + return requestEntry$.pipe( + map((request: RequestEntry) => isStale(request.state)), + filter((stale: boolean) => stale), + take(1) + ); } /** @@ -344,10 +366,10 @@ export class RequestService { // if it's not a GET request if (request.method !== RestRequestMethod.GET) { return true; - // if it is a GET request, check it isn't pending + // if it is a GET request, check it isn't pending } else if (this.isPending(request)) { return false; - // if it is pending, check if we're allowed to use a cached version + // if it is pending, check if we're allowed to use a cached version } else if (!useCachedVersionIfAvailable) { return true; } else { diff --git a/src/app/core/data/root-data.service.spec.ts b/src/app/core/data/root-data.service.spec.ts index b65449d007..c34ad37531 100644 --- a/src/app/core/data/root-data.service.spec.ts +++ b/src/app/core/data/root-data.service.spec.ts @@ -1,16 +1,18 @@ import { RootDataService } from './root-data.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; -import { Observable, of } from 'rxjs'; +import { + createSuccessfulRemoteDataObject$, + createFailedRemoteDataObject$ +} from '../../shared/remote-data.utils'; +import { Observable } from 'rxjs'; import { RemoteData } from './remote-data'; import { Root } from './root.model'; -import { RawRestResponse } from '../dspace-rest/raw-rest-response.model'; import { cold } from 'jasmine-marbles'; describe('RootDataService', () => { let service: RootDataService; let halService: HALEndpointService; - let restService; + let requestService; let rootEndpoint; let findByHrefSpy; @@ -19,10 +21,10 @@ describe('RootDataService', () => { halService = jasmine.createSpyObj('halService', { getRootHref: rootEndpoint, }); - restService = jasmine.createSpyObj('halService', { - get: jasmine.createSpy('get'), - }); - service = new RootDataService(null, null, null, halService, restService); + requestService = jasmine.createSpyObj('requestService', [ + 'setStaleByHref', + ]); + service = new RootDataService(requestService, null, null, halService); findByHrefSpy = spyOn(service as any, 'findByHref'); findByHrefSpy.and.returnValue(createSuccessfulRemoteDataObject$({})); @@ -47,12 +49,8 @@ describe('RootDataService', () => { let result$: Observable; it('should return observable of true when root endpoint is available', () => { - const mockResponse = { - statusCode: 200, - statusText: 'OK' - } as RawRestResponse; + spyOn(service, 'findRoot').and.returnValue(createSuccessfulRemoteDataObject$({} as any)); - restService.get.and.returnValue(of(mockResponse)); result$ = service.checkServerAvailability(); expect(result$).toBeObservable(cold('(a|)', { @@ -61,12 +59,8 @@ describe('RootDataService', () => { }); it('should return observable of false when root endpoint is not available', () => { - const mockResponse = { - statusCode: 500, - statusText: 'Internal Server Error' - } as RawRestResponse; + spyOn(service, 'findRoot').and.returnValue(createFailedRemoteDataObject$('500')); - restService.get.and.returnValue(of(mockResponse)); result$ = service.checkServerAvailability(); expect(result$).toBeObservable(cold('(a|)', { @@ -75,4 +69,12 @@ describe('RootDataService', () => { }); }); + + describe(`invalidateRootCache`, () => { + it(`should set the cached root request to stale`, () => { + service.invalidateRootCache(); + expect(halService.getRootHref).toHaveBeenCalled(); + expect(requestService.setStaleByHref).toHaveBeenCalledWith(rootEndpoint); + }); + }); }); diff --git a/src/app/core/data/root-data.service.ts b/src/app/core/data/root-data.service.ts index 54fe614d3e..5431a2d1fb 100644 --- a/src/app/core/data/root-data.service.ts +++ b/src/app/core/data/root-data.service.ts @@ -7,12 +7,11 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Observable, of as observableOf } from 'rxjs'; import { RemoteData } from './remote-data'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; -import { DspaceRestService } from '../dspace-rest/dspace-rest.service'; -import { RawRestResponse } from '../dspace-rest/raw-rest-response.model'; import { catchError, map } from 'rxjs/operators'; import { BaseDataService } from './base/base-data.service'; import { ObjectCacheService } from '../cache/object-cache.service'; import { dataService } from './base/data-service.decorator'; +import { getFirstCompletedRemoteData } from '../shared/operators'; /** * A service to retrieve the {@link Root} object from the REST API. @@ -25,7 +24,6 @@ export class RootDataService extends BaseDataService { protected rdbService: RemoteDataBuildService, protected objectCache: ObjectCacheService, protected halService: HALEndpointService, - protected restService: DspaceRestService, ) { super('', requestService, rdbService, objectCache, halService, 6 * 60 * 60 * 1000); } @@ -34,12 +32,13 @@ export class RootDataService extends BaseDataService { * Check if root endpoint is available */ checkServerAvailability(): Observable { - return this.restService.get(this.halService.getRootHref()).pipe( + return this.findRoot().pipe( catchError((err ) => { console.error(err); return observableOf(false); }), - map((res: RawRestResponse) => res.statusCode === 200) + getFirstCompletedRemoteData(), + map((rootRd: RemoteData) => rootRd.statusCode === 200) ); } @@ -60,6 +59,6 @@ export class RootDataService extends BaseDataService { * Set to sale the root endpoint cache hit */ invalidateRootCache() { - this.requestService.setStaleByHrefSubstring(this.halService.getRootHref()); + this.requestService.setStaleByHref(this.halService.getRootHref()); } } diff --git a/src/app/core/server-check/server-check.guard.spec.ts b/src/app/core/server-check/server-check.guard.spec.ts index 1f126be5e5..f65a7deca7 100644 --- a/src/app/core/server-check/server-check.guard.spec.ts +++ b/src/app/core/server-check/server-check.guard.spec.ts @@ -1,68 +1,88 @@ import { ServerCheckGuard } from './server-check.guard'; -import { Router } from '@angular/router'; +import { Router, NavigationStart, UrlTree, NavigationEnd, RouterEvent } from '@angular/router'; -import { of } from 'rxjs'; -import { take } from 'rxjs/operators'; - -import { getPageInternalServerErrorRoute } from '../../app-routing-paths'; +import { of, ReplaySubject } from 'rxjs'; import { RootDataService } from '../data/root-data.service'; +import { TestScheduler } from 'rxjs/testing'; import SpyObj = jasmine.SpyObj; describe('ServerCheckGuard', () => { let guard: ServerCheckGuard; - let router: SpyObj; + let router: Router; + let eventSubject: ReplaySubject; let rootDataServiceStub: SpyObj; - - rootDataServiceStub = jasmine.createSpyObj('RootDataService', { - checkServerAvailability: jasmine.createSpy('checkServerAvailability'), - invalidateRootCache: jasmine.createSpy('invalidateRootCache') - }); - router = jasmine.createSpyObj('Router', { - navigateByUrl: jasmine.createSpy('navigateByUrl') - }); + let testScheduler: TestScheduler; + let redirectUrlTree: UrlTree; beforeEach(() => { + testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + rootDataServiceStub = jasmine.createSpyObj('RootDataService', { + checkServerAvailability: jasmine.createSpy('checkServerAvailability'), + invalidateRootCache: jasmine.createSpy('invalidateRootCache'), + findRoot: jasmine.createSpy('findRoot') + }); + redirectUrlTree = new UrlTree(); + eventSubject = new ReplaySubject(1); + router = { + events: eventSubject.asObservable(), + navigateByUrl: jasmine.createSpy('navigateByUrl'), + parseUrl: jasmine.createSpy('parseUrl').and.returnValue(redirectUrlTree) + } as any; guard = new ServerCheckGuard(router, rootDataServiceStub); }); - afterEach(() => { - router.navigateByUrl.calls.reset(); - rootDataServiceStub.invalidateRootCache.calls.reset(); - }); - it('should be created', () => { expect(guard).toBeTruthy(); }); - describe('when root endpoint has succeeded', () => { + describe('when root endpoint request has succeeded', () => { beforeEach(() => { rootDataServiceStub.checkServerAvailability.and.returnValue(of(true)); }); - it('should not redirect to error page', () => { - guard.canActivateChild({} as any, {} as any).pipe( - take(1) - ).subscribe((canActivate: boolean) => { - expect(canActivate).toEqual(true); - expect(rootDataServiceStub.invalidateRootCache).not.toHaveBeenCalled(); - expect(router.navigateByUrl).not.toHaveBeenCalled(); + it('should return true', () => { + testScheduler.run(({ expectObservable }) => { + const result$ = guard.canActivateChild({} as any, {} as any); + expectObservable(result$).toBe('(a|)', { a: true }); }); }); }); - describe('when root endpoint has not succeeded', () => { + describe('when root endpoint request has not succeeded', () => { beforeEach(() => { rootDataServiceStub.checkServerAvailability.and.returnValue(of(false)); }); - it('should redirect to error page', () => { - guard.canActivateChild({} as any, {} as any).pipe( - take(1) - ).subscribe((canActivate: boolean) => { - expect(canActivate).toEqual(false); - expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalled(); - expect(router.navigateByUrl).toHaveBeenCalledWith(getPageInternalServerErrorRoute()); + it('should return a UrlTree with the route to the 500 error page', () => { + testScheduler.run(({ expectObservable }) => { + const result$ = guard.canActivateChild({} as any, {} as any); + expectObservable(result$).toBe('(b|)', { b: redirectUrlTree }); }); + expect(router.parseUrl).toHaveBeenCalledWith('/500'); + }); + }); + + describe(`listenForRouteChanges`, () => { + it(`should invalidate the root cache, when the method is first called`, () => { + testScheduler.run(() => { + guard.listenForRouteChanges(); + expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1); + }); + }); + + it(`should invalidate the root cache on every NavigationStart event`, () => { + testScheduler.run(() => { + guard.listenForRouteChanges(); + eventSubject.next(new NavigationStart(1,'')); + eventSubject.next(new NavigationEnd(1,'', '')); + eventSubject.next(new NavigationStart(2,'')); + eventSubject.next(new NavigationEnd(2,'', '')); + eventSubject.next(new NavigationStart(3,'')); + }); + // once when the method is first called, and then 3 times for NavigationStart events + expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1 + 3); }); }); }); diff --git a/src/app/core/server-check/server-check.guard.ts b/src/app/core/server-check/server-check.guard.ts index 8a0e26c01d..79c34c3659 100644 --- a/src/app/core/server-check/server-check.guard.ts +++ b/src/app/core/server-check/server-check.guard.ts @@ -1,8 +1,15 @@ import { Injectable } from '@angular/core'; -import { ActivatedRouteSnapshot, CanActivateChild, Router, RouterStateSnapshot } from '@angular/router'; +import { + ActivatedRouteSnapshot, + CanActivateChild, + Router, + RouterStateSnapshot, + UrlTree, + NavigationStart +} from '@angular/router'; import { Observable } from 'rxjs'; -import { take, tap } from 'rxjs/operators'; +import { take, map, filter } from 'rxjs/operators'; import { RootDataService } from '../data/root-data.service'; import { getPageInternalServerErrorRoute } from '../../app-routing-paths'; @@ -23,17 +30,36 @@ export class ServerCheckGuard implements CanActivateChild { */ canActivateChild( route: ActivatedRouteSnapshot, - state: RouterStateSnapshot): Observable { + state: RouterStateSnapshot + ): Observable { return this.rootDataService.checkServerAvailability().pipe( take(1), - tap((isAvailable: boolean) => { + map((isAvailable: boolean) => { if (!isAvailable) { - this.rootDataService.invalidateRootCache(); - this.router.navigateByUrl(getPageInternalServerErrorRoute()); + return this.router.parseUrl(getPageInternalServerErrorRoute()); + } else { + return true; } }) ); + } + /** + * Listen to all router events. Every time a new navigation starts, invalidate the cache + * for the root endpoint. That way we retrieve it once per routing operation to ensure the + * backend is not down. But if the guard is called multiple times during the same routing + * operation, the cached version is used. + */ + listenForRouteChanges(): void { + // we'll always be too late for the first NavigationStart event with the router subscribe below, + // so this statement is for the very first route operation. + this.rootDataService.invalidateRootCache(); + + this.router.events.pipe( + filter(event => event instanceof NavigationStart), + ).subscribe(() => { + this.rootDataService.invalidateRootCache(); + }); } } diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index 56e890b318..b81d0806df 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -1,4 +1,3 @@ -import { cold, hot } from 'jasmine-marbles'; import { getMockRequestService } from '../../shared/mocks/request.service.mock'; import { RequestService } from '../data/request.service'; import { HALEndpointService } from './hal-endpoint.service'; @@ -7,12 +6,17 @@ import { combineLatest as observableCombineLatest, of as observableOf } from 'rx import { environment } from '../../../environments/environment'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { TestScheduler } from 'rxjs/testing'; +import { RemoteData } from '../data/remote-data'; +import { RequestEntryState } from '../data/request-entry-state.model'; describe('HALEndpointService', () => { let service: HALEndpointService; let requestService: RequestService; let rdbService: RemoteDataBuildService; let envConfig; + let testScheduler; + let remoteDataMocks; const endpointMap = { test: { href: 'https://rest.api/test' @@ -68,7 +72,30 @@ describe('HALEndpointService', () => { }; const linkPath = 'test'; + const timeStamp = new Date().getTime(); + const msToLive = 15 * 60 * 1000; + const payload = { + _links: endpointMaps[one] + }; + 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), + ResponsePendingStale: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePendingStale, 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), + }; + beforeEach(() => { + testScheduler = new TestScheduler((actual, expected) => { + // asserting the two objects are equal + // e.g. using chai. + expect(actual).toEqual(expected); + }); requestService = getMockRequestService(); rdbService = jasmine.createSpyObj('rdbService', { buildFromHref: createSuccessfulRemoteDataObject$({ @@ -111,20 +138,28 @@ describe('HALEndpointService', () => { }); it(`should return the endpoint URL for the service's linkPath`, () => { - spyOn(service as any, 'getEndpointAt').and - .returnValue(hot('a-', { a: 'https://rest.api/test' })); - const result = service.getEndpoint(linkPath); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getEndpointAt').and + .returnValue(cold('a-', { a: 'https://rest.api/test' })); + const result = service.getEndpoint(linkPath); - const expected = cold('(b|)', { b: endpointMap.test.href }); - expect(result).toBeObservable(expected); + const expected = '(b|)'; + const values = { + b: endpointMap.test.href + }; + expectObservable(result).toBe(expected, values); + }); }); it('should return undefined for a linkPath that isn\'t in the endpoint map', () => { - spyOn(service as any, 'getEndpointAt').and - .returnValue(hot('a-', { a: undefined })); - const result = service.getEndpoint('unknown'); - const expected = cold('(b|)', { b: undefined }); - expect(result).toBeObservable(expected); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getEndpointAt').and + .returnValue(cold('a-', { a: undefined })); + const result = service.getEndpoint('unknown'); + const expected = '(b|)'; + const values = { b: undefined }; + expectObservable(result).toBe(expected, values); + }); }); }); @@ -183,29 +218,118 @@ describe('HALEndpointService', () => { }); it('should return undefined as long as getRootEndpointMap hasn\'t fired', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('----')); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('----')); - const result = service.isEnabledOnRestApi(linkPath); - const expected = cold('b---', { b: undefined }); - expect(result).toBeObservable(expected); + const result = service.isEnabledOnRestApi(linkPath); + const expected = 'b---'; + const values = { b: undefined }; + expectObservable(result).toBe(expected, values); + }); }); it('should return true if the service\'s linkPath is in the endpoint map', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('--a-', { a: endpointMap })); - const result = service.isEnabledOnRestApi(linkPath); - const expected = cold('b-c-', { b: undefined, c: true }); - expect(result).toBeObservable(expected); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('--a-', { a: endpointMap })); + const result = service.isEnabledOnRestApi(linkPath); + const expected = 'b-c-'; + const values = { b: undefined, c: true }; + expectObservable(result).toBe(expected, values); + }); }); it('should return false if the service\'s linkPath isn\'t in the endpoint map', () => { - spyOn(service as any, 'getRootEndpointMap').and - .returnValue(hot('--a-', { a: endpointMap })); + testScheduler.run(({ cold, expectObservable }) => { + spyOn(service as any, 'getRootEndpointMap').and + .returnValue(cold('--a-', { a: endpointMap })); - const result = service.isEnabledOnRestApi('unknown'); - const expected = cold('b-c-', { b: undefined, c: false }); - expect(result).toBeObservable(expected); + const result = service.isEnabledOnRestApi('unknown'); + const expected = 'b-c-'; + const values = { b: undefined, c: false }; + expectObservable(result).toBe(expected, values); + }); + }); + + }); + + describe(`getEndpointMapAt`, () => { + const href = 'https://rest.api/some/sub/path'; + + it(`should call requestService.send with a new EndpointMapRequest for the given href. useCachedVersionIfAvailable should be true`, () => { + testScheduler.run(() => { + (service as any).getEndpointMapAt(href); + }); + const expected = new EndpointMapRequest(requestService.generateRequestId(), href); + expect(requestService.send).toHaveBeenCalledWith(expected, true); + }); + + it(`should call rdbService.buildFromHref with the given href`, () => { + testScheduler.run(() => { + (service as any).getEndpointMapAt(href); + }); + expect(rdbService.buildFromHref).toHaveBeenCalledWith(href); + }); + + describe(`when the RemoteData returned from rdbService is stale`, () => { + it(`should re-request it`, () => { + spyOn(service as any, 'getEndpointMapAt').and.callThrough(); + testScheduler.run(({ cold }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { a: remoteDataMocks.ResponsePendingStale })); + // we need to subscribe to the result, to ensure the "tap" that does the re-request can fire + (service as any).getEndpointMapAt(href).subscribe(); + }); + expect((service as any).getEndpointMapAt).toHaveBeenCalledTimes(2); + }); + }); + + describe(`when the RemoteData returned from rdbService isn't stale`, () => { + it(`should not re-request it`, () => { + spyOn(service as any, 'getEndpointMapAt').and.callThrough(); + testScheduler.run(({ cold }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { a: remoteDataMocks.ResponsePending })); + // we need to subscribe to the result, to ensure the "tap" that does the re-request can fire + (service as any).getEndpointMapAt(href).subscribe(); + }); + expect((service as any).getEndpointMapAt).toHaveBeenCalledTimes(1); + }); + }); + + it(`should emit exactly once, returning the endpoint map in the response, when the RemoteData completes`, () => { + testScheduler.run(({ cold, expectObservable }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a-b-c-d-e-f-g-h-i-j-k-l', { + a: remoteDataMocks.RequestPending, + b: remoteDataMocks.ResponsePending, + c: remoteDataMocks.ResponsePendingStale, + d: remoteDataMocks.SuccessStale, + e: remoteDataMocks.RequestPending, + f: remoteDataMocks.ResponsePending, + g: remoteDataMocks.Success, + h: remoteDataMocks.SuccessStale, + i: remoteDataMocks.RequestPending, + k: remoteDataMocks.ResponsePending, + l: remoteDataMocks.Error, + })); + const expected = '------------(g|)'; + const values = { + g: endpointMaps[one] + }; + expectObservable((service as any).getEndpointMapAt(one)).toBe(expected, values); + }); + }); + + it(`should emit undefined when the response doesn't have a payload`, () => { + testScheduler.run(({ cold, expectObservable }) => { + (rdbService.buildFromHref as jasmine.Spy).and.returnValue(cold('a', { + a: remoteDataMocks.Error, + })); + const expected = '(a|)'; + const values = { + g: undefined + }; + expectObservable((service as any).getEndpointMapAt(href)).toBe(expected, values); + }); }); }); diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index 8b6316a6ce..07754616c7 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -1,5 +1,12 @@ import { Observable } from 'rxjs'; -import { distinctUntilChanged, map, startWith, switchMap, take } from 'rxjs/operators'; +import { + distinctUntilChanged, + map, + startWith, + switchMap, + take, + tap, filter +} from 'rxjs/operators'; import { RequestService } from '../data/request.service'; import { EndpointMapRequest } from '../data/request.models'; import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; @@ -9,7 +16,7 @@ import { EndpointMap } from '../cache/response.models'; import { getFirstCompletedRemoteData } from './operators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RemoteData } from '../data/remote-data'; -import { UnCacheableObject } from './uncacheable-object.model'; +import { CacheableObject } from '../cache/cacheable-object.model'; @Injectable() export class HALEndpointService { @@ -33,9 +40,18 @@ export class HALEndpointService { this.requestService.send(request, true); - return this.rdbService.buildFromHref(href).pipe( + return this.rdbService.buildFromHref(href).pipe( + // Re-request stale responses + tap((rd: RemoteData) => { + if (hasValue(rd) && rd.isStale) { + this.getEndpointMapAt(href); + } + }), + // Filter out all stale responses. We're only interested in a single, non-stale, + // completed RemoteData + filter((rd: RemoteData) => !rd.isStale), getFirstCompletedRemoteData(), - map((response: RemoteData) => { + map((response: RemoteData) => { if (hasValue(response.payload)) { return response.payload._links; } else { diff --git a/src/modules/app/browser-init.service.ts b/src/modules/app/browser-init.service.ts index 61d57f10f9..bf40f0c68b 100644 --- a/src/modules/app/browser-init.service.ts +++ b/src/modules/app/browser-init.service.ts @@ -32,6 +32,7 @@ import { logStartupMessage } from '../../../startup-message'; import { MenuService } from '../../app/shared/menu/menu.service'; import { RootDataService } from '../../app/core/data/root-data.service'; import { firstValueFrom, Subscription } from 'rxjs'; +import { ServerCheckGuard } from '../../app/core/server-check/server-check.guard'; /** * Performs client-side initialization. @@ -56,7 +57,8 @@ export class BrowserInitService extends InitService { protected authService: AuthService, protected themeService: ThemeService, protected menuService: MenuService, - private rootDataService: RootDataService + private rootDataService: RootDataService, + protected serverCheckGuard: ServerCheckGuard, ) { super( store, @@ -172,4 +174,13 @@ export class BrowserInitService extends InitService { }); } + /** + * Start route-listening subscriptions + * @protected + */ + protected initRouteListeners(): void { + super.initRouteListeners(); + this.serverCheckGuard.listenForRouteChanges(); + } + }