Split up NotebookProvider into separate providers for handling file serialization and cell execution. (#17176)

This commit is contained in:
Cory Rivera
2021-09-29 16:15:28 -07:00
committed by GitHub
parent dfc2635aa7
commit 14904bb671
51 changed files with 1426 additions and 971 deletions

View File

@@ -13,19 +13,21 @@ import { IMainContext } from 'vs/workbench/api/common/extHost.protocol';
import { ExtHostNotebook } from 'sql/workbench/api/common/extHostNotebook';
import { MainThreadNotebookShape } from 'sql/workbench/api/common/sqlExtHost.protocol';
import { INotebookManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes';
import { mssqlProviderName } from 'sql/platform/connection/common/constants';
import { IExecuteManagerDetails, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes';
suite('ExtHostNotebook Tests', () => {
let extHostNotebook: ExtHostNotebook;
let mockProxy: TypeMoq.Mock<MainThreadNotebookShape>;
let notebookUri: URI;
let notebookProviderMock: TypeMoq.Mock<NotebookProviderStub>;
let serializationProviderMock: TypeMoq.Mock<SerializationProviderStub>;
let executeProviderMock: TypeMoq.Mock<ExecuteProviderStub>;
setup(() => {
mockProxy = TypeMoq.Mock.ofInstance(<MainThreadNotebookShape>{
$registerNotebookProvider: (providerId, handle) => undefined,
$unregisterNotebookProvider: (handle) => undefined,
$registerSerializationProvider: (providerId, handle) => undefined,
$registerExecuteProvider: (providerId, handle) => undefined,
$unregisterSerializationProvider: (handle) => undefined,
$unregisterExecuteProvider: (handle) => undefined,
dispose: () => undefined
});
let mainContext = <IMainContext>{
@@ -33,71 +35,115 @@ suite('ExtHostNotebook Tests', () => {
};
extHostNotebook = new ExtHostNotebook(mainContext);
notebookUri = URI.parse('file:/user/default/my.ipynb');
notebookProviderMock = TypeMoq.Mock.ofType(NotebookProviderStub, TypeMoq.MockBehavior.Loose);
notebookProviderMock.callBase = true;
serializationProviderMock = TypeMoq.Mock.ofType(SerializationProviderStub, TypeMoq.MockBehavior.Loose);
serializationProviderMock.callBase = true;
executeProviderMock = TypeMoq.Mock.ofType(ExecuteProviderStub, TypeMoq.MockBehavior.Loose);
executeProviderMock.callBase = true;
});
suite('getNotebookManager', () => {
test('Should throw if no matching provider is defined', async () => {
suite('get notebook managers', () => {
test('Should throw if no matching serialization provider is defined', async () => {
try {
await extHostNotebook.$getNotebookManager(-1, notebookUri);
await extHostNotebook.$getSerializationManagerDetails(-1, notebookUri);
assert.fail('expected to throw');
} catch (e) { }
});
test('Should throw if no matching execute provider is defined', async () => {
try {
await extHostNotebook.$getExecuteManagerDetails(-1, notebookUri);
assert.fail('expected to throw');
} catch (e) { }
});
suite('with provider', () => {
let providerHandle: number = -1;
let serializationProviderHandle: number = -1;
let executeProviderHandle: number = -1;
setup(() => {
mockProxy.setup(p =>
p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
.returns((providerId, handle) => {
providerHandle = handle;
serializationProviderHandle = handle;
return undefined;
});
mockProxy.setup(p =>
p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
.returns((providerId, handle) => {
executeProviderHandle = handle;
return undefined;
});
// Register the provider so we can test behavior with this present
extHostNotebook.registerNotebookProvider(notebookProviderMock.object);
extHostNotebook.registerSerializationProvider(serializationProviderMock.object);
extHostNotebook.registerExecuteProvider(executeProviderMock.object);
});
test('Should return a notebook manager with correct info on content and server manager existence', async () => {
test('Should return a serialization manager with correct info on content manager existence', async () => {
// Given the provider returns a manager with no
let expectedManager = new NotebookManagerStub();
notebookProviderMock.setup(p => p.getNotebookManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
let expectedManager = new SerializationManagerStub();
serializationProviderMock.setup(p => p.getSerializationManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
// When I call through using the handle provided during registration
let managerDetails: INotebookManagerDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri);
let managerDetails: ISerializationManagerDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri);
// Then I expect the same manager to be returned
assert.ok(managerDetails.hasContentManager === false, 'Expect no content manager defined');
assert.ok(managerDetails.handle > 0, 'Expect a valid handle defined');
});
test('Should return an execute manager with correct info on whether server manager exists', async () => {
// An execute manager with no server manager
let expectedManager = new ExecuteManagerStub();
executeProviderMock.setup(p => p.getExecuteManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
// When I call through using the handle provided during registration
let managerDetails: IExecuteManagerDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri);
// Then I expect the same manager to be returned
assert.ok(managerDetails.hasServerManager === false, 'Expect no server manager defined');
assert.ok(managerDetails.handle > 0, 'Expect a valid handle defined');
});
test('Should have a unique handle for each notebook URI', async () => {
test('Should have a unique serialization provider handle for each notebook URI', async () => {
// Given the we request 2 URIs
let expectedManager = new NotebookManagerStub();
notebookProviderMock.setup(p => p.getNotebookManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
let expectedManager = new SerializationManagerStub();
serializationProviderMock.setup(p => p.getSerializationManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
// When I call through using the handle provided during registration
let originalManagerDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri);
let differentDetails = await extHostNotebook.$getNotebookManager(providerHandle, URI.parse('file://other/file.ipynb'));
let sameDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri);
let originalManagerDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri);
let differentDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, URI.parse('file://other/file.ipynb'));
let sameDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri);
// Then I expect the 2 different handles in the managers returned.
// This is because we can't easily track identity of the managers, so just track which one is assigned to
// a notebook by the handle ID
assert.notStrictEqual(originalManagerDetails.handle, differentDetails.handle, 'Should have unique handle for each manager');
assert.strictEqual(originalManagerDetails.handle, sameDetails.handle, 'Should have same handle when same URI is passed in');
});
test('Should have a unique execute provider handle for each notebook URI', async () => {
// Given the we request 2 URIs
let expectedManager = new ExecuteManagerStub();
executeProviderMock.setup(p => p.getExecuteManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager));
// When I call through using the handle provided during registration
let originalManagerDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri);
let differentDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, URI.parse('file://other/file.ipynb'));
let sameDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri);
// Then I expect the 2 different handles in the managers returned.
// This is because we can't easily track identity of the managers, so just track which one is assigned to
// a notebook by the handle ID
assert.notStrictEqual(originalManagerDetails.handle, differentDetails.handle, 'Should have unique handle for each manager');
assert.strictEqual(originalManagerDetails.handle, sameDetails.handle, 'Should have same handle when same URI is passed in');
});
});
});
suite('registerNotebookProvider', () => {
suite('registerSerializationProvider', () => {
let savedHandle: number = -1;
setup(() => {
mockProxy.setup(p =>
p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
.returns((providerId, handle) => {
savedHandle = handle;
return undefined;
@@ -105,28 +151,61 @@ suite('ExtHostNotebook Tests', () => {
});
test('Should register with a new handle to the proxy', () => {
extHostNotebook.registerNotebookProvider(notebookProviderMock.object);
extHostNotebook.registerSerializationProvider(serializationProviderMock.object);
mockProxy.verify(p =>
p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId),
p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId),
TypeMoq.It.isAnyNumber()), TypeMoq.Times.once());
// It shouldn't unregister until requested
mockProxy.verify(p => p.$unregisterNotebookProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
mockProxy.verify(p => p.$unregisterSerializationProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
});
test('Should not call unregister on disposing', () => {
let disposable = extHostNotebook.registerNotebookProvider(notebookProviderMock.object);
let disposable = extHostNotebook.registerSerializationProvider(serializationProviderMock.object);
disposable.dispose();
mockProxy.verify(p => p.$unregisterNotebookProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
mockProxy.verify(p => p.$unregisterSerializationProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
});
});
suite('registerExecuteProvider', () => {
let savedHandle: number = -1;
setup(() => {
mockProxy.setup(p =>
p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId), TypeMoq.It.isAnyNumber()))
.returns((providerId, handle) => {
savedHandle = handle;
return undefined;
});
});
test('Should register with a new handle to the proxy', () => {
extHostNotebook.registerExecuteProvider(executeProviderMock.object);
mockProxy.verify(p =>
p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId),
TypeMoq.It.isAnyNumber()), TypeMoq.Times.once());
// It shouldn't unregister until requested
mockProxy.verify(p => p.$unregisterExecuteProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
});
test('Should not call unregister on disposing', () => {
let disposable = extHostNotebook.registerExecuteProvider(executeProviderMock.object);
disposable.dispose();
mockProxy.verify(p => p.$unregisterExecuteProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never());
});
});
});
class NotebookProviderStub implements azdata.nb.NotebookProvider {
class SerializationProviderStub implements azdata.nb.NotebookSerializationProvider {
providerId: string = 'TestProvider';
standardKernels: azdata.nb.IStandardKernel[] = [{ name: 'fakeKernel', displayName: 'fakeKernel', connectionProviderIds: [mssqlProviderName] }];
getNotebookManager(notebookUri: vscode.Uri): Thenable<azdata.nb.NotebookManager> {
getSerializationManager(notebookUri: vscode.Uri): Thenable<azdata.nb.SerializationManager> {
throw new Error('Method not implemented.');
}
}
class ExecuteProviderStub implements azdata.nb.NotebookExecuteProvider {
providerId: string = 'TestProvider';
getExecuteManager(notebookUri: vscode.Uri): Thenable<azdata.nb.ExecuteManager> {
throw new Error('Method not implemented.');
}
handleNotebookClosed(notebookUri: vscode.Uri): void {
@@ -134,11 +213,13 @@ class NotebookProviderStub implements azdata.nb.NotebookProvider {
}
}
class NotebookManagerStub implements azdata.nb.NotebookManager {
class SerializationManagerStub implements azdata.nb.SerializationManager {
get contentManager(): azdata.nb.ContentManager {
return undefined;
}
}
class ExecuteManagerStub implements azdata.nb.ExecuteManager {
get sessionManager(): azdata.nb.SessionManager {
return undefined;
}

View File

@@ -12,8 +12,8 @@ import { IExtHostContext } from 'vs/workbench/api/common/extHost.protocol';
import { MainThreadNotebook } from 'sql/workbench/api/browser/mainThreadNotebook';
import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl';
import { INotebookProvider } from 'sql/workbench/services/notebook/browser/notebookService';
import { INotebookManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails } from 'sql/workbench/api/common/sqlExtHostTypes';
import { IExecuteProvider, ISerializationProvider } from 'sql/workbench/services/notebook/browser/notebookService';
import { IExecuteManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes';
import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager';
import { TestLifecycleService } from 'vs/workbench/test/browser/workbenchTestServices';
import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
@@ -57,80 +57,123 @@ suite('MainThreadNotebook Tests', () => {
mainThreadNotebook = new MainThreadNotebook(extContext, mockNotebookService.object, instantiationService);
});
suite('On registering a provider', () => {
let provider: INotebookProvider;
suite('On registering a serialization provider', () => {
let provider: ISerializationProvider;
setup(() => {
mockNotebookService.setup(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
mockNotebookService.setup(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
provider = providerImpl;
});
});
test('should call through to notebook service', () => {
// When I register a provider
mainThreadNotebook.$registerNotebookProvider(providerId, 1);
mainThreadNotebook.$registerSerializationProvider(providerId, 1);
// Then I expect a provider implementation to be passed to the service
mockNotebookService.verify(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once());
mockNotebookService.verify(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once());
assert.strictEqual(provider.providerId, providerId);
});
test('should unregister in service', () => {
// Given we have a provider
mainThreadNotebook.$registerNotebookProvider(providerId, 1);
mainThreadNotebook.$registerSerializationProvider(providerId, 1);
// When I unregister a provider twice
mainThreadNotebook.$unregisterNotebookProvider(1);
mainThreadNotebook.$unregisterNotebookProvider(1);
mainThreadNotebook.$unregisterSerializationProvider(1);
mainThreadNotebook.$unregisterSerializationProvider(1);
// Then I expect it to be unregistered in the service just 1 time
mockNotebookService.verify(s => s.unregisterProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once());
mockNotebookService.verify(s => s.unregisterSerializationProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once());
});
});
suite('getNotebookManager', () => {
let managerWithAllFeatures: INotebookManagerDetails;
let provider: INotebookProvider;
suite('On registering an execute provider', () => {
let provider: IExecuteProvider;
setup(() => {
managerWithAllFeatures = {
handle: 2,
hasContentManager: true,
hasServerManager: true
};
mockNotebookService.setup(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
mockNotebookService.setup(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
provider = providerImpl;
});
mainThreadNotebook.$registerNotebookProvider(providerId, 1);
});
test('should call through to notebook service', () => {
// When I register a provider
mainThreadNotebook.$registerExecuteProvider(providerId, 1);
// Then I expect a provider implementation to be passed to the service
mockNotebookService.verify(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once());
assert.strictEqual(provider.providerId, providerId);
});
test('should unregister in service', () => {
// Given we have a provider
mainThreadNotebook.$registerExecuteProvider(providerId, 1);
// When I unregister a provider twice
mainThreadNotebook.$unregisterExecuteProvider(1);
mainThreadNotebook.$unregisterExecuteProvider(1);
// Then I expect it to be unregistered in the service just 1 time
mockNotebookService.verify(s => s.unregisterExecuteProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once());
});
});
suite('get notebook managers', () => {
let serializationManagerWithAllFeatures: ISerializationManagerDetails;
let executeManagerWithAllFeatures: IExecuteManagerDetails;
let serializationProvider: ISerializationProvider;
let executeProvider: IExecuteProvider;
setup(() => {
serializationManagerWithAllFeatures = {
handle: 3,
hasContentManager: true,
};
executeManagerWithAllFeatures = {
handle: 4,
hasServerManager: true
};
mockNotebookService.setup(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
serializationProvider = providerImpl;
});
mainThreadNotebook.$registerSerializationProvider(providerId, 1);
mockNotebookService.setup(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => {
executeProvider = providerImpl;
});
mainThreadNotebook.$registerExecuteProvider(providerId, 2);
// Always return empty specs in this test suite
mockProxy.setup(p => p.$refreshSpecs(TypeMoq.It.isAnyNumber())).returns(() => Promise.resolve(undefined));
});
test('should return manager with default content manager & undefined server manager if extension host has none', async () => {
test('should return execute manager with undefined server manager if extension host has none', async () => {
// Given the extension provider doesn't have acontent or server manager
let details: INotebookManagerDetails = {
let details: IExecuteManagerDetails = {
handle: 2,
hasContentManager: false,
hasServerManager: false
};
mockProxy.setup(p => p.$getNotebookManager(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri)))
mockProxy.setup(p => p.$getExecuteManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri)))
.returns(() => Promise.resolve(details));
// When I get the notebook manager
let manager = await provider.getNotebookManager(notebookUri);
let manager = await executeProvider.getExecuteManager(notebookUri);
// Then it should use the built-in content manager
assert.ok(manager.contentManager instanceof LocalContentManager);
// And it should not define a server manager
assert.strictEqual(manager.serverManager, undefined);
});
test('should return manager with a content & server manager if extension host has these', async () => {
test('should return serialization manager with a content manager if extension host has these', async () => {
// Given the extension provider doesn't have acontent or server manager
mockProxy.setup(p => p.$getNotebookManager(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri)))
.returns(() => Promise.resolve(managerWithAllFeatures));
mockProxy.setup(p => p.$getSerializationManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri)))
.returns(() => Promise.resolve(serializationManagerWithAllFeatures));
// When I get the notebook manager
let manager = await provider.getNotebookManager(notebookUri);
let manager = await serializationProvider.getSerializationManager(notebookUri);
// Then it shouldn't have wrappers for the content or server manager
// Then it shouldn't have wrappers for the content manager
assert.ok(!(manager.contentManager instanceof LocalContentManager));
});
test('should return execute manager with a server manager if extension host has these', async () => {
// Given the extension provider doesn't have a content or server manager
mockProxy.setup(p => p.$getExecuteManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri)))
.returns(() => Promise.resolve(executeManagerWithAllFeatures));
// When I get the notebook manager
let manager = await executeProvider.getExecuteManager(notebookUri);
// Then it shouldn't have wrappers for the server manager
assert.notStrictEqual(manager.serverManager, undefined);
});
});
@@ -138,7 +181,10 @@ suite('MainThreadNotebook Tests', () => {
});
class ExtHostNotebookStub implements ExtHostNotebookShape {
$getNotebookManager(providerHandle: number, notebookUri: UriComponents): Thenable<INotebookManagerDetails> {
$getSerializationManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable<ISerializationManagerDetails> {
throw new Error('Method not implemented.');
}
$getExecuteManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable<IExecuteManagerDetails> {
throw new Error('Method not implemented.');
}
$handleNotebookClosed(notebookUri: UriComponents): void {
@@ -150,10 +196,10 @@ class ExtHostNotebookStub implements ExtHostNotebookShape {
$doStopServer(managerHandle: number): Thenable<void> {
throw new Error('Method not implemented.');
}
$getNotebookContents(managerHandle: number, notebookUri: UriComponents): Thenable<azdata.nb.INotebookContents> {
$deserializeNotebook(managerHandle: number, contents: string): Thenable<azdata.nb.INotebookContents> {
throw new Error('Method not implemented.');
}
$save(managerHandle: number, notebookUri: UriComponents, notebook: azdata.nb.INotebookContents): Thenable<azdata.nb.INotebookContents> {
$serializeNotebook(managerHandle: number, notebook: azdata.nb.INotebookContents): Thenable<string> {
throw new Error('Method not implemented.');
}
$refreshSpecs(managerHandle: number): Thenable<azdata.nb.IAllKernels> {