Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental notebook save delegate on remote extension host #186123

Merged
merged 2 commits into from
Jun 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/vs/workbench/api/browser/mainThreadNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,15 @@ export class MainThreadNotebooks implements MainThreadNotebookShape {
extensionId: extension.id.value,
});
return result;
}
},
save: async (uri, versionId, options, token) => {
const stat = await this._proxy.$saveNotebook(handle, uri, versionId, options, token);
return {
...stat,
children: undefined,
resource: uri
};
},
}));

if (data) {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/common/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
const extHostDocuments = rpcProtocol.set(ExtHostContext.ExtHostDocuments, new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors));
const extHostDocumentContentProviders = rpcProtocol.set(ExtHostContext.ExtHostDocumentContentProviders, new ExtHostDocumentContentProvider(rpcProtocol, extHostDocumentsAndEditors, extHostLogService));
const extHostDocumentSaveParticipant = rpcProtocol.set(ExtHostContext.ExtHostDocumentSaveParticipant, new ExtHostDocumentSaveParticipant(extHostLogService, extHostDocuments, rpcProtocol.getProxy(MainContext.MainThreadBulkEdits)));
const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments));
const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem));
const extHostNotebookDocuments = rpcProtocol.set(ExtHostContext.ExtHostNotebookDocuments, new ExtHostNotebookDocuments(extHostNotebook));
const extHostNotebookEditors = rpcProtocol.set(ExtHostContext.ExtHostNotebookEditors, new ExtHostNotebookEditors(extHostLogService, extHostNotebook));
const extHostNotebookKernels = rpcProtocol.set(ExtHostContext.ExtHostNotebookKernels, new ExtHostNotebookKernels(rpcProtocol, initData, extHostNotebook, extHostCommands, extHostLogService));
Expand Down
3 changes: 3 additions & 0 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2274,12 +2274,15 @@ export interface NotebookCellDto {
internalMetadata?: notebookCommon.NotebookCellInternalMetadata;
}

export type INotebookPartialFileStatsWithMetadata = Omit<files.IFileStatWithMetadata, 'resource' | 'children'>;

export interface ExtHostNotebookShape extends ExtHostNotebookDocumentsAndEditorsShape {
$provideNotebookCellStatusBarItems(handle: number, uri: UriComponents, index: number, token: CancellationToken): Promise<INotebookCellStatusBarListDto | undefined>;
$releaseNotebookCellStatusBarItems(id: number): void;

$dataToNotebook(handle: number, data: VSBuffer, token: CancellationToken): Promise<SerializableObjectWithBuffers<NotebookDataDto>>;
$notebookToData(handle: number, data: SerializableObjectWithBuffers<NotebookDataDto>, token: CancellationToken): Promise<VSBuffer>;
$saveNotebook(handle: number, uri: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata>;
}

export interface ExtHostNotebookDocumentSaveParticipantShape {
Expand Down
72 changes: 67 additions & 5 deletions src/vs/workbench/api/common/extHostNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import { isFalsyOrWhitespace } from 'vs/base/common/strings';
import { assertIsDefined } from 'vs/base/common/types';
import { URI, UriComponents } from 'vs/base/common/uri';
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
import * as files from 'vs/platform/files/common/files';
import { Cache } from 'vs/workbench/api/common/cache';
import { ExtHostNotebookShape, IMainContext, IModelAddedData, INotebookCellStatusBarListDto, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, MainContext, MainThreadNotebookDocumentsShape, MainThreadNotebookEditorsShape, MainThreadNotebookShape, NotebookDataDto } from 'vs/workbench/api/common/extHost.protocol';
import { ExtHostNotebookShape, IMainContext, IModelAddedData, INotebookCellStatusBarListDto, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorAddData, INotebookPartialFileStatsWithMetadata, MainContext, MainThreadNotebookDocumentsShape, MainThreadNotebookEditorsShape, MainThreadNotebookShape, NotebookDataDto } from 'vs/workbench/api/common/extHost.protocol';
import { ApiCommand, ApiCommandArgument, ApiCommandResult, CommandsConverter, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands';
import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments';
import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors';
Expand All @@ -27,6 +28,9 @@ import type * as vscode from 'vscode';
import { ExtHostCell, ExtHostNotebookDocument } from './extHostNotebookDocument';
import { ExtHostNotebookEditor } from './extHostNotebookEditor';
import { onUnexpectedExternalError } from 'vs/base/common/errors';
import { IExtHostConsumerFileSystem } from 'vs/workbench/api/common/extHostFileSystemConsumer';
import { basename } from 'vs/base/common/resources';
import { filter } from 'vs/base/common/objects';



Expand Down Expand Up @@ -69,6 +73,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
commands: ExtHostCommands,
private _textDocumentsAndEditors: ExtHostDocumentsAndEditors,
private _textDocuments: ExtHostDocuments,
private _extHostFileSystem: IExtHostConsumerFileSystem
) {
this._notebookProxy = mainContext.getProxy(MainContext.MainThreadNotebook);
this._notebookDocumentsProxy = mainContext.getProxy(MainContext.MainThreadNotebookDocuments);
Expand Down Expand Up @@ -263,14 +268,14 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
// --- serialize/deserialize

private _handlePool = 0;
private readonly _notebookSerializer = new Map<number, vscode.NotebookSerializer>();
private readonly _notebookSerializer = new Map<number, { viewType: string; serializer: vscode.NotebookSerializer; options: vscode.NotebookDocumentContentOptions | undefined }>();

registerNotebookSerializer(extension: IExtensionDescription, viewType: string, serializer: vscode.NotebookSerializer, options?: vscode.NotebookDocumentContentOptions, registration?: vscode.NotebookRegistrationData): vscode.Disposable {
if (isFalsyOrWhitespace(viewType)) {
throw new Error(`viewType cannot be empty or just whitespace`);
}
const handle = this._handlePool++;
this._notebookSerializer.set(handle, serializer);
this._notebookSerializer.set(handle, { viewType, serializer, options });
this._notebookProxy.$registerNotebookSerializer(
handle,
{ id: extension.identifier, location: extension.extensionLocation },
Expand All @@ -288,7 +293,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
if (!serializer) {
throw new Error('NO serializer found');
}
const data = await serializer.deserializeNotebook(bytes.buffer, token);
const data = await serializer.serializer.deserializeNotebook(bytes.buffer, token);
return new SerializableObjectWithBuffers(typeConverters.NotebookData.from(data));
}

Expand All @@ -297,10 +302,67 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
if (!serializer) {
throw new Error('NO serializer found');
}
const bytes = await serializer.serializeNotebook(typeConverters.NotebookData.to(data.value), token);
const bytes = await serializer.serializer.serializeNotebook(typeConverters.NotebookData.to(data.value), token);
return VSBuffer.wrap(bytes);
}

async $saveNotebook(handle: number, uriComponents: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata> {
const uri = URI.revive(uriComponents);
const serializer = this._notebookSerializer.get(handle);
if (!serializer) {
throw new Error('NO serializer found');
}

const document = this._documents.get(uri);
if (!document) {
throw new Error('Document NOT found');
}

if (document.versionId !== versionId) {
throw new Error('Document version mismatch');
}

const data: vscode.NotebookData = {
metadata: filter(document.apiNotebook.metadata, key => !(serializer.options?.transientDocumentMetadata ?? {})[key]),
cells: [],
};

for (const cell of document.apiNotebook.getCells()) {
const cellData = new extHostTypes.NotebookCellData(
cell.kind,
cell.document.getText(),
cell.document.languageId,
cell.mime,
!(serializer.options?.transientOutputs) ? [...cell.outputs] : [],
cell.metadata,
cell.executionSummary
);

cellData.metadata = filter(cell.metadata, key => !(serializer.options?.transientCellMetadata ?? {})[key]);
data.cells.push(cellData);
}

const bytes = await serializer.serializer.serializeNotebook(data, token);
await this._extHostFileSystem.value.writeFile(uri, bytes);
const stat = await this._extHostFileSystem.value.stat(uri);

const fileStats = {
name: basename(uri), // providerExtUri.basename(resource)
isFile: (stat.type & files.FileType.File) !== 0,
isDirectory: (stat.type & files.FileType.Directory) !== 0,
isSymbolicLink: (stat.type & files.FileType.SymbolicLink) !== 0,
mtime: stat.mtime,
ctime: stat.ctime,
size: stat.size,
readonly: Boolean((stat.permissions ?? 0) & files.FilePermission.Readonly) || !this._extHostFileSystem.value.isWritableFileSystem(uri.scheme),
locked: Boolean((stat.permissions ?? 0) & files.FilePermission.Locked),
etag: files.etag({ mtime: stat.mtime, size: stat.size }),
children: undefined
};

return fileStats;
}

// --- open, save, saveAs, backup


Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/api/common/extHostNotebookDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ export class ExtHostNotebookDocument {
this._disposed = true;
}

get versionId(): number {
return this._versionId;
}

get apiNotebook(): vscode.NotebookDocument {
if (!this._notebook) {
const that = this;
Expand Down
6 changes: 5 additions & 1 deletion src/vs/workbench/api/test/browser/extHostNotebook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { ExtHostNotebookDocuments } from 'vs/workbench/api/common/extHostNoteboo
import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier';
import { VSBuffer } from 'vs/base/common/buffer';
import { IExtHostTelemetry } from 'vs/workbench/api/common/extHostTelemetry';
import { ExtHostConsumerFileSystem } from 'vs/workbench/api/common/extHostFileSystemConsumer';
import { ExtHostFileSystemInfo } from 'vs/workbench/api/common/extHostFileSystemInfo';

suite('NotebookCell#Document', function () {

Expand All @@ -34,6 +36,7 @@ suite('NotebookCell#Document', function () {
let extHostDocuments: ExtHostDocuments;
let extHostNotebooks: ExtHostNotebookController;
let extHostNotebookDocuments: ExtHostNotebookDocuments;
let extHostConsumerFileSystem: ExtHostConsumerFileSystem;

const notebookUri = URI.parse('test:///notebook.file');
const disposables = new DisposableStore();
Expand All @@ -53,11 +56,12 @@ suite('NotebookCell#Document', function () {
});
extHostDocumentsAndEditors = new ExtHostDocumentsAndEditors(rpcProtocol, new NullLogService());
extHostDocuments = new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors);
extHostConsumerFileSystem = new ExtHostConsumerFileSystem(rpcProtocol, new ExtHostFileSystemInfo());
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, new ExtHostCommands(rpcProtocol, new NullLogService(), new class extends mock<IExtHostTelemetry>() {
override onExtensionError(): boolean {
return true;
}
}), extHostDocumentsAndEditors, extHostDocuments);
}), extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem);
extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks);

const reg = extHostNotebooks.registerNotebookSerializer(nullExtensionDescription, 'test', new class extends mock<vscode.NotebookSerializer>() { });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/
import { TestRPCProtocol } from 'vs/workbench/api/test/common/testRPCProtocol';
import { mock } from 'vs/workbench/test/common/workbenchTestServices';
import { IExtHostTelemetry } from 'vs/workbench/api/common/extHostTelemetry';
import { ExtHostConsumerFileSystem } from 'vs/workbench/api/common/extHostFileSystemConsumer';
import { ExtHostFileSystemInfo } from 'vs/workbench/api/common/extHostFileSystemInfo';

suite('NotebookKernel', function () {

Expand All @@ -37,6 +39,7 @@ suite('NotebookKernel', function () {
let extHostNotebooks: ExtHostNotebookController;
let extHostNotebookDocuments: ExtHostNotebookDocuments;
let extHostCommands: ExtHostCommands;
let extHostConsumerFileSystem: ExtHostConsumerFileSystem;

const notebookUri = URI.parse('test:///notebook.file');
const kernelData = new Map<number, INotebookKernelDto2>();
Expand Down Expand Up @@ -94,7 +97,8 @@ suite('NotebookKernel', function () {
return true;
}
});
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments);
extHostConsumerFileSystem = new ExtHostConsumerFileSystem(rpcProtocol, new ExtHostFileSystemInfo());
extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem);

extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks);

Expand Down
23 changes: 21 additions & 2 deletions src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { Schemas } from 'vs/base/common/network';
import { filter } from 'vs/base/common/objects';
import { assertType } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IWriteFileOptions, IFileStatWithMetadata } from 'vs/platform/files/common/files';
import { IRevertOptions, ISaveOptions, IUntypedEditorInput } from 'vs/workbench/common/editor';
import { EditorModel } from 'vs/workbench/common/editor/editorModel';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
Expand Down Expand Up @@ -175,10 +177,12 @@ export class NotebookFileWorkingCopyModel extends Disposable implements IStoredF
readonly onWillDispose: Event<void>;

readonly configuration: IFileWorkingCopyModelConfiguration | undefined = undefined;
save: ((options: IWriteFileOptions, token: CancellationToken) => Promise<IFileStatWithMetadata>) | undefined;

constructor(
private readonly _notebookModel: NotebookTextModel,
private readonly _notebookService: INotebookService
private readonly _notebookService: INotebookService,
private readonly _configurationService: IConfigurationService
) {
super();

Expand Down Expand Up @@ -209,6 +213,20 @@ export class NotebookFileWorkingCopyModel extends Disposable implements IStoredF
// remote hosts the extension of the notebook with the contents truth
backupDelay: 10000
};

// Override save behavior to avoid transferring the buffer across the wire 3 times
if (this._configurationService.getValue('notebook.experimental.remoteSave')) {
this.save = async (options: IWriteFileOptions, token: CancellationToken) => {
const serializer = await this.getNotebookSerializer();

if (token.isCancellationRequested) {
throw new CancellationError();
}

const stat = await serializer.save(this._notebookModel.uri, this._notebookModel.versionId, options, token);
return stat;
};
}
}
}

Expand Down Expand Up @@ -287,6 +305,7 @@ export class NotebookFileWorkingCopyModelFactory implements IStoredFileWorkingCo
constructor(
private readonly _viewType: string,
@INotebookService private readonly _notebookService: INotebookService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
) { }

async createModel(resource: URI, stream: VSBufferReadableStream, token: CancellationToken): Promise<NotebookFileWorkingCopyModel> {
Expand All @@ -304,7 +323,7 @@ export class NotebookFileWorkingCopyModelFactory implements IStoredFileWorkingCo
}

const notebookModel = this._notebookService.createNotebookTextModel(info.viewType, resource, data, info.serializer.options);
return new NotebookFileWorkingCopyModel(notebookModel, this._notebookService);
return new NotebookFileWorkingCopyModel(notebookModel, this._notebookService, this._configurationService);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Schemas } from 'vs/base/common/network';
import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
import { assertIsDefined } from 'vs/base/common/types';
import { CancellationToken } from 'vs/base/common/cancellation';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';

class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IResolvedNotebookEditorModel>> {

Expand All @@ -40,6 +41,7 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@INotebookService private readonly _notebookService: INotebookService,
@ILogService private readonly _logService: ILogService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
) {
super();
}
Expand All @@ -65,7 +67,7 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso
const workingCopyTypeId = NotebookWorkingCopyTypeIdentifier.create(viewType);
let workingCopyManager = this._workingCopyManagers.get(workingCopyTypeId);
if (!workingCopyManager) {
const factory = new NotebookFileWorkingCopyModelFactory(viewType, this._notebookService);
const factory = new NotebookFileWorkingCopyModelFactory(viewType, this._notebookService, this._configurationService);
workingCopyManager = <IFileWorkingCopyManager<NotebookFileWorkingCopyModel, NotebookFileWorkingCopyModel>><any>this._instantiationService.createInstance(
FileWorkingCopyManager,
workingCopyTypeId,
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/contrib/notebook/common/notebookService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/mode
import { IDisposable } from 'vs/base/common/lifecycle';
import { VSBuffer } from 'vs/base/common/buffer';
import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration';
import { IFileStatWithMetadata, IWriteFileOptions } from 'vs/platform/files/common/files';


export const INotebookService = createDecorator<INotebookService>('notebookService');
Expand All @@ -29,6 +30,7 @@ export interface INotebookSerializer {
options: TransientOptions;
dataToNotebook(data: VSBuffer): Promise<NotebookData>;
notebookToData(data: NotebookData): Promise<VSBuffer>;
save(uri: URI, versionId: number, options: IWriteFileOptions, token: CancellationToken): Promise<IFileStatWithMetadata>;
}

export interface INotebookRawData {
Expand Down