From 8d36de27d19959c5b470adb0817afa0c8c932b49 Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Fri, 16 Sep 2022 20:56:38 +0200 Subject: [PATCH] 94207: Update DeleteDate to use buildFromRequestUUIDAndAwait --- src/app/core/data/base/delete-data.spec.ts | 105 +++++++++------------ src/app/core/data/base/delete-data.ts | 27 +----- 2 files changed, 48 insertions(+), 84 deletions(-) diff --git a/src/app/core/data/base/delete-data.spec.ts b/src/app/core/data/base/delete-data.spec.ts index 7bffef18e6..a076473b0f 100644 --- a/src/app/core/data/base/delete-data.spec.ts +++ b/src/app/core/data/base/delete-data.spec.ts @@ -22,7 +22,7 @@ import { RequestEntryState } from '../request-entry-state.model'; import { DeleteData, DeleteDataImpl } from './delete-data'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { createFailedRemoteDataObject, createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; -import { fakeAsync, tick } from '@angular/core/testing'; +import { RestRequestMethod } from '../rest-request-method'; /** * Tests whether calls to `DeleteData` methods are correctly patched through in a concrete data service that implements it @@ -63,8 +63,6 @@ export function testDeleteDataImplementation(serviceFactory: () => DeleteData { constructor( protected requestService: RequestService, @@ -155,13 +153,13 @@ describe('DeleteDataImpl', () => { let MOCK_FAILED_RD; let invalidateByHrefSpy: jasmine.Spy; - let buildFromRequestUUIDSpy: jasmine.Spy; + let buildFromRequestUUIDAndAwaitSpy: jasmine.Spy; let getIDHrefObsSpy: jasmine.Spy; let deleteByHrefSpy: 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(); @@ -171,7 +169,7 @@ describe('DeleteDataImpl', () => { 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'); @@ -180,66 +178,53 @@ describe('DeleteDataImpl', () => { }); describe('deleteByHref', () => { - it('should call invalidateByHref if the DELETE request succeeds', (done) => { - buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + it('should send a DELETE request', (done) => { + buildFromRequestUUIDAndAwaitSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.deleteByHref('some-href').subscribe(() => { + expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ + method: RestRequestMethod.DELETE, + href: 'some-href', + })); + done(); + }); + }); + + it('should include the virtual metadata to be copied in the DELETE request', (done) => { + buildFromRequestUUIDAndAwaitSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); + + service.deleteByHref('some-href', ['a', 'b', 'c']).subscribe(() => { + expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ + method: RestRequestMethod.DELETE, + href: 'some-href?copyVirtualMetadata=a©VirtualMetadata=b©VirtualMetadata=c', + })); + done(); + }); + }); + + it('should invalidate the currently cached object', (done) => { + service.deleteByHref('some-href').subscribe(() => { + expect(buildFromRequestUUIDAndAwaitSpy).toHaveBeenCalledWith( + requestService.generateRequestId(), + jasmine.anything(), + ); + + const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1]; + callback(); + expect(service.invalidateByHref).toHaveBeenCalledWith('some-href'); + + done(); + }); + }); + + it('should return the RemoteData of the response', (done) => { + buildFromRequestUUIDAndAwaitSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD)); service.deleteByHref('some-href').subscribe(rd => { expect(rd).toBe(MOCK_SUCCEEDED_RD); - expect(invalidateByHrefSpy).toHaveBeenCalled(); done(); }); }); - - 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)); - - service.deleteByHref('some-href').subscribe(rd => { - expect(rd).toBe(MOCK_FAILED_RD); - expect(invalidateByHrefSpy).not.toHaveBeenCalled(); - done(); - }); - }); - - 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 - ); - - 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 - ); - }); - }); }); }); }); diff --git a/src/app/core/data/base/delete-data.ts b/src/app/core/data/base/delete-data.ts index 2e34f0957c..807d9d838e 100644 --- a/src/app/core/data/base/delete-data.ts +++ b/src/app/core/data/base/delete-data.ts @@ -6,13 +6,12 @@ * http://www.dspace.org/license/ */ import { CacheableObject } from '../../cache/cacheable-object.model'; -import { AsyncSubject, combineLatest, Observable } from 'rxjs'; +import { Observable } from 'rxjs'; import { RemoteData } from '../remote-data'; import { NoContent } from '../../shared/NoContent.model'; -import { filter, map, switchMap } from 'rxjs/operators'; +import { switchMap } from 'rxjs/operators'; import { DeleteRequest } from '../request.models'; import { hasNoValue, hasValue } from '../../../shared/empty.util'; -import { getFirstCompletedRemoteData } from '../../shared/operators'; import { RequestService } from '../request.service'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; @@ -83,26 +82,6 @@ export class DeleteDataImpl extends IdentifiableDataS } 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 combineLatest([response$, invalidated$]).pipe( - filter(([_, invalidated]) => invalidated), - map(([response, _]) => response), - ); + return this.rdbService.buildFromRequestUUIDAndAwait(requestId, () => this.invalidateByHref(href)); } }