From 78d0bdd336f65204bafca66f1c1734f62906a042 Mon Sep 17 00:00:00 2001 From: Koen Pauwels Date: Fri, 18 Aug 2023 14:18:41 +0200 Subject: [PATCH 001/112] 104938 Implementation and tests (WIP) --- ...llection-source-controls.component.spec.ts | 8 +- .../collection-source-controls.component.ts | 194 +++++++++++------- .../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, 418 insertions(+), 204 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 3eb83ebe8a..37a5d8642d 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 7113c25e9f..e8c3666da0 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); - }); - }); - this.testConfigRunning$.next(false); - } + 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); + } + })); + + // 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 88e5bd5791..6e7ce51502 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 3bf34eb650..f550407a59 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 9552f9a092..68b97d0e5c 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 a379dfe337..18992eae2f 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 002/112] 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 6e7ce51502..bf42b6b9cf 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 f550407a59..f0c0829e85 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 003/112] 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 bf42b6b9cf..ccdb65c417 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 004/112] 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 | 107 +++++++++------- .../data/processes/process-data.service.ts | 114 +++++++++++++----- .../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, 183 insertions(+), 170 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 37a5d8642d..f717943e8e 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 e8c3666da0..ea2cb3e2f4 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 ccdb65c417..f58752ee97 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; - spyOn(processDataService, 'findById').and.returnValue( - cold('(c|)', { - 'c': new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess) - }) - ); + const completedProcessRD = new RemoteData(0, 0, 0, RequestEntryState.Success, null, completedProcess); - let process$ = processDataService.notifyOnCompletion('instantly'); - process$.subscribe((rd) => { - expect(processDataService.findById).toHaveBeenCalledTimes(1); - expect(processDataService.invalidateByHref).not.toHaveBeenCalled(); + spyOn(processDataService, 'findById').and.returnValue( + cold('c', { + 'c': completedProcessRD + }) + ); + + 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; + 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('p 150ms (c|)', { - 'p': new RemoteData(0, 0, 0, RequestEntryState.Success, null, runningProcess), - 'c': 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 + }); + }); - let process$ = processDataService.notifyOnCompletion('foo', 100); - 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); }); - }); -}); + }); // /** // * 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 f0c0829e85..a639ef24ea 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; + // 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); - // 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); + this.activelyBeingPolled.set(processId, nextTimeout); + }); - // 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) - ); - } - }); - } + // 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 5f905cbfff..c25a6ad50a 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 18992eae2f..b1d3241422 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(); - // } - // } + 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), + ); - // 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.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 b43340bffb..1ff42789d8 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 6c91bae4c1..aa327c7934 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 005/112] 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 f58752ee97..fe632096f5 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 68b97d0e5c..ba99342191 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 b1d3241422..d96c47b371 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 006/112] 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 ea2cb3e2f4..58f41acf34 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 fe632096f5..d66560b083 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 a639ef24ea..e17b0b1f19 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 ba99342191..1af1edca99 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 d96c47b371..c8e4507fd2 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 007/112] 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 58f41acf34..185a1f938e 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 1af1edca99..241af9fd10 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 008/112] 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 e17b0b1f19..3af755561c 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 a55eb97a6ac5bd4ee4a32d07b09369d5d2fa286a Mon Sep 17 00:00:00 2001 From: Vlad Nouski Date: Fri, 20 Oct 2023 16:04:40 +0200 Subject: [PATCH 009/112] [CST-12043] fix: normalization for boolean --- src/app/shared/form/builder/form-builder.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/shared/form/builder/form-builder.service.ts b/src/app/shared/form/builder/form-builder.service.ts index cf6f38bf7b..8bd449a07d 100644 --- a/src/app/shared/form/builder/form-builder.service.ts +++ b/src/app/shared/form/builder/form-builder.service.ts @@ -196,6 +196,7 @@ export class FormBuilderService extends DynamicFormService { return new FormFieldMetadataValueObject((controlValue as any).value, controlLanguage, authority, (controlValue as any).display, place, (controlValue as any).confidence); } } + return controlValue; }; const iterateControlModels = (findGroupModel: DynamicFormControlModel[], controlModelIndex: number = 0): void => { From ffdeca69f45ff4b0ce811f237a314fc672816777 Mon Sep 17 00:00:00 2001 From: Vlad Nouski Date: Fri, 20 Oct 2023 16:07:31 +0200 Subject: [PATCH 010/112] [CST-12043] feature: add primary bitstream switch --- .../workspaceitem-section-upload.model.ts | 5 +- .../custom-switch.component.html | 2 +- .../objects/submission-objects.actions.ts | 25 +++++ .../objects/submission-objects.reducer.ts | 47 ++++++++- .../section-upload-file-edit.component.html | 6 +- .../section-upload-file-edit.component.ts | 69 +++++++------ .../edit/section-upload-file-edit.model.ts | 13 +++ .../file/section-upload-file.component.html | 9 +- .../file/section-upload-file.component.ts | 97 ++++++++++++++++++- .../themed-section-upload-file.component.ts | 8 ++ .../upload/section-upload.component.html | 28 +++--- .../upload/section-upload.component.ts | 35 +++---- .../sections/upload/section-upload.service.ts | 36 ++++++- 13 files changed, 303 insertions(+), 77 deletions(-) diff --git a/src/app/core/submission/models/workspaceitem-section-upload.model.ts b/src/app/core/submission/models/workspaceitem-section-upload.model.ts index f98e0584eb..d992567df4 100644 --- a/src/app/core/submission/models/workspaceitem-section-upload.model.ts +++ b/src/app/core/submission/models/workspaceitem-section-upload.model.ts @@ -4,7 +4,10 @@ import { WorkspaceitemSectionUploadFileObject } from './workspaceitem-section-up * An interface to represent submission's upload section data. */ export interface WorkspaceitemSectionUploadObject { - + /** + * Primary bitstream flag + */ + primary: string | null; /** * A list of [[WorkspaceitemSectionUploadFileObject]] */ diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.html index ed117a5021..572c99ad3a 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.component.html @@ -13,7 +13,7 @@ (blur)="onBlur($event)" (change)="onChange($event)" (focus)="onFocus($event)"/> -