From f8404c540e305b2a8afd32c13ff1db0ab248c007 Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Thu, 8 Sep 2022 12:10:41 +0200 Subject: [PATCH] 94207: Extract 'wait for invalidation' pattern into a new method --- .../remote-data-build.service.spec.ts | 95 ++++++++++- .../builders/remote-data-build.service.ts | 45 +++++ .../bitstream-format-data.service.spec.ts | 3 +- src/app/core/data/data.service.spec.ts | 78 +++------ src/app/core/data/data.service.ts | 30 +--- .../core/eperson/group-data.service.spec.ts | 21 +++ src/app/core/eperson/group-data.service.ts | 160 +++--------------- .../resource-policy.service.spec.ts | 41 ++--- .../resource-policy.service.ts | 30 +--- .../mocks/remote-data-build.service.mock.ts | 2 + 10 files changed, 217 insertions(+), 288 deletions(-) diff --git a/src/app/core/cache/builders/remote-data-build.service.spec.ts b/src/app/core/cache/builders/remote-data-build.service.spec.ts index befeabdd90..d9b856bb77 100644 --- a/src/app/core/cache/builders/remote-data-build.service.spec.ts +++ b/src/app/core/cache/builders/remote-data-build.service.spec.ts @@ -1,4 +1,4 @@ -import { createSuccessfulRemoteDataObject } from '../../../shared/remote-data.utils'; +import { createFailedRemoteDataObject, createPendingRemoteDataObject, createSuccessfulRemoteDataObject } from '../../../shared/remote-data.utils'; import { buildPaginatedList, PaginatedList } from '../../data/paginated-list.model'; import { Item } from '../../shared/item.model'; import { PageInfo } from '../../shared/page-info.model'; @@ -19,6 +19,8 @@ import { HALLink } from '../../shared/hal-link.model'; import { RequestEntryState } from '../../data/request-entry-state.model'; import { RequestEntry } from '../../data/request-entry.model'; import { cold } from 'jasmine-marbles'; +import { TestScheduler } from 'rxjs/testing'; +import { fakeAsync, tick } from '@angular/core/testing'; describe('RemoteDataBuildService', () => { let service: RemoteDataBuildService; @@ -763,4 +765,95 @@ describe('RemoteDataBuildService', () => { }); }); }); + + describe('buildFromRequestUUIDAndAwait', () => { + let testScheduler; + + let callback: jasmine.Spy; + let buildFromRequestUUIDSpy; + + const BOOLEAN = { t: true, f: false }; + + const MOCK_PENDING_RD = createPendingRemoteDataObject(); + const MOCK_SUCCEEDED_RD = createSuccessfulRemoteDataObject({}); + const MOCK_FAILED_RD = createFailedRemoteDataObject('failed'); + + const RDs = { + p: MOCK_PENDING_RD, + s: MOCK_SUCCEEDED_RD, + f: MOCK_FAILED_RD, + }; + + + beforeEach(() => { + testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + callback = jasmine.createSpy('callback'); + callback.and.returnValue(observableOf(undefined)); + buildFromRequestUUIDSpy = spyOn(service, 'buildFromRequestUUID').and.callThrough(); + }); + + it('should patch through href & followLinks to buildFromRequestUUID', () => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + service.buildFromRequestUUIDAndAwait('some-href', callback, ...linksToFollow); + expect(buildFromRequestUUIDSpy).toHaveBeenCalledWith('some-href', ...linksToFollow); + }); + + it('should trigger the callback on successful RD', (done) => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.buildFromRequestUUIDAndAwait('some-href', callback).subscribe(rd => { + expect(rd).toBe(MOCK_SUCCEEDED_RD); + expect(callback).toHaveBeenCalled(); + done(); + }); + }); + + it('should trigger the callback on successful RD even if nothing subscribes to the returned Observable', fakeAsync(() => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.buildFromRequestUUIDAndAwait('some-href', callback); + tick(); + + expect(callback).toHaveBeenCalled(); + })); + + it('should not trigger the callback on pending RD', (done) => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_PENDING_RD)); + + service.buildFromRequestUUIDAndAwait('some-href', callback).subscribe(rd => { + expect(rd).toBe(MOCK_PENDING_RD); + expect(callback).not.toHaveBeenCalled(); + done(); + }); + }); + + it('should not trigger the callback on failed RD', (done) => { + buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_FAILED_RD)); + + service.buildFromRequestUUIDAndAwait('some-href', callback).subscribe(rd => { + expect(rd).toBe(MOCK_FAILED_RD); + expect(callback).not.toHaveBeenCalled(); + done(); + }); + }); + + it('should only emit after the callback is done', () => { + testScheduler.run(({ cold: tsCold, expectObservable }) => { + buildFromRequestUUIDSpy.and.returnValue( + tsCold('-p----s', RDs) + ); + callback.and.returnValue( + tsCold(' --t', BOOLEAN) + ); + + const done$ = service.buildFromRequestUUIDAndAwait('some-href', callback); + expectObservable(done$).toBe( + ' -p------s', RDs // resulting duration between pending & successful includes the callback + ); + }); + }); + }); }); diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index 8ceab63e03..76529891d4 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -1,5 +1,6 @@ import { Injectable } from '@angular/core'; import { + AsyncSubject, combineLatest as observableCombineLatest, Observable, of as observableOf, @@ -24,6 +25,7 @@ import { hasSucceeded, isStale, RequestEntryState } from '../../data/request-ent import { getRequestFromRequestHref, getRequestFromRequestUUID } from '../../shared/request.operators'; import { RequestEntry } from '../../data/request-entry.model'; import { ResponseState } from '../../data/response-state.model'; +import { getFirstCompletedRemoteData } from '../../shared/operators'; @Injectable() export class RemoteDataBuildService { @@ -188,6 +190,49 @@ export class RemoteDataBuildService { return this.toRemoteDataObservable(requestEntry$, payload$); } + /** + * Creates a {@link RemoteData} object for a rest request and its response + * and emits it only after the callback function is completed. + * + * @param requestUUID$ The UUID of the request we want to retrieve + * @param callback A function that returns an Observable. It will only be called once the request has succeeded. + * Then, the response will only be emitted after this callback function has emitted. + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved + */ + buildFromRequestUUIDAndAwait(requestUUID$: string | Observable, callback: (rd?: RemoteData) => Observable, ...linksToFollow: FollowLinkConfig[]): Observable> { + const response$ = this.buildFromRequestUUID(requestUUID$, ...linksToFollow); + + const callbackDone$ = new AsyncSubject(); + response$.pipe( + getFirstCompletedRemoteData(), + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + // if the request succeeded, execute the callback + return callback(rd); + } else { + // otherwise, emit right away so the subscription doesn't stick around + return [true]; + } + }), + ).subscribe(() => { + callbackDone$.next(true); + callbackDone$.complete(); + }); + + return response$.pipe( + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + // if the request succeeded, wait for the callback to finish + return callbackDone$.pipe( + map(() => rd), + ); + } else { + return [rd]; + } + }) + ); + } + /** * Creates a {@link RemoteData} object for a rest request and its response * diff --git a/src/app/core/data/bitstream-format-data.service.spec.ts b/src/app/core/data/bitstream-format-data.service.spec.ts index 30ef79ee6d..9dd176fd11 100644 --- a/src/app/core/data/bitstream-format-data.service.spec.ts +++ b/src/app/core/data/bitstream-format-data.service.spec.ts @@ -59,7 +59,8 @@ describe('BitstreamFormatDataService', () => { function initTestService(halService) { rd = createSuccessfulRemoteDataObject({}); rdbService = jasmine.createSpyObj('rdbService', { - buildFromRequestUUID: observableOf(rd) + buildFromRequestUUID: observableOf(rd), + buildFromRequestUUIDAndAwait: observableOf(rd), }); return new BitstreamFormatDataService( diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index dc661e12d7..b8c41dee9d 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -920,16 +920,16 @@ describe('DataService', () => { let MOCK_SUCCEEDED_RD; let MOCK_FAILED_RD; - let invalidateByHrefSpy: jasmine.Spy; - let buildFromRequestUUIDSpy: jasmine.Spy; + let buildFromRequestUUIDAndAwaitSpy: jasmine.Spy; let getIDHrefObsSpy: jasmine.Spy; let deleteByHrefSpy: jasmine.Spy; + let invalidateByHrefSpy: jasmine.Spy; beforeEach(() => { - invalidateByHrefSpy = spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true)); - buildFromRequestUUIDSpy = spyOn(rdbService, 'buildFromRequestUUID').and.callThrough(); + buildFromRequestUUIDAndAwaitSpy = spyOn(rdbService, 'buildFromRequestUUIDAndAwait').and.callThrough(); getIDHrefObsSpy = spyOn(service, 'getIDHrefObs').and.callThrough(); deleteByHrefSpy = spyOn(service, 'deleteByHref').and.callThrough(); + invalidateByHrefSpy = spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true)); MOCK_SUCCEEDED_RD = createSuccessfulRemoteDataObject({}); MOCK_FAILED_RD = createFailedRemoteDataObject('something went wrong'); @@ -937,7 +937,7 @@ describe('DataService', () => { it('should retrieve href by ID and call deleteByHref', () => { getIDHrefObsSpy.and.returnValue(observableOf('some-href')); - buildFromRequestUUIDSpy.and.returnValue(createSuccessfulRemoteDataObject$({})); + buildFromRequestUUIDAndAwaitSpy.and.returnValue(createSuccessfulRemoteDataObject$({})); service.delete('some-id', ['a', 'b', 'c']).subscribe(rd => { expect(getIDHrefObsSpy).toHaveBeenCalledWith('some-id'); @@ -946,65 +946,27 @@ describe('DataService', () => { }); describe('deleteByHref', () => { - it('should call invalidateByHref if the DELETE request succeeds', (done) => { - buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); - - service.deleteByHref('some-href').subscribe(rd => { - expect(rd).toBe(MOCK_SUCCEEDED_RD); - expect(invalidateByHrefSpy).toHaveBeenCalled(); - done(); - }); + beforeEach(() => { + buildFromRequestUUIDAndAwaitSpy.and.returnValue(createSuccessfulRemoteDataObject$({})); }); - it('should call invalidateByHref even if not subscribing to returned Observable', fakeAsync(() => { - buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + it('should send a DELETE request', () => { + service.deleteByHref('https://somewhere.org/something/123', ['things', 'stuff']); - 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)); - - service.deleteByHref('some-href').subscribe(rd => { - expect(rd).toBe(MOCK_FAILED_RD); - expect(invalidateByHrefSpy).not.toHaveBeenCalled(); - done(); - }); + expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ + method: 'DELETE', + href: 'https://somewhere.org/something/123?copyVirtualMetadata=things©VirtualMetadata=stuff' + })); }); - it('should wait for invalidateByHref before emitting', () => { - testScheduler.run(({ cold, expectObservable }) => { - buildFromRequestUUIDSpy.and.returnValue( - cold('(r|)', { r: MOCK_SUCCEEDED_RD}) // RD emits right away - ); - invalidateByHrefSpy.and.returnValue( - cold('----(t|)', BOOLEAN) // but we pretend that setting requests to stale takes longer - ); + it('should retrieve response via buildFromRequestUUIDAndAwait & invalidate within the callback', () => { + service.deleteByHref('https://somewhere.org/something/123'); - const done$ = service.deleteByHref('some-href'); - expectObservable(done$).toBe( - '----(r|)', { r: MOCK_SUCCEEDED_RD} // ...and expect the returned Observable to wait until that's done - ); - }); - }); - - it('should wait for the DELETE request to resolve before emitting', () => { - testScheduler.run(({ cold, expectObservable }) => { - buildFromRequestUUIDSpy.and.returnValue( - cold('----(r|)', { r: MOCK_SUCCEEDED_RD}) // the request takes a while - ); - invalidateByHrefSpy.and.returnValue( - cold('(t|)', BOOLEAN) // but we pretend that setting to stale happens sooner - ); // e.g.: maybe already stale before this call? - - const done$ = service.deleteByHref('some-href'); - expectObservable(done$).toBe( - '----(r|)', { r: MOCK_SUCCEEDED_RD} // ...and expect the returned Observable to wait for the request - ); - }); + expect(buildFromRequestUUIDAndAwaitSpy).toHaveBeenCalled(); + expect(buildFromRequestUUIDAndAwaitSpy.calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = buildFromRequestUUIDAndAwaitSpy.calls.argsFor(0)[1]; + callback(); + expect(invalidateByHrefSpy).toHaveBeenCalledWith('https://somewhere.org/something/123'); }); }); }); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 695b2e5fea..279791d326 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -649,35 +649,7 @@ export abstract class DataService implements UpdateDa } this.requestService.send(request); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return this.invalidateByHref(href); - } else { - return [true]; - } - }) - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => this.invalidateByHref(href)); } /** diff --git a/src/app/core/eperson/group-data.service.spec.ts b/src/app/core/eperson/group-data.service.spec.ts index 01c8632afa..c1b9e59d5b 100644 --- a/src/app/core/eperson/group-data.service.spec.ts +++ b/src/app/core/eperson/group-data.service.spec.ts @@ -95,6 +95,7 @@ describe('GroupDataService', () => { store = new Store(undefined, undefined, undefined); service = initTestService(); spyOn(store, 'dispatch'); + spyOn(rdbService, 'buildFromRequestUUIDAndAwait').and.callThrough(); }); describe('searchGroups', () => { @@ -136,6 +137,11 @@ describe('GroupDataService', () => { expect(requestService.send).toHaveBeenCalledWith(expected); }); it('should invalidate the previous requests of the parent group', () => { + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect(rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[1]; + callback(); + expect(objectCache.getByHref).toHaveBeenCalledWith(GroupMock._links.self.href); expect(requestService.setStaleByUUID).toHaveBeenCalledTimes(2); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); @@ -156,6 +162,11 @@ describe('GroupDataService', () => { expect(requestService.send).toHaveBeenCalledWith(expected); }); it('should invalidate the previous requests of the parent group\'', () => { + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect(rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[1]; + callback(); + expect(objectCache.getByHref).toHaveBeenCalledWith(GroupMock._links.self.href); expect(requestService.setStaleByUUID).toHaveBeenCalledTimes(2); expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1'); @@ -180,6 +191,11 @@ describe('GroupDataService', () => { expect(requestService.send).toHaveBeenCalledWith(expected); }); it('should invalidate the previous requests of the EPerson and the group', () => { + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect(rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[1]; + callback(); + expect(objectCache.getByHref).toHaveBeenCalledWith(EPersonMock2._links.self.href); expect(objectCache.getByHref).toHaveBeenCalledWith(GroupMock._links.self.href); expect(requestService.setStaleByUUID).toHaveBeenCalledTimes(4); @@ -201,6 +217,11 @@ describe('GroupDataService', () => { expect(requestService.send).toHaveBeenCalledWith(expected); }); it('should invalidate the previous requests of the EPerson and the group', () => { + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect(rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = rdbService.buildFromRequestUUIDAndAwait.calls.argsFor(0)[1]; + callback(); + expect(objectCache.getByHref).toHaveBeenCalledWith(EPersonMock._links.self.href); expect(objectCache.getByHref).toHaveBeenCalledWith(GroupMock._links.self.href); expect(requestService.setStaleByUUID).toHaveBeenCalledTimes(4); diff --git a/src/app/core/eperson/group-data.service.ts b/src/app/core/eperson/group-data.service.ts index a994170281..df1f5bcb09 100644 --- a/src/app/core/eperson/group-data.service.ts +++ b/src/app/core/eperson/group-data.service.ts @@ -124,40 +124,10 @@ export class GroupDataService extends DataService { const postRequest = new PostRequest(requestId, activeGroup.self + '/' + this.subgroupsEndpoint, subgroup.self, options); this.requestService.send(postRequest); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return observableZip( - this.invalidateByHref(activeGroup._links.self.href), - this.requestService.setStaleByHrefSubstring(activeGroup._links.subgroups.href).pipe(take(1)), - ).pipe( - map((arr: boolean[]) => arr.every((b: boolean) => b === true)) - ); - } else { - return [true]; - } - }) - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => observableZip( + this.invalidateByHref(activeGroup._links.self.href), + this.requestService.setStaleByHrefSubstring(activeGroup._links.subgroups.href).pipe(take(1)), + )); } /** @@ -172,40 +142,10 @@ export class GroupDataService extends DataService { const deleteRequest = new DeleteRequest(requestId, activeGroup.self + '/' + this.subgroupsEndpoint + '/' + subgroup.id); this.requestService.send(deleteRequest); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return observableZip( - this.invalidateByHref(activeGroup._links.self.href), - this.requestService.setStaleByHrefSubstring(activeGroup._links.subgroups.href).pipe(take(1)), - ).pipe( - map((arr: boolean[]) => arr.every((b: boolean) => b === true)) - ); - } else { - return [true]; - } - }) - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => observableZip( + this.invalidateByHref(activeGroup._links.self.href), + this.requestService.setStaleByHrefSubstring(activeGroup._links.subgroups.href).pipe(take(1)), + )); } /** @@ -223,42 +163,12 @@ export class GroupDataService extends DataService { const postRequest = new PostRequest(requestId, activeGroup.self + '/' + this.ePersonsEndpoint, ePerson.self, options); this.requestService.send(postRequest); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return observableZip( - this.invalidateByHref(ePerson._links.self.href), - this.invalidateByHref(activeGroup._links.self.href), - this.requestService.setStaleByHrefSubstring(ePerson._links.groups.href).pipe(take(1)), - this.requestService.setStaleByHrefSubstring(activeGroup._links.epersons.href).pipe(take(1)), - ).pipe( - map((arr: boolean[]) => arr.every((b: boolean) => b === true)) - ); - } else { - return [true]; - } - }) - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => observableZip( + this.invalidateByHref(ePerson._links.self.href), + this.invalidateByHref(activeGroup._links.self.href), + this.requestService.setStaleByHrefSubstring(ePerson._links.groups.href).pipe(take(1)), + this.requestService.setStaleByHrefSubstring(activeGroup._links.epersons.href).pipe(take(1)), + )); } /** @@ -272,42 +182,12 @@ export class GroupDataService extends DataService { const deleteRequest = new DeleteRequest(requestId, activeGroup.self + '/' + this.ePersonsEndpoint + '/' + ePerson.id); this.requestService.send(deleteRequest); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return observableZip( - this.invalidateByHref(ePerson._links.self.href), - this.invalidateByHref(activeGroup._links.self.href), - this.requestService.setStaleByHrefSubstring(ePerson._links.groups.href).pipe(take(1)), - this.requestService.setStaleByHrefSubstring(activeGroup._links.epersons.href).pipe(take(1)), - ).pipe( - map((arr: boolean[]) => arr.every((b: boolean) => b === true)) - ); - } else { - return [true]; - } - }) - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => observableZip( + this.invalidateByHref(ePerson._links.self.href), + this.invalidateByHref(activeGroup._links.self.href), + this.requestService.setStaleByHrefSubstring(ePerson._links.groups.href).pipe(take(1)), + this.requestService.setStaleByHrefSubstring(activeGroup._links.epersons.href).pipe(take(1)), + )); } /** diff --git a/src/app/core/resource-policy/resource-policy.service.spec.ts b/src/app/core/resource-policy/resource-policy.service.spec.ts index e4d3358d55..357a9ec659 100644 --- a/src/app/core/resource-policy/resource-policy.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy.service.spec.ts @@ -123,6 +123,9 @@ describe('ResourcePolicyService', () => { }), buildFromRequestUUID: hot('a|', { a: resourcePolicyRD + }), + buildFromRequestUUIDAndAwait: hot('a|', { + a: resourcePolicyRD }) }); ePersonService = jasmine.createSpyObj('ePersonService', { @@ -346,6 +349,8 @@ describe('ResourcePolicyService', () => { }); describe('updateTarget', () => { + let buildFromRequestUUIDAndAwaitSpy; + beforeEach(() => { scheduler.schedule(() => service.create(resourcePolicy, resourceUUID, epersonUUID)); }); @@ -362,41 +367,17 @@ describe('ResourcePolicyService', () => { })); }); - it('should send a PUT request to update the Group', () => { - service.updateTarget(resourcePolicyId, requestURL, groupUUID, 'group'); - scheduler.flush(); - - expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ - method: RestRequestMethod.PUT, - uuid: requestUUID, - href: `${resourcePolicy._links.self.href}/group`, - body: 'https://rest.api/rest/api/eperson/groups/' + groupUUID, - })); - }); - - it('should invalidate the ResourcePolicy if the PUT request succeeds', () => { + it('should invalidate the ResourcePolicy', () => { service.updateTarget(resourcePolicyId, requestURL, epersonUUID, 'eperson'); scheduler.flush(); + expect(rdbService.buildFromRequestUUIDAndAwait).toHaveBeenCalled(); + expect((rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[0]).toBe(requestService.generateRequestId()); + const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1]; + callback(); + expect((service as any).dataService.invalidateByHref).toHaveBeenCalledWith(resourcePolicy._links.self.href); }); - - it('should only emit when invalidation is complete', () => { - const RD = { - p: createPendingRemoteDataObject(), - s: createSuccessfulRemoteDataObject({}), - }; - const response$ = cold('(s|)', RD); - const invalidate$ = cold('--(d|)', { d: true }); - - (rdbService.buildFromRequestUUID as any).and.returnValue(response$); - ((service as any).dataService.invalidateByHref).and.returnValue(invalidate$); - - const out$ = service.updateTarget(resourcePolicyId, requestURL, groupUUID, 'group'); - scheduler.flush(); - - expect(out$).toBeObservable(cold('--(s|)', RD)); - }); }); }); diff --git a/src/app/core/resource-policy/resource-policy.service.ts b/src/app/core/resource-policy/resource-policy.service.ts index 0178c460f1..615a9470d9 100644 --- a/src/app/core/resource-policy/resource-policy.service.ts +++ b/src/app/core/resource-policy/resource-policy.service.ts @@ -263,35 +263,7 @@ export class ResourcePolicyService { this.requestService.send(request); }); - const response$ = this.rdbService.buildFromRequestUUID(requestId); - - const invalidated$ = new AsyncSubject(); - response$.pipe( - getFirstCompletedRemoteData(), - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return this.dataService.invalidateByHref(resourcePolicyHref); - } else { - return [undefined]; - } - }), - ).subscribe(() => { - invalidated$.next(true); - invalidated$.complete(); - }); - - return response$.pipe( - switchMap((rd: RemoteData) => { - if (rd.hasSucceeded) { - return invalidated$.pipe( - filter((invalidated: boolean) => invalidated), - map(() => rd) - ); - } else { - return [rd]; - } - }) - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => this.dataService.invalidateByHref(resourcePolicyHref)); } } diff --git a/src/app/shared/mocks/remote-data-build.service.mock.ts b/src/app/shared/mocks/remote-data-build.service.mock.ts index 7b96c5a67a..a0fc74a753 100644 --- a/src/app/shared/mocks/remote-data-build.service.mock.ts +++ b/src/app/shared/mocks/remote-data-build.service.mock.ts @@ -29,6 +29,7 @@ export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observab } }, buildFromRequestUUID: (id: string) => createSuccessfulRemoteDataObject$({}), + buildFromRequestUUIDAndAwait: (id: string, callback: (rd?: RemoteData) => Observable) => createSuccessfulRemoteDataObject$({}), buildFromHref: (href: string) => createSuccessfulRemoteDataObject$({}) } as RemoteDataBuildService; @@ -66,6 +67,7 @@ export function getMockRemoteDataBuildServiceHrefMap(toRemoteDataObservable$?: O ); }, buildFromRequestUUID: (id: string) => createSuccessfulRemoteDataObject$({}), + buildFromRequestUUIDAndAwait: (id: string, callback: (rd?: RemoteData) => Observable) => createSuccessfulRemoteDataObject$({}), buildFromHref: (href: string) => createSuccessfulRemoteDataObject$({}) } as RemoteDataBuildService;