Skip to content

Commit d00ee13

Browse files
alexdimaalexdima
committed
Add more validation
1 parent 9ae8d33 commit d00ee13

1 file changed

Lines changed: 75 additions & 26 deletions

File tree

‎src/vs/platform/undoRedo/common/undoRedoService.ts‎

Lines changed: 75 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,15 @@ class ResourceEditStack {
157157
private _past: StackElement[];
158158
private _future: StackElement[];
159159
public locked: boolean;
160+
public versionId: number;
160161

161162
constructor(resource: URI, strResource: string) {
162163
this.resource = resource;
163164
this.strResource = strResource;
164165
this._past = [];
165166
this._future = [];
166167
this.locked = false;
168+
this.versionId = 1;
167169
}
168170

169171
public dispose(): void {
@@ -177,11 +179,13 @@ class ResourceEditStack {
177179
element.removeResource(this.resource, this.strResource, RemovedResourceReason.ExternalRemoval);
178180
}
179181
}
182+
this.versionId++;
180183
}
181184

182185
public flushAllElements(): void {
183186
this._past = [];
184187
this._future = [];
188+
this.versionId++;
185189
}
186190

187191
public setElementsIsValid(isValid: boolean): void {
@@ -217,6 +221,7 @@ class ResourceEditStack {
217221
}
218222
}
219223
this._past.push(element);
224+
this.versionId++;
220225
}
221226

222227
public getElements(): IPastFutureElements {
@@ -268,6 +273,7 @@ class ResourceEditStack {
268273
break;
269274
}
270275
}
276+
this.versionId++;
271277
}
272278

273279
public splitFutureWorkspaceElement(toRemove: WorkspaceStackElement, individualMap: Map<string, ResourceStackElement>): void {
@@ -283,16 +289,42 @@ class ResourceEditStack {
283289
break;
284290
}
285291
}
292+
this.versionId++;
286293
}
287294

288295
public moveBackward(element: StackElement): void {
289296
this._past.pop();
290297
this._future.push(element);
298+
this.versionId++;
291299
}
292300

293301
public moveForward(element: StackElement): void {
294302
this._future.pop();
295303
this._past.push(element);
304+
this.versionId++;
305+
}
306+
}
307+
308+
class EditStackSnapshot {
309+
310+
public readonly editStacks: ResourceEditStack[];
311+
private readonly _versionIds: number[];
312+
313+
constructor(editStacks: ResourceEditStack[]) {
314+
this.editStacks = editStacks;
315+
this._versionIds = [];
316+
for (let i = 0, len = this.editStacks.length; i < len; i++) {
317+
this._versionIds[i] = this.editStacks[i].versionId;
318+
}
319+
}
320+
321+
public isValid(): boolean {
322+
for (let i = 0, len = this.editStacks.length; i < len; i++) {
323+
if (this._versionIds[i] !== this.editStacks[i].versionId) {
324+
return false;
325+
}
326+
}
327+
return true;
296328
}
297329
}
298330

@@ -428,29 +460,29 @@ export class UndoRedoService implements IUndoRedoService {
428460
this._notificationService.error(err);
429461
}
430462

431-
private _acquireLocks(affectedEditStacks: ResourceEditStack[]): () => void {
463+
private _acquireLocks(editStackSnapshot: EditStackSnapshot): () => void {
432464
// first, check if all locks can be acquired
433-
for (const editStack of affectedEditStacks) {
465+
for (const editStack of editStackSnapshot.editStacks) {
434466
if (editStack.locked) {
435467
throw new Error('Cannot acquire edit stack lock');
436468
}
437469
}
438470

439471
// can acquire all locks
440-
for (const editStack of affectedEditStacks) {
472+
for (const editStack of editStackSnapshot.editStacks) {
441473
editStack.locked = true;
442474
}
443475

444476
return () => {
445477
// release all locks
446-
for (const editStack of affectedEditStacks) {
478+
for (const editStack of editStackSnapshot.editStacks) {
447479
editStack.locked = false;
448480
}
449481
};
450482
}
451483

452-
private _safeInvokeWithLocks(element: StackElement, invoke: () => Promise<void> | void, affectedEditStacks: ResourceEditStack[], cleanup: IDisposable = Disposable.None): Promise<void> | void {
453-
const releaseLocks = this._acquireLocks(affectedEditStacks);
484+
private _safeInvokeWithLocks(element: StackElement, invoke: () => Promise<void> | void, editStackSnapshot: EditStackSnapshot, cleanup: IDisposable = Disposable.None): Promise<void> | void {
485+
const releaseLocks = this._acquireLocks(editStackSnapshot);
454486

455487
let result: Promise<void> | void;
456488
try {
@@ -492,15 +524,15 @@ export class UndoRedoService implements IUndoRedoService {
492524
return result;
493525
}
494526

495-
private _getAffectedEditStacks(element: WorkspaceStackElement): ResourceEditStack[] {
527+
private _getAffectedEditStacks(element: WorkspaceStackElement): EditStackSnapshot {
496528
const affectedEditStacks: ResourceEditStack[] = [];
497529
for (const strResource of element.strResources) {
498530
affectedEditStacks.push(this._editStacks.get(strResource)!);
499531
}
500-
return affectedEditStacks;
532+
return new EditStackSnapshot(affectedEditStacks);
501533
}
502534

503-
private _checkWorkspaceUndo(resource: URI, element: WorkspaceStackElement, affectedEditStacks: ResourceEditStack[], checkInvalidatedResources: boolean): WorkspaceVerificationError | null {
535+
private _checkWorkspaceUndo(resource: URI, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot, checkInvalidatedResources: boolean): WorkspaceVerificationError | null {
504536
if (element.removedResources) {
505537
this._splitPastWorkspaceElement(element, element.removedResources);
506538
const message = nls.localize('cannotWorkspaceUndo', "Could not undo '{0}' across all files. {1}", element.label, element.removedResources.createMessage());
@@ -516,7 +548,7 @@ export class UndoRedoService implements IUndoRedoService {
516548

517549
// this must be the last past element in all the impacted resources!
518550
const cannotUndoDueToResources: URI[] = [];
519-
for (const editStack of affectedEditStacks) {
551+
for (const editStack of editStackSnapshot.editStacks) {
520552
if (editStack.getClosestPastElement() !== element) {
521553
cannotUndoDueToResources.push(editStack.resource);
522554
}
@@ -530,7 +562,7 @@ export class UndoRedoService implements IUndoRedoService {
530562
}
531563

532564
const cannotLockDueToResources: URI[] = [];
533-
for (const editStack of affectedEditStacks) {
565+
for (const editStack of editStackSnapshot.editStacks) {
534566
if (editStack.locked) {
535567
cannotLockDueToResources.push(editStack.resource);
536568
}
@@ -543,6 +575,14 @@ export class UndoRedoService implements IUndoRedoService {
543575
return new WorkspaceVerificationError(this.undo(resource));
544576
}
545577

578+
// check if new stack elements were added in the meantime...
579+
if (!editStackSnapshot.isValid()) {
580+
this._splitPastWorkspaceElement(element, null);
581+
const message = nls.localize('cannotWorkspaceUndoDueToInMeantimeUndoRedo', "Could not undo '{0}' across all files because an undo or redo operation occurred in the meantime", element.label);
582+
this._notificationService.info(message);
583+
return new WorkspaceVerificationError(this.undo(resource));
584+
}
585+
546586
return null;
547587
}
548588

@@ -555,12 +595,13 @@ export class UndoRedoService implements IUndoRedoService {
555595
return this._confirmAndExecuteWorkspaceUndo(resource, element, affectedEditStacks);
556596
}
557597

558-
private async _confirmAndExecuteWorkspaceUndo(resource: URI, element: WorkspaceStackElement, affectedEditStacks: ResourceEditStack[]): Promise<void> {
598+
private async _confirmAndExecuteWorkspaceUndo(resource: URI, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot): Promise<void> {
599+
559600
const result = await this._dialogService.show(
560601
Severity.Info,
561602
nls.localize('confirmWorkspace', "Would you like to undo '{0}' across all files?", element.label),
562603
[
563-
nls.localize('ok', "Undo in {0} Files", affectedEditStacks.length),
604+
nls.localize('ok', "Undo in {0} Files", editStackSnapshot.editStacks.length),
564605
nls.localize('nok', "Undo this File"),
565606
nls.localize('cancel', "Cancel"),
566607
],
@@ -583,7 +624,7 @@ export class UndoRedoService implements IUndoRedoService {
583624
// choice: undo in all files
584625

585626
// At this point, it is possible that the element has been made invalid in the meantime (due to the confirmation await)
586-
const verificationError1 = this._checkWorkspaceUndo(resource, element, affectedEditStacks, /*invalidated resources will be checked after the prepare call*/false);
627+
const verificationError1 = this._checkWorkspaceUndo(resource, element, editStackSnapshot, /*invalidated resources will be checked after the prepare call*/false);
587628
if (verificationError1) {
588629
return verificationError1.returnValue;
589630
}
@@ -597,16 +638,16 @@ export class UndoRedoService implements IUndoRedoService {
597638
}
598639

599640
// At this point, it is possible that the element has been made invalid in the meantime (due to the prepare await)
600-
const verificationError2 = this._checkWorkspaceUndo(resource, element, affectedEditStacks, /*now also check that there are no more invalidated resources*/true);
641+
const verificationError2 = this._checkWorkspaceUndo(resource, element, editStackSnapshot, /*now also check that there are no more invalidated resources*/true);
601642
if (verificationError2) {
602643
cleanup.dispose();
603644
return verificationError2.returnValue;
604645
}
605646

606-
for (const editStack of affectedEditStacks) {
647+
for (const editStack of editStackSnapshot.editStacks) {
607648
editStack.moveBackward(element);
608649
}
609-
return this._safeInvokeWithLocks(element, () => element.actual.undo(), affectedEditStacks, cleanup);
650+
return this._safeInvokeWithLocks(element, () => element.actual.undo(), editStackSnapshot, cleanup);
610651
}
611652

612653
private _resourceUndo(editStack: ResourceEditStack, element: ResourceStackElement): Promise<void> | void {
@@ -621,7 +662,7 @@ export class UndoRedoService implements IUndoRedoService {
621662
return;
622663
}
623664
editStack.moveBackward(element);
624-
return this._safeInvokeWithLocks(element, () => element.actual.undo(), [editStack]);
665+
return this._safeInvokeWithLocks(element, () => element.actual.undo(), new EditStackSnapshot([editStack]));
625666
}
626667

627668
public undo(resource: URI): Promise<void> | void {
@@ -652,7 +693,7 @@ export class UndoRedoService implements IUndoRedoService {
652693
return false;
653694
}
654695

655-
private _checkWorkspaceRedo(resource: URI, element: WorkspaceStackElement, affectedEditStacks: ResourceEditStack[], checkInvalidatedResources: boolean): WorkspaceVerificationError | null {
696+
private _checkWorkspaceRedo(resource: URI, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot, checkInvalidatedResources: boolean): WorkspaceVerificationError | null {
656697
if (element.removedResources) {
657698
this._splitFutureWorkspaceElement(element, element.removedResources);
658699
const message = nls.localize('cannotWorkspaceRedo', "Could not redo '{0}' across all files. {1}", element.label, element.removedResources.createMessage());
@@ -668,7 +709,7 @@ export class UndoRedoService implements IUndoRedoService {
668709

669710
// this must be the last future element in all the impacted resources!
670711
const cannotRedoDueToResources: URI[] = [];
671-
for (const editStack of affectedEditStacks) {
712+
for (const editStack of editStackSnapshot.editStacks) {
672713
if (editStack.getClosestFutureElement() !== element) {
673714
cannotRedoDueToResources.push(editStack.resource);
674715
}
@@ -682,7 +723,7 @@ export class UndoRedoService implements IUndoRedoService {
682723
}
683724

684725
const cannotLockDueToResources: URI[] = [];
685-
for (const editStack of affectedEditStacks) {
726+
for (const editStack of editStackSnapshot.editStacks) {
686727
if (editStack.locked) {
687728
cannotLockDueToResources.push(editStack.resource);
688729
}
@@ -695,6 +736,14 @@ export class UndoRedoService implements IUndoRedoService {
695736
return new WorkspaceVerificationError(this.redo(resource));
696737
}
697738

739+
// check if new stack elements were added in the meantime...
740+
if (!editStackSnapshot.isValid()) {
741+
this._splitPastWorkspaceElement(element, null);
742+
const message = nls.localize('cannotWorkspaceRedoDueToInMeantimeUndoRedo', "Could not redo '{0}' across all files because an undo or redo operation occurred in the meantime", element.label);
743+
this._notificationService.info(message);
744+
return new WorkspaceVerificationError(this.undo(resource));
745+
}
746+
698747
return null;
699748
}
700749

@@ -707,7 +756,7 @@ export class UndoRedoService implements IUndoRedoService {
707756
return this._executeWorkspaceRedo(resource, element, affectedEditStacks);
708757
}
709758

710-
private async _executeWorkspaceRedo(resource: URI, element: WorkspaceStackElement, affectedEditStacks: ResourceEditStack[]): Promise<void> {
759+
private async _executeWorkspaceRedo(resource: URI, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot): Promise<void> {
711760
// prepare
712761
let cleanup: IDisposable;
713762
try {
@@ -717,16 +766,16 @@ export class UndoRedoService implements IUndoRedoService {
717766
}
718767

719768
// At this point, it is possible that the element has been made invalid in the meantime (due to the prepare await)
720-
const verificationError = this._checkWorkspaceRedo(resource, element, affectedEditStacks, /*now also check that there are no more invalidated resources*/true);
769+
const verificationError = this._checkWorkspaceRedo(resource, element, editStackSnapshot, /*now also check that there are no more invalidated resources*/true);
721770
if (verificationError) {
722771
cleanup.dispose();
723772
return verificationError.returnValue;
724773
}
725774

726-
for (const editStack of affectedEditStacks) {
775+
for (const editStack of editStackSnapshot.editStacks) {
727776
editStack.moveForward(element);
728777
}
729-
return this._safeInvokeWithLocks(element, () => element.actual.redo(), affectedEditStacks, cleanup);
778+
return this._safeInvokeWithLocks(element, () => element.actual.redo(), editStackSnapshot, cleanup);
730779
}
731780

732781
private _resourceRedo(editStack: ResourceEditStack, element: ResourceStackElement): Promise<void> | void {
@@ -741,7 +790,7 @@ export class UndoRedoService implements IUndoRedoService {
741790
return;
742791
}
743792
editStack.moveForward(element);
744-
return this._safeInvokeWithLocks(element, () => element.actual.redo(), [editStack]);
793+
return this._safeInvokeWithLocks(element, () => element.actual.redo(), new EditStackSnapshot([editStack]));
745794
}
746795

747796
public redo(resource: URI): Promise<void> | void {

0 commit comments

Comments
 (0)