From 78d0bdd336f65204bafca66f1c1734f62906a042 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Fri, 18 Aug 2023 14:18:41 +0200 Subject: [PATCH 01/19] 104938 Implementation and tests (WIP) --- ...llection-source-controls.component.spec.ts | 8 +- .../collection-source-controls.component.ts | 192 +++++++++++------- .../processes/process-data.service.spec.ts | 113 +++++++++++ .../data/processes/process-data.service.ts | 44 +++- .../detail/process-detail.component.spec.ts | 124 +++++------ .../detail/process-detail.component.ts | 139 +++++++------ 6 files changed, 417 insertions(+), 203 deletions(-) diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts index 3eb83ebe8ac..37a5d8642de 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts @@ -88,7 +88,7 @@ describe('CollectionSourceControlsComponent', () => { invoke: createSuccessfulRemoteDataObject$(process), }); processDataService = jasmine.createSpyObj('processDataService', { - findById: createSuccessfulRemoteDataObject$(process), + notifyOnCompletion: createSuccessfulRemoteDataObject$(process), }); bitstreamService = jasmine.createSpyObj('bitstreamService', { findByHref: createSuccessfulRemoteDataObject$(bitstream), @@ -137,7 +137,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-i', value: new ContentSourceSetSerializer().Serialize(contentSource.oaiSetId)}, ], []); - expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); expect(bitstreamService.findByHref).toHaveBeenCalledWith(process._links.output.href); expect(notificationsService.info).toHaveBeenCalledWith(jasmine.anything() as any, 'Script text'); }); @@ -151,7 +151,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-r', value: null}, {name: '-c', value: collection.uuid}, ], []); - expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); expect(notificationsService.success).toHaveBeenCalled(); }); }); @@ -164,7 +164,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-o', value: null}, {name: '-c', value: collection.uuid}, ], []); - expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); expect(notificationsService.success).toHaveBeenCalled(); }); }); diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index 7113c25e9f6..e8c3666da08 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -95,36 +95,55 @@ export class CollectionSourceControlsComponent implements OnDestroy { }), // filter out responses that aren't successful since the pinging of the process only needs to happen when the invocation was successful. filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), - getAllCompletedRemoteData(), - filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - map((rd) => rd.payload), - hasValueOperator(), + switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + map((rd) => rd.payload) ).subscribe((process: Process) => { - if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // Ping the current process state every 5s - setTimeout(() => { - this.requestService.setStaleByHrefSubstring(process._links.self.href); - }, 5000); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); - this.testConfigRunning$.next(false); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - this.bitstreamService.findByHref(process._links.output.href).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { - this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { - const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') - .replaceAll('The script has started', '') - .replaceAll('The script has completed', ''); - this.notificationsService.info(this.translateService.get('collection.source.controls.test.completed'), output); - }); + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); + this.testConfigRunning$.next(false); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.bitstreamService.findByHref(process._links.output.href).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { + this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { + const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') + .replaceAll('The script has started', '') + .replaceAll('The script has completed', ''); + this.notificationsService.info(this.translateService.get('collection.source.controls.test.completed'), output); }); - this.testConfigRunning$.next(false); - } + }); + this.testConfigRunning$.next(false); } - )); + })); + + // getAllCompletedRemoteData(), + // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + // map((rd) => rd.payload), + // hasValueOperator(), + // ).subscribe((process: Process) => { + // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // // Ping the current process state every 5s + // setTimeout(() => { + // this.requestService.setStaleByHrefSubstring(process._links.self.href); + // }, 5000); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + // this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); + // this.testConfigRunning$.next(false); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + // this.bitstreamService.findByHref(process._links.output.href).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { + // this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { + // const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') + // .replaceAll('The script has started', '') + // .replaceAll('The script has completed', ''); + // this.notificationsService.info(this.translateService.get('collection.source.controls.test.completed'), output); + // }); + // }); + // this.testConfigRunning$.next(false); + // } + // } + // )); } /** @@ -147,31 +166,44 @@ export class CollectionSourceControlsComponent implements OnDestroy { } }), filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), - getAllCompletedRemoteData(), - filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - map((rd) => rd.payload), - hasValueOperator(), + switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + map((rd) => rd.payload) ).subscribe((process) => { - if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // Ping the current process state every 5s - setTimeout(() => { - this.requestService.setStaleByHrefSubstring(process._links.self.href); - this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - }, 5000); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - this.notificationsService.error(this.translateService.get('collection.source.controls.import.failed')); - this.importRunning$.next(false); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); - this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - this.importRunning$.next(false); - } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.import.failed')); + this.importRunning$.next(false); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + this.importRunning$.next(false); } - )); + })); + + // getAllCompletedRemoteData(), + // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + // map((rd) => rd.payload), + // hasValueOperator(), + // ).subscribe((process) => { + // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // // Ping the current process state every 5s + // setTimeout(() => { + // this.requestService.setStaleByHrefSubstring(process._links.self.href); + // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + // }, 5000); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + // this.notificationsService.error(this.translateService.get('collection.source.controls.import.failed')); + // this.importRunning$.next(false); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + // this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); + // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + // this.importRunning$.next(false); + // } + // } + // )); } /** @@ -194,31 +226,45 @@ export class CollectionSourceControlsComponent implements OnDestroy { } }), filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), - getAllCompletedRemoteData(), - filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - map((rd) => rd.payload), - hasValueOperator(), + switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + map((rd) => rd.payload) ).subscribe((process) => { - if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // Ping the current process state every 5s - setTimeout(() => { - this.requestService.setStaleByHrefSubstring(process._links.self.href); - this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - }, 5000); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - this.notificationsService.error(this.translateService.get('collection.source.controls.reset.failed')); - this.reImportRunning$.next(false); - } - if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - this.notificationsService.success(this.translateService.get('collection.source.controls.reset.completed')); - this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - this.reImportRunning$.next(false); - } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.reset.failed')); + this.reImportRunning$.next(false); } - )); + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.notificationsService.success(this.translateService.get('collection.source.controls.reset.completed')); + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + this.reImportRunning$.next(false); + } + })); + + // switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), + // getAllCompletedRemoteData(), + // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + // map((rd) => rd.payload), + // hasValueOperator(), + // ).subscribe((process) => { + // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // // Ping the current process state every 5s + // setTimeout(() => { + // this.requestService.setStaleByHrefSubstring(process._links.self.href); + // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + // }, 5000); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + // this.notificationsService.error(this.translateService.get('collection.source.controls.reset.failed')); + // this.reImportRunning$.next(false); + // } + // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + // this.notificationsService.success(this.translateService.get('collection.source.controls.reset.completed')); + // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + // this.reImportRunning$.next(false); + // } + // } + // )); } ngOnDestroy(): void { diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index 88e5bd57915..6e7ce515027 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -9,6 +9,21 @@ import { testFindAllDataImplementation } from '../base/find-all-data.spec'; import { ProcessDataService } from './process-data.service'; import { testDeleteDataImplementation } from '../base/delete-data.spec'; +import { cold } from 'jasmine-marbles'; +import { waitForAsync, TestBed } from '@angular/core/testing'; +import { RequestService } from '../request.service'; +import { RemoteData } from '../remote-data'; +import { RequestEntryState } from '../request-entry-state.model'; +import { Process } from '../../../process-page/processes/process.model'; +import { ProcessStatus } from '../../../process-page/processes/process-status.model'; +import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; +import { ObjectCacheService } from '../../cache/object-cache.service'; +import { CoreModule } from '../../core.module'; +import { ReducerManager } from '@ngrx/store'; +import { HALEndpointService } from '../../shared/hal-endpoint.service'; +import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; +import { BitstreamFormatDataService } from '../bitstream-format-data.service'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; describe('ProcessDataService', () => { describe('composition', () => { @@ -16,4 +31,102 @@ describe('ProcessDataService', () => { testFindAllDataImplementation(initService); testDeleteDataImplementation(initService); }); + + let requestService; + let processDataService; + let remoteDataBuildService; + + describe('notifyOnCompletion', () => { + beforeEach(waitForAsync(() => { + requestService = jasmine.createSpyObj('requestService', ['setStaleByHrefSubstring']); + + TestBed.configureTestingModule({ + imports: [], + providers: [ + ProcessDataService, + { provide: RequestService, useValue: requestService }, + { provide: RemoteDataBuildService, useValue: null }, + { provide: ObjectCacheService, useValue: null }, + { provide: ReducerManager, useValue: null }, + { provide: HALEndpointService, useValue: null }, + { provide: DSOChangeAnalyzer, useValue: null }, + { provide: BitstreamFormatDataService, useValue: null }, + { provide: NotificationsService, useValue: null }, + ] + }); + + processDataService = TestBed.inject(ProcessDataService); + })); + + it('TODO', () => { + let completedProcess = new Process(); + completedProcess.processStatus = ProcessStatus.COMPLETED; + + spyOn(processDataService, 'findById').and.returnValue( + cold('(c|)', { + 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + }) + ); + + let process$ = processDataService.notifyOnCompletion('instantly'); + process$.subscribe((rd) => { + expect(processDataService.findById).toHaveBeenCalledTimes(1); + expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); + }); + expect(process$).toBeObservable(cold('(c|)', { + 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + })); + }); + + // it('TODO2', () => { + // let completedProcess = new Process(); + // completedProcess.processStatus = ProcessStatus.COMPLETED; + + // spyOn(processDataService, 'findById').and.returnValue( + // cold('p 150ms (c|)', { + // 'p': new RemoteData(0, 0, 0, RequestEntryState., null, completedProcess), + // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + // }) + // ); + + // let process$ = processDataService.notifyOnCompletion('foo', 100); + // expect(process$).toBeObservable(cold('---(c|)', { + // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + // })); + // process$.subscribe((rd) => { + // expect(processDataService.findById).toHaveBeenCalledTimes(1); + // expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); + // }); + // }); + }); + }); + +// /** +// * Tests whether calls to `FindAllData` methods are correctly patched through in a concrete data service that implements it +// */ +// export function testFindAllDataImplementation(serviceFactory: () => FindAllData) { +// let service; +// +// describe('FindAllData implementation', () => { +// const OPTIONS = Object.assign(new FindListOptions(), { elementsPerPage: 10, currentPage: 3 }); +// const FOLLOWLINKS = [ +// followLink('test'), +// followLink('something'), +// ]; +// +// beforeAll(() => { +// service = serviceFactory(); +// (service as any).findAllData = jasmine.createSpyObj('findAllData', { +// findAll: 'TEST findAll', +// }); +// }); +// +// it('should handle calls to findAll', () => { +// const out: any = service.findAll(OPTIONS, false, true, ...FOLLOWLINKS); +// +// expect((service as any).findAllData.findAll).toHaveBeenCalledWith(OPTIONS, false, true, ...FOLLOWLINKS); +// expect(out).toBe('TEST findAll'); +// }); +// }); +// } diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index 3bf34eb650d..f550407a59a 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -6,25 +6,28 @@ import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { Process } from '../../../process-page/processes/process.model'; import { PROCESS } from '../../../process-page/processes/process.resource-type'; import { Observable } from 'rxjs'; -import { switchMap } from 'rxjs/operators'; +import { switchMap, filter, take } from 'rxjs/operators'; import { PaginatedList } from '../paginated-list.model'; import { Bitstream } from '../../shared/bitstream.model'; import { RemoteData } from '../remote-data'; import { BitstreamDataService } from '../bitstream-data.service'; import { IdentifiableDataService } from '../base/identifiable-data.service'; -import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; +import { FollowLinkConfig, followLink } from '../../../shared/utils/follow-link-config.model'; import { FindAllData, FindAllDataImpl } from '../base/find-all-data'; import { FindListOptions } from '../find-list-options.model'; import { dataService } from '../base/data-service.decorator'; import { DeleteData, DeleteDataImpl } from '../base/delete-data'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { NoContent } from '../../shared/NoContent.model'; +import { getAllCompletedRemoteData } from '../../shared/operators'; +import { ProcessStatus } from 'src/app/process-page/processes/process-status.model'; @Injectable() @dataService(PROCESS) export class ProcessDataService extends IdentifiableDataService implements FindAllData, DeleteData { private findAllData: FindAllData; private deleteData: DeleteData; + protected activelyBeingPolled: Set = new Set(); constructor( protected requestService: RequestService, @@ -101,4 +104,41 @@ export class ProcessDataService extends IdentifiableDataService impleme public deleteByHref(href: string, copyVirtualMetadata?: string[]): Observable> { return this.deleteData.deleteByHref(href, copyVirtualMetadata); } + + // TODO + public notifyOnCompletion(processId: string, pollingIntervalInMs = 5000): Observable> { + const process$ = this.findById(processId, false, true, followLink('script')) + .pipe( + getAllCompletedRemoteData(), + ); + + // TODO: this is horrible + const statusIs = (process: Process, status: ProcessStatus) => + process.processStatus === status; + + // If we have to wait too long for the result, we should mark the result as stale. + // However, we should make sure this happens only once (in case there are multiple observers waiting + // for the result). + if (!this.activelyBeingPolled.has(processId)) { + this.activelyBeingPolled.add(processId); + + // Create a subscription that marks the data as stale if the polling interval time has been exceeded. + const sub = process$.subscribe((rd) => { + const process = rd.payload; + if (statusIs(process, ProcessStatus.COMPLETED) || statusIs(process, ProcessStatus.FAILED)) { + this.activelyBeingPolled.delete(processId); + sub.unsubscribe(); + } else { + setTimeout(() => { + this.requestService.setStaleByHrefSubstring(process._links.self.href); + }, pollingIntervalInMs); + } + }); + } + + return process$.pipe( + filter(rd => statusIs(rd.payload, ProcessStatus.COMPLETED) || statusIs(rd.payload, ProcessStatus.FAILED)), + take(1) + ); + } } diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 9552f9a092a..68b97d0e5cd 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -273,92 +273,92 @@ describe('ProcessDetailComponent', () => { }); }); - describe('refresh counter', () => { - const queryRefreshCounter = () => fixture.debugElement.query(By.css('.refresh-counter')); + // describe('refresh counter', () => { + // const queryRefreshCounter = () => fixture.debugElement.query(By.css('.refresh-counter')); - describe('if process is completed', () => { - beforeEach(() => { - process.processStatus = ProcessStatus.COMPLETED; - route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); - }); + // describe('if process is completed', () => { + // beforeEach(() => { + // process.processStatus = ProcessStatus.COMPLETED; + // route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); + // }); - it('should not show', () => { - spyOn(component, 'startRefreshTimer'); + // it('should not show', () => { + // spyOn(component, 'startRefreshTimer'); - const refreshCounter = queryRefreshCounter(); - expect(refreshCounter).toBeNull(); + // const refreshCounter = queryRefreshCounter(); + // expect(refreshCounter).toBeNull(); - expect(component.startRefreshTimer).not.toHaveBeenCalled(); - }); - }); + // expect(component.startRefreshTimer).not.toHaveBeenCalled(); + // }); + // }); - describe('if process is not finished', () => { - beforeEach(() => { - process.processStatus = ProcessStatus.RUNNING; - route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); - fixture.detectChanges(); - component.stopRefreshTimer(); - }); + // describe('if process is not finished', () => { + // beforeEach(() => { + // process.processStatus = ProcessStatus.RUNNING; + // route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); + // fixture.detectChanges(); + // component.stopRefreshTimer(); + // }); - it('should call startRefreshTimer', () => { - spyOn(component, 'startRefreshTimer'); + // it('should call startRefreshTimer', () => { + // spyOn(component, 'startRefreshTimer'); - component.ngOnInit(); - fixture.detectChanges(); // subscribe to process observable with async pipe + // component.ngOnInit(); + // fixture.detectChanges(); // subscribe to process observable with async pipe - expect(component.startRefreshTimer).toHaveBeenCalled(); - }); + // expect(component.startRefreshTimer).toHaveBeenCalled(); + // }); - it('should call refresh method every 5 seconds, until process is completed', fakeAsync(() => { - spyOn(component, 'refresh'); - spyOn(component, 'stopRefreshTimer'); + // it('should call refresh method every 5 seconds, until process is completed', fakeAsync(() => { + // spyOn(component, 'refresh'); + // spyOn(component, 'stopRefreshTimer'); - process.processStatus = ProcessStatus.COMPLETED; - // set findbyId to return a completed process - (processService.findById as jasmine.Spy).and.returnValue(observableOf(createSuccessfulRemoteDataObject(process))); + // process.processStatus = ProcessStatus.COMPLETED; + // // set findbyId to return a completed process + // (processService.findById as jasmine.Spy).and.returnValue(observableOf(createSuccessfulRemoteDataObject(process))); - component.ngOnInit(); - fixture.detectChanges(); // subscribe to process observable with async pipe + // component.ngOnInit(); + // fixture.detectChanges(); // subscribe to process observable with async pipe - expect(component.refresh).not.toHaveBeenCalled(); + // expect(component.refresh).not.toHaveBeenCalled(); - expect(component.refreshCounter$.value).toBe(0); + // expect(component.refreshCounter$.value).toBe(0); - tick(1001); // 1 second + 1 ms by the setTimeout - expect(component.refreshCounter$.value).toBe(5); // 5 - 0 + // tick(1001); // 1 second + 1 ms by the setTimeout + // expect(component.refreshCounter$.value).toBe(5); // 5 - 0 - tick(2001); // 2 seconds + 1 ms by the setTimeout - expect(component.refreshCounter$.value).toBe(3); // 5 - 2 + // tick(2001); // 2 seconds + 1 ms by the setTimeout + // expect(component.refreshCounter$.value).toBe(3); // 5 - 2 - tick(2001); // 2 seconds + 1 ms by the setTimeout - expect(component.refreshCounter$.value).toBe(1); // 3 - 2 + // tick(2001); // 2 seconds + 1 ms by the setTimeout + // expect(component.refreshCounter$.value).toBe(1); // 3 - 2 - tick(1001); // 1 second + 1 ms by the setTimeout - expect(component.refreshCounter$.value).toBe(0); // 1 - 1 + // tick(1001); // 1 second + 1 ms by the setTimeout + // expect(component.refreshCounter$.value).toBe(0); // 1 - 1 - tick(1000); // 1 second + // tick(1000); // 1 second - expect(component.refresh).toHaveBeenCalledTimes(1); - expect(component.stopRefreshTimer).toHaveBeenCalled(); + // expect(component.refresh).toHaveBeenCalledTimes(1); + // expect(component.stopRefreshTimer).toHaveBeenCalled(); - expect(component.refreshCounter$.value).toBe(0); + // expect(component.refreshCounter$.value).toBe(0); - tick(1001); // 1 second + 1 ms by the setTimeout - // startRefreshTimer not called again - expect(component.refreshCounter$.value).toBe(0); + // tick(1001); // 1 second + 1 ms by the setTimeout + // // startRefreshTimer not called again + // expect(component.refreshCounter$.value).toBe(0); - discardPeriodicTasks(); // discard any periodic tasks that have not yet executed - })); + // discardPeriodicTasks(); // discard any periodic tasks that have not yet executed + // })); - it('should show if refreshCounter is different from 0', () => { - component.refreshCounter$.next(1); - fixture.detectChanges(); + // it('should show if refreshCounter is different from 0', () => { + // component.refreshCounter$.next(1); + // fixture.detectChanges(); - const refreshCounter = queryRefreshCounter(); - expect(refreshCounter).not.toBeNull(); - }); + // const refreshCounter = queryRefreshCounter(); + // expect(refreshCounter).not.toBeNull(); + // }); - }); + // }); - }); + // }); }); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index a379dfe3376..18992eae2f3 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http'; -import { Component, Inject, NgZone, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; +import { Component, Inject, NgZone, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, interval, Observable, shareReplay, Subscription } from 'rxjs'; +import { BehaviorSubject, Observable, Subscription } from 'rxjs'; import { finalize, map, switchMap, take, tap } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; @@ -26,8 +26,6 @@ import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import { getProcessListRoute } from '../process-page-routing.paths'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { followLink } from '../../shared/utils/follow-link-config.model'; -import { isPlatformBrowser } from '@angular/common'; @Component({ selector: 'ds-process-detail', @@ -36,7 +34,7 @@ import { isPlatformBrowser } from '@angular/common'; /** * A component displaying detailed information about a DSpace Process */ -export class ProcessDetailComponent implements OnInit, OnDestroy { +export class ProcessDetailComponent implements OnInit { /** * The AlertType enumeration @@ -107,18 +105,22 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { * Display a 404 if the process doesn't exist */ ngOnInit(): void { - this.processRD$ = this.route.data.pipe( - map((data) => { - if (isPlatformBrowser(this.platformId)) { - if (!this.isProcessFinished(data.process.payload)) { - this.startRefreshTimer(); - } - } + // this.processRD$ = this.route.data.pipe( + // map((data) => { + // if (isPlatformBrowser(this.platformId)) { + // if (!this.isProcessFinished(data.process.payload)) { + // this.startRefreshTimer(); + // } + // } - return data.process as RemoteData; - }), - redirectOn4xx(this.router, this.authService), - shareReplay(1) + // return data.process as RemoteData; + // }), + // redirectOn4xx(this.router, this.authService), + // shareReplay(1) + // ); + + this.processRD$ = this.processService.notifyOnCompletion(this.route.snapshot.params.id).pipe( + redirectOn4xx(this.router, this.authService) ); this.filesRD$ = this.processRD$.pipe( @@ -127,52 +129,69 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { ); } - refresh() { - this.processRD$ = this.processService.findById( - this.route.snapshot.params.id, - false, - true, - followLink('script') - ).pipe( - getFirstSucceededRemoteData(), - redirectOn4xx(this.router, this.authService), - tap((processRemoteData: RemoteData) => { - if (!this.isProcessFinished(processRemoteData.payload)) { - this.startRefreshTimer(); - } - }), - shareReplay(1) - ); + // refresh() { - this.filesRD$ = this.processRD$.pipe( - getFirstSucceededRemoteDataPayload(), - switchMap((process: Process) => this.processService.getFiles(process.processId)) - ); - } + //////////////////////////////////////////////////////////////////////////////// - startRefreshTimer() { - this.refreshCounter$.next(0); + // this.processRD$ = this.processService.findById( + // this.route.snapshot.params.id, + // false, + // true, + // followLink('script') + // ).pipe( + // // First get the process state + // getFirstSucceededRemoteData(), - this.refreshTimerSub = interval(1000).subscribe( - value => { - if (value > 5) { - setTimeout(() => { - this.refresh(); - this.stopRefreshTimer(); - this.refreshCounter$.next(0); - }, 1); - } else { - this.refreshCounter$.next(5 - value); - } - }); - } + // // Error if it goes wrong + // redirectOn4xx(this.router, this.authService), - stopRefreshTimer() { - if (hasValue(this.refreshTimerSub)) { - this.refreshTimerSub.unsubscribe(); - this.refreshTimerSub = undefined; - } - } + // // If process is not finished, start the refresh timer + // tap((processRemoteData: RemoteData) => { + // if (!this.isProcessFinished(processRemoteData.payload)) { + // this.startRefreshTimer(); + // } + // }), + + // // ??? + // shareReplay(1) + // ); + // this.filesRD$ = this.processRD$.pipe( + // getFirstSucceededRemoteDataPayload(), + // switchMap((process: Process) => this.processService.getFiles(process.processId)) + // ); + // } + + // // TODO delete + // // call refresh after 5 sec + // startRefreshTimer() { + // this.refreshCounter$.next(0); + // + // // TODO delete comment + // // This fires every 1000 ms with an incrementing value. + // // So the first time this fires, it adds the value 5 to the refresh counter + // // the second time, it adds the value 4, + // // etc. + // // If the value exceeds 5, the refresh timer is stopped and this.refresh is called. + // this.refreshTimerSub = interval(1000).subscribe( + // value => { + // if (value > 5) { + // setTimeout(() => { + // this.refresh(); + // this.stopRefreshTimer(); + // this.refreshCounter$.next(0); + // }, 1); + // } else { + // this.refreshCounter$.next(5 - value); + // } + // }); + // } + // + // stopRefreshTimer() { + // if (hasValue(this.refreshTimerSub)) { + // this.refreshTimerSub.unsubscribe(); + // this.refreshTimerSub = undefined; + // } + // } /** * Get the name of a bitstream @@ -276,8 +295,4 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { closeModal() { this.modalRef.close(); } - - ngOnDestroy(): void { - this.stopRefreshTimer(); - } } From c4d57770c74959dbd73ec5fb338692db279942eb Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Thu, 24 Aug 2023 10:00:21 +0200 Subject: [PATCH 02/19] ProcessDataService.notifyOnCompletion tests (WIP) --- .../processes/process-data.service.spec.ts | 48 +++++++++---------- .../data/processes/process-data.service.ts | 11 +++-- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index 6e7ce515027..bf42b6b9cff 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -18,7 +18,6 @@ import { Process } from '../../../process-page/processes/process.model'; import { ProcessStatus } from '../../../process-page/processes/process-status.model'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../../cache/object-cache.service'; -import { CoreModule } from '../../core.module'; import { ReducerManager } from '@ngrx/store'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; @@ -27,7 +26,7 @@ import { NotificationsService } from '../../../shared/notifications/notification describe('ProcessDataService', () => { describe('composition', () => { - const initService = () => new ProcessDataService(null, null, null, null, null, null); + const initService = () => new ProcessDataService(null, null, null, null, null, null, null); testFindAllDataImplementation(initService); testDeleteDataImplementation(initService); }); @@ -38,13 +37,11 @@ describe('ProcessDataService', () => { describe('notifyOnCompletion', () => { beforeEach(waitForAsync(() => { - requestService = jasmine.createSpyObj('requestService', ['setStaleByHrefSubstring']); - TestBed.configureTestingModule({ imports: [], providers: [ ProcessDataService, - { provide: RequestService, useValue: requestService }, + { provide: RequestService, useValue: null }, { provide: RemoteDataBuildService, useValue: null }, { provide: ObjectCacheService, useValue: null }, { provide: ReducerManager, useValue: null }, @@ -56,6 +53,7 @@ describe('ProcessDataService', () => { }); processDataService = TestBed.inject(ProcessDataService); + spyOn(processDataService, 'invalidateByHref'); })); it('TODO', () => { @@ -71,33 +69,35 @@ describe('ProcessDataService', () => { let process$ = processDataService.notifyOnCompletion('instantly'); process$.subscribe((rd) => { expect(processDataService.findById).toHaveBeenCalledTimes(1); - expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); + expect(processDataService.invalidateByHref).not.toHaveBeenCalled(); }); expect(process$).toBeObservable(cold('(c|)', { 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) })); }); - // it('TODO2', () => { - // let completedProcess = new Process(); - // completedProcess.processStatus = ProcessStatus.COMPLETED; + it('TODO2', () => { + let runningProcess = new Process(); + runningProcess.processStatus = ProcessStatus.RUNNING; + let completedProcess = new Process(); + completedProcess.processStatus = ProcessStatus.COMPLETED; - // spyOn(processDataService, 'findById').and.returnValue( - // cold('p 150ms (c|)', { - // 'p': new RemoteData(0, 0, 0, RequestEntryState., null, completedProcess), - // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - // }) - // ); + spyOn(processDataService, 'findById').and.returnValue( + cold('p 150ms (c|)', { + 'p': new RemoteData(0, 0, 0, RequestEntryState.Success, null, runningProcess), + 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + }) + ); - // let process$ = processDataService.notifyOnCompletion('foo', 100); - // expect(process$).toBeObservable(cold('---(c|)', { - // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - // })); - // process$.subscribe((rd) => { - // expect(processDataService.findById).toHaveBeenCalledTimes(1); - // expect(requestService.setStaleByHrefSubstring).not.toHaveBeenCalled(); - // }); - // }); + let process$ = processDataService.notifyOnCompletion('foo', 100); + // expect(process$).toBeObservable(cold('- 800ms (c|)', { + // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + // })); + process$.subscribe((rd) => { + expect(processDataService.findById).toHaveBeenCalledTimes(1); + expect(processDataService.invalidateByHref).toHaveBeenCalledTimes(1); + }); + }); }); }); diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index f550407a59a..f0c0829e855 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@angular/core'; +import { Injectable, NgZone } from '@angular/core'; import { RequestService } from '../request.service'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../../cache/object-cache.service'; @@ -36,6 +36,7 @@ export class ProcessDataService extends IdentifiableDataService impleme protected halService: HALEndpointService, protected bitstreamDataService: BitstreamDataService, protected notificationsService: NotificationsService, + protected zone: NgZone, ) { super('processes', requestService, rdbService, objectCache, halService); @@ -129,9 +130,11 @@ export class ProcessDataService extends IdentifiableDataService impleme this.activelyBeingPolled.delete(processId); sub.unsubscribe(); } else { - setTimeout(() => { - this.requestService.setStaleByHrefSubstring(process._links.self.href); - }, pollingIntervalInMs); + this.zone.runOutsideAngular(() => + setTimeout(() => { + this.invalidateByHref(process._links.self.href); + }, pollingIntervalInMs) + ); } }); } From bd6648703c2c73ec552adf3bec13e7bf771fcfe6 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Fri, 25 Aug 2023 11:17:36 +0200 Subject: [PATCH 03/19] 104938 Add flush to process polling tests --- .../processes/process-data.service.spec.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index bf42b6b9cff..ccdb65c4172 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -9,7 +9,7 @@ import { testFindAllDataImplementation } from '../base/find-all-data.spec'; import { ProcessDataService } from './process-data.service'; import { testDeleteDataImplementation } from '../base/delete-data.spec'; -import { cold } from 'jasmine-marbles'; +import { cold, getTestScheduler } from 'jasmine-marbles'; import { waitForAsync, TestBed } from '@angular/core/testing'; import { RequestService } from '../request.service'; import { RemoteData } from '../remote-data'; @@ -23,6 +23,7 @@ import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; import { BitstreamFormatDataService } from '../bitstream-format-data.service'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { TestScheduler } from 'rxjs/testing'; describe('ProcessDataService', () => { describe('composition', () => { @@ -34,9 +35,11 @@ describe('ProcessDataService', () => { let requestService; let processDataService; let remoteDataBuildService; + let scheduler: TestScheduler; describe('notifyOnCompletion', () => { beforeEach(waitForAsync(() => { + scheduler = getTestScheduler(); TestBed.configureTestingModule({ imports: [], providers: [ @@ -90,13 +93,12 @@ describe('ProcessDataService', () => { ); let process$ = processDataService.notifyOnCompletion('foo', 100); - // expect(process$).toBeObservable(cold('- 800ms (c|)', { - // 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - // })); - process$.subscribe((rd) => { - expect(processDataService.findById).toHaveBeenCalledTimes(1); - expect(processDataService.invalidateByHref).toHaveBeenCalledTimes(1); - }); + expect(process$).toBeObservable(cold('- 150ms (c|)', { + 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) + })); + scheduler.flush(); + expect(processDataService.findById).toHaveBeenCalledTimes(1); + expect(processDataService.invalidateByHref).toHaveBeenCalledTimes(1); }); }); From 3be90ebe460b9910ab2c86bbc15d27918a7c22a4 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 29 Aug 2023 18:39:39 +0200 Subject: [PATCH 04/19] rewrite notifyOnCompletion as autoRefreshUntilCompletion, fix ProcessDetailComponent, and the ProcessDataService tests --- ...llection-source-controls.component.spec.ts | 8 +- .../collection-source-controls.component.ts | 6 +- .../processes/process-data.service.spec.ts | 111 +++++++++------- .../data/processes/process-data.service.ts | 118 +++++++++++++----- .../detail/process-detail.component.html | 4 +- .../detail/process-detail.component.ts | 104 +++------------ .../processes/process-status.model.ts | 8 +- src/assets/i18n/en.json5 | 2 + 8 files changed, 187 insertions(+), 174 deletions(-) diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts index 37a5d8642de..f717943e8eb 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts @@ -88,7 +88,7 @@ describe('CollectionSourceControlsComponent', () => { invoke: createSuccessfulRemoteDataObject$(process), }); processDataService = jasmine.createSpyObj('processDataService', { - notifyOnCompletion: createSuccessfulRemoteDataObject$(process), + autoRefreshUntilCompletion: createSuccessfulRemoteDataObject$(process), }); bitstreamService = jasmine.createSpyObj('bitstreamService', { findByHref: createSuccessfulRemoteDataObject$(bitstream), @@ -137,7 +137,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-i', value: new ContentSourceSetSerializer().Serialize(contentSource.oaiSetId)}, ], []); - expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); + expect(processDataService.autoRefreshUntilCompletion).toHaveBeenCalledWith(process.processId); expect(bitstreamService.findByHref).toHaveBeenCalledWith(process._links.output.href); expect(notificationsService.info).toHaveBeenCalledWith(jasmine.anything() as any, 'Script text'); }); @@ -151,7 +151,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-r', value: null}, {name: '-c', value: collection.uuid}, ], []); - expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); + expect(processDataService.autoRefreshUntilCompletion).toHaveBeenCalledWith(process.processId); expect(notificationsService.success).toHaveBeenCalled(); }); }); @@ -164,7 +164,7 @@ describe('CollectionSourceControlsComponent', () => { {name: '-o', value: null}, {name: '-c', value: collection.uuid}, ], []); - expect(processDataService.notifyOnCompletion).toHaveBeenCalledWith(process.processId); + expect(processDataService.autoRefreshUntilCompletion).toHaveBeenCalledWith(process.processId); expect(notificationsService.success).toHaveBeenCalled(); }); }); diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index e8c3666da08..ea2cb3e2f49 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -95,7 +95,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { }), // filter out responses that aren't successful since the pinging of the process only needs to happen when the invocation was successful. filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + switchMap((rd) => this.processDataService.autoRefreshUntilCompletion(rd.payload.processId)), map((rd) => rd.payload) ).subscribe((process: Process) => { if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { @@ -166,7 +166,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { } }), filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + switchMap((rd) => this.processDataService.autoRefreshUntilCompletion(rd.payload.processId)), map((rd) => rd.payload) ).subscribe((process) => { if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { @@ -226,7 +226,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { } }), filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), - switchMap((rd) => this.processDataService.notifyOnCompletion(rd.payload.processId)), + switchMap((rd) => this.processDataService.autoRefreshUntilCompletion(rd.payload.processId)), map((rd) => rd.payload) ).subscribe((process) => { if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index ccdb65c4172..f58752ee973 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -7,10 +7,9 @@ */ import { testFindAllDataImplementation } from '../base/find-all-data.spec'; -import { ProcessDataService } from './process-data.service'; +import { ProcessDataService, TIMER_FACTORY } from './process-data.service'; import { testDeleteDataImplementation } from '../base/delete-data.spec'; -import { cold, getTestScheduler } from 'jasmine-marbles'; -import { waitForAsync, TestBed } from '@angular/core/testing'; +import { waitForAsync, TestBed, fakeAsync, tick } from '@angular/core/testing'; import { RequestService } from '../request.service'; import { RemoteData } from '../remote-data'; import { RequestEntryState } from '../request-entry-state.model'; @@ -23,11 +22,20 @@ import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; import { BitstreamFormatDataService } from '../bitstream-format-data.service'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; -import { TestScheduler } from 'rxjs/testing'; +import { TestScheduler, RunHelpers } from 'rxjs/testing'; +import { cold } from 'jasmine-marbles'; +import { of } from 'rxjs'; describe('ProcessDataService', () => { + let testScheduler; + + const mockTimer = (fn: () => {}, interval: number) => { + fn(); + return 555; + }; + describe('composition', () => { - const initService = () => new ProcessDataService(null, null, null, null, null, null, null); + const initService = () => new ProcessDataService(null, null, null, null, null, null, null, null); testFindAllDataImplementation(initService); testDeleteDataImplementation(initService); }); @@ -35,11 +43,12 @@ describe('ProcessDataService', () => { let requestService; let processDataService; let remoteDataBuildService; - let scheduler: TestScheduler; - describe('notifyOnCompletion', () => { + describe('autoRefreshUntilCompletion', () => { beforeEach(waitForAsync(() => { - scheduler = getTestScheduler(); + testScheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); TestBed.configureTestingModule({ imports: [], providers: [ @@ -52,6 +61,7 @@ describe('ProcessDataService', () => { { provide: DSOChangeAnalyzer, useValue: null }, { provide: BitstreamFormatDataService, useValue: null }, { provide: NotificationsService, useValue: null }, + { provide: TIMER_FACTORY, useValue: mockTimer }, ] }); @@ -59,50 +69,63 @@ describe('ProcessDataService', () => { spyOn(processDataService, 'invalidateByHref'); })); - it('TODO', () => { - let completedProcess = new Process(); - completedProcess.processStatus = ProcessStatus.COMPLETED; + it('should not do any polling when the process is already completed', () => { + testScheduler.run(({ cold, expectObservable }) => { + let completedProcess = new Process(); + completedProcess.processStatus = ProcessStatus.COMPLETED; + + const completedProcessRD = new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess); - spyOn(processDataService, 'findById').and.returnValue( - cold('(c|)', { - 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - }) - ); + spyOn(processDataService, 'findById').and.returnValue( + cold('c', { + 'c': completedProcessRD + }) + ); - let process$ = processDataService.notifyOnCompletion('instantly'); - process$.subscribe((rd) => { - expect(processDataService.findById).toHaveBeenCalledTimes(1); - expect(processDataService.invalidateByHref).not.toHaveBeenCalled(); + let process$ = processDataService.autoRefreshUntilCompletion('instantly'); + expectObservable(process$).toBe('c', { + c: completedProcessRD + }); }); - expect(process$).toBeObservable(cold('(c|)', { - 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - })); + + expect(processDataService.findById).toHaveBeenCalledTimes(1); + expect(processDataService.invalidateByHref).not.toHaveBeenCalled(); }); - it('TODO2', () => { - let runningProcess = new Process(); - runningProcess.processStatus = ProcessStatus.RUNNING; - let completedProcess = new Process(); - completedProcess.processStatus = ProcessStatus.COMPLETED; - - spyOn(processDataService, 'findById').and.returnValue( - cold('p 150ms (c|)', { - 'p': new RemoteData(0, 0, 0, RequestEntryState.Success, null, runningProcess), - 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - }) - ); - - let process$ = processDataService.notifyOnCompletion('foo', 100); - expect(process$).toBeObservable(cold('- 150ms (c|)', { - 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - })); - scheduler.flush(); + it('should poll until a process completes', () => { + testScheduler.run(({ cold, expectObservable }) => { + const runningProcess = Object.assign(new Process(), { + _links: { + self: { + href: 'https://rest.api/processes/123' + } + } + }); + runningProcess.processStatus = ProcessStatus.RUNNING; + const completedProcess = new Process(); + completedProcess.processStatus = ProcessStatus.COMPLETED; + const runningProcessRD = new RemoteData(0, 0, 0, RequestEntryState.Success, null, runningProcess); + const completedProcessRD = new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess); + + spyOn(processDataService, 'findById').and.returnValue( + cold('r 150ms c', { + 'r': runningProcessRD, + 'c': completedProcessRD + }) + ); + + let process$ = processDataService.autoRefreshUntilCompletion('foo', 100); + expectObservable(process$).toBe('r 150ms c', { + 'r': runningProcessRD, + 'c': completedProcessRD + }); + }); + expect(processDataService.findById).toHaveBeenCalledTimes(1); expect(processDataService.invalidateByHref).toHaveBeenCalledTimes(1); }); - }); -}); + }); // /** // * Tests whether calls to `FindAllData` methods are correctly patched through in a concrete data service that implements it @@ -131,4 +154,4 @@ describe('ProcessDataService', () => { // expect(out).toBe('TEST findAll'); // }); // }); -// } +}); diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index f0c0829e855..a639ef24eae 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -1,4 +1,4 @@ -import { Injectable, NgZone } from '@angular/core'; +import { Injectable, NgZone, Inject, InjectionToken } from '@angular/core'; import { RequestService } from '../request.service'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../../cache/object-cache.service'; @@ -6,7 +6,7 @@ import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { Process } from '../../../process-page/processes/process.model'; import { PROCESS } from '../../../process-page/processes/process.resource-type'; import { Observable } from 'rxjs'; -import { switchMap, filter, take } from 'rxjs/operators'; +import { switchMap, filter, distinctUntilChanged, find } from 'rxjs/operators'; import { PaginatedList } from '../paginated-list.model'; import { Bitstream } from '../../shared/bitstream.model'; import { RemoteData } from '../remote-data'; @@ -21,13 +21,23 @@ import { NotificationsService } from '../../../shared/notifications/notification import { NoContent } from '../../shared/NoContent.model'; import { getAllCompletedRemoteData } from '../../shared/operators'; import { ProcessStatus } from 'src/app/process-page/processes/process-status.model'; +import { hasValue } from '../../../shared/empty.util'; + +/** + * Create an InjectionToken for the default JS setTimeout function, purely so we can mock it during + * testing. (fakeAsync isn't working for this case) + */ +export const TIMER_FACTORY = new InjectionToken<(callback: (...args: any[]) => void, ms?: number, ...args: any[]) => NodeJS.Timeout>('timer', { + providedIn: 'root', + factory: () => setTimeout +}); @Injectable() @dataService(PROCESS) export class ProcessDataService extends IdentifiableDataService implements FindAllData, DeleteData { private findAllData: FindAllData; private deleteData: DeleteData; - protected activelyBeingPolled: Set = new Set(); + protected activelyBeingPolled: Map = new Map(); constructor( protected requestService: RequestService, @@ -37,6 +47,7 @@ export class ProcessDataService extends IdentifiableDataService impleme protected bitstreamDataService: BitstreamDataService, protected notificationsService: NotificationsService, protected zone: NgZone, + @Inject(TIMER_FACTORY) protected timer: (callback: (...args: any[]) => void, ms?: number, ...args: any[]) => NodeJS.Timeout ) { super('processes', requestService, rdbService, objectCache, halService); @@ -106,42 +117,83 @@ export class ProcessDataService extends IdentifiableDataService impleme return this.deleteData.deleteByHref(href, copyVirtualMetadata); } - // TODO - public notifyOnCompletion(processId: string, pollingIntervalInMs = 5000): Observable> { - const process$ = this.findById(processId, false, true, followLink('script')) + /** + * Return true if the given process has the given status + * @protected + */ + protected statusIs(process: Process, status: ProcessStatus): boolean { + return hasValue(process) && process.processStatus === status; + } + + /** + * Return true if the given process has the status COMPLETED or FAILED + */ + public hasCompletedOrFailed(process: Process): boolean { + return this.statusIs(process, ProcessStatus.COMPLETED) || + this.statusIs(process, ProcessStatus.FAILED); + } + + /** + * Clear the timeout for the given process, if that timeout exists + * @protected + */ + protected clearCurrentTimeout(processId: string): void { + const timeout = this.activelyBeingPolled.get(processId); + if (hasValue(timeout)) { + clearTimeout(timeout); + } + }; + + /** + * Poll the process with the given ID, using the given interval, until that process either + * completes successfully or fails + * + * Return an Observable for the Process. Note that this will also emit while the + * process is still running. It will only emit again when the process (not the RemoteData!) changes + * status. That makes it more convenient to retrieve that process for a component: you can replace + * a findByID call with this method, rather than having to do a separate findById, and then call + * this method + * @param processId + * @param pollingIntervalInMs + */ + public autoRefreshUntilCompletion(processId: string, pollingIntervalInMs = 5000): Observable> { + const process$ = this.findById(processId, true, true, followLink('script')) .pipe( getAllCompletedRemoteData(), ); - // TODO: this is horrible - const statusIs = (process: Process, status: ProcessStatus) => - process.processStatus === status; - - // If we have to wait too long for the result, we should mark the result as stale. - // However, we should make sure this happens only once (in case there are multiple observers waiting - // for the result). - if (!this.activelyBeingPolled.has(processId)) { - this.activelyBeingPolled.add(processId); - - // Create a subscription that marks the data as stale if the polling interval time has been exceeded. - const sub = process$.subscribe((rd) => { - const process = rd.payload; - if (statusIs(process, ProcessStatus.COMPLETED) || statusIs(process, ProcessStatus.FAILED)) { - this.activelyBeingPolled.delete(processId); - sub.unsubscribe(); - } else { - this.zone.runOutsideAngular(() => - setTimeout(() => { - this.invalidateByHref(process._links.self.href); - }, pollingIntervalInMs) - ); - } - }); - } + // Create a subscription that marks the data as stale if the process hasn't been completed and + // the polling interval time has been exceeded. + const sub = process$.pipe( + filter((processRD: RemoteData) => + !this.hasCompletedOrFailed(processRD.payload) && + !this.activelyBeingPolled.has(processId) + ) + ).subscribe((processRD: RemoteData) => { + this.clearCurrentTimeout(processId); + const nextTimeout = this.timer(() => { + this.activelyBeingPolled.delete(processId); + this.invalidateByHref(processRD.payload._links.self.href); + }, pollingIntervalInMs); + + this.activelyBeingPolled.set(processId, nextTimeout); + }); + + // When the process completes create a one off subscription (the `find` completes the + // observable) that unsubscribes the previous one, removes the processId from the list of + // processes being polled and clears any running timeouts + process$.pipe( + find((processRD: RemoteData) => this.hasCompletedOrFailed(processRD.payload)) + ).subscribe(() => { + this.clearCurrentTimeout(processId); + this.activelyBeingPolled.delete(processId); + sub.unsubscribe(); + }); return process$.pipe( - filter(rd => statusIs(rd.payload, ProcessStatus.COMPLETED) || statusIs(rd.payload, ProcessStatus.FAILED)), - take(1) + distinctUntilChanged((previous: RemoteData, current: RemoteData) => + previous.payload.processStatus === current.payload.processStatus + ) ); } } diff --git a/src/app/process-page/detail/process-detail.component.html b/src/app/process-page/detail/process-detail.component.html index 5f905cbfff3..c25a6ad50a9 100644 --- a/src/app/process-page/detail/process-detail.component.html +++ b/src/app/process-page/detail/process-detail.component.html @@ -5,8 +5,8 @@

{{ 'process.detail.title' | translate:{ id: process?.processId, name: process?.scriptName } }}

-
- Refreshing in {{ seconds }}s +
+ {{ 'process.detail.refreshing' | translate }}
diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index 18992eae2f3..b1d3241422e 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,8 +1,8 @@ import { HttpClient } from '@angular/common/http'; import { Component, Inject, NgZone, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, Observable, Subscription } from 'rxjs'; -import { finalize, map, switchMap, take, tap } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subscription, interval } from 'rxjs'; +import { finalize, map, switchMap, take, tap, filter, find, startWith } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; @@ -14,7 +14,7 @@ import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteData, - getFirstSucceededRemoteDataPayload + getFirstSucceededRemoteDataPayload, getAllSucceededRemoteDataPayload } from '../../core/shared/operators'; import { URLCombiner } from '../../core/url-combiner/url-combiner'; import { AlertType } from '../../shared/alert/aletr-type'; @@ -26,6 +26,7 @@ import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import { getProcessListRoute } from '../process-page-routing.paths'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; +import { isPlatformBrowser } from '@angular/common'; @Component({ selector: 'ds-process-detail', @@ -76,7 +77,7 @@ export class ProcessDetailComponent implements OnInit { */ dateFormat = 'yyyy-MM-dd HH:mm:ss ZZZZ'; - refreshCounter$ = new BehaviorSubject(0); + isRefreshing$: Observable; /** * Reference to NgbModal @@ -105,94 +106,29 @@ export class ProcessDetailComponent implements OnInit { * Display a 404 if the process doesn't exist */ ngOnInit(): void { - // this.processRD$ = this.route.data.pipe( - // map((data) => { - // if (isPlatformBrowser(this.platformId)) { - // if (!this.isProcessFinished(data.process.payload)) { - // this.startRefreshTimer(); - // } - // } - - // return data.process as RemoteData; - // }), - // redirectOn4xx(this.router, this.authService), - // shareReplay(1) - // ); + this.processRD$ = this.route.data.pipe( + switchMap((data) => { + if (isPlatformBrowser(this.platformId)) { + return this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); + } else { + return [data.process as RemoteData]; + } + }), + redirectOn4xx(this.router, this.authService), + ); - this.processRD$ = this.processService.notifyOnCompletion(this.route.snapshot.params.id).pipe( - redirectOn4xx(this.router, this.authService) + this.isRefreshing$ = this.processRD$.pipe( + find((processRD: RemoteData) => this.processService.hasCompletedOrFailed(processRD.payload)), + map(() => false), + startWith(true) ); this.filesRD$ = this.processRD$.pipe( - getFirstSucceededRemoteDataPayload(), + getAllSucceededRemoteDataPayload(), switchMap((process: Process) => this.processService.getFiles(process.processId)) ); } - // refresh() { - - //////////////////////////////////////////////////////////////////////////////// - - // this.processRD$ = this.processService.findById( - // this.route.snapshot.params.id, - // false, - // true, - // followLink('script') - // ).pipe( - // // First get the process state - // getFirstSucceededRemoteData(), - - // // Error if it goes wrong - // redirectOn4xx(this.router, this.authService), - - // // If process is not finished, start the refresh timer - // tap((processRemoteData: RemoteData) => { - // if (!this.isProcessFinished(processRemoteData.payload)) { - // this.startRefreshTimer(); - // } - // }), - - // // ??? - // shareReplay(1) - // ); - // this.filesRD$ = this.processRD$.pipe( - // getFirstSucceededRemoteDataPayload(), - // switchMap((process: Process) => this.processService.getFiles(process.processId)) - // ); - // } - - // // TODO delete - // // call refresh after 5 sec - // startRefreshTimer() { - // this.refreshCounter$.next(0); - // - // // TODO delete comment - // // This fires every 1000 ms with an incrementing value. - // // So the first time this fires, it adds the value 5 to the refresh counter - // // the second time, it adds the value 4, - // // etc. - // // If the value exceeds 5, the refresh timer is stopped and this.refresh is called. - // this.refreshTimerSub = interval(1000).subscribe( - // value => { - // if (value > 5) { - // setTimeout(() => { - // this.refresh(); - // this.stopRefreshTimer(); - // this.refreshCounter$.next(0); - // }, 1); - // } else { - // this.refreshCounter$.next(5 - value); - // } - // }); - // } - // - // stopRefreshTimer() { - // if (hasValue(this.refreshTimerSub)) { - // this.refreshTimerSub.unsubscribe(); - // this.refreshTimerSub = undefined; - // } - // } - /** * Get the name of a bitstream * @param bitstream diff --git a/src/app/process-page/processes/process-status.model.ts b/src/app/process-page/processes/process-status.model.ts index b43340bffb7..1ff42789d81 100644 --- a/src/app/process-page/processes/process-status.model.ts +++ b/src/app/process-page/processes/process-status.model.ts @@ -2,8 +2,8 @@ * List of process statuses */ export enum ProcessStatus { - SCHEDULED, - RUNNING, - COMPLETED, - FAILED + SCHEDULED = 'SCHEDULED', + RUNNING = 'RUNNING', + COMPLETED = 'COMPLETED', + FAILED = 'FAILED' } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 6c91bae4c16..aa327c79341 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3216,6 +3216,8 @@ "process.detail.delete.error": "Something went wrong when deleting the process", + "process.detail.refreshing": "Auto-refreshing…", + "process.overview.table.finish": "Finish time (UTC)", "process.overview.table.id": "Process ID", From a59776d5a00b50497e987b372c91b28b17bf5152 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Fri, 1 Sep 2023 15:10:39 +0200 Subject: [PATCH 05/19] Failed attempt at fixing process-detail.component.spec.ts tests --- .../processes/process-data.service.spec.ts | 29 ------------------- .../detail/process-detail.component.spec.ts | 9 ++++-- .../detail/process-detail.component.ts | 6 +++- 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index f58752ee973..fe632096f5d 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -124,34 +124,5 @@ describe('ProcessDataService', () => { expect(processDataService.findById).toHaveBeenCalledTimes(1); expect(processDataService.invalidateByHref).toHaveBeenCalledTimes(1); }); - }); - -// /** -// * Tests whether calls to `FindAllData` methods are correctly patched through in a concrete data service that implements it -// */ -// export function testFindAllDataImplementation(serviceFactory: () => FindAllData) { -// let service; -// -// describe('FindAllData implementation', () => { -// const OPTIONS = Object.assign(new FindListOptions(), { elementsPerPage: 10, currentPage: 3 }); -// const FOLLOWLINKS = [ -// followLink('test'), -// followLink('something'), -// ]; -// -// beforeAll(() => { -// service = serviceFactory(); -// (service as any).findAllData = jasmine.createSpyObj('findAllData', { -// findAll: 'TEST findAll', -// }); -// }); -// -// it('should handle calls to findAll', () => { -// const out: any = service.findAll(OPTIONS, false, true, ...FOLLOWLINKS); -// -// expect((service as any).findAllData.findAll).toHaveBeenCalledWith(OPTIONS, false, true, ...FOLLOWLINKS); -// expect(out).toBe('TEST findAll'); -// }); -// }); }); diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 68b97d0e5cd..ba99342191a 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -106,10 +106,12 @@ describe('ProcessDetailComponent', () => { content: { href: 'log-selflink' } } }); + const processRD$ = createSuccessfulRemoteDataObject$(process); processService = jasmine.createSpyObj('processService', { getFiles: createSuccessfulRemoteDataObject$(createPaginatedList(files)), delete: createSuccessfulRemoteDataObject$(null), - findById: createSuccessfulRemoteDataObject$(process), + findById: processRD$, + autoRefreshUntilCompletion: processRD$ }); bitstreamDataService = jasmine.createSpyObj('bitstreamDataService', { findByHref: createSuccessfulRemoteDataObject$(logBitstream) @@ -132,7 +134,7 @@ describe('ProcessDetailComponent', () => { }); route = jasmine.createSpyObj('route', { - data: observableOf({ process: createSuccessfulRemoteDataObject(process) }), + data: observableOf({ process: processRD$ }), snapshot: { params: { id: process.processId } } @@ -158,7 +160,8 @@ describe('ProcessDetailComponent', () => { { provide: NotificationsService, useValue: notificationsService }, { provide: Router, useValue: router }, ], - schemas: [CUSTOM_ELEMENTS_SCHEMA] + // schemas: [CUSTOM_ELEMENTS_SCHEMA] + schemas: [] }).compileComponents(); })); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index b1d3241422e..d96c47b3713 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -109,7 +109,10 @@ export class ProcessDetailComponent implements OnInit { this.processRD$ = this.route.data.pipe( switchMap((data) => { if (isPlatformBrowser(this.platformId)) { - return this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); + const x = this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); + //[data.process as RemoteData]; + console.log("ASDF", x); + return x; } else { return [data.process as RemoteData]; } @@ -117,6 +120,7 @@ export class ProcessDetailComponent implements OnInit { redirectOn4xx(this.router, this.authService), ); + this.processRD$.subscribe(x => console.log("QWER", x)); this.isRefreshing$ = this.processRD$.pipe( find((processRD: RemoteData) => this.processService.hasCompletedOrFailed(processRD.payload)), map(() => false), From 53b0af100d5dcbd07d5ac2f8c8744da7ed7240be Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Fri, 1 Sep 2023 16:11:32 +0200 Subject: [PATCH 06/19] 104938 Fix ProcessDetailComponent tests --- .../collection-source-controls.component.ts | 3 +- .../processes/process-data.service.spec.ts | 6 +-- .../data/processes/process-data.service.ts | 38 +++++++++---------- .../detail/process-detail.component.spec.ts | 18 +++++---- .../detail/process-detail.component.ts | 12 ++---- 5 files changed, 36 insertions(+), 41 deletions(-) diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index ea2cb3e2f49..58f41acf344 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -3,13 +3,12 @@ import { ScriptDataService } from '../../../../core/data/processes/script-data.s import { ContentSource } from '../../../../core/shared/content-source.model'; import { ProcessDataService } from '../../../../core/data/processes/process-data.service'; import { - getAllCompletedRemoteData, getAllSucceededRemoteDataPayload, getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators'; import { filter, map, switchMap, tap } from 'rxjs/operators'; -import { hasValue, hasValueOperator } from '../../../../shared/empty.util'; +import { hasValue } from '../../../../shared/empty.util'; import { ProcessStatus } from '../../../../process-page/processes/process-status.model'; import { BehaviorSubject, Observable, Subscription } from 'rxjs'; import { RequestService } from '../../../../core/data/request.service'; diff --git a/src/app/core/data/processes/process-data.service.spec.ts b/src/app/core/data/processes/process-data.service.spec.ts index fe632096f5d..d66560b0834 100644 --- a/src/app/core/data/processes/process-data.service.spec.ts +++ b/src/app/core/data/processes/process-data.service.spec.ts @@ -9,7 +9,7 @@ import { testFindAllDataImplementation } from '../base/find-all-data.spec'; import { ProcessDataService, TIMER_FACTORY } from './process-data.service'; import { testDeleteDataImplementation } from '../base/delete-data.spec'; -import { waitForAsync, TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { waitForAsync, TestBed } from '@angular/core/testing'; import { RequestService } from '../request.service'; import { RemoteData } from '../remote-data'; import { RequestEntryState } from '../request-entry-state.model'; @@ -22,9 +22,7 @@ import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; import { BitstreamFormatDataService } from '../bitstream-format-data.service'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; -import { TestScheduler, RunHelpers } from 'rxjs/testing'; -import { cold } from 'jasmine-marbles'; -import { of } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; describe('ProcessDataService', () => { let testScheduler; diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index a639ef24eae..e17b0b1f196 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -35,6 +35,22 @@ export const TIMER_FACTORY = new InjectionToken<(callback: (...args: any[]) => v @Injectable() @dataService(PROCESS) export class ProcessDataService extends IdentifiableDataService implements FindAllData, DeleteData { + /** + * Return true if the given process has the given status + * @protected + */ + protected static statusIs(process: Process, status: ProcessStatus): boolean { + return hasValue(process) && process.processStatus === status; + } + + /** + * Return true if the given process has the status COMPLETED or FAILED + */ + public static hasCompletedOrFailed(process: Process): boolean { + return ProcessDataService.statusIs(process, ProcessStatus.COMPLETED) || + ProcessDataService.statusIs(process, ProcessStatus.FAILED); + } + private findAllData: FindAllData; private deleteData: DeleteData; protected activelyBeingPolled: Map = new Map(); @@ -117,22 +133,6 @@ export class ProcessDataService extends IdentifiableDataService impleme return this.deleteData.deleteByHref(href, copyVirtualMetadata); } - /** - * Return true if the given process has the given status - * @protected - */ - protected statusIs(process: Process, status: ProcessStatus): boolean { - return hasValue(process) && process.processStatus === status; - } - - /** - * Return true if the given process has the status COMPLETED or FAILED - */ - public hasCompletedOrFailed(process: Process): boolean { - return this.statusIs(process, ProcessStatus.COMPLETED) || - this.statusIs(process, ProcessStatus.FAILED); - } - /** * Clear the timeout for the given process, if that timeout exists * @protected @@ -142,7 +142,7 @@ export class ProcessDataService extends IdentifiableDataService impleme if (hasValue(timeout)) { clearTimeout(timeout); } - }; + } /** * Poll the process with the given ID, using the given interval, until that process either @@ -166,7 +166,7 @@ export class ProcessDataService extends IdentifiableDataService impleme // the polling interval time has been exceeded. const sub = process$.pipe( filter((processRD: RemoteData) => - !this.hasCompletedOrFailed(processRD.payload) && + !ProcessDataService.hasCompletedOrFailed(processRD.payload) && !this.activelyBeingPolled.has(processId) ) ).subscribe((processRD: RemoteData) => { @@ -183,7 +183,7 @@ export class ProcessDataService extends IdentifiableDataService impleme // observable) that unsubscribes the previous one, removes the processId from the list of // processes being polled and clears any running timeouts process$.pipe( - find((processRD: RemoteData) => this.hasCompletedOrFailed(processRD.payload)) + find((processRD: RemoteData) => ProcessDataService.hasCompletedOrFailed(processRD.payload)) ).subscribe(() => { this.clearCurrentTimeout(processId); this.activelyBeingPolled.delete(processId); diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index ba99342191a..1af1edca995 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -35,7 +35,6 @@ import { NotificationsServiceStub } from '../../shared/testing/notifications-ser import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { getProcessListRoute } from '../process-page-routing.paths'; -import {ProcessStatus} from '../processes/process-status.model'; describe('ProcessDetailComponent', () => { let component: ProcessDetailComponent; @@ -106,12 +105,11 @@ describe('ProcessDetailComponent', () => { content: { href: 'log-selflink' } } }); - const processRD$ = createSuccessfulRemoteDataObject$(process); processService = jasmine.createSpyObj('processService', { getFiles: createSuccessfulRemoteDataObject$(createPaginatedList(files)), delete: createSuccessfulRemoteDataObject$(null), - findById: processRD$, - autoRefreshUntilCompletion: processRD$ + findById: createSuccessfulRemoteDataObject$(process), + autoRefreshUntilCompletion: createSuccessfulRemoteDataObject$(process) }); bitstreamDataService = jasmine.createSpyObj('bitstreamDataService', { findByHref: createSuccessfulRemoteDataObject$(logBitstream) @@ -134,7 +132,7 @@ describe('ProcessDetailComponent', () => { }); route = jasmine.createSpyObj('route', { - data: observableOf({ process: processRD$ }), + data: observableOf({ process: createSuccessfulRemoteDataObject$(process) }), snapshot: { params: { id: process.processId } } @@ -149,7 +147,12 @@ describe('ProcessDetailComponent', () => { providers: [ { provide: ActivatedRoute, - useValue: { data: observableOf({ process: createSuccessfulRemoteDataObject(process) }) } + useValue: { + data: observableOf({ process: createSuccessfulRemoteDataObject(process) }), + snapshot: { + params: { id: process.processId } + } + } }, { provide: ProcessDataService, useValue: processService }, { provide: BitstreamDataService, useValue: bitstreamDataService }, @@ -160,8 +163,7 @@ describe('ProcessDetailComponent', () => { { provide: NotificationsService, useValue: notificationsService }, { provide: Router, useValue: router }, ], - // schemas: [CUSTOM_ELEMENTS_SCHEMA] - schemas: [] + schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); })); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index d96c47b3713..c8e4507fd29 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,8 +1,8 @@ import { HttpClient } from '@angular/common/http'; import { Component, Inject, NgZone, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, Observable, Subscription, interval } from 'rxjs'; -import { finalize, map, switchMap, take, tap, filter, find, startWith } from 'rxjs/operators'; +import { BehaviorSubject, Observable, Subscription } from 'rxjs'; +import { finalize, map, switchMap, take, tap, find, startWith } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; @@ -109,10 +109,7 @@ export class ProcessDetailComponent implements OnInit { this.processRD$ = this.route.data.pipe( switchMap((data) => { if (isPlatformBrowser(this.platformId)) { - const x = this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); - //[data.process as RemoteData]; - console.log("ASDF", x); - return x; + return this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); } else { return [data.process as RemoteData]; } @@ -120,9 +117,8 @@ export class ProcessDetailComponent implements OnInit { redirectOn4xx(this.router, this.authService), ); - this.processRD$.subscribe(x => console.log("QWER", x)); this.isRefreshing$ = this.processRD$.pipe( - find((processRD: RemoteData) => this.processService.hasCompletedOrFailed(processRD.payload)), + find((processRD: RemoteData) => ProcessDataService.hasCompletedOrFailed(processRD.payload)), map(() => false), startWith(true) ); From 9b5001e1d987bde1dd57248c35a69ae60bb8ea48 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Thu, 7 Sep 2023 10:19:33 +0200 Subject: [PATCH 07/19] 104938 Cleanup --- .../collection-source-controls.component.ts | 81 ----------------- .../detail/process-detail.component.spec.ts | 89 ------------------- 2 files changed, 170 deletions(-) diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index 58f41acf344..185a1f938ef 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -113,36 +113,6 @@ export class CollectionSourceControlsComponent implements OnDestroy { this.testConfigRunning$.next(false); } })); - - // getAllCompletedRemoteData(), - // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - // map((rd) => rd.payload), - // hasValueOperator(), - // ).subscribe((process: Process) => { - // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // // Ping the current process state every 5s - // setTimeout(() => { - // this.requestService.setStaleByHrefSubstring(process._links.self.href); - // }, 5000); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - // this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); - // this.testConfigRunning$.next(false); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - // this.bitstreamService.findByHref(process._links.output.href).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { - // this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { - // const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') - // .replaceAll('The script has started', '') - // .replaceAll('The script has completed', ''); - // this.notificationsService.info(this.translateService.get('collection.source.controls.test.completed'), output); - // }); - // }); - // this.testConfigRunning$.next(false); - // } - // } - // )); } /** @@ -178,31 +148,6 @@ export class CollectionSourceControlsComponent implements OnDestroy { this.importRunning$.next(false); } })); - - // getAllCompletedRemoteData(), - // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - // map((rd) => rd.payload), - // hasValueOperator(), - // ).subscribe((process) => { - // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // // Ping the current process state every 5s - // setTimeout(() => { - // this.requestService.setStaleByHrefSubstring(process._links.self.href); - // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - // }, 5000); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - // this.notificationsService.error(this.translateService.get('collection.source.controls.import.failed')); - // this.importRunning$.next(false); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - // this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); - // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - // this.importRunning$.next(false); - // } - // } - // )); } /** @@ -238,32 +183,6 @@ export class CollectionSourceControlsComponent implements OnDestroy { this.reImportRunning$.next(false); } })); - - // switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), - // getAllCompletedRemoteData(), - // filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), - // map((rd) => rd.payload), - // hasValueOperator(), - // ).subscribe((process) => { - // if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && - // process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { - // // Ping the current process state every 5s - // setTimeout(() => { - // this.requestService.setStaleByHrefSubstring(process._links.self.href); - // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - // }, 5000); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { - // this.notificationsService.error(this.translateService.get('collection.source.controls.reset.failed')); - // this.reImportRunning$.next(false); - // } - // if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - // this.notificationsService.success(this.translateService.get('collection.source.controls.reset.completed')); - // this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - // this.reImportRunning$.next(false); - // } - // } - // )); } ngOnDestroy(): void { diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 1af1edca995..241af9fd100 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -277,93 +277,4 @@ describe('ProcessDetailComponent', () => { expect(router.navigateByUrl).not.toHaveBeenCalled(); }); }); - - // describe('refresh counter', () => { - // const queryRefreshCounter = () => fixture.debugElement.query(By.css('.refresh-counter')); - - // describe('if process is completed', () => { - // beforeEach(() => { - // process.processStatus = ProcessStatus.COMPLETED; - // route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); - // }); - - // it('should not show', () => { - // spyOn(component, 'startRefreshTimer'); - - // const refreshCounter = queryRefreshCounter(); - // expect(refreshCounter).toBeNull(); - - // expect(component.startRefreshTimer).not.toHaveBeenCalled(); - // }); - // }); - - // describe('if process is not finished', () => { - // beforeEach(() => { - // process.processStatus = ProcessStatus.RUNNING; - // route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); - // fixture.detectChanges(); - // component.stopRefreshTimer(); - // }); - - // it('should call startRefreshTimer', () => { - // spyOn(component, 'startRefreshTimer'); - - // component.ngOnInit(); - // fixture.detectChanges(); // subscribe to process observable with async pipe - - // expect(component.startRefreshTimer).toHaveBeenCalled(); - // }); - - // it('should call refresh method every 5 seconds, until process is completed', fakeAsync(() => { - // spyOn(component, 'refresh'); - // spyOn(component, 'stopRefreshTimer'); - - // process.processStatus = ProcessStatus.COMPLETED; - // // set findbyId to return a completed process - // (processService.findById as jasmine.Spy).and.returnValue(observableOf(createSuccessfulRemoteDataObject(process))); - - // component.ngOnInit(); - // fixture.detectChanges(); // subscribe to process observable with async pipe - - // expect(component.refresh).not.toHaveBeenCalled(); - - // expect(component.refreshCounter$.value).toBe(0); - - // tick(1001); // 1 second + 1 ms by the setTimeout - // expect(component.refreshCounter$.value).toBe(5); // 5 - 0 - - // tick(2001); // 2 seconds + 1 ms by the setTimeout - // expect(component.refreshCounter$.value).toBe(3); // 5 - 2 - - // tick(2001); // 2 seconds + 1 ms by the setTimeout - // expect(component.refreshCounter$.value).toBe(1); // 3 - 2 - - // tick(1001); // 1 second + 1 ms by the setTimeout - // expect(component.refreshCounter$.value).toBe(0); // 1 - 1 - - // tick(1000); // 1 second - - // expect(component.refresh).toHaveBeenCalledTimes(1); - // expect(component.stopRefreshTimer).toHaveBeenCalled(); - - // expect(component.refreshCounter$.value).toBe(0); - - // tick(1001); // 1 second + 1 ms by the setTimeout - // // startRefreshTimer not called again - // expect(component.refreshCounter$.value).toBe(0); - - // discardPeriodicTasks(); // discard any periodic tasks that have not yet executed - // })); - - // it('should show if refreshCounter is different from 0', () => { - // component.refreshCounter$.next(1); - // fixture.detectChanges(); - - // const refreshCounter = queryRefreshCounter(); - // expect(refreshCounter).not.toBeNull(); - // }); - - // }); - - // }); }); From 11e98f7e20894e4badb6d2ed3a78d155f56e6a2c Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Thu, 7 Sep 2023 11:36:52 +0200 Subject: [PATCH 08/19] Fix lint issue. --- src/app/core/data/processes/process-data.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index e17b0b1f196..3af755561c1 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -35,6 +35,10 @@ export const TIMER_FACTORY = new InjectionToken<(callback: (...args: any[]) => v @Injectable() @dataService(PROCESS) export class ProcessDataService extends IdentifiableDataService implements FindAllData, DeleteData { + private findAllData: FindAllData; + private deleteData: DeleteData; + protected activelyBeingPolled: Map = new Map(); + /** * Return true if the given process has the given status * @protected @@ -51,10 +55,6 @@ export class ProcessDataService extends IdentifiableDataService impleme ProcessDataService.statusIs(process, ProcessStatus.FAILED); } - private findAllData: FindAllData; - private deleteData: DeleteData; - protected activelyBeingPolled: Map = new Map(); - constructor( protected requestService: RequestService, protected rdbService: RemoteDataBuildService, From 6b0f2e7c44f4ef80df05edd89d65e55539eb9c5b Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 30 Nov 2023 17:30:22 +0100 Subject: [PATCH 09/19] 108915: Refactored code to use followLinks to retrieve the files of a process instead of a second request --- .../data/processes/process-data.service.ts | 44 +++++----- .../detail/process-detail.component.spec.ts | 86 +++++++++---------- .../detail/process-detail.component.ts | 9 +- src/app/process-page/process-page.resolver.ts | 6 +- .../process-page/processes/process.model.ts | 9 ++ 5 files changed, 83 insertions(+), 71 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index e17b0b1f196..4cd18caad9a 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -12,7 +12,7 @@ import { Bitstream } from '../../shared/bitstream.model'; import { RemoteData } from '../remote-data'; import { BitstreamDataService } from '../bitstream-data.service'; import { IdentifiableDataService } from '../base/identifiable-data.service'; -import { FollowLinkConfig, followLink } from '../../../shared/utils/follow-link-config.model'; +import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; import { FindAllData, FindAllDataImpl } from '../base/find-all-data'; import { FindListOptions } from '../find-list-options.model'; import { dataService } from '../base/data-service.decorator'; @@ -35,21 +35,6 @@ export const TIMER_FACTORY = new InjectionToken<(callback: (...args: any[]) => v @Injectable() @dataService(PROCESS) export class ProcessDataService extends IdentifiableDataService implements FindAllData, DeleteData { - /** - * Return true if the given process has the given status - * @protected - */ - protected static statusIs(process: Process, status: ProcessStatus): boolean { - return hasValue(process) && process.processStatus === status; - } - - /** - * Return true if the given process has the status COMPLETED or FAILED - */ - public static hasCompletedOrFailed(process: Process): boolean { - return ProcessDataService.statusIs(process, ProcessStatus.COMPLETED) || - ProcessDataService.statusIs(process, ProcessStatus.FAILED); - } private findAllData: FindAllData; private deleteData: DeleteData; @@ -71,6 +56,22 @@ export class ProcessDataService extends IdentifiableDataService impleme this.deleteData = new DeleteDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, notificationsService, this.responseMsToLive, this.constructIdEndpoint); } + /** + * Return true if the given process has the given status + * @protected + */ + protected static statusIs(process: Process, status: ProcessStatus): boolean { + return hasValue(process) && process.processStatus === status; + } + + /** + * Return true if the given process has the status COMPLETED or FAILED + */ + public static hasCompletedOrFailed(process: Process): boolean { + return ProcessDataService.statusIs(process, ProcessStatus.COMPLETED) || + ProcessDataService.statusIs(process, ProcessStatus.FAILED); + } + /** * Get the endpoint for the files of the process * @param processId The ID of the process @@ -153,11 +154,14 @@ export class ProcessDataService extends IdentifiableDataService impleme * status. That makes it more convenient to retrieve that process for a component: you can replace * a findByID call with this method, rather than having to do a separate findById, and then call * this method - * @param processId - * @param pollingIntervalInMs + * + * @param processId The ID of the {@link Process} to poll + * @param pollingIntervalInMs The interval for how often the request needs to be polled + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be + * automatically resolved */ - public autoRefreshUntilCompletion(processId: string, pollingIntervalInMs = 5000): Observable> { - const process$ = this.findById(processId, true, true, followLink('script')) + public autoRefreshUntilCompletion(processId: string, pollingIntervalInMs = 5000, ...linksToFollow: FollowLinkConfig[]): Observable> { + const process$: Observable> = this.findById(processId, true, true, ...linksToFollow) .pipe( getAllCompletedRemoteData(), ); diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 241af9fd100..9ba5d6e94d0 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -27,7 +27,6 @@ import { ProcessDataService } from '../../core/data/processes/process-data.servi import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { createFailedRemoteDataObject$, - createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { createPaginatedList } from '../../shared/testing/utils.test'; @@ -35,6 +34,10 @@ import { NotificationsServiceStub } from '../../shared/testing/notifications-ser import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { getProcessListRoute } from '../process-page-routing.paths'; +import { PaginatedList } from '../../core/data/paginated-list.model'; +import { RouterTestingModule } from '@angular/router/testing'; +import { RouterStub } from '../../shared/testing/router.stub'; +import { ActivatedRouteStub } from '../../shared/testing/active-router.stub'; describe('ProcessDetailComponent', () => { let component: ProcessDetailComponent; @@ -44,20 +47,35 @@ describe('ProcessDetailComponent', () => { let nameService: DSONameService; let bitstreamDataService: BitstreamDataService; let httpClient: HttpClient; - let route: ActivatedRoute; + let route: ActivatedRouteStub; + let router: RouterStub; + let modalService; + let notificationsService: NotificationsServiceStub; let process: Process; let fileName: string; let files: Bitstream[]; - let processOutput; - - let modalService; - let notificationsService; - - let router; + let processOutput: string; function init() { + fileName = 'fake-file-name'; + files = [ + Object.assign(new Bitstream(), { + sizeBytes: 10000, + metadata: { + 'dc.title': [ + { + value: fileName, + language: null + } + ] + }, + _links: { + content: { href: 'file-selflink' } + } + }) + ]; processOutput = 'Process Started'; process = Object.assign(new Process(), { processId: 1, @@ -73,6 +91,9 @@ describe('ProcessDetailComponent', () => { value: 'identifier' } ], + files: createSuccessfulRemoteDataObject$(Object.assign(new PaginatedList(), { + page: files, + })), _links: { self: { href: 'https://rest.api/processes/1' @@ -80,25 +101,8 @@ describe('ProcessDetailComponent', () => { output: { href: 'https://rest.api/processes/1/output' } - } + }, }); - fileName = 'fake-file-name'; - files = [ - Object.assign(new Bitstream(), { - sizeBytes: 10000, - metadata: { - 'dc.title': [ - { - value: fileName, - language: null - } - ] - }, - _links: { - content: { href: 'file-selflink' } - } - }) - ]; const logBitstream = Object.assign(new Bitstream(), { id: 'output.log', _links: { @@ -127,33 +131,22 @@ describe('ProcessDetailComponent', () => { notificationsService = new NotificationsServiceStub(); - router = jasmine.createSpyObj('router', { - navigateByUrl:{} - }); + router = new RouterStub(); - route = jasmine.createSpyObj('route', { - data: observableOf({ process: createSuccessfulRemoteDataObject$(process) }), - snapshot: { - params: { id: process.processId } - } + route = new ActivatedRouteStub({ + id: process.processId, + }, { + process: createSuccessfulRemoteDataObject$(process), }); } beforeEach(waitForAsync(() => { init(); - TestBed.configureTestingModule({ + void TestBed.configureTestingModule({ declarations: [ProcessDetailComponent, ProcessDetailFieldComponent, VarDirective, FileSizePipe], - imports: [TranslateModule.forRoot()], + imports: [TranslateModule.forRoot(), RouterTestingModule], providers: [ - { - provide: ActivatedRoute, - useValue: { - data: observableOf({ process: createSuccessfulRemoteDataObject(process) }), - snapshot: { - params: { id: process.processId } - } - } - }, + { provide: ActivatedRoute, useValue: route }, { provide: ProcessDataService, useValue: processService }, { provide: BitstreamDataService, useValue: bitstreamDataService }, { provide: DSONameService, useValue: nameService }, @@ -258,6 +251,8 @@ describe('ProcessDetailComponent', () => { describe('deleteProcess', () => { it('should delete the process and navigate back to the overview page on success', () => { spyOn(component, 'closeModal'); + spyOn(router, 'navigateByUrl').and.callThrough(); + component.deleteProcess(process); expect(processService.delete).toHaveBeenCalledWith(process.processId); @@ -268,6 +263,7 @@ describe('ProcessDetailComponent', () => { it('should delete the process and not navigate on error', () => { (processService.delete as jasmine.Spy).and.returnValue(createFailedRemoteDataObject$()); spyOn(component, 'closeModal'); + spyOn(router, 'navigateByUrl').and.callThrough(); component.deleteProcess(process); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index c8e4507fd29..e86797d66e2 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http'; import { Component, Inject, NgZone, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, Observable, Subscription } from 'rxjs'; +import { BehaviorSubject, Observable } from 'rxjs'; import { finalize, map, switchMap, take, tap, find, startWith } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; @@ -27,6 +27,7 @@ import { getProcessListRoute } from '../process-page-routing.paths'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; import { isPlatformBrowser } from '@angular/common'; +import { PROCESS_PAGE_FOLLOW_LINKS } from '../process-page.resolver'; @Component({ selector: 'ds-process-detail', @@ -84,8 +85,6 @@ export class ProcessDetailComponent implements OnInit { */ protected modalRef: NgbModalRef; - private refreshTimerSub?: Subscription; - constructor( @Inject(PLATFORM_ID) protected platformId: object, protected route: ActivatedRoute, @@ -109,7 +108,7 @@ export class ProcessDetailComponent implements OnInit { this.processRD$ = this.route.data.pipe( switchMap((data) => { if (isPlatformBrowser(this.platformId)) { - return this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000); + return this.processService.autoRefreshUntilCompletion(this.route.snapshot.params.id, 5000, ...PROCESS_PAGE_FOLLOW_LINKS); } else { return [data.process as RemoteData]; } @@ -125,7 +124,7 @@ export class ProcessDetailComponent implements OnInit { this.filesRD$ = this.processRD$.pipe( getAllSucceededRemoteDataPayload(), - switchMap((process: Process) => this.processService.getFiles(process.processId)) + switchMap((process: Process) => process.files), ); } diff --git a/src/app/process-page/process-page.resolver.ts b/src/app/process-page/process-page.resolver.ts index ba872302b30..2e4843646b0 100644 --- a/src/app/process-page/process-page.resolver.ts +++ b/src/app/process-page/process-page.resolver.ts @@ -7,6 +7,10 @@ import { followLink } from '../shared/utils/follow-link-config.model'; import { ProcessDataService } from '../core/data/processes/process-data.service'; import { getFirstCompletedRemoteData } from '../core/shared/operators'; +export const PROCESS_PAGE_FOLLOW_LINKS = [ + followLink('files'), +]; + /** * This class represents a resolver that requests a specific process before the route is activated */ @@ -23,7 +27,7 @@ export class ProcessPageResolver implements Resolve> { * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { - return this.processService.findById(route.params.id, false, true, followLink('script')).pipe( + return this.processService.findById(route.params.id, false, true, ...PROCESS_PAGE_FOLLOW_LINKS).pipe( getFirstCompletedRemoteData(), ); } diff --git a/src/app/process-page/processes/process.model.ts b/src/app/process-page/processes/process.model.ts index d5f6e77d32a..da396759e90 100644 --- a/src/app/process-page/processes/process.model.ts +++ b/src/app/process-page/processes/process.model.ts @@ -13,6 +13,8 @@ import { RemoteData } from '../../core/data/remote-data'; import { SCRIPT } from '../scripts/script.resource-type'; import { Script } from '../scripts/script.model'; import { CacheableObject } from '../../core/cache/cacheable-object.model'; +import { BITSTREAM } from '../../core/shared/bitstream.resource-type'; +import { PaginatedList } from '../../core/data/paginated-list.model'; /** * Object representing a process @@ -94,4 +96,11 @@ export class Process implements CacheableObject { */ @link(PROCESS_OUTPUT_TYPE) output?: Observable>; + + /** + * The files created by this Process + * Will be undefined unless the output {@link HALLink} has been resolved. + */ + @link(BITSTREAM, true) + files?: Observable>>; } From e339b46228915f31f6a58bd8f085f5200186eee7 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 30 Nov 2023 17:56:34 +0100 Subject: [PATCH 10/19] 108915: Added the missing Filetypes followLink to Process --- .../process-page/processes/filetypes.model.ts | 34 +++++++++++++++++++ .../processes/filetypes.resource-type.ts | 8 +++++ .../process-page/processes/process.model.ts | 13 ++++++- 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/app/process-page/processes/filetypes.model.ts create mode 100644 src/app/process-page/processes/filetypes.resource-type.ts diff --git a/src/app/process-page/processes/filetypes.model.ts b/src/app/process-page/processes/filetypes.model.ts new file mode 100644 index 00000000000..28e9df71cdc --- /dev/null +++ b/src/app/process-page/processes/filetypes.model.ts @@ -0,0 +1,34 @@ +import { typedObject } from '../../core/cache/builders/build-decorators'; +import { excludeFromEquals } from '../../core/utilities/equals.decorators'; +import { autoserialize } from 'cerialize'; +import { ResourceType } from '../../core/shared/resource-type'; +import { FILETYPES } from './filetypes.resource-type'; + +/** + * Object representing the file types of the {@link Bitstream}s of a {@link Process} + */ +@typedObject +export class Filetypes { + + static type = FILETYPES; + + /** + * The id of this {@link Filetypes} + */ + @autoserialize + id: string; + + /** + * The values of this {@link Filetypes} + */ + @autoserialize + values: string[]; + + /** + * The object type + */ + @excludeFromEquals + @autoserialize + type: ResourceType; + +} diff --git a/src/app/process-page/processes/filetypes.resource-type.ts b/src/app/process-page/processes/filetypes.resource-type.ts new file mode 100644 index 00000000000..29f9636208d --- /dev/null +++ b/src/app/process-page/processes/filetypes.resource-type.ts @@ -0,0 +1,8 @@ +/** + * The resource type for {@link Filetypes} + * + * Needs to be in a separate file to prevent circular dependencies in webpack. + */ +import { ResourceType } from '../../core/shared/resource-type'; + +export const FILETYPES = new ResourceType('filetypes'); diff --git a/src/app/process-page/processes/process.model.ts b/src/app/process-page/processes/process.model.ts index da396759e90..609182d6ca4 100644 --- a/src/app/process-page/processes/process.model.ts +++ b/src/app/process-page/processes/process.model.ts @@ -15,6 +15,8 @@ import { Script } from '../scripts/script.model'; import { CacheableObject } from '../../core/cache/cacheable-object.model'; import { BITSTREAM } from '../../core/shared/bitstream.resource-type'; import { PaginatedList } from '../../core/data/paginated-list.model'; +import { Filetypes } from './filetypes.model'; +import { FILETYPES } from './filetypes.resource-type'; /** * Object representing a process @@ -80,7 +82,8 @@ export class Process implements CacheableObject { self: HALLink, script: HALLink, output: HALLink, - files: HALLink + files: HALLink, + filetypes: HALLink, }; /** @@ -103,4 +106,12 @@ export class Process implements CacheableObject { */ @link(BITSTREAM, true) files?: Observable>>; + + /** + * The filetypes present in this Process + * Will be undefined unless the output {@link HALLink} has been resolved. + */ + @link(FILETYPES) + filetypes?: Observable>; + } From 24eb5b4bc0d32157d2bf75a098c064cfe677eacb Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 8 Dec 2023 17:05:00 +0100 Subject: [PATCH 11/19] 108915: Always invalidate all followLinks when invalidating linked cached object --- .../core/data/base/base-data.service.spec.ts | 27 +++--------- src/app/core/data/base/base-data.service.ts | 41 ++++++++++++++++++- .../core/data/collection-data.service.spec.ts | 9 ++-- .../data/relationship-data.service.spec.ts | 12 ++---- .../relationship-type-data.service.spec.ts | 14 ++----- .../resource-policy-data.service.spec.ts | 7 ++-- .../vocabularies/vocabulary.service.spec.ts | 2 + .../supervision-order-data.service.spec.ts | 7 ++-- .../testing/object-cache-service.stub.ts | 31 ++++++++++++++ 9 files changed, 97 insertions(+), 53 deletions(-) create mode 100644 src/app/shared/testing/object-cache-service.stub.ts 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 098f075c101..bfd148e90a3 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -21,6 +21,8 @@ import { RequestEntryState } from '../request-entry-state.model'; import { fakeAsync, tick } from '@angular/core/testing'; import { BaseDataService } from './base-data.service'; import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { ObjectCacheServiceStub } from '../../../shared/testing/object-cache-service.stub'; +import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; const endpoint = 'https://rest.api/core'; @@ -46,7 +48,7 @@ describe('BaseDataService', () => { let requestService; let halService; let rdbService; - let objectCache; + let objectCache: ObjectCacheServiceStub; let selfLink; let linksToFollow; let testScheduler; @@ -56,24 +58,7 @@ describe('BaseDataService', () => { requestService = getMockRequestService(); halService = new HALEndpointServiceStub('url') as any; rdbService = getMockRemoteDataBuildService(); - objectCache = { - - addPatch: () => { - /* empty */ - }, - getObjectBySelfLink: () => { - /* empty */ - }, - getByHref: () => { - /* empty */ - }, - addDependency: () => { - /* empty */ - }, - removeDependents: () => { - /* empty */ - }, - } as any; + objectCache = new ObjectCacheServiceStub(); selfLink = 'https://rest.api/endpoint/1698f1d3-be98-4c51-9fd8-6bfedcbd59b7'; linksToFollow = [ followLink('a'), @@ -104,7 +89,7 @@ describe('BaseDataService', () => { return new TestService( requestService, rdbService, - objectCache, + objectCache as ObjectCacheService, halService, ); } @@ -567,7 +552,7 @@ describe('BaseDataService', () => { getByHrefSpy = spyOn(objectCache, 'getByHref').and.returnValue(observableOf({ requestUUIDs: ['request1', 'request2', 'request3'], dependentRequestUUIDs: ['request4', 'request5'] - })); + } as ObjectCacheEntry)); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index edd6d9e2a42..473dbae0c73 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -24,6 +24,7 @@ import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; import { ObjectCacheService } from '../../cache/object-cache.service'; import { HALDataService } from './hal-data-service.interface'; import { getFirstCompletedRemoteData } from '../../shared/operators'; +import { HALLink } from '../../shared/hal-link.model'; export const EMBED_SEPARATOR = '%2F'; /** @@ -268,7 +269,7 @@ export class BaseDataService implements HALDataServic this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); - return this.rdbService.buildSingle(requestHref$, ...linksToFollow).pipe( + const response$: Observable> = this.rdbService.buildSingle(requestHref$, ...linksToFollow).pipe( // This skip ensures that if a stale object is present in the cache when you do a // 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 @@ -277,6 +278,22 @@ export class BaseDataService implements HALDataServic this.reRequestStaleRemoteData(reRequestOnStale, () => this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); + return response$.pipe( + // Ensure all followLinks from the cached object are automatically invalidated when invalidating the cached object + tap((remoteDataObject: RemoteData) => { + if (hasValue(remoteDataObject?.payload?._links)) { + for (const followLink of Object.values(remoteDataObject.payload._links)) { + // followLink can be either an individual HALLink or a HALLink[] + const followLinksList: HALLink[] = [].concat(followLink); + for (const individualFollowLink of followLinksList) { + if (hasValue(individualFollowLink?.href)) { + this.addDependency(response$, individualFollowLink.href); + } + } + } + } + }), + ); } /** @@ -302,7 +319,7 @@ export class BaseDataService implements HALDataServic this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); - return this.rdbService.buildList(requestHref$, ...linksToFollow).pipe( + const response$: Observable>> = this.rdbService.buildList(requestHref$, ...linksToFollow).pipe( // This skip ensures that if a stale object is present in the cache when you do a // 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 @@ -311,6 +328,26 @@ export class BaseDataService implements HALDataServic this.reRequestStaleRemoteData(reRequestOnStale, () => this.findListByHref(href$, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); + return response$.pipe( + // Ensure all followLinks from the cached object are automatically invalidated when invalidating the cached object + tap((remoteDataObject: RemoteData>) => { + if (hasValue(remoteDataObject?.payload?.page)) { + for (const object of remoteDataObject.payload.page) { + if (hasValue(object?._links)) { + for (const followLink of Object.values(object._links)) { + // followLink can be either an individual HALLink or a HALLink[] + const followLinksList: HALLink[] = [].concat(followLink); + for (const individualFollowLink of followLinksList) { + if (hasValue(individualFollowLink?.href)) { + this.addDependency(response$, individualFollowLink.href); + } + } + } + } + } + } + }), + ); } /** diff --git a/src/app/core/data/collection-data.service.spec.ts b/src/app/core/data/collection-data.service.spec.ts index 65f8b3ab2cd..c1a7ac64c26 100644 --- a/src/app/core/data/collection-data.service.spec.ts +++ b/src/app/core/data/collection-data.service.spec.ts @@ -24,6 +24,7 @@ import { testFindAllDataImplementation } from './base/find-all-data.spec'; import { testSearchDataImplementation } from './base/search-data.spec'; import { testPatchDataImplementation } from './base/patch-data.spec'; import { testDeleteDataImplementation } from './base/delete-data.spec'; +import { ObjectCacheServiceStub } from '../../shared/testing/object-cache-service.stub'; const url = 'fake-url'; const collectionId = 'fake-collection-id'; @@ -35,7 +36,7 @@ describe('CollectionDataService', () => { let translate: TranslateService; let notificationsService: any; let rdbService: RemoteDataBuildService; - let objectCache: ObjectCacheService; + let objectCache: ObjectCacheServiceStub; let halService: any; const mockCollection1: Collection = Object.assign(new Collection(), { @@ -205,14 +206,12 @@ describe('CollectionDataService', () => { buildFromRequestUUID: buildResponse$, buildSingle: buildResponse$ }); - objectCache = jasmine.createSpyObj('objectCache', { - remove: jasmine.createSpy('remove') - }); + objectCache = new ObjectCacheServiceStub(); halService = new HALEndpointServiceStub(url); notificationsService = new NotificationsServiceStub(); translate = getMockTranslateService(); - service = new CollectionDataService(requestService, rdbService, objectCache, halService, null, notificationsService, null, null, translate); + service = new CollectionDataService(requestService, rdbService, objectCache as ObjectCacheService, halService, null, notificationsService, null, null, translate); } }); diff --git a/src/app/core/data/relationship-data.service.spec.ts b/src/app/core/data/relationship-data.service.spec.ts index 4432d5213ae..8ce67d19e09 100644 --- a/src/app/core/data/relationship-data.service.spec.ts +++ b/src/app/core/data/relationship-data.service.spec.ts @@ -23,6 +23,7 @@ import { FindListOptions } from './find-list-options.model'; import { testSearchDataImplementation } from './base/search-data.spec'; import { MetadataValue } from '../shared/metadata.models'; import { MetadataRepresentationType } from '../shared/metadata-representation/metadata-representation.model'; +import { ObjectCacheServiceStub } from '../../shared/testing/object-cache-service.stub'; describe('RelationshipDataService', () => { let service: RelationshipDataService; @@ -114,14 +115,7 @@ describe('RelationshipDataService', () => { 'href': buildList$, 'https://rest.api/core/publication/relationships': relationships$ }); - const objectCache = Object.assign({ - /* eslint-disable no-empty,@typescript-eslint/no-empty-function */ - remove: () => { - }, - hasBySelfLinkObservable: () => observableOf(false), - hasByHref$: () => observableOf(false) - /* eslint-enable no-empty, @typescript-eslint/no-empty-function */ - }) as ObjectCacheService; + const objectCache = new ObjectCacheServiceStub(); const itemService = jasmine.createSpyObj('itemService', { findById: (uuid) => createSuccessfulRemoteDataObject(relatedItems.find((relatedItem) => relatedItem.id === uuid)), @@ -133,7 +127,7 @@ describe('RelationshipDataService', () => { requestService, rdbService, halService, - objectCache, + objectCache as ObjectCacheService, itemService, null, jasmine.createSpy('paginatedRelationsToItems').and.returnValue((v) => v), diff --git a/src/app/core/data/relationship-type-data.service.spec.ts b/src/app/core/data/relationship-type-data.service.spec.ts index 6a788446d8a..ecd84f82885 100644 --- a/src/app/core/data/relationship-type-data.service.spec.ts +++ b/src/app/core/data/relationship-type-data.service.spec.ts @@ -10,6 +10,7 @@ import { RequestService } from './request.service'; import { createPaginatedList } from '../../shared/testing/utils.test'; import { hasValueOperator } from '../../shared/empty.util'; import { ObjectCacheService } from '../cache/object-cache.service'; +import { ObjectCacheServiceStub } from '../../shared/testing/object-cache-service.stub'; describe('RelationshipTypeDataService', () => { let service: RelationshipTypeDataService; @@ -28,7 +29,7 @@ describe('RelationshipTypeDataService', () => { let buildList; let rdbService; - let objectCache; + let objectCache: ObjectCacheServiceStub; function init() { restEndpointURL = 'https://rest.api/relationshiptypes'; @@ -60,21 +61,14 @@ describe('RelationshipTypeDataService', () => { buildList = createSuccessfulRemoteDataObject(createPaginatedList([relationshipType1, relationshipType2])); rdbService = getMockRemoteDataBuildService(undefined, observableOf(buildList)); - objectCache = Object.assign({ - /* eslint-disable no-empty,@typescript-eslint/no-empty-function */ - remove: () => { - }, - hasBySelfLinkObservable: () => observableOf(false) - /* eslint-enable no-empty, @typescript-eslint/no-empty-function */ - }) as ObjectCacheService; - + objectCache = new ObjectCacheServiceStub(); } function initTestService() { return new RelationshipTypeDataService( requestService, rdbService, - objectCache, + objectCache as ObjectCacheService, halService, ); } diff --git a/src/app/core/resource-policy/resource-policy-data.service.spec.ts b/src/app/core/resource-policy/resource-policy-data.service.spec.ts index 7cfcaabb5d9..e4c54d862cf 100644 --- a/src/app/core/resource-policy/resource-policy-data.service.spec.ts +++ b/src/app/core/resource-policy/resource-policy-data.service.spec.ts @@ -20,13 +20,14 @@ import { FindListOptions } from '../data/find-list-options.model'; import { EPersonDataService } from '../eperson/eperson-data.service'; import { GroupDataService } from '../eperson/group-data.service'; import { RestRequestMethod } from '../data/rest-request-method'; +import { ObjectCacheServiceStub } from '../../shared/testing/object-cache-service.stub'; describe('ResourcePolicyService', () => { let scheduler: TestScheduler; let service: ResourcePolicyDataService; let requestService: RequestService; let rdbService: RemoteDataBuildService; - let objectCache: ObjectCacheService; + let objectCache: ObjectCacheServiceStub; let halService: HALEndpointService; let responseCacheEntry: RequestEntry; let ePersonService: EPersonDataService; @@ -139,14 +140,14 @@ describe('ResourcePolicyService', () => { a: 'https://rest.api/rest/api/eperson/groups/' + groupUUID }), }); - objectCache = {} as ObjectCacheService; + objectCache = new ObjectCacheServiceStub(); const notificationsService = {} as NotificationsService; const comparator = {} as any; service = new ResourcePolicyDataService( requestService, rdbService, - objectCache, + objectCache as ObjectCacheService, halService, notificationsService, comparator, diff --git a/src/app/core/submission/vocabularies/vocabulary.service.spec.ts b/src/app/core/submission/vocabularies/vocabulary.service.spec.ts index faa58235203..e8ff2b479d8 100644 --- a/src/app/core/submission/vocabularies/vocabulary.service.spec.ts +++ b/src/app/core/submission/vocabularies/vocabulary.service.spec.ts @@ -25,6 +25,7 @@ import { createPaginatedList } from '../../../shared/testing/utils.test'; import { RequestEntry } from '../../data/request-entry.model'; import { VocabularyDataService } from './vocabulary.data.service'; import { VocabularyEntryDetailsDataService } from './vocabulary-entry-details.data.service'; +import { ObjectCacheServiceStub } from '../../../shared/testing/object-cache-service.stub'; describe('VocabularyService', () => { let scheduler: TestScheduler; @@ -205,6 +206,7 @@ describe('VocabularyService', () => { function initTestService() { hrefOnlyDataService = getMockHrefOnlyDataService(); + objectCache = new ObjectCacheServiceStub() as ObjectCacheService; return new VocabularyService( requestService, diff --git a/src/app/core/supervision-order/supervision-order-data.service.spec.ts b/src/app/core/supervision-order/supervision-order-data.service.spec.ts index b12817fa1af..b25d440fa23 100644 --- a/src/app/core/supervision-order/supervision-order-data.service.spec.ts +++ b/src/app/core/supervision-order/supervision-order-data.service.spec.ts @@ -17,13 +17,14 @@ import { RestResponse } from '../cache/response.models'; import { RequestEntry } from '../data/request-entry.model'; import { FindListOptions } from '../data/find-list-options.model'; import { GroupDataService } from '../eperson/group-data.service'; +import { ObjectCacheServiceStub } from '../../shared/testing/object-cache-service.stub'; describe('SupervisionOrderService', () => { let scheduler: TestScheduler; let service: SupervisionOrderDataService; let requestService: RequestService; let rdbService: RemoteDataBuildService; - let objectCache: ObjectCacheService; + let objectCache: ObjectCacheServiceStub; let halService: HALEndpointService; let responseCacheEntry: RequestEntry; let groupService: GroupDataService; @@ -127,14 +128,14 @@ describe('SupervisionOrderService', () => { a: 'https://rest.api/rest/api/group/groups/' + groupUUID }), }); - objectCache = {} as ObjectCacheService; + objectCache = new ObjectCacheServiceStub(); const notificationsService = {} as NotificationsService; const comparator = {} as any; service = new SupervisionOrderDataService( requestService, rdbService, - objectCache, + objectCache as ObjectCacheService, halService, notificationsService, comparator, diff --git a/src/app/shared/testing/object-cache-service.stub.ts b/src/app/shared/testing/object-cache-service.stub.ts new file mode 100644 index 00000000000..f62f3575c35 --- /dev/null +++ b/src/app/shared/testing/object-cache-service.stub.ts @@ -0,0 +1,31 @@ +import { Observable, of as observableOf } from 'rxjs'; +import { CacheableObject } from '../../core/cache/cacheable-object.model'; +import { ObjectCacheEntry } from '../../core/cache/object-cache.reducer'; + +/* eslint-disable @typescript-eslint/no-empty-function */ +/** + * Stub class of {@link ObjectCacheService} + */ +export class ObjectCacheServiceStub { + + add(_object: CacheableObject, _msToLive: number, _requestUUID: string, _alternativeLink?: string): void { + } + + remove(_href: string): void { + } + + getByHref(_href: string): Observable { + return observableOf(undefined); + } + + hasByHref$(_href: string): Observable { + return observableOf(false); + } + + addDependency(_href$: string | Observable, _dependsOnHref$: string | Observable): void { + } + + removeDependents(_href: string): void { + } + +} From 540098175199f0ac4fb4344862fceb0dd349661c Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 19 Dec 2023 15:58:11 +0100 Subject: [PATCH 12/19] 108915: Added tests proving that the addDependency is called on all the followLinks Also fixed the mock values used by findListByHref, because they didn't use PaginatedLists but regular items --- .../core/data/base/base-data.service.spec.ts | 169 +++++++++++++----- 1 file changed, 120 insertions(+), 49 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 bfd148e90a3..c10f52d8a50 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -23,6 +23,8 @@ import { BaseDataService } from './base-data.service'; import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { ObjectCacheServiceStub } from '../../../shared/testing/object-cache-service.stub'; import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; +import { HALLink } from '../../shared/hal-link.model'; +import { createPaginatedList } from '../../../shared/testing/utils.test'; const endpoint = 'https://rest.api/core'; @@ -52,7 +54,8 @@ describe('BaseDataService', () => { let selfLink; let linksToFollow; let testScheduler; - let remoteDataMocks; + let remoteDataMocks: { [responseType: string]: RemoteData }; + let remoteDataPageMocks: { [responseType: string]: RemoteData }; function initTestService(): TestService { requestService = getMockRequestService(); @@ -73,7 +76,25 @@ describe('BaseDataService', () => { const timeStamp = new Date().getTime(); const msToLive = 15 * 60 * 1000; - const payload = { foo: 'bar' }; + const payload = { + foo: 'bar', + _links: { + self: Object.assign(new HALLink(), { + href: 'self-test-link', + }), + followLink1: Object.assign(new HALLink(), { + href: 'follow-link-1', + }), + followLink2: [ + Object.assign(new HALLink(), { + href: 'follow-link-2-1', + }), + Object.assign(new HALLink(), { + href: 'follow-link-2-2', + }), + ], + } + }; const statusCodeSuccess = 200; const statusCodeError = 404; const errorMessage = 'not found'; @@ -85,6 +106,14 @@ describe('BaseDataService', () => { Error: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError), ErrorStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError), }; + remoteDataPageMocks = { + RequestPending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.RequestPending, undefined, undefined, undefined), + ResponsePending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePending, undefined, undefined, undefined), + Success: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Success, undefined, createPaginatedList([payload]), statusCodeSuccess), + SuccessStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.SuccessStale, undefined, createPaginatedList([payload]), statusCodeSuccess), + Error: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError), + ErrorStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError), + }; return new TestService( requestService, @@ -360,6 +389,27 @@ describe('BaseDataService', () => { }); + it('should link all the followLinks of a cached object by calling addDependency', () => { + spyOn(objectCache, 'addDependency').and.callThrough(); + testScheduler.run(({ cold, expectObservable, flush }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d', { + a: remoteDataMocks.Success, + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + })); + const expected = '--b-c-d'; + const values = { + b: remoteDataMocks.RequestPending, + c: remoteDataMocks.ResponsePending, + d: remoteDataMocks.Success, + }; + + expectObservable(service.findByHref(selfLink, false, false, ...linksToFollow)).toBe(expected, values); + flush(); + expect(objectCache.addDependency).toHaveBeenCalledTimes(4); + }); + }); }); describe(`findListByHref`, () => { @@ -372,8 +422,8 @@ describe('BaseDataService', () => { it(`should call buildHrefFromFindOptions with href and linksToFollow`, () => { testScheduler.run(({ cold }) => { spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); - spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); - spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataPageMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataPageMocks.Success })); service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow); expect(service.buildHrefFromFindOptions).toHaveBeenCalledWith(selfLink, findListOptions, [], ...linksToFollow); @@ -383,8 +433,8 @@ describe('BaseDataService', () => { it(`should call createAndSendGetRequest with the result from buildHrefFromFindOptions and useCachedVersionIfAvailable`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); - spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); - spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataPageMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataPageMocks.Success })); service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow); expect((service as any).createAndSendGetRequest).toHaveBeenCalledWith(jasmine.anything(), true); @@ -399,8 +449,8 @@ describe('BaseDataService', () => { it(`should call rdbService.buildList with the result from buildHrefFromFindOptions and linksToFollow`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); - spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); - spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.Success })); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataPageMocks.Success })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataPageMocks.Success })); service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow); expect(rdbService.buildList).toHaveBeenCalledWith(jasmine.anything() as any, ...linksToFollow); @@ -411,12 +461,12 @@ describe('BaseDataService', () => { it(`should call reRequestStaleRemoteData with reRequestOnStale and the exact same findListByHref call as a callback`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(service, 'buildHrefFromFindOptions').and.returnValue('bingo!'); - spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); - spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataMocks.SuccessStale })); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataPageMocks.SuccessStale })); + spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: remoteDataPageMocks.SuccessStale })); service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow); expect((service as any).reRequestStaleRemoteData.calls.argsFor(0)[0]).toBeTrue(); - spyOn(service, 'findListByHref').and.returnValue(cold('a', { a: remoteDataMocks.SuccessStale })); + spyOn(service, 'findListByHref').and.returnValue(cold('a', { a: remoteDataPageMocks.SuccessStale })); // prove that the spy we just added hasn't been called yet expect(service.findListByHref).not.toHaveBeenCalled(); // call the callback passed to reRequestStaleRemoteData @@ -431,7 +481,7 @@ describe('BaseDataService', () => { it(`should return a the output from reRequestStaleRemoteData`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(service, 'buildHrefFromFindOptions').and.returnValue(selfLink); - spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataMocks.Success })); + spyOn(rdbService, 'buildList').and.returnValue(cold('a', { a: remoteDataPageMocks.Success })); spyOn(service as any, 'reRequestStaleRemoteData').and.returnValue(() => cold('a', { a: 'bingo!' })); const expected = 'a'; const values = { @@ -451,19 +501,19 @@ describe('BaseDataService', () => { it(`should emit a cached completed RemoteData immediately, and keep emitting if it gets rerequested`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { - a: remoteDataMocks.Success, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + a: remoteDataPageMocks.Success, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, })); const expected = 'a-b-c-d-e'; const values = { - a: remoteDataMocks.Success, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + a: remoteDataPageMocks.Success, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); @@ -473,18 +523,18 @@ 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, + a: remoteDataPageMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, })); const expected = '--b-c-d-e'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, true, true, ...linksToFollow)).toBe(expected, values); @@ -503,18 +553,18 @@ describe('BaseDataService', () => { it(`should not emit a cached completed 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.Success, - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + a: remoteDataPageMocks.Success, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, })); const expected = '--b-c-d-e'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); @@ -524,24 +574,45 @@ 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, + a: remoteDataPageMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, })); const expected = '--b-c-d-e'; const values = { - b: remoteDataMocks.RequestPending, - c: remoteDataMocks.ResponsePending, - d: remoteDataMocks.Success, - e: remoteDataMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + e: remoteDataPageMocks.SuccessStale, }; expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); }); }); + it('should link all the followLinks of the cached objects by calling addDependency', () => { + spyOn(objectCache, 'addDependency').and.callThrough(); + testScheduler.run(({ cold, expectObservable, flush }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d', { + a: remoteDataPageMocks.SuccessStale, + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + })); + const expected = '--b-c-d'; + const values = { + b: remoteDataPageMocks.RequestPending, + c: remoteDataPageMocks.ResponsePending, + d: remoteDataPageMocks.Success, + }; + + expectObservable(service.findListByHref(selfLink, findListOptions, false, false, ...linksToFollow)).toBe(expected, values); + flush(); + expect(objectCache.addDependency).toHaveBeenCalledTimes(4); + }); + }); }); }); From 9e31f73f5e0dcba2944df8d39b807dcd2d3edeb0 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 5 Jan 2024 16:51:23 +0100 Subject: [PATCH 13/19] 108915: Fixed issue where the observable would emit itself again even when the lastUpdated didn't change --- src/app/core/cache/builders/remote-data-build.service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 075bf3ca0ca..42cff05abe1 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -272,12 +272,13 @@ export class RemoteDataBuildService { return isStale(r2.state) ? r1 : r2; } }), - distinctUntilKeyChanged('lastUpdated') ); const payload$ = this.buildPayload(requestEntry$, href$, ...linksToFollow); - return this.toRemoteDataObservable(requestEntry$, payload$); + return this.toRemoteDataObservable(requestEntry$, payload$).pipe( + distinctUntilKeyChanged('lastUpdated'), + ); } /** From c91b99feceb1f5bc9c51c587cf84e9511865bf26 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 5 Jan 2024 16:53:05 +0100 Subject: [PATCH 14/19] 108915: Fixed delete process error on ProcessDetailComponent --- src/app/core/data/processes/process-data.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index 4cd18caad9a..f11367f6ecf 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -196,7 +196,7 @@ export class ProcessDataService extends IdentifiableDataService impleme return process$.pipe( distinctUntilChanged((previous: RemoteData, current: RemoteData) => - previous.payload.processStatus === current.payload.processStatus + previous.payload?.processStatus === current.payload?.processStatus, ) ); } From 252b3673eea2a9bcd68f35af7fbe51b0c05fc5e3 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 8 Feb 2024 16:14:38 +0100 Subject: [PATCH 15/19] 108915: Fixed issue that redirected you to 404 page when deleting the process on the process page --- src/app/process-page/detail/process-detail.component.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index e86797d66e2..a9348f6589c 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -2,7 +2,7 @@ import { HttpClient } from '@angular/common/http'; import { Component, Inject, NgZone, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; import { BehaviorSubject, Observable } from 'rxjs'; -import { finalize, map, switchMap, take, tap, find, startWith } from 'rxjs/operators'; +import { finalize, map, switchMap, take, tap, find, startWith, filter } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { BitstreamDataService } from '../../core/data/bitstream-data.service'; @@ -80,6 +80,8 @@ export class ProcessDetailComponent implements OnInit { isRefreshing$: Observable; + isDeleting: boolean; + /** * Reference to NgbModal */ @@ -113,6 +115,7 @@ export class ProcessDetailComponent implements OnInit { return [data.process as RemoteData]; } }), + filter(() => !this.isDeleting), redirectOn4xx(this.router, this.authService), ); @@ -203,15 +206,17 @@ export class ProcessDetailComponent implements OnInit { * @param process */ deleteProcess(process: Process) { + this.isDeleting = true; this.processService.delete(process.processId).pipe( getFirstCompletedRemoteData() ).subscribe((rd) => { if (rd.hasSucceeded) { this.notificationsService.success(this.translateService.get('process.detail.delete.success')); this.closeModal(); - this.router.navigateByUrl(getProcessListRoute()); + void this.router.navigateByUrl(getProcessListRoute()); } else { this.notificationsService.error(this.translateService.get('process.detail.delete.error')); + this.isDeleting = false; } }); } From ecb20bbcbfa8c35bc2d60fb62bd93546a1ef60a9 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 8 Feb 2024 16:16:06 +0100 Subject: [PATCH 16/19] 108915: Fixed Proxy Timout error when retrieving a non-existing process --- .../core/data/processes/process-data.service.ts | 14 ++++++++------ .../process-page/process-breadcrumb.resolver.ts | 10 +++++----- .../process-page/process-breadcrumbs.service.ts | 7 ++++++- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/app/core/data/processes/process-data.service.ts b/src/app/core/data/processes/process-data.service.ts index f11367f6ecf..ac459068b1b 100644 --- a/src/app/core/data/processes/process-data.service.ts +++ b/src/app/core/data/processes/process-data.service.ts @@ -175,12 +175,14 @@ export class ProcessDataService extends IdentifiableDataService impleme ) ).subscribe((processRD: RemoteData) => { this.clearCurrentTimeout(processId); - const nextTimeout = this.timer(() => { - this.activelyBeingPolled.delete(processId); - this.invalidateByHref(processRD.payload._links.self.href); - }, pollingIntervalInMs); - - this.activelyBeingPolled.set(processId, nextTimeout); + if (processRD.hasSucceeded) { + const nextTimeout = this.timer(() => { + this.activelyBeingPolled.delete(processId); + this.invalidateByHref(processRD.payload._links.self.href); + }, pollingIntervalInMs); + + this.activelyBeingPolled.set(processId, nextTimeout); + } }); // When the process completes create a one off subscription (the `find` completes the diff --git a/src/app/process-page/process-breadcrumb.resolver.ts b/src/app/process-page/process-breadcrumb.resolver.ts index 23e3dca0a8a..fd0c1ad735a 100644 --- a/src/app/process-page/process-breadcrumb.resolver.ts +++ b/src/app/process-page/process-breadcrumb.resolver.ts @@ -6,8 +6,9 @@ import { Process } from './processes/process.model'; import { followLink } from '../shared/utils/follow-link-config.model'; import { ProcessDataService } from '../core/data/processes/process-data.service'; import { BreadcrumbConfig } from '../breadcrumbs/breadcrumb/breadcrumb-config.model'; -import { getRemoteDataPayload, getFirstSucceededRemoteData } from '../core/shared/operators'; +import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { ProcessBreadcrumbsService } from './process-breadcrumbs.service'; +import { RemoteData } from '../core/data/remote-data'; /** * This class represents a resolver that requests a specific process before the route is activated @@ -28,12 +29,11 @@ export class ProcessBreadcrumbResolver implements Resolve { + getFirstCompletedRemoteData(), + map((object: RemoteData) => { const fullPath = state.url; const url = fullPath.substr(0, fullPath.indexOf(id)) + id; - return { provider: this.breadcrumbService, key: object, url: url }; + return { provider: this.breadcrumbService, key: object.payload, url: url }; }) ); } diff --git a/src/app/process-page/process-breadcrumbs.service.ts b/src/app/process-page/process-breadcrumbs.service.ts index 26b0787a536..ac490138b8f 100644 --- a/src/app/process-page/process-breadcrumbs.service.ts +++ b/src/app/process-page/process-breadcrumbs.service.ts @@ -3,6 +3,7 @@ import { Injectable } from '@angular/core'; import { BreadcrumbsProviderService } from '../core/breadcrumbs/breadcrumbsProviderService'; import { Breadcrumb } from '../breadcrumbs/breadcrumb/breadcrumb.model'; import { Process } from './processes/process.model'; +import { hasValue } from '../shared/empty.util'; /** * Service to calculate process breadcrumbs for a single part of the route @@ -16,6 +17,10 @@ export class ProcessBreadcrumbsService implements BreadcrumbsProviderService { - return observableOf([new Breadcrumb(key.processId + ' - ' + key.scriptName, url)]); + if (hasValue(key)) { + return observableOf([new Breadcrumb(key.processId + ' - ' + key.scriptName, url)]); + } else { + return observableOf([]); + } } } From 8ddc62168ef82f32d00a89ce387003c49ad21af1 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Mon, 19 Feb 2024 18:56:43 +0100 Subject: [PATCH 17/19] 108915: Prevent self links & unresolved followLinks to be added as dependencies --- .../core/data/base/base-data.service.spec.ts | 6 ++-- src/app/core/data/base/base-data.service.ts | 30 +++++++++++-------- 2 files changed, 22 insertions(+), 14 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 c10f52d8a50..1ea35756d33 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -78,6 +78,8 @@ describe('BaseDataService', () => { const msToLive = 15 * 60 * 1000; const payload = { foo: 'bar', + followLink1: {}, + followLink2: {}, _links: { self: Object.assign(new HALLink(), { href: 'self-test-link', @@ -407,7 +409,7 @@ describe('BaseDataService', () => { expectObservable(service.findByHref(selfLink, false, false, ...linksToFollow)).toBe(expected, values); flush(); - expect(objectCache.addDependency).toHaveBeenCalledTimes(4); + expect(objectCache.addDependency).toHaveBeenCalledTimes(3); }); }); }); @@ -610,7 +612,7 @@ describe('BaseDataService', () => { expectObservable(service.findListByHref(selfLink, findListOptions, false, false, ...linksToFollow)).toBe(expected, values); flush(); - expect(objectCache.addDependency).toHaveBeenCalledTimes(4); + expect(objectCache.addDependency).toHaveBeenCalledTimes(3); }); }); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index 473dbae0c73..aef680578b2 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -282,12 +282,15 @@ export class BaseDataService implements HALDataServic // Ensure all followLinks from the cached object are automatically invalidated when invalidating the cached object tap((remoteDataObject: RemoteData) => { if (hasValue(remoteDataObject?.payload?._links)) { - for (const followLink of Object.values(remoteDataObject.payload._links)) { - // followLink can be either an individual HALLink or a HALLink[] - const followLinksList: HALLink[] = [].concat(followLink); - for (const individualFollowLink of followLinksList) { - if (hasValue(individualFollowLink?.href)) { - this.addDependency(response$, individualFollowLink.href); + for (const followLinkName of Object.keys(remoteDataObject.payload._links)) { + // only add the followLinks if they are embedded + if (hasValue(remoteDataObject.payload[followLinkName]) && followLinkName !== 'self') { + // followLink can be either an individual HALLink or a HALLink[] + const followLinksList: HALLink[] = [].concat(remoteDataObject.payload._links[followLinkName]); + for (const individualFollowLink of followLinksList) { + if (hasValue(individualFollowLink?.href)) { + this.addDependency(response$, individualFollowLink.href); + } } } } @@ -334,12 +337,15 @@ export class BaseDataService implements HALDataServic if (hasValue(remoteDataObject?.payload?.page)) { for (const object of remoteDataObject.payload.page) { if (hasValue(object?._links)) { - for (const followLink of Object.values(object._links)) { - // followLink can be either an individual HALLink or a HALLink[] - const followLinksList: HALLink[] = [].concat(followLink); - for (const individualFollowLink of followLinksList) { - if (hasValue(individualFollowLink?.href)) { - this.addDependency(response$, individualFollowLink.href); + for (const followLinkName of Object.keys(object._links)) { + // only add the followLinks if they are embedded + if (hasValue(object[followLinkName]) && followLinkName !== 'self') { + // followLink can be either an individual HALLink or a HALLink[] + const followLinksList: HALLink[] = [].concat(object._links[followLinkName]); + for (const individualFollowLink of followLinksList) { + if (hasValue(individualFollowLink?.href)) { + this.addDependency(response$, individualFollowLink.href); + } } } } From d5aca7633c666e8827eb6645868d4fffd6b04bb6 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Thu, 22 Feb 2024 09:10:02 +0100 Subject: [PATCH 18/19] site search should not adopt existing URL query string parameters (#2818) src/app/search-navbar: do not merge existing query params Do not merge existing query params when using site search from MyDSpace workflow. --- src/app/search-navbar/search-navbar.component.spec.ts | 4 ++-- src/app/search-navbar/search-navbar.component.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/search-navbar/search-navbar.component.spec.ts b/src/app/search-navbar/search-navbar.component.spec.ts index 7edae293e12..ece2391dcb0 100644 --- a/src/app/search-navbar/search-navbar.component.spec.ts +++ b/src/app/search-navbar/search-navbar.component.spec.ts @@ -88,7 +88,7 @@ describe('SearchNavbarComponent', () => { fixture.detectChanges(); })); it('to search page with empty query', () => { - const extras: NavigationExtras = { queryParams: { query: '' }, queryParamsHandling: 'merge' }; + const extras: NavigationExtras = { queryParams: { query: '' } }; expect(component.onSubmit).toHaveBeenCalledWith({ query: '' }); expect(router.navigate).toHaveBeenCalledWith(['search'], extras); }); @@ -113,7 +113,7 @@ describe('SearchNavbarComponent', () => { fixture.detectChanges(); })); it('to search page with query', async () => { - const extras: NavigationExtras = { queryParams: { query: 'test' }, queryParamsHandling: 'merge' }; + const extras: NavigationExtras = { queryParams: { query: 'test' } }; expect(component.onSubmit).toHaveBeenCalledWith({ query: 'test' }); expect(router.navigate).toHaveBeenCalledWith(['search'], extras); diff --git a/src/app/search-navbar/search-navbar.component.ts b/src/app/search-navbar/search-navbar.component.ts index 98e64f6e10f..7f8f951073f 100644 --- a/src/app/search-navbar/search-navbar.component.ts +++ b/src/app/search-navbar/search-navbar.component.ts @@ -66,8 +66,7 @@ export class SearchNavbarComponent { this.searchForm.reset(); this.router.navigate(linkToNavigateTo, { - queryParams: queryParams, - queryParamsHandling: 'merge' + queryParams: queryParams }); } } From 10e8768fc2a3ffb9224028ec91e9326629b8a3bd Mon Sep 17 00:00:00 2001 From: FrancescoMolinaro Date: Thu, 22 Feb 2024 16:25:47 +0100 Subject: [PATCH 19/19] [issue-9355] fix error on e2e tests --- .../suggestion-notifications/suggestions.service.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/suggestion-notifications/suggestions.service.ts b/src/app/suggestion-notifications/suggestions.service.ts index 874659eab5f..3b2ce081f1d 100644 --- a/src/app/suggestion-notifications/suggestions.service.ts +++ b/src/app/suggestion-notifications/suggestions.service.ts @@ -11,7 +11,7 @@ import { hasValue, isNotEmpty } from '../shared/empty.util'; import { ResearcherProfile } from '../core/profile/model/researcher-profile.model'; import { getAllSucceededRemoteDataPayload, - getFinishedRemoteData, + getFinishedRemoteData, getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload, getFirstSucceededRemoteListPayload } from '../core/shared/operators'; @@ -155,10 +155,10 @@ export class SuggestionsService { */ public retrieveCurrentUserSuggestions(userUuid: string): Observable { return this.researcherProfileService.findById(userUuid, true).pipe( - getFirstSucceededRemoteDataPayload(), - mergeMap((profile: ResearcherProfile) => { - if (isNotEmpty(profile)) { - return this.researcherProfileService.findRelatedItemId(profile).pipe( + getFirstCompletedRemoteData(), + mergeMap((profile: RemoteData ) => { + if (isNotEmpty(profile) && profile.hasSucceeded && isNotEmpty(profile.payload)) { + return this.researcherProfileService.findRelatedItemId(profile.payload).pipe( mergeMap((itemId: string) => { return this.suggestionsDataService.getTargetsByUser(itemId).pipe( getFirstSucceededRemoteListPayload() @@ -169,7 +169,7 @@ export class SuggestionsService { return of([]); } }), - take(1) + catchError(() => of([])) ); }