Skip to content

Commit fd5b39b

Browse files
committed
Submission save+sub stability / performance
* Debug console output added for testing * Debouncer added to save operations * More thorough sub management / tracking
1 parent 6a49cdb commit fd5b39b

File tree

3 files changed

+176
-17
lines changed

3 files changed

+176
-17
lines changed

src/app/submission/objects/submission-objects.actions.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ export class SaveSubmissionFormAction implements Action {
433433
payload: {
434434
submissionId: string;
435435
isManual?: boolean;
436+
timestamp: number;
436437
};
437438

438439
/**
@@ -442,7 +443,8 @@ export class SaveSubmissionFormAction implements Action {
442443
* the submission's ID
443444
*/
444445
constructor(submissionId: string, isManual: boolean = false) {
445-
this.payload = { submissionId, isManual };
446+
this.payload = { submissionId, isManual, timestamp: Date.now() };
447+
console.log(`Creating SaveSubmissionFormAction for submission ${submissionId} at ${new Date().toISOString()}`);
446448
}
447449
}
448450

src/app/submission/sections/form/section-form.component.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,9 @@ export class SubmissionSectionFormComponent extends SectionModelComponent {
402402
* Initialize all subscriptions
403403
*/
404404
subscriptions(): void {
405+
// clear existing subscriptions first
406+
this.onSectionDestroy();
407+
this.subs = [];
405408
this.subs.push(
406409
/**
407410
* Subscribe to form's data
@@ -426,6 +429,7 @@ export class SubmissionSectionFormComponent extends SectionModelComponent {
426429
this.updateForm(sectionState);
427430
}),
428431
);
432+
console.log(`Section ${this.sectionData.id} has ${this.subs.length} active subscriptions`);
429433
}
430434

431435
/**

src/app/submission/submission.service.ts

Lines changed: 169 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,20 @@ import {
99
} from '@ngrx/store';
1010
import { TranslateService } from '@ngx-translate/core';
1111
import {
12-
Observable,
13-
of as observableOf,
14-
Subscription,
12+
EMPTY,
13+
Observable, of,
14+
of as observableOf, ReplaySubject, share, Subject,
15+
Subscription, timeout, TimeoutError,
1516
timer as observableTimer,
1617
} from 'rxjs';
1718
import {
1819
catchError,
19-
concatMap,
20+
concatMap, debounceTime,
2021
distinctUntilChanged,
21-
filter,
22+
filter, finalize,
2223
find,
2324
map,
24-
startWith,
25+
startWith, switchMap,
2526
take,
2627
tap,
2728
} from 'rxjs/operators';
@@ -102,13 +103,20 @@ export class SubmissionService {
102103
*/
103104
protected autoSaveSub: Subscription;
104105

106+
private _currentSaveSubmissionId;
107+
private _saveDebouncer = new Subject<{id: string, manual: boolean}>();
108+
private activeStateSubscriptions: Map<string, Subscription[]> = new Map();
109+
105110
/**
106111
* Observable used as timer
107112
*/
108113
protected timer$: Observable<any>;
109114

110115
private workspaceLinkPath = 'workspaceitems';
111116
private workflowLinkPath = 'workflowitems';
117+
118+
// testing sub stuff
119+
private submissionStateSubjects: Map<string, ReplaySubject<SubmissionObjectEntry>>;
112120
/**
113121
* Initialize service variables
114122
* @param {NotificationsService} notificationsService
@@ -130,8 +138,48 @@ export class SubmissionService {
130138
protected searchService: SearchService,
131139
protected requestService: RequestService,
132140
protected jsonPatchOperationService: SubmissionJsonPatchOperationsService) {
141+
this._saveDebouncer.pipe(
142+
debounceTime(300),
143+
//distinctUntilChanged((prev, curr) => prev.id === curr.id),
144+
).subscribe(({ id, manual }) => {
145+
this._dispatchSaveImpl(id, manual);
146+
});
147+
}
148+
149+
// testing sub stuff
150+
// Add this method to manage subscriptions better
151+
private getOrCreateStateSubscription(submissionId: string): Observable<SubmissionObjectEntry> {
152+
// Check if we already have an active subject for this submission
153+
if (!this.submissionStateSubjects) {
154+
this.submissionStateSubjects = new Map();
155+
}
156+
157+
if (!this.submissionStateSubjects.has(submissionId)) {
158+
// Create a new ReplaySubject to cache the latest state
159+
const subject = new ReplaySubject<SubmissionObjectEntry>(1);
160+
161+
// Set up a single subscription to the store that feeds our subject
162+
const subscription = this.store.select(submissionObjectFromIdSelector(submissionId))
163+
.pipe(
164+
filter((submission: SubmissionObjectEntry) => isNotUndefined(submission))
165+
)
166+
.subscribe(subject);
167+
168+
// Track this subscription so we can clean it up later
169+
if (!this.activeStateSubscriptions.has(submissionId)) {
170+
this.activeStateSubscriptions.set(submissionId, []);
171+
}
172+
this.activeStateSubscriptions.get(submissionId).push(subscription);
173+
174+
// Store our subject
175+
this.submissionStateSubjects.set(submissionId, subject);
176+
}
177+
178+
// Return the subject as an observable
179+
return this.submissionStateSubjects.get(submissionId).asObservable();
133180
}
134181

182+
135183
/**
136184
* Dispatch a new [ChangeSubmissionCollectionAction]
137185
*
@@ -269,11 +317,60 @@ export class SubmissionService {
269317
* whether is a manual save, default false
270318
*/
271319
dispatchSave(submissionId, manual?: boolean) {
272-
this.getSubmissionSaveProcessingStatus(submissionId).pipe(
273-
find((isPending: boolean) => !isPending),
274-
).subscribe(() => {
275-
this.store.dispatch(new SaveSubmissionFormAction(submissionId, manual));
276-
});
320+
console.log('qeueing save for', submissionId);
321+
this._saveDebouncer.next({id: submissionId, manual: manual});
322+
}
323+
_dispatchSaveImpl(submissionId, manual?: boolean) {
324+
console.log('Dispatching save for', submissionId);
325+
const saveInProgress = this._currentSaveSubmissionId === submissionId;
326+
if (saveInProgress) {
327+
console.log(`Save already in progress, ignoring save for ${submissionId}`);
328+
return;
329+
}
330+
331+
this._currentSaveSubmissionId = submissionId;
332+
333+
// Check the current save processing status first
334+
this.getSubmissionObject(submissionId).pipe(
335+
take(1),
336+
map((state: SubmissionObjectEntry) => state.savePending),
337+
switchMap((isPending: boolean) => {
338+
if (!isPending) {
339+
// If not pending, proceed immediately
340+
return of(false);
341+
} else {
342+
// Otherwise wait for non-pending state
343+
return this.getSubmissionSaveProcessingStatus(submissionId).pipe(
344+
find((pendingState: boolean) => !pendingState),
345+
timeout(10000)
346+
);
347+
}
348+
}),
349+
finalize(() => {
350+
console.log(`Clearing save in progress flag for ${submissionId}`);
351+
this._currentSaveSubmissionId = null;
352+
})
353+
).subscribe({
354+
next: () => {
355+
console.log('Proceeding with save dispatch');
356+
this.store.dispatch(new SaveSubmissionFormAction(submissionId, manual));
357+
},
358+
error: error => {
359+
console.error('Error waiting for non-pending state', error);
360+
// Error handling logic - potentially still dispatch the save if it was a timeout
361+
if (error instanceof TimeoutError) {
362+
console.log('Timeout occurred, forcing save dispatch');
363+
this.store.dispatch(new SaveSubmissionFormAction(submissionId, manual));
364+
}
365+
}
366+
})
367+
// ).subscribe(() => {
368+
// console.log('Proceeding with save dispatch');
369+
// this.store.dispatch(new SaveSubmissionFormAction(submissionId, manual));
370+
// }, error => {
371+
// console.error('Error waiting for non-pending state', error);
372+
// this._currentSaveSubmissionId = null;
373+
// });
277374
}
278375

279376
/**
@@ -319,9 +416,18 @@ export class SubmissionService {
319416
* @return Observable<SubmissionObjectEntry>
320417
* observable of SubmissionObjectEntry
321418
*/
419+
// getSubmissionObject(submissionId: string): Observable<SubmissionObjectEntry> {
420+
// console.log(`Getting submission object for submission ${submissionId}, active subs:`, this.activeStateSubscriptions.get(submissionId)?.length || 0);
421+
// return this.store.select(submissionObjectFromIdSelector(submissionId)).pipe(
422+
// filter((submission: SubmissionObjectEntry) => isNotUndefined(submission)),
423+
// share()
424+
// );
425+
// }
426+
//
322427
getSubmissionObject(submissionId: string): Observable<SubmissionObjectEntry> {
323-
return this.store.select(submissionObjectFromIdSelector(submissionId)).pipe(
324-
filter((submission: SubmissionObjectEntry) => isNotUndefined(submission)));
428+
console.log(`Getting submission object for submission ${submissionId}, active subs:`,
429+
this.activeStateSubscriptions.get(submissionId)?.length || 0);
430+
return this.getOrCreateStateSubscription(submissionId);
325431
}
326432

327433
/**
@@ -464,13 +570,26 @@ export class SubmissionService {
464570
* @return Observable<boolean>
465571
* observable with submission save processing status
466572
*/
573+
// getSubmissionSaveProcessingStatus(submissionId: string): Observable<boolean> {
574+
// const requestId = Date.now(); // unique req id
575+
// console.log(`${requestId}: checking save status for ${submissionId}`);
576+
// return this.getSubmissionObject(submissionId).pipe(
577+
// tap((submission) => {
578+
// console.log('Current submission state:', submission.savePending ? 'PENDING' : 'NOT PENDING');
579+
// }),
580+
// map((state: SubmissionObjectEntry) => state.savePending),
581+
// distinctUntilChanged(),
582+
// tap(isPending => console.log(`Save pending status changed to: ${isPending ? 'PENDING' : 'NOT PENDING'}`)),
583+
// startWith(false));
584+
// }
467585
getSubmissionSaveProcessingStatus(submissionId: string): Observable<boolean> {
468586
return this.getSubmissionObject(submissionId).pipe(
469587
map((state: SubmissionObjectEntry) => state.savePending),
470588
distinctUntilChanged(),
471-
startWith(false));
589+
// Only log when the status actually changes
590+
tap(isPending => console.log(`Save pending status: ${isPending ? 'PENDING' : 'NOT PENDING'}`))
591+
);
472592
}
473-
474593
/**
475594
* Return the deposit processing status of the submission
476595
*
@@ -568,13 +687,47 @@ export class SubmissionService {
568687
).subscribe();
569688
}
570689

690+
691+
692+
693+
// clearSubmissionSubscriptions(submissionId: string) {
694+
// const subs = this.activeStateSubscriptions.get(submissionId) || [];
695+
// console.log(`Clearing ${subs.length} subscriptions for submission ${submissionId}`);
696+
//
697+
// subs.forEach(sub => {
698+
// if (!sub.closed) {
699+
// sub.unsubscribe();
700+
// }
701+
// });
702+
//
703+
// this.activeStateSubscriptions.set(submissionId, []);
704+
// }
705+
clearSubmissionSubscriptions(submissionId: string) {
706+
const subs = this.activeStateSubscriptions.get(submissionId) || [];
707+
console.log(`Clearing ${subs.length} subscriptions for submission ${submissionId}`);
708+
709+
subs.forEach(sub => {
710+
if (!sub.closed) {
711+
sub.unsubscribe();
712+
}
713+
});
714+
715+
this.activeStateSubscriptions.set(submissionId, []);
716+
717+
// Also clear any state subjects
718+
if (this.submissionStateSubjects && this.submissionStateSubjects.has(submissionId)) {
719+
this.submissionStateSubjects.delete(submissionId);
720+
}
721+
}
571722
/**
572723
* Dispatch a new [CancelSubmissionFormAction]
573724
*/
574725
resetAllSubmissionObjects() {
726+
Array.from(this.activeStateSubscriptions.keys()).forEach(submissionId => {
727+
this.clearSubmissionSubscriptions(submissionId);
728+
});
575729
this.store.dispatch(new CancelSubmissionFormAction());
576730
}
577-
578731
/**
579732
* Dispatch a new [ResetSubmissionFormAction]
580733
*

0 commit comments

Comments
 (0)