From 9f6616a5ce94202c10f33cdf6d33eb3b0dbb8697 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Tue, 28 Mar 2023 14:38:44 +0200 Subject: [PATCH] 100302: Add support to check if a request has a cached value --- .../core/data/base/base-data.service.spec.ts | 56 +++++++++++++++++++ src/app/core/data/base/base-data.service.ts | 31 ++++++++++ .../data/external-source-data.service.spec.ts | 42 +++++++++++--- .../core/data/external-source-data.service.ts | 7 ++- src/app/shared/mocks/request.service.mock.ts | 3 +- .../submission-import-external.component.ts | 7 +-- 6 files changed, 129 insertions(+), 17 deletions(-) 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 17532f477a..098f075c10 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -641,6 +641,62 @@ describe('BaseDataService', () => { }); }); + describe('hasCachedResponse', () => { + it('should return false when the request will be dispatched', (done) => { + const result = service.hasCachedResponse('test-href'); + + result.subscribe((hasCachedResponse) => { + expect(hasCachedResponse).toBeFalse(); + done(); + }); + }); + + it('should return true when the request will not be dispatched', (done) => { + (requestService.shouldDispatchRequest as jasmine.Spy).and.returnValue(false); + const result = service.hasCachedResponse('test-href'); + + result.subscribe((hasCachedResponse) => { + expect(hasCachedResponse).toBeTrue(); + done(); + }); + }); + }); + + describe('hasCachedErrorResponse', () => { + it('should return false when no response is cached', (done) => { + spyOn(service,'hasCachedResponse').and.returnValue(observableOf(false)); + const result = service.hasCachedErrorResponse('test-href'); + + result.subscribe((hasCachedErrorResponse) => { + expect(hasCachedErrorResponse).toBeFalse(); + done(); + }); + }); + it('should return false when no error response is cached', (done) => { + spyOn(service,'hasCachedResponse').and.returnValue(observableOf(true)); + spyOn(rdbService,'buildSingle').and.returnValue(createSuccessfulRemoteDataObject$({})); + + const result = service.hasCachedErrorResponse('test-href'); + + result.subscribe((hasCachedErrorResponse) => { + expect(hasCachedErrorResponse).toBeFalse(); + done(); + }); + }); + + it('should return true when an error response is cached', (done) => { + spyOn(service,'hasCachedResponse').and.returnValue(observableOf(true)); + spyOn(rdbService,'buildSingle').and.returnValue(createFailedRemoteDataObject$()); + + const result = service.hasCachedErrorResponse('test-href'); + + result.subscribe((hasCachedErrorResponse) => { + expect(hasCachedErrorResponse).toBeTrue(); + done(); + }); + }); + }); + describe('addDependency', () => { let addDependencySpy; diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index 85603580a4..e373c8cfac 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -341,6 +341,37 @@ export class BaseDataService implements HALDataServic } } + hasCachedResponse(href$: string | Observable): Observable { + if (isNotEmpty(href$)) { + if (typeof href$ === 'string') { + href$ = observableOf(href$); + } + return href$.pipe( + isNotEmptyOperator(), + take(1), + map((href: string) => { + const requestId = this.requestService.generateRequestId(); + const request = new GetRequest(requestId, href); + return !this.requestService.shouldDispatchRequest(request, true); + }), + ); + } + } + + hasCachedErrorResponse(href$: string | Observable): Observable { + return this.hasCachedResponse(href$).pipe( + switchMap((hasCachedResponse) => { + if (hasCachedResponse) { + return this.rdbService.buildSingle(href$).pipe( + getFirstCompletedRemoteData(), + map((rd => rd.isError || rd.isErrorStale)) + ); + } + return observableOf(false); + }) + ); + } + /** * Return the links to traverse from the root of the api to the * endpoint this DataService represents diff --git a/src/app/core/data/external-source-data.service.spec.ts b/src/app/core/data/external-source-data.service.spec.ts index cdbdbaa006..723d7f9bed 100644 --- a/src/app/core/data/external-source-data.service.spec.ts +++ b/src/app/core/data/external-source-data.service.spec.ts @@ -5,6 +5,7 @@ import { ExternalSourceEntry } from '../shared/external-source-entry.model'; import { of as observableOf } from 'rxjs'; import { GetRequest } from './request.models'; import { testSearchDataImplementation } from './base/search-data.spec'; +import { take } from 'rxjs/operators'; describe('ExternalSourceService', () => { let service: ExternalSourceDataService; @@ -64,19 +65,42 @@ describe('ExternalSourceService', () => { }); describe('getExternalSourceEntries', () => { - let result; - beforeEach(() => { - result = service.getExternalSourceEntries('test'); + describe('when no error response is cached', () => { + let result; + beforeEach(() => { + spyOn(service, 'hasCachedErrorResponse').and.returnValue(observableOf(false)); + result = service.getExternalSourceEntries('test'); + }); + + it('should send a GetRequest', () => { + result.pipe(take(1)).subscribe(); + expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), true); + }); + + it('should return the entries', () => { + result.subscribe((resultRD) => { + expect(resultRD.payload.page).toBe(entries); + }); + }); }); - it('should send a GetRequest', () => { - expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), true); - }); + describe('when an error response is cached', () => { + let result; + beforeEach(() => { + spyOn(service, 'hasCachedErrorResponse').and.returnValue(observableOf(true)); + result = service.getExternalSourceEntries('test'); + }); - it('should return the entries', () => { - result.subscribe((resultRD) => { - expect(resultRD.payload.page).toBe(entries); + it('should send a GetRequest', () => { + result.pipe(take(1)).subscribe(); + expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), false); + }); + + it('should return the entries', () => { + result.subscribe((resultRD) => { + expect(resultRD.payload.page).toBe(entries); + }); }); }); }); diff --git a/src/app/core/data/external-source-data.service.ts b/src/app/core/data/external-source-data.service.ts index c0552aeaec..02c5e4a53c 100644 --- a/src/app/core/data/external-source-data.service.ts +++ b/src/app/core/data/external-source-data.service.ts @@ -74,7 +74,12 @@ export class ExternalSourceDataService extends IdentifiableDataService { + return this.findListByHref(href$, undefined, !hasCachedErrorResponse, reRequestOnStale, ...linksToFollow as any); + }) + ) as any; } /** diff --git a/src/app/shared/mocks/request.service.mock.ts b/src/app/shared/mocks/request.service.mock.ts index bce5b2d466..90db66dcd7 100644 --- a/src/app/shared/mocks/request.service.mock.ts +++ b/src/app/shared/mocks/request.service.mock.ts @@ -14,6 +14,7 @@ export function getMockRequestService(requestEntry$: Observable = removeByHrefSubstring: observableOf(true), setStaleByHrefSubstring: observableOf(true), setStaleByUUID: observableOf(true), - hasByHref$: observableOf(false) + hasByHref$: observableOf(false), + shouldDispatchRequest: true }); } diff --git a/src/app/submission/import-external/submission-import-external.component.ts b/src/app/submission/import-external/submission-import-external.component.ts index 8c2c5eca9e..25b1d5d1aa 100644 --- a/src/app/submission/import-external/submission-import-external.component.ts +++ b/src/app/submission/import-external/submission-import-external.component.ts @@ -188,17 +188,12 @@ export class SubmissionImportExternalComponent implements OnInit, OnDestroy { this.retrieveExternalSourcesSub = this.reload$.pipe( filter((sourceQueryObject: ExternalSourceData) => isNotEmpty(sourceQueryObject.sourceId) && isNotEmpty(sourceQueryObject.query)), switchMap((sourceQueryObject: ExternalSourceData) => { - const currentEntry = this.entriesRD$.getValue(); - let useCache = true; - if (hasValue(currentEntry) && currentEntry.isError) { - useCache = false; - } const query = sourceQueryObject.query; this.routeData = sourceQueryObject; return this.searchConfigService.paginatedSearchOptions.pipe( tap(() => this.isLoading$.next(true)), filter((searchOptions) => searchOptions.query === query), - mergeMap((searchOptions) => this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions, useCache).pipe( + mergeMap((searchOptions) => this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions).pipe( getFinishedRemoteData(), )) );