From a364af5c4c404192a146f28ebeeb6d8888ceaaf5 Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Fri, 31 May 2019 10:21:30 -0700 Subject: [PATCH] Fix #5765 Trusted state handling (#5775) - Notebooks Can be Improperly Trusted After Save + Reopen Fix was to only save if actually trusted. Also fixed condition where it wasn't correctly skipping if in the queue --- .../parts/notebook/notebookActions.ts | 4 ++-- .../notebook/common/notebookServiceImpl.ts | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/sql/workbench/parts/notebook/notebookActions.ts b/src/sql/workbench/parts/notebook/notebookActions.ts index 6558169b4f..a4e55a61dd 100644 --- a/src/sql/workbench/parts/notebook/notebookActions.ts +++ b/src/sql/workbench/parts/notebook/notebookActions.ts @@ -377,9 +377,9 @@ export class AttachToDropdown extends SelectBox { }).catch(err => this.logService.error(err)); } - model.onValidConnectionSelected(validConnection => { + this._register(model.onValidConnectionSelected(validConnection => { this.handleContextsChanged(!validConnection); - }); + })); } private getKernelDisplayName(): string { diff --git a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts index 4f5ddee24e..362bef6926 100644 --- a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts @@ -553,16 +553,21 @@ export class NotebookService extends Disposable implements INotebookService { } serializeNotebookStateChange(notebookUri: URI, changeType: SerializationStateChangeType): void { - let updateTrustState = changeType === SerializationStateChangeType.Saved; - if (notebookUri.scheme !== Schemas.untitled) { - // Cache state for non-untitled notebooks only. + // Conditions for saving: + // 1. Not untitled. They're always trusted as we open them + // 2. Serialization action was a save, since don't need to update on execution etc. + // 3. Not already saving (e.g. isn't in the queue to be cached) + // 4. Notebook is trusted. Don't need to save state of untrusted notebooks let notebookUriString = notebookUri.toString(); - if (updateTrustState && this._trustedCacheQueue.findIndex(uri => uri.toString() === notebookUriString)) { - this._trustedCacheQueue.push(notebookUri); - this._updateTrustCacheScheduler.schedule(); + if (changeType === SerializationStateChangeType.Saved && this._trustedCacheQueue.findIndex(uri => uri.toString() === notebookUriString) < 0) { + // Only save if it's trusted + let notebook = this.listNotebookEditors().find(n => n.id === notebookUriString); + if (notebook && notebook.model.trustedMode) { + this._trustedCacheQueue.push(notebookUri); + this._updateTrustCacheScheduler.schedule(); + } } - // TODO add history notification if a non-untitled notebook has a state change } }