diff --git a/src/app/core/cache/object-cache.service.spec.ts b/src/app/core/cache/object-cache.service.spec.ts index bde6831967..f18c262524 100644 --- a/src/app/core/cache/object-cache.service.spec.ts +++ b/src/app/core/cache/object-cache.service.spec.ts @@ -211,25 +211,69 @@ describe('ObjectCacheService', () => { }); }); - describe('has', () => { + describe('hasByHref', () => { + describe('with requestUUID not specified', () => { + describe('getByHref emits an object', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(cacheEntry)); + }); - describe('getByHref emits an object', () => { - beforeEach(() => { - spyOn(service, 'getByHref').and.returnValue(observableOf(cacheEntry)); + it('should return true', () => { + expect(service.hasByHref(selfLink)).toBe(true); + }); }); - it('should return true', () => { - expect(service.hasByHref(selfLink)).toBe(true); + describe('getByHref emits nothing', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(empty()); + }); + + it('should return false', () => { + expect(service.hasByHref(selfLink)).toBe(false); + }); }); }); - describe('getByHref emits nothing', () => { - beforeEach(() => { - spyOn(service, 'getByHref').and.returnValue(empty()); + describe('with requestUUID specified', () => { + describe('getByHref emits an object that includes the specified requestUUID', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, { + requestUUIDs: [ + 'something', + 'something-else', + 'specific-request', + ] + }))); + }); + + it('should return true', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(true); + }); }); - it('should return false', () => { - expect(service.hasByHref(selfLink)).toBe(false); + describe('getByHref emits an object that doesn\'t include the specified requestUUID', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, { + requestUUIDs: [ + 'something', + 'something-else', + ] + }))); + }); + + it('should return true', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(false); + }); + }); + + describe('getByHref emits nothing', () => { + beforeEach(() => { + spyOn(service, 'getByHref').and.returnValue(empty()); + }); + + it('should return false', () => { + expect(service.hasByHref(selfLink, 'specific-request')).toBe(false); + }); }); }); }); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 00f124b2f6..cdf87e5c1a 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -282,7 +282,7 @@ export class ObjectCacheService { let result = false; this.getByHref(href).subscribe((entry: ObjectCacheEntry) => { if (isNotEmpty(requestUUID)) { - result = entry.requestUUIDs[0] === requestUUID; // todo: may make more sense to do entry.requestUUIDs.includes(requestUUID) instead + result = entry.requestUUIDs.includes(requestUUID); } else { result = true; } diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index 7022196c1a..c7721d29e2 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -864,6 +864,16 @@ describe('DataService', () => { }); }); + it('should call setStaleByUUID even if not subscribing to returned Observable', fakeAsync(() => { + service.invalidateByHref('some-href'); + tick(); + + expect(getByHrefSpy).toHaveBeenCalledWith('some-href'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2'); + expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3'); + })); + it('should return an Observable that only emits true once all requests are stale', () => { testScheduler.run(({ cold, expectObservable }) => { requestService.setStaleByUUID.and.callFake((uuid) => { @@ -906,11 +916,11 @@ describe('DataService', () => { it('should retrieve href by ID and call deleteByHref', () => { getIDHrefObsSpy.and.returnValue(observableOf('some-href')); - buildFromRequestUUIDSpy.and.returnValue(null); + buildFromRequestUUIDSpy.and.returnValue(createSuccessfulRemoteDataObject$({})); - service.delete('some-id').subscribe(rd => { + service.delete('some-id', ['a', 'b', 'c']).subscribe(rd => { expect(getIDHrefObsSpy).toHaveBeenCalledWith('some-id'); - expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href'); + expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href', ['a', 'b', 'c']); }); }); @@ -925,6 +935,15 @@ describe('DataService', () => { }); }); + it('should call invalidateByHref even if not subscribing to returned Observable', fakeAsync(() => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.deleteByHref('some-href'); + tick(); + + expect(invalidateByHrefSpy).toHaveBeenCalled(); + })); + it('should not call invalidateByHref if the DELETE request fails', (done) => { buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_FAILED_RD)); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index ca4d7e42de..909bc8d14b 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; import { Operation } from 'fast-json-patch'; -import { combineLatest, from, Observable, of as observableOf } from 'rxjs'; +import { AsyncSubject, combineLatest, from as observableFrom, Observable, of as observableOf } from 'rxjs'; import { distinctUntilChanged, filter, @@ -21,6 +21,7 @@ import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { getClassForType } from '../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RequestParam } from '../cache/models/request-param.model'; +import { ObjectCacheEntry } from '../cache/object-cache.reducer'; import { ObjectCacheService } from '../cache/object-cache.service'; import { DSpaceSerializer } from '../dspace-rest/dspace.serializer'; import { DSpaceObject } from '../shared/dspace-object.model'; @@ -596,18 +597,22 @@ export abstract class DataService implements UpdateDa * @return An Observable that will emit `true` once all requests are stale */ invalidateByHref(href: string): Observable { - return this.objectCache.getByHref(href).pipe( - map(oce => oce.requestUUIDs), - switchMap(requestUUIDs => { - return from(requestUUIDs).pipe( - mergeMap(requestUUID => this.requestService.setStaleByUUID(requestUUID)), - toArray(), - ); - }), - map(areRequestsStale => areRequestsStale.every(Boolean)), + const done$ = new AsyncSubject(); + + this.objectCache.getByHref(href).pipe( + switchMap((oce: ObjectCacheEntry) => observableFrom(oce.requestUUIDs)), + mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)), + toArray(), + map((areRequestsStale: boolean[]) => areRequestsStale.every(Boolean)), distinctUntilChanged(), - takeWhile(allStale => allStale === false, true), - ); + ).subscribe((done: boolean) => { + if (done) { + done$.next(true); + done$.complete(); + } + }); + + return done$; } /** @@ -652,22 +657,25 @@ export abstract class DataService implements UpdateDa const response$ = this.rdbService.buildFromRequestUUID(requestId); - const invalidated$ = response$.pipe( + const invalidated$ = new AsyncSubject(); + response$.pipe( getFirstCompletedRemoteData(), - switchMap(rd => { + switchMap((rd: RemoteData) => { if (rd.hasSucceeded) { return this.invalidateByHref(href); } else { return [true]; } }) - ); + ).subscribe((invalidated: boolean) => { + if (invalidated) { + invalidated$.next(true); + invalidated$.complete(); + } + }); return combineLatest([response$, invalidated$]).pipe( filter(([_, invalidated]) => invalidated), - tap(() => { - console.log(`DataService.deleteByHref() href=${href} done.`); - }), map(([response, _]) => response), ); } diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index b8b1d1eba9..2d5acb2cb3 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -320,8 +320,8 @@ export class RequestService { this.store.dispatch(new RequestStaleAction(uuid)); return this.getByUUID(uuid).pipe( - map(request => isStale(request.state)), - filter(stale => stale === true), + map((request: RequestEntry) => isStale(request.state)), + filter((stale: boolean) => stale), take(1), ); }