diff --git a/package.json b/package.json index d3853149e3..61cc67143c 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,12 @@ "category": "Deepnote", "icon": "$(plug)" }, + { + "command": "deepnote.addExistingIntegration", + "title": "%deepnote.commands.addExistingIntegration.title%", + "category": "Deepnote", + "icon": "$(link)" + }, { "command": "deepnote.authenticateIntegration", "title": "%deepnote.commands.authenticateIntegration.title%", diff --git a/package.nls.json b/package.nls.json index 63e53ab0f7..388ab8d32c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -257,6 +257,7 @@ "deepnote.commands.enableSnapshots.title": "Enable Snapshots", "deepnote.commands.disableSnapshots.title": "Disable Snapshots", "deepnote.commands.manageIntegrations.title": "Manage Integrations", + "deepnote.commands.addExistingIntegration.title": "Add Existing Integration", "deepnote.commands.authenticateIntegration.title": "Authenticate Integration", "deepnote.commands.newProject.title": "New Project", "deepnote.commands.importNotebook.title": "Import Notebook", diff --git a/specs/INTEGRATIONS_CREDENTIALS.md b/specs/INTEGRATIONS_CREDENTIALS.md index 8edc5e2233..0693f22c8e 100644 --- a/specs/INTEGRATIONS_CREDENTIALS.md +++ b/specs/INTEGRATIONS_CREDENTIALS.md @@ -433,6 +433,32 @@ Orchestrates the integration management UI and commands. 3. Manager opens webview with integration list 4. Optionally pre-selects a specific integration for configuration +Both commands act only on the notebook a menu or the panel names, when one is given, and refuse it once it is closed +rather than acting on whichever editor has focus. They also refuse a snapshot (`*.snapshot.deepnote`): it matches the +notebook selector, but it records a past run, and the integrations writer never edits one. + +**Add Existing Integration** (`deepnote.addExistingIntegration`, also the "Add Existing Integration" button in the +webview, implemented in `existingIntegrationPicker.ts`): + +1. Scans every `.deepnote` file in the workspace folders (snapshots excluded) and collects the roster entries of + _other_ projects whose id has a config in SecretStorage; ids already on the active project's roster are skipped, + as are file-only (`.deepnote.env.yaml`) integrations, which already apply workspace-wide. +2. Shows a QuickPick (name, type, "Used in: "). An id whose roster type disagrees with the stored config's + type is dropped with a warning rather than offered. +3. On selection, appends `{ id, name, type }` to the active project's roster via `addProjectIntegration` (see "Project + files" below). Nothing is copied in SecretStorage: configs (and federated refresh tokens) are keyed by integration + id alone, and the roster entry is what scopes an integration to a project, so the linked project resolves the same + credentials. +4. Refreshes the integrations panel when one is open for this project, without revealing it; run from the command + palette, the command never opens one. Then re-runs the integration env refresh in the project's running kernels, + since no storage change event fires for a roster-only edit. The panel goes first: a busy kernel answers the refresh + only after its running cell. + +The panel's Delete only takes a linked integration off the active project's roster, and keeps its credentials, while +another project in the open workspace folders still declares the id. No storage change event fires then either, so the +panel refreshes the project's running kernels itself. Reset, Sign out and Save go ahead on shared credentials, which +are the same for every project declaring the id, and their success message names the other projects they affected. + #### 4. **Integration Webview** (`integrationWebview.ts`) Provides the webview-based UI for managing integration credentials. @@ -444,6 +470,17 @@ Provides the webview-based UI for managing integration credentials. - Configuration forms for each integration type - Delete/reset functionality +**Project files:** + +Each panel action edits the roster entry it acts on, through `projectIntegrationsWriter.ts`: Save adds the saved +integration or updates its entry in place (`addProjectIntegration`), Delete removes it (`removeProjectIntegration`), +and Reset leaves the roster alone. Every edit merges into what each of the project's `.deepnote` files holds rather +than writing the panel's list, so entries the panel does not list survive it: types it cannot configure, like +`pandas-dataframe`, and integrations linked after it opened. The active file goes first, and the siblings are only +swept once it is on disk; each open notebook's cached project then takes the list its own file now holds. A Save +refreshes the project's running kernels once the roster holds the integration, since the refresh its credential save +triggers may have read the roster before. + **Connection Status:** `IntegrationItem.tsx` derives the status pill from two inputs: `config` (the SecretStorage credentials the panel @@ -480,6 +517,9 @@ Extension → Webview: Webview → Extension: ```typescript +// Posted on every mount, once the listener is attached; answered with the loc strings, then the list +{ type: 'started' } + // Save configuration { type: 'save', integrationId: string, config: IntegrationConfig } diff --git a/specs/INTEGRATION_ENV_LIVE_REFRESH.md b/specs/INTEGRATION_ENV_LIVE_REFRESH.md index f5a1fb238b..0de59270e2 100644 --- a/specs/INTEGRATION_ENV_LIVE_REFRESH.md +++ b/specs/INTEGRATION_ENV_LIVE_REFRESH.md @@ -42,7 +42,10 @@ Reacts to **file** changes. ### 3. `IntegrationEnvLiveRefresher` (`integrationEnvLiveRefresher.node.ts`) -Performs the refresh, for both triggers. +Performs the refresh, for both triggers above and for edits to a project's `integrations` list, which decides the +stored configs its kernels get. Such an edit writes no storage, so no event fires and the code making it calls the +refresher for the project's open notebooks itself: `IntegrationManager` after Add Existing Integration, and +`IntegrationWebviewProvider` after a Save, or after a Delete that only takes a shared integration off the project. - Skips notebooks with no kernel, or whose kernel has never started - Runs a hidden execution in each remaining kernel: @@ -127,7 +130,7 @@ web counterpart, since the loopback server and the file reads both require Node - `IntegrationEnvLiveRefresher.refreshNotebook()` never throws; a per-notebook failure is logged and the other notebooks still refresh -- Both triggers wrap their async entry point and log rather than surfacing an unhandled rejection +- Every trigger wraps its async entry point and logs rather than surfacing an unhandled rejection - A refresh that produces `error` outputs is counted as failed, so the success message only appears when at least one kernel actually applied the new environment - `UserpodApiEndpoints` keeps a persistent `error` listener (an `error` with no listener would crash the diff --git a/src/messageTypes.ts b/src/messageTypes.ts index 76d893cd53..01fea32ffe 100644 --- a/src/messageTypes.ts +++ b/src/messageTypes.ts @@ -10,6 +10,7 @@ import { import { KernelSocketOptions } from './kernels/types'; import { IJupyterVariable, IJupyterVariablesRequest, IJupyterVariablesResponse } from './kernels/variables/types'; import { WidgetScriptSource } from './notebooks/controllers/ipywidgets/types'; +import type { IntegrationTypeLabelKey } from './platform/notebooks/deepnote/integrationTypeLabels'; export type NotifyIPyWidgetWidgetVersionNotSupportedAction = { moduleName: 'qgrid'; @@ -120,7 +121,7 @@ export enum SharedMessages { LocInit = 'loc_init' } -export type LocalizedMessages = { +export type LocalizedMessages = { [K in IntegrationTypeLabelKey]: string } & { collapseSingle: string; expandSingle: string; openExportFileYes: string; @@ -186,29 +187,10 @@ export type LocalizedMessages = { integrationsCancel: string; integrationsSave: string; integrationsAddNewIntegration: string; + integrationsAddExistingIntegration: string; integrationsDatabase: string; integrationsDataWarehousesLakes: string; integrationsDatabases: string; - // Integration type labels - integrationsPostgresTypeLabel: string; - integrationsBigQueryTypeLabel: string; - integrationsSnowflakeTypeLabel: string; - integrationsAlloyDBTypeLabel: string; - integrationsAthenaTypeLabel: string; - integrationsClickHouseTypeLabel: string; - integrationsCloudSqlTypeLabel: string; - integrationsDatabricksTypeLabel: string; - integrationsDremioTypeLabel: string; - integrationsMariaDBTypeLabel: string; - integrationsMaterializeTypeLabel: string; - integrationsMindsDBTypeLabel: string; - integrationsMongoDBTypeLabel: string; - integrationsMySQLTypeLabel: string; - integrationsDuckDBTypeLabel: string; - integrationsRedshiftTypeLabel: string; - integrationsSpannerTypeLabel: string; - integrationsSQLServerTypeLabel: string; - integrationsTrinoTypeLabel: string; // PostgreSQL form strings integrationsPostgresNameLabel: string; integrationsPostgresNamePlaceholder: string; diff --git a/src/notebooks/deepnote/deepnoteNotebookManager.ts b/src/notebooks/deepnote/deepnoteNotebookManager.ts index 3772f20098..b166be0845 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.ts @@ -1,7 +1,7 @@ import { injectable } from 'inversify'; import type { DeepnoteFile } from '@deepnote/blocks'; -import { IDeepnoteNotebookManager, ProjectIntegration } from '../types'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../types'; /** * Centralized manager for tracking Deepnote notebook selections and project state. @@ -36,24 +36,22 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { notebookEntries.set(notebookId, clonedProject); } - /** - * Updates the integrations list across every cached notebook entry under the project (cache-only). - * @returns `true` if at least one cached entry was updated, `false` otherwise. - */ - updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean { + /** Replaces the integrations list of one cached (projectId, notebookId) entry (cache-only). */ + updateProjectIntegrationsForNotebook( + projectId: string, + notebookId: string, + integrations: RawProjectIntegration[] + ): void { const notebookEntries = this.originalProjects.get(projectId); + const project = notebookEntries?.get(notebookId); - if (!notebookEntries || notebookEntries.size === 0) { - return false; + if (!notebookEntries || !project) { + return; } - for (const [notebookId, project] of notebookEntries) { - const updatedProject = structuredClone(project); - updatedProject.project.integrations = structuredClone(integrations); - - notebookEntries.set(notebookId, updatedProject); - } + const updatedProject = structuredClone(project); + updatedProject.project.integrations = structuredClone(integrations); - return true; + notebookEntries.set(notebookId, updatedProject); } } diff --git a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts index ef1f9b5421..ff3b322057 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts @@ -52,91 +52,33 @@ suite('DeepnoteNotebookManager', () => { }); }); - suite('updateProjectIntegrations', () => { - test('should update integrations list for existing project and return true', () => { - manager.storeOriginalProject('project-123', 'notebook-456', mockProject); - - const integrations: ProjectIntegration[] = [ - { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }, - { id: 'int-2', name: 'BigQuery', type: 'big-query' } - ]; - - const result = manager.updateProjectIntegrations('project-123', integrations); - - assert.strictEqual(result, true); - - const updatedProject = manager.getProjectForNotebook('project-123', 'notebook-456'); - assert.deepStrictEqual(updatedProject?.project.integrations, integrations); - }); - - test('should replace existing integrations list and return true', () => { - const projectWithIntegrations: DeepnoteFile = { + suite('updateProjectIntegrationsForNotebook', () => { + test('replaces the list of the entry and keeps every other field of it', () => { + manager.storeOriginalProject('project-123', 'notebook-456', { ...mockProject, - project: { - ...mockProject.project, - integrations: [{ id: 'old-int', name: 'Old Integration', type: 'pgsql' }] - } - }; - - manager.storeOriginalProject('project-123', 'notebook-456', projectWithIntegrations); - - const newIntegrations: ProjectIntegration[] = [ - { id: 'new-int-1', name: 'New Integration 1', type: 'pgsql' }, - { id: 'new-int-2', name: 'New Integration 2', type: 'big-query' } - ]; - - const result = manager.updateProjectIntegrations('project-123', newIntegrations); + project: { ...mockProject.project, integrations: [{ id: 'old-int', name: 'Old', type: 'pgsql' }] } + }); - assert.strictEqual(result, true); + const integrations: ProjectIntegration[] = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }]; - const updatedProject = manager.getProjectForNotebook('project-123', 'notebook-456'); - assert.deepStrictEqual(updatedProject?.project.integrations, newIntegrations); - }); + manager.updateProjectIntegrationsForNotebook('project-123', 'notebook-456', integrations); - test('should handle empty integrations array and return true', () => { - const projectWithIntegrations: DeepnoteFile = { + assert.deepStrictEqual(manager.getProjectForNotebook('project-123', 'notebook-456'), { ...mockProject, - project: { - ...mockProject.project, - integrations: [{ id: 'int-1', name: 'Integration 1', type: 'pgsql' }] - } - }; - - manager.storeOriginalProject('project-123', 'notebook-456', projectWithIntegrations); - - const result = manager.updateProjectIntegrations('project-123', []); - - assert.strictEqual(result, true); - - const updatedProject = manager.getProjectForNotebook('project-123', 'notebook-456'); - assert.deepStrictEqual(updatedProject?.project.integrations, []); - }); - - test('should return false for unknown project', () => { - const result = manager.updateProjectIntegrations('unknown-project', [ - { id: 'int-1', name: 'Integration', type: 'pgsql' } - ]); - - assert.strictEqual(result, false); - - const project = manager.getProjectForNotebook('unknown-project', 'notebook-456'); - assert.strictEqual(project, undefined); + project: { ...mockProject.project, integrations } + }); }); - test('should preserve other project properties and return true', () => { + test('caches nothing for a project or notebook that is not cached', () => { manager.storeOriginalProject('project-123', 'notebook-456', mockProject); - const integrations: ProjectIntegration[] = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }]; - const result = manager.updateProjectIntegrations('project-123', integrations); + manager.updateProjectIntegrationsForNotebook('unknown-project', 'notebook-456', integrations); + manager.updateProjectIntegrationsForNotebook('project-123', 'unknown-notebook', integrations); - assert.strictEqual(result, true); - - const updatedProject = manager.getProjectForNotebook('project-123', 'notebook-456'); - assert.strictEqual(updatedProject?.project.id, mockProject.project.id); - assert.strictEqual(updatedProject?.project.name, mockProject.project.name); - assert.strictEqual(updatedProject?.version, mockProject.version); - assert.deepStrictEqual(updatedProject?.metadata, mockProject.metadata); + assert.strictEqual(manager.getProjectForNotebook('unknown-project', 'notebook-456'), undefined); + assert.strictEqual(manager.getProjectForNotebook('project-123', 'unknown-notebook'), undefined); + assert.deepStrictEqual(manager.getProjectForNotebook('project-123', 'notebook-456'), mockProject); }); }); @@ -180,42 +122,38 @@ suite('DeepnoteNotebookManager', () => { assert.strictEqual(result, undefined); }); - test('updateProjectIntegrations updates every cached notebook entry under the project', () => { + test('updateProjectIntegrationsForNotebook updates that entry alone, leaving the siblings on their own lists', () => { + const siblingIntegrations: ProjectIntegration[] = [{ id: 'int-b', name: 'MySQL', type: 'mysql' }]; + const projectB = siblingProject(nbB, 'Sibling B'); + projectB.project.integrations = siblingIntegrations; manager.storeOriginalProject(projectId, nbA, siblingProject(nbA, 'Sibling A')); - manager.storeOriginalProject(projectId, nbB, siblingProject(nbB, 'Sibling B')); + manager.storeOriginalProject(projectId, nbB, projectB); - const integrations: ProjectIntegration[] = [ - { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }, - { id: 'int-2', name: 'BigQuery', type: 'big-query' } - ]; + const integrations: ProjectIntegration[] = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }]; - const updated = manager.updateProjectIntegrations(projectId, integrations); + manager.updateProjectIntegrationsForNotebook(projectId, nbA, integrations); - assert.strictEqual(updated, true); - // BOTH siblings of the project must see the new integrations. assert.deepStrictEqual(manager.getProjectForNotebook(projectId, nbA)?.project.integrations, integrations); - assert.deepStrictEqual(manager.getProjectForNotebook(projectId, nbB)?.project.integrations, integrations); + assert.deepStrictEqual( + manager.getProjectForNotebook(projectId, nbB)?.project.integrations, + siblingIntegrations + ); }); - test('updateProjectIntegrations deep-clones integrations so cached siblings are isolated from the caller and each other', () => { + test('updateProjectIntegrationsForNotebook deep-clones integrations so the cache is isolated from the caller', () => { manager.storeOriginalProject(projectId, nbA, siblingProject(nbA, 'Sibling A')); - manager.storeOriginalProject(projectId, nbB, siblingProject(nbB, 'Sibling B')); const pg: ProjectIntegration = { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }; const integrations: ProjectIntegration[] = [pg]; - manager.updateProjectIntegrations(projectId, integrations); + manager.updateProjectIntegrationsForNotebook(projectId, nbA, integrations); - // Mutating the caller-owned array and its contents after the call must not leak into the cache. pg.name = 'MUTATED'; integrations.push({ id: 'int-2', name: 'BigQuery', type: 'big-query' }); - const cachedA = manager.getProjectForNotebook(projectId, nbA)?.project.integrations; - const cachedB = manager.getProjectForNotebook(projectId, nbB)?.project.integrations; - - assert.deepStrictEqual(cachedA, [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }]); - // Each sibling holds its own array instance rather than a shared reference. - assert.notStrictEqual(cachedA, cachedB); + assert.deepStrictEqual(manager.getProjectForNotebook(projectId, nbA)?.project.integrations, [ + { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' } + ]); }); }); }); diff --git a/src/notebooks/deepnote/integrations/existingIntegrationPicker.ts b/src/notebooks/deepnote/integrations/existingIntegrationPicker.ts new file mode 100644 index 0000000000..a390eb1296 --- /dev/null +++ b/src/notebooks/deepnote/integrations/existingIntegrationPicker.ts @@ -0,0 +1,230 @@ +import type { DeepnoteFile } from '@deepnote/blocks'; +import { CancellationToken, RelativePattern, Uri, workspace, WorkspaceFolder } from 'vscode'; + +import { Cancellation } from '../../../platform/common/cancellation'; +import { readDeepnoteProjectFile } from '../../../platform/deepnote/deepnoteProjectFileReader'; +import { logger } from '../../../platform/logging'; +import { + ConfigurableDatabaseIntegrationType, + isConfigurableDatabaseIntegrationType +} from '../../../platform/notebooks/deepnote/integrationTypes'; +import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types'; +import { isSnapshotFile } from '../snapshots/snapshotFiles'; +import { addProjectIntegration, PersistIntegrationsResult } from './projectIntegrationsWriter'; +import { IIntegrationStorage } from './types'; + +/** An integration another project in the workspace has credentials stored for, so linking it needs no re-entry. */ +export interface ReusableIntegration { + id: string; + /** Name from the stored config — the same source the panel writes to the project integrations on save. */ + name: string; + /** The other projects declaring this integration; deduped and sorted. */ + projectNames: string[]; + type: ConfigurableDatabaseIntegrationType; +} + +export interface CollectReusableIntegrationsParams { + /** Ids the current project already declares. */ + excludeIntegrationIds: ReadonlySet; + integrationStorage: IIntegrationStorage; + /** The project being extended; its own `.deepnote` files are skipped. */ + projectId: string; + /** Aborts the scan with a `CancellationError` rather than handing back a partial result. */ + token?: CancellationToken; +} + +export interface CollectReusableIntegrationsResult { + /** + * Ids skipped because some project declares a type the stored config disagrees with: linking one would put a + * type on this project that the credentials cannot back. + */ + conflictingIds: string[]; + integrations: ReusableIntegration[]; +} + +export interface AttachExistingIntegrationParams { + activeFileUri: Uri; + integration: ReusableIntegration; + notebookManager: IDeepnoteNotebookManager; + projectId: string; +} + +export interface FindOtherProjectsDeclaringParams { + integrationId: string; + /** The project asking; its own `.deepnote` files are skipped. */ + projectId: string; +} + +type OtherProjectFileVisitor = (project: DeepnoteFile['project'], fileUri: Uri) => Promise | void; + +/** + * Scans every `.deepnote` file in the open workspace folders for integrations other projects declare. + * + * Reuse is a link, not a copy: `IntegrationStorage` and `FederatedAuthTokenStorage` both key configs by integration + * id alone, so the project's own entry is the only thing that scopes an integration to a project. + * + * Integrations configured only in `.deepnote.env.yaml` are not offered: that file already applies to every project + * under it, and the panel cannot write that layer. + * + * @throws `CancellationError` when `token` trips, so a half-finished scan can never be mistaken for a complete one. + */ +export async function collectReusableIntegrations( + params: CollectReusableIntegrationsParams +): Promise { + const { excludeIntegrationIds, integrationStorage, projectId, token } = params; + + const candidates = new Map }>(); + const conflictingIds = new Set(); + + await forEachOtherProjectFile(projectId, token, async (project, fileUri) => { + const projectName = project.name || project.id; + + for (const entry of project.integrations ?? []) { + if (excludeIntegrationIds.has(entry.id) || !isConfigurableDatabaseIntegrationType(entry.type)) { + continue; + } + + const storedConfig = await integrationStorage.getIntegrationConfig(entry.id); + + if (!storedConfig) { + // File-only or never configured — no stored credentials to reuse. + continue; + } + + if (storedConfig.type !== entry.type) { + logger.warn( + `collectReusableIntegrations: ${entry.id} is declared as ${entry.type} in ${fileUri.path} but stored as ${storedConfig.type}; skipping` + ); + conflictingIds.add(entry.id); + + continue; + } + + const existing = candidates.get(entry.id); + + if (existing) { + existing.projectNameSet.add(projectName); + } else { + candidates.set(entry.id, { + id: entry.id, + name: storedConfig.name || entry.name || entry.id, + projectNameSet: new Set([projectName]), + projectNames: [], + type: storedConfig.type + }); + } + } + }); + + // A conflict in any project disqualifies the id everywhere: the stored config is the single shared truth. + for (const id of conflictingIds) { + candidates.delete(id); + } + + const integrations = Array.from(candidates.values()) + .map(({ projectNameSet, ...candidate }) => ({ + ...candidate, + projectNames: Array.from(projectNameSet).sort((a, b) => a.localeCompare(b)) + })) + .sort((a, b) => a.name.localeCompare(b.name) || a.id.localeCompare(b.id)); + + return { conflictingIds: Array.from(conflictingIds).sort(), integrations }; +} + +/** + * Links `integration` into the project's integrations through the same writer the panel uses, so the cache, the active + * file and every sibling `.deepnote` file are updated together. Re-linking an id already there replaces its entry. + */ +export function attachExistingIntegration(params: AttachExistingIntegrationParams): Promise { + const { activeFileUri, integration, notebookManager, projectId } = params; + + const linked: ProjectIntegration = { id: integration.id, name: integration.name, type: integration.type }; + + return addProjectIntegration({ activeFileUri, integration: linked, notebookManager, projectId }); +} + +/** + * Names of the other projects in the open workspace folders whose `.deepnote` files declare `integrationId`, deduped + * and sorted; empty when none does. Only open folders are scanned, while SecretStorage is shared machine-wide. + */ +export async function findOtherProjectsDeclaring(params: FindOtherProjectsDeclaringParams): Promise { + const { integrationId, projectId } = params; + const projectNames = new Set(); + + await forEachOtherProjectFile(projectId, undefined, (project) => { + if (project.integrations?.some((entry) => entry.id === integrationId)) { + projectNames.add(project.name || project.id); + } + }); + + return Array.from(projectNames).sort((a, b) => a.localeCompare(b)); +} + +/** A folder that cannot be enumerated contributes no files instead of ending the walk. */ +async function findDeepnoteFiles( + workspaceFolder: WorkspaceFolder, + token: CancellationToken | undefined +): Promise { + try { + return await workspace.findFiles( + new RelativePattern(workspaceFolder, '**/*.deepnote'), + undefined, + undefined, + token + ); + } catch (error) { + logger.error('existingIntegrationPicker: failed to enumerate .deepnote files', error); + + return []; + } +} + +/** + * Visits each `.deepnote` file in the open workspace folders that belongs to a project other than `projectId`, once; + * snapshots are skipped. + * + * @throws `CancellationError` when `token` trips, including once the last file has been visited. + */ +async function forEachOtherProjectFile( + projectId: string, + token: CancellationToken | undefined, + visit: OtherProjectFileVisitor +): Promise { + const visited = new Set(); + + for (const workspaceFolder of workspace.workspaceFolders || []) { + Cancellation.throwIfCanceled(token); + + for (const fileUri of await findDeepnoteFiles(workspaceFolder, token)) { + Cancellation.throwIfCanceled(token); + + const key = fileUri.toString(); + + if (visited.has(key) || isSnapshotFile(fileUri)) { + continue; + } + + visited.add(key); + await visitOtherProjectFile(fileUri, projectId, visit); + } + } + + // `workspace.findFiles` resolves empty when its token trips and the per-file awaits are not token-aware. + Cancellation.throwIfCanceled(token); +} + +/** `visit` runs inside the per-file catch: a visitor that throws costs only this file, like a failed read. */ +async function visitOtherProjectFile(fileUri: Uri, projectId: string, visit: OtherProjectFileVisitor): Promise { + // One unreadable file must not hide every other project's integrations. + try { + const projectData = await readDeepnoteProjectFile(fileUri); + + if (!projectData?.project || projectData.project.id === projectId) { + return; + } + + await visit(projectData.project, fileUri); + } catch (error) { + logger.error(`existingIntegrationPicker: failed to read ${fileUri.path}`, error); + } +} diff --git a/src/notebooks/deepnote/integrations/existingIntegrationPicker.unit.test.ts b/src/notebooks/deepnote/integrations/existingIntegrationPicker.unit.test.ts new file mode 100644 index 0000000000..2b20ceeffb --- /dev/null +++ b/src/notebooks/deepnote/integrations/existingIntegrationPicker.unit.test.ts @@ -0,0 +1,436 @@ +import type { DeepnoteFile } from '@deepnote/blocks'; +import { assert, use } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { CancellationError, CancellationToken, CancellationTokenSource, Uri } from 'vscode'; + +import { ConfigurableDatabaseIntegrationConfig } from '../../../platform/notebooks/deepnote/integrationTypes'; +import { stubDeepnoteFiles } from '../../../test/mocks/vscodeFs'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../../types'; +import { + createDeepnoteFile, + createDeepnoteProject, + createMockNotebook, + createWorkspaceFolder +} from '../deepnoteTestHelpers'; +import { + attachExistingIntegration, + collectReusableIntegrations, + findOtherProjectsDeclaring, + ReusableIntegration +} from './existingIntegrationPicker'; +import { buildGoogleOauthIntegration, buildPostgresIntegration } from './federatedAuth/federatedAuthTestHelpers'; +import { IIntegrationStorage } from './types'; + +use(chaiAsPromised); + +const CURRENT_PROJECT_ID = 'project-current'; + +// `thenCall` checks no signature, so doubles take their parameter types from here; annotating them by hand undoes it. +type UpdateProjectIntegrationsFn = IDeepnoteNotebookManager['updateProjectIntegrationsForNotebook']; + +interface OnDiskProject { + uri: Uri; + projectId: string; + projectName?: string; + integrations?: Array<{ id: string; name: string; type: string }>; +} + +function projectFile(project: OnDiskProject): DeepnoteFile { + return createDeepnoteFile({ + project: createDeepnoteProject({ + id: project.projectId, + name: project.projectName ?? project.projectId, + integrations: project.integrations + }) + }); +} + +/** Stubs `workspace.findFiles` + `workspace.fs` over the given files; `unreadable` URIs reject on read. */ +function stubWorkspace(opts: { + projects: OnDiskProject[]; + unreadable?: Uri[]; + hasWorkspaceFolder?: boolean; + onFindFiles?: () => void; + onRead?: (uri: Uri) => void; +}): { + reads: string[]; + writes: Map; +} { + when(mockedVSCodeNamespaces.workspace.workspaceFolders).thenReturn( + opts.hasWorkspaceFolder === false ? undefined : [createWorkspaceFolder(Uri.file('/ws'))] + ); + + const discovered = [...opts.projects.map((project) => project.uri), ...(opts.unreadable ?? [])]; + when(mockedVSCodeNamespaces.workspace.findFiles(anything(), anything(), anything(), anything())).thenCall( + (_include: unknown, _exclude: unknown, _maxResults: unknown, token?: CancellationToken) => { + opts.onFindFiles?.(); + + // Mirrors the real API, which resolves to no results when its token trips instead of rejecting. + return Promise.resolve(token?.isCancellationRequested ? [] : discovered); + } + ); + // The writer enumerates without a token; the scan passes one. + when(mockedVSCodeNamespaces.workspace.findFiles(anything())).thenReturn(Promise.resolve(discovered)); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + + const byPath = new Map(opts.projects.map((project) => [project.uri.fsPath, projectFile(project)] as const)); + const reads: string[] = []; + const writes = stubDeepnoteFiles((uri) => byPath.get(uri.fsPath), { + onRead: (uri) => { + reads.push(uri.fsPath); + opts.onRead?.(uri); + } + }); + + return { reads, writes }; +} + +function stubStorage(configs: ConfigurableDatabaseIntegrationConfig[]): IIntegrationStorage { + const byId = new Map(configs.map((config) => [config.id, config] as const)); + const storage = mock(); + + when(storage.getIntegrationConfig(anything())).thenCall((id: string) => Promise.resolve(byId.get(id))); + + return instance(storage); +} + +suite('existingIntegrationPicker', () => { + setup(() => { + resetVSCodeMocks(); + }); + + suite('collectReusableIntegrations', () => { + const pgConfig = buildPostgresIntegration({ id: 'pg-shared', name: 'Shared Postgres' }); + const bqConfig = buildGoogleOauthIntegration({ id: 'bq-oauth', name: 'Team BigQuery' }); + + async function collect( + excludeIntegrationIds: string[] = [], + configs: ConfigurableDatabaseIntegrationConfig[] = [pgConfig, bqConfig], + token?: CancellationToken + ) { + return collectReusableIntegrations({ + excludeIntegrationIds: new Set(excludeIntegrationIds), + integrationStorage: stubStorage(configs), + projectId: CURRENT_PROJECT_ID, + token + }); + } + + test('lists integrations other projects declare, deduped by id with every using project named', async () => { + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + projectName: 'Alpha', + integrations: [{ id: 'pg-shared', name: 'Alpha Postgres', type: 'pgsql' }] + }, + { + uri: Uri.file('/ws/b.deepnote'), + projectId: 'project-b', + projectName: 'Beta', + integrations: [ + { id: 'pg-shared', name: 'Beta Postgres', type: 'pgsql' }, + { id: 'bq-oauth', name: 'Team BigQuery', type: 'big-query' } + ] + }, + // A second notebook file of Beta must not list Beta twice. + { + uri: Uri.file('/ws/b-2.deepnote'), + projectId: 'project-b', + projectName: 'Beta', + integrations: [{ id: 'pg-shared', name: 'Beta Postgres', type: 'pgsql' }] + } + ] + }); + + const result = await collect(); + + const expected: ReusableIntegration[] = [ + { id: 'pg-shared', name: 'Shared Postgres', projectNames: ['Alpha', 'Beta'], type: 'pgsql' }, + { id: 'bq-oauth', name: 'Team BigQuery', projectNames: ['Beta'], type: 'big-query' } + ]; + assert.deepStrictEqual(result, { conflictingIds: [], integrations: expected }); + }); + + test('excludes ids the current project already declares and the current project files themselves', async () => { + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/current.deepnote'), + projectId: CURRENT_PROJECT_ID, + integrations: [{ id: 'bq-oauth', name: 'Team BigQuery', type: 'big-query' }] + }, + { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + integrations: [ + { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' }, + { id: 'bq-oauth', name: 'Team BigQuery', type: 'big-query' } + ] + } + ] + }); + + const { integrations } = await collect(['pg-shared']); + + const expected: ReusableIntegration[] = [ + { id: 'bq-oauth', name: 'Team BigQuery', projectNames: ['project-a'], type: 'big-query' } + ]; + assert.deepStrictEqual( + integrations, + expected, + 'pg-shared is already attached; bq-oauth is offered because project-a (not the current project) declares it' + ); + }); + + test('skips entries with no stored config (file-only or never configured) and unsupported types', async () => { + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + integrations: [ + { id: 'file-only', name: 'From env yaml', type: 'pgsql' }, + { id: 'duckdb', name: 'DuckDB', type: 'pandas-dataframe' }, + { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' } + ] + }, + // A type this build cannot manage is skipped, not taken for a conflict that hides pg-shared. + { + uri: Uri.file('/ws/b.deepnote'), + projectId: 'project-b', + integrations: [{ id: 'pg-shared', name: 'Shared Postgres', type: 'some-future-type' }] + } + ] + }); + + const { integrations } = await collect(); + + assert.deepStrictEqual( + integrations.map((integration) => integration.id), + ['pg-shared'] + ); + }); + + test('reports an id whose declared type disagrees with the stored config as conflicting and drops it everywhere', async () => { + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + integrations: [{ id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' }] + }, + { + uri: Uri.file('/ws/b.deepnote'), + projectId: 'project-b', + // Same id, but declared as a different database than the credentials are for. + integrations: [{ id: 'pg-shared', name: 'Not really Postgres', type: 'mysql' }] + } + ] + }); + + const result = await collect(); + + assert.deepStrictEqual(result, { conflictingIds: ['pg-shared'], integrations: [] }); + }); + + test('ignores snapshot files and keeps going past an unreadable file', async () => { + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/snapshots/a_project-a_2024.snapshot.deepnote'), + projectId: 'project-snapshot', + integrations: [{ id: 'bq-oauth', name: 'Team BigQuery', type: 'big-query' }] + }, + { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + integrations: [{ id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' }] + } + ], + unreadable: [Uri.file('/ws/broken.deepnote')] + }); + + const { integrations } = await collect(); + + assert.deepStrictEqual( + integrations.map((integration) => integration.id), + ['pg-shared'] + ); + }); + + const projectA: OnDiskProject = { + uri: Uri.file('/ws/a.deepnote'), + projectId: 'project-a', + integrations: [{ id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' }] + }; + const projectB: OnDiskProject = { + uri: Uri.file('/ws/b.deepnote'), + projectId: 'project-b', + integrations: [{ id: 'bq-oauth', name: 'Team BigQuery', type: 'big-query' }] + }; + const cancellations: { + name: string; + projects: OnDiskProject[]; + readsBeforeStop: Uri[]; + trip: 'discovery' | 'read'; + }[] = [ + { name: 'mid-scan', projects: [projectA, projectB], readsBeforeStop: [projectA.uri], trip: 'read' }, + { + name: 'while the last file is read', + projects: [projectA], + readsBeforeStop: [projectA.uri], + trip: 'read' + }, + { name: 'during file discovery', projects: [projectA], readsBeforeStop: [], trip: 'discovery' } + ]; + + for (const { name, projects, readsBeforeStop, trip } of cancellations) { + test(`reports cancellation and reads no further when the token trips ${name}`, async () => { + const cts = new CancellationTokenSource(); + + try { + const { reads } = stubWorkspace({ + projects, + onFindFiles: trip === 'discovery' ? () => cts.cancel() : undefined, + onRead: trip === 'read' ? () => cts.cancel() : undefined + }); + + await assert.isRejected(collect([], [pgConfig, bqConfig], cts.token), CancellationError); + + assert.deepStrictEqual( + reads, + readsBeforeStop.map((uri) => uri.fsPath) + ); + } finally { + cts.dispose(); + } + }); + } + + test('returns nothing without an open workspace folder', async () => { + stubWorkspace({ projects: [], hasWorkspaceFolder: false }); + + const result = await collect(); + + assert.deepStrictEqual(result, { conflictingIds: [], integrations: [] }); + }); + }); + + suite('attachExistingIntegration', () => { + const activeUri = Uri.file('/ws/current.deepnote'); + const shared: ReusableIntegration = { + id: 'pg-shared', + name: 'Shared Postgres', + projectNames: ['Alpha'], + type: 'pgsql' + }; + + let notebookManager: IDeepnoteNotebookManager; + let cacheUpdates: Parameters[]; + + setup(() => { + cacheUpdates = []; + const recordCacheUpdate: UpdateProjectIntegrationsFn = (...update) => { + cacheUpdates.push(update); + }; + const mockManager = mock(); + when(mockManager.updateProjectIntegrationsForNotebook(anything(), anything(), anything())).thenCall( + recordCacheUpdate + ); + notebookManager = instance(mockManager); + }); + + test('appends the linked entry in the cache and on disk, keeping every existing entry verbatim, even of types it cannot manage', async () => { + const currentIntegrations: RawProjectIntegration[] = [ + { id: 'bq-own', name: 'Own BigQuery', type: 'big-query' }, + { id: 'duckdb', name: 'DuckDB', type: 'pandas-dataframe' }, + { id: 'future', name: 'Unknown to this build', type: 'some-future-type' } + ]; + const { writes } = stubWorkspace({ + projects: [{ uri: activeUri, projectId: CURRENT_PROJECT_ID, integrations: currentIntegrations }] + }); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([ + createMockNotebook({ + metadata: { deepnoteNotebookId: 'notebook-current', deepnoteProjectId: CURRENT_PROJECT_ID }, + uri: activeUri + }) + ]); + + const result = await attachExistingIntegration({ + activeFileUri: activeUri, + integration: shared, + notebookManager, + projectId: CURRENT_PROJECT_ID + }); + + const expectedIntegrations = [ + ...currentIntegrations, + { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' } + ]; + assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); + assert.deepStrictEqual(cacheUpdates, [[CURRENT_PROJECT_ID, 'notebook-current', expectedIntegrations]]); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, expectedIntegrations); + }); + }); + + suite('findOtherProjectsDeclaring', () => { + test('names every other project declaring the id once, ignoring its own files, snapshots and unreadable files', async () => { + // Catches: the current project or a snapshot counted as "another project", which blocks every delete, or + // a project listed twice. + const shared = { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' }; + + stubWorkspace({ + projects: [ + { + uri: Uri.file('/ws/beta.deepnote'), + projectId: 'project-beta', + projectName: 'Beta', + integrations: [shared] + }, + { + uri: Uri.file('/ws/alpha.deepnote'), + projectId: 'project-alpha', + projectName: 'Alpha', + integrations: [shared] + }, + { + uri: Uri.file('/ws/alpha-2.deepnote'), + projectId: 'project-alpha', + projectName: 'Alpha', + integrations: [shared] + }, + { + uri: Uri.file('/ws/current.deepnote'), + projectId: CURRENT_PROJECT_ID, + projectName: 'Current', + integrations: [shared] + }, + { + uri: Uri.file('/ws/snapshots/gamma_project-gamma_latest.snapshot.deepnote'), + projectId: 'project-gamma', + projectName: 'Gamma', + integrations: [shared] + }, + { + uri: Uri.file('/ws/delta.deepnote'), + projectId: 'project-delta', + projectName: 'Delta', + integrations: [{ id: 'pg-other', name: 'Other Postgres', type: 'pgsql' }] + } + ], + unreadable: [Uri.file('/ws/broken.deepnote')] + }); + + const projectNames = await findOtherProjectsDeclaring({ + integrationId: 'pg-shared', + projectId: CURRENT_PROJECT_ID + }); + + assert.deepStrictEqual(projectNames, ['Alpha', 'Beta']); + }); + }); +}); diff --git a/src/notebooks/deepnote/integrations/integrationDetector.ts b/src/notebooks/deepnote/integrations/integrationDetector.ts index d860175e96..e35044fd84 100644 --- a/src/notebooks/deepnote/integrations/integrationDetector.ts +++ b/src/notebooks/deepnote/integrations/integrationDetector.ts @@ -26,11 +26,11 @@ export class IntegrationDetector implements IIntegrationDetector { /** * Detect all integrations for the notebook's project. Three inputs, three roles: - * - `project.integrations` is the roster (ids, names and types only — never credentials), so it decides + * - `project.integrations` holds ids, names and types only — never credentials — so it decides * the order and the names the panel shows. * - SecretStorage supplies the editable config for each one; integrations configured only in * `.deepnote.env.yaml` stay `null` here, since those configs are never persisted through it. - * - `.deepnote.env.yaml` entries missing from the roster are appended, matching what actually applies at + * - `.deepnote.env.yaml` entries the project omits are appended, matching what actually applies at * execution time. Without this a file-only integration works but is invisible, and a federated one is * unusable outright — its Authenticate action exists only as a row in this panel. */ @@ -76,9 +76,9 @@ export class IntegrationDetector implements IIntegrationDetector { } /** - * Adds `.deepnote.env.yaml` integrations the roster omits. `config` stays `null` because the panel edits + * Adds `.deepnote.env.yaml` integrations the project omits. `config` stays `null` because the panel edits * SecretStorage only and the file layer cannot be written back; the name and type are carried so the row - * renders. A failed lookup leaves the roster-only result rather than blocking the panel. + * renders. A failed lookup leaves the project's own integrations rather than blocking the panel. */ private async appendFileOnlyIntegrations( notebookUri: Uri, @@ -89,13 +89,16 @@ export class IntegrationDetector implements IIntegrationDetector { try { mergedIntegrationConfigs = await this.sqlIntegrationEnvVars.getMergedIntegrationConfigs(notebookUri); } catch (error) { - logger.error('IntegrationDetector: failed to read file integrations; listing the roster only', error); + logger.error( + "IntegrationDetector: failed to read file integrations; listing the project's own only", + error + ); return; } for (const config of mergedIntegrationConfigs) { - // Anything merged but absent here came from the file alone — the merge resolves roster ids first. + // Anything merged but absent here came from the file alone — the merge resolves the project's ids first. if (integrations.has(config.id) || !isConfigurableDatabaseIntegrationType(config.type)) { continue; } diff --git a/src/notebooks/deepnote/integrations/integrationManager.ts b/src/notebooks/deepnote/integrations/integrationManager.ts index 72b58dfada..a449f2f476 100644 --- a/src/notebooks/deepnote/integrations/integrationManager.ts +++ b/src/notebooks/deepnote/integrations/integrationManager.ts @@ -1,12 +1,32 @@ -import { inject, injectable } from 'inversify'; -import { commands, l10n, NotebookDocument, window, workspace } from 'vscode'; +import { inject, injectable, optional } from 'inversify'; +import { commands, l10n, NotebookDocument, ProgressLocation, QuickPickItem, window, workspace } from 'vscode'; +import { CommandOutcome, ITelemetryService } from '../../../platform/analytics/types'; +import { isCancellationError } from '../../../platform/common/cancellation'; import { IExtensionContext } from '../../../platform/common/types'; import { Commands } from '../../../platform/common/constants'; +import * as localize from '../../../platform/common/utils/localize'; import { logger } from '../../../platform/logging'; -import { IIntegrationDetector, IIntegrationManager, IIntegrationStorage, IIntegrationWebviewProvider } from './types'; -import { IDeepnoteNotebookManager } from '../../types'; +import { + IIntegrationDetector, + IIntegrationEnvLiveRefresher, + IIntegrationManager, + IIntegrationStorage, + IIntegrationWebviewProvider +} from './types'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../../types'; import { DatabaseIntegrationType, databaseIntegrationTypes } from '@deepnote/database-integrations'; +import { + attachExistingIntegration, + collectReusableIntegrations, + CollectReusableIntegrationsResult, + ReusableIntegration +} from './existingIntegrationPicker'; +import { isSnapshotFile } from '../snapshots/snapshotFiles'; + +interface ReusableIntegrationQuickPickItem extends QuickPickItem { + integration: ReusableIntegration; +} /** * Manages integration UI and commands for Deepnote notebooks @@ -18,7 +38,12 @@ export class IntegrationManager implements IIntegrationManager { @inject(IIntegrationDetector) private readonly integrationDetector: IIntegrationDetector, @inject(IIntegrationStorage) private readonly integrationStorage: IIntegrationStorage, @inject(IIntegrationWebviewProvider) private readonly webviewProvider: IIntegrationWebviewProvider, - @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(ITelemetryService) private readonly analytics: ITelemetryService, + // Node-only service: the web extension has no kernels to refresh. + @inject(IIntegrationEnvLiveRefresher) + @optional() + private readonly liveRefresher?: IIntegrationEnvLiveRefresher ) {} public activate(): void { @@ -48,6 +73,151 @@ export class IntegrationManager implements IIntegrationManager { return this.showIntegrationsUI(integrationId, notebookUri); }) ); + + // Accepts the same argument shapes as ManageIntegrations so the panel and menus can pass a notebook URI. + this.extensionContext.subscriptions.push( + commands.registerCommand(Commands.AddExistingIntegration, (...args: unknown[]) => { + let notebookUri: string | undefined; + + for (const arg of args) { + notebookUri ??= this.extractNotebookUri(arg); + } + + return this.addExistingIntegration(notebookUri); + }) + ); + } + + /** + * Offers the integrations other projects in the workspace declare and links the chosen one into this project; + * no credentials are copied (see `collectReusableIntegrations`). Public so tests can drive it without + * `commands.executeCommand`. + */ + public async addExistingIntegration(notebookUri?: string): Promise { + const activeNotebook = this.resolveDeepnoteNotebook(notebookUri); + + if (!activeNotebook) { + return 'failed'; + } + + const projectId = activeNotebook.metadata?.deepnoteProjectId; + const notebookId = activeNotebook.metadata?.deepnoteNotebookId; + + if (!projectId || !notebookId) { + void window.showErrorMessage(l10n.t('Cannot determine project or notebook ID')); + + return 'failed'; + } + + const currentIntegrations = this.getCachedProjectIntegrations(projectId, notebookId); + + if (!currentIntegrations) { + void window.showErrorMessage(localize.Integrations.addExistingIntegrationFailed); + + return 'failed'; + } + + let scan: CollectReusableIntegrationsResult; + + try { + scan = await window.withProgress( + { + cancellable: true, + location: ProgressLocation.Notification, + title: localize.Integrations.addExistingIntegrationScanning + }, + (_progress, token) => + collectReusableIntegrations({ + excludeIntegrationIds: new Set(currentIntegrations.map((entry) => entry.id)), + integrationStorage: this.integrationStorage, + projectId, + token + }) + ); + } catch (error) { + if (error instanceof Error && isCancellationError(error)) { + this.trackAddExistingIntegration('cancelled'); + + return 'cancelled'; + } + + throw error; + } + + const { conflictingIds, integrations } = scan; + + if (conflictingIds.length > 0) { + void window.showWarningMessage( + localize.Integrations.addExistingIntegrationConflictsSkipped(conflictingIds.length) + ); + } + + if (integrations.length === 0) { + void window.showInformationMessage(localize.Integrations.addExistingIntegrationNoneAvailable); + + return 'completed'; + } + + const items: ReusableIntegrationQuickPickItem[] = integrations.map((integration) => ({ + description: localize.Integrations.typeLabel(integration.type), + detail: localize.Integrations.addExistingIntegrationUsedIn(integration.projectNames.join(', ')), + integration, + label: integration.name + })); + + const picked = await window.showQuickPick(items, { + matchOnDescription: true, + matchOnDetail: true, + placeHolder: localize.Integrations.addExistingIntegrationPlaceholder + }); + + if (!picked) { + this.trackAddExistingIntegration('cancelled'); + + return 'cancelled'; + } + + const { integration } = picked; + + let outcome: CommandOutcome = 'failed'; + + try { + const { activePersisted, siblingsFailed } = await attachExistingIntegration({ + activeFileUri: activeNotebook.uri, + integration, + notebookManager: this.notebookManager, + projectId + }); + + if (activePersisted) { + outcome = 'completed'; + void window.showInformationMessage( + localize.Integrations.addExistingIntegrationSucceeded(integration.name) + ); + + if (siblingsFailed > 0) { + void window.showWarningMessage( + l10n.t( + 'Integrations saved, but {0} related notebook file(s) could not be updated.', + siblingsFailed + ) + ); + } + + // Storage did not change, so the storage-change listeners that normally refresh kernels and the + // panel after a save stay silent; do both explicitly for this project. + await this.refreshAfterProjectIntegrationsChange(projectId, notebookId, activeNotebook); + } else { + void window.showErrorMessage(localize.Integrations.addExistingIntegrationFailed); + } + } catch (error) { + logger.error(`IntegrationManager: failed to add existing integration ${integration.id}`, error); + void window.showErrorMessage(localize.Integrations.addExistingIntegrationFailed); + } + + this.trackAddExistingIntegration(outcome, integration.type); + + return outcome; } /** The notebook URI a menu contribution passed; `notebook/toolbar` sends `{ notebookEditor: { notebookUri } }`. */ @@ -62,20 +232,78 @@ export class IntegrationManager implements IIntegrationManager { return uri ? String(uri) : undefined; } + /** The integrations the project declares, or `undefined` when it is not cached at all. */ + private getCachedProjectIntegrations(projectId: string, notebookId: string): RawProjectIntegration[] | undefined { + const project = this.notebookManager.getProjectForNotebook(projectId, notebookId); + + return project ? [...(project.project.integrations ?? [])] : undefined; + } + + private async refreshAfterProjectIntegrationsChange( + projectId: string, + notebookId: string, + activeNotebook: NotebookDocument + ): Promise { + // Panel first: a busy kernel holds the refresh behind its running cell. Only a panel already open for this + // project is refreshed: run from the palette, the command must not open one and take focus from the notebook. + try { + const integrations = await this.integrationDetector.detectIntegrations({ + notebookId, + notebookUri: activeNotebook.uri, + projectId + }); + + await this.webviewProvider.refresh(projectId, integrations); + } catch (error) { + logger.error('IntegrationManager: failed to refresh the integrations panel', error); + } + + const projectNotebooks = workspace.notebookDocuments.filter( + (notebook) => notebook.notebookType === 'deepnote' && notebook.metadata?.deepnoteProjectId === projectId + ); + + try { + await this.liveRefresher?.refresh(projectNotebooks, 'integration_config'); + } catch (error) { + logger.error('IntegrationManager: failed to refresh integration env after adding an integration', error); + } + } + /** - * The Deepnote notebook to act on: `window.activeNotebookEditor` is unset until an editor is focused, so a - * restored but not yet focused notebook resolves via the menu's URI or the one visible editor instead. + * The Deepnote notebook to act on, or `undefined` after telling the user why there is none. A URI from a menu or + * the panel names the only notebook to act on: once it is closed, the focused editor may belong to another project. + * A snapshot is refused: `*.snapshot.deepnote` matches the notebook selector, but it records a past run, so the + * writer never edits one, while credentials are saved and deleted all the same. */ private resolveDeepnoteNotebook(notebookUri: string | undefined): NotebookDocument | undefined { - if (notebookUri) { - const fromUri = workspace.notebookDocuments.find( - (notebook) => notebook.notebookType === 'deepnote' && notebook.uri.toString() === notebookUri + const notebook = notebookUri + ? workspace.notebookDocuments.find( + (candidate) => candidate.notebookType === 'deepnote' && candidate.uri.toString() === notebookUri + ) + : this.findFocusedDeepnoteNotebook(); + + if (!notebook) { + void window.showErrorMessage( + notebookUri ? localize.Integrations.commandNotebookClosed : l10n.t('No active Deepnote notebook') ); - if (fromUri) { - return fromUri; - } + + return undefined; } + if (isSnapshotFile(notebook.uri)) { + void window.showErrorMessage(localize.Integrations.snapshotIntegrationsUnsupported); + + return undefined; + } + + return notebook; + } + + /** + * `window.activeNotebookEditor` is unset until an editor is focused, so a restored but not yet focused notebook + * resolves via the one visible editor instead. + */ + private findFocusedDeepnoteNotebook(): NotebookDocument | undefined { const active = window.activeNotebookEditor?.notebook; if (active?.notebookType === 'deepnote') { return active; @@ -98,7 +326,6 @@ export class IntegrationManager implements IIntegrationManager { const activeNotebook = this.resolveDeepnoteNotebook(notebookUri); if (!activeNotebook) { - void window.showErrorMessage(l10n.t('No active Deepnote notebook')); return; } @@ -162,4 +389,11 @@ export class IntegrationManager implements IIntegrationManager { activeNotebook.metadata?.deepnoteProjectName ); } + + private trackAddExistingIntegration(outcome: CommandOutcome, integrationType?: string): void { + this.analytics.trackEvent({ + eventName: 'add_existing_integration', + properties: { integrationType: integrationType ?? 'unknown', outcome } + }); + } } diff --git a/src/notebooks/deepnote/integrations/integrationManager.unit.test.ts b/src/notebooks/deepnote/integrations/integrationManager.unit.test.ts new file mode 100644 index 0000000000..731203cdab --- /dev/null +++ b/src/notebooks/deepnote/integrations/integrationManager.unit.test.ts @@ -0,0 +1,575 @@ +import type { DeepnoteFile } from '@deepnote/blocks'; +import { assert } from 'chai'; +import sinon from 'sinon'; +import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; +import { + CancellationToken, + CancellationTokenSource, + NotebookDocument, + NotebookEditor, + QuickPickItem, + Uri +} from 'vscode'; + +import { ITelemetryService } from '../../../platform/analytics/types'; +import { Commands } from '../../../platform/common/constants'; +import { IExtensionContext } from '../../../platform/common/types'; +import { createDeferred } from '../../../platform/common/utils/async'; +import { Integrations } from '../../../platform/common/utils/localize'; +import { ConfigurableDatabaseIntegrationConfig } from '../../../platform/notebooks/deepnote/integrationTypes'; +import { waitForCondition } from '../../../test/common'; +import { stubDeepnoteFiles } from '../../../test/mocks/vscodeFs'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; +import { IDeepnoteNotebookManager, ProjectIntegration, RawProjectIntegration } from '../../types'; +import { + createDeepnoteFile, + createDeepnoteProject, + createMockNotebook, + createWorkspaceFolder +} from '../deepnoteTestHelpers'; +import { buildPostgresIntegration } from './federatedAuth/federatedAuthTestHelpers'; +import { IntegrationManager } from './integrationManager'; +import { + IIntegrationDetector, + IIntegrationEnvLiveRefresher, + IIntegrationStorage, + IIntegrationWebviewProvider +} from './types'; + +const CURRENT_PROJECT_ID = 'project-current'; +const CURRENT_NOTEBOOK_ID = 'notebook-current'; +const OTHER_PROJECT_ID = 'project-other'; +const CURRENT_URI = Uri.file('/ws/current.deepnote'); +const OTHER_URI = Uri.file('/ws/other.deepnote'); +/** A second file of the CURRENT project, so the sibling-write branch has something to fail on. */ +const SIBLING_URI = Uri.file('/ws/sibling.deepnote'); +const SNAPSHOT_URI = Uri.file('/ws/snapshots/current_project-current_latest.snapshot.deepnote'); + +const SHARED_CONFIG = buildPostgresIntegration({ id: 'pg-shared', name: 'Shared Postgres' }); +/** Below mocha's 2s test timeout, so a refresh that never starts fails with its own message. */ +const REFRESH_START_TIMEOUT_MS = 1_000; + +type RefreshFn = IIntegrationEnvLiveRefresher['refresh']; +// `thenCall` checks no signature, so doubles take their parameter types from here; annotating them by hand undoes it. +type UpdateProjectIntegrationsFn = IDeepnoteNotebookManager['updateProjectIntegrationsForNotebook']; + +function projectFile(projectId: string, notebookId: string, integrations: RawProjectIntegration[]): DeepnoteFile { + return createDeepnoteFile({ + project: createDeepnoteProject({ + id: projectId, + name: projectId, + notebooks: [{ id: notebookId, name: 'Notebook', blocks: [] }], + integrations + }) + }); +} + +suite('IntegrationManager.addExistingIntegration', () => { + let currentNotebook: NotebookDocument; + let otherNotebook: NotebookDocument; + let currentProject: DeepnoteFile | undefined; + let writes: Map; + let cacheUpdates: Parameters[]; + // Typed off the interface so a signature change fails the compile, not just the `firstCall.args` assertion. + let refreshSpy: sinon.SinonSpy, ReturnType>; + let quickPickItems: QuickPickItem[] | undefined; + let scanProgress: CancellationTokenSource; + let writeFailures: Set; + + let detector: IIntegrationDetector; + let webviewProvider: IIntegrationWebviewProvider; + let notebookManager: IDeepnoteNotebookManager; + let telemetry: ITelemetryService; + let cacheUpdateError: Error | undefined; + let storedConfigs: ConfigurableDatabaseIntegrationConfig[]; + let onDiskOther: DeepnoteFile | undefined; + let onDiskSibling: DeepnoteFile | undefined; + let onDiskCurrent: DeepnoteFile | undefined; + + setup(() => { + resetVSCodeMocks(); + + currentNotebook = createMockNotebook({ + uri: CURRENT_URI, + metadata: { deepnoteProjectId: CURRENT_PROJECT_ID, deepnoteNotebookId: CURRENT_NOTEBOOK_ID } + }); + otherNotebook = createMockNotebook({ + uri: OTHER_URI, + metadata: { deepnoteProjectId: OTHER_PROJECT_ID, deepnoteNotebookId: 'notebook-other' } + }); + currentProject = projectFile(CURRENT_PROJECT_ID, CURRENT_NOTEBOOK_ID, []); + onDiskOther = projectFile(OTHER_PROJECT_ID, 'notebook-other', [ + { id: SHARED_CONFIG.id, name: SHARED_CONFIG.name, type: SHARED_CONFIG.type } + ]); + onDiskSibling = undefined; + onDiskCurrent = undefined; + cacheUpdateError = undefined; + storedConfigs = [SHARED_CONFIG]; + cacheUpdates = []; + quickPickItems = undefined; + scanProgress = new CancellationTokenSource(); + writeFailures = new Set(); + + when(mockedVSCodeNamespaces.workspace.workspaceFolders).thenReturn([createWorkspaceFolder(Uri.file('/ws'))]); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([currentNotebook, otherNotebook]); + const discovered = () => + Promise.resolve([ + CURRENT_URI, + ...(onDiskOther ? [OTHER_URI] : []), + ...(onDiskSibling ? [SIBLING_URI] : []) + ]); + + // The writer enumerates without a token; the scan passes one. + when(mockedVSCodeNamespaces.workspace.findFiles(anything())).thenCall(discovered); + when(mockedVSCodeNamespaces.workspace.findFiles(anything(), anything(), anything(), anything())).thenCall( + discovered + ); + + when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( + (_options: unknown, task: (progress: unknown, token: CancellationToken) => unknown) => + task({ report: () => undefined }, scanProgress.token) + ); + + writes = stubDeepnoteFiles( + (uri) => + new Map([ + [CURRENT_URI.fsPath, onDiskCurrent ?? currentProject], + [OTHER_URI.fsPath, onDiskOther], + [SIBLING_URI.fsPath, onDiskSibling] + ]).get(uri.fsPath), + { failWriteFor: writeFailures } + ); + + // Picks the first offered item; tests that want a cancel override this. + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenCall((items: QuickPickItem[]) => { + quickPickItems = items; + + return Promise.resolve(items[0]); + }); + + const mockDetector = mock(); + when(mockDetector.detectIntegrations(anything())).thenResolve(new Map()); + detector = mockDetector; + + webviewProvider = mock(); + when(webviewProvider.refresh(anything(), anything())).thenResolve(); + when(webviewProvider.show(anything(), anything(), anything(), anything(), anything())).thenResolve(); + + const mockManager = mock(); + when(mockManager.getProjectForNotebook(CURRENT_PROJECT_ID, CURRENT_NOTEBOOK_ID)).thenCall(() => currentProject); + const recordCacheUpdate: UpdateProjectIntegrationsFn = (projectId, notebookId, integrations) => { + if (cacheUpdateError) { + throw cacheUpdateError; + } + + cacheUpdates.push([projectId, notebookId, integrations]); + }; + when(mockManager.updateProjectIntegrationsForNotebook(anything(), anything(), anything())).thenCall( + recordCacheUpdate + ); + notebookManager = mockManager; + + telemetry = mock(); + refreshSpy = sinon.spy(async () => undefined); + }); + + teardown(() => { + scanProgress.dispose(); + }); + + function buildManager(): IntegrationManager { + const extensionContext = mock(); + when(extensionContext.subscriptions).thenReturn([]); + + const storage = mock(); + when(storage.getIntegrationConfig(anything())).thenCall((id: string) => + Promise.resolve(storedConfigs.find((config) => config.id === id)) + ); + + const liveRefresher: IIntegrationEnvLiveRefresher = { refresh: refreshSpy }; + + return new IntegrationManager( + instance(extensionContext), + instance(detector), + instance(storage), + instance(webviewProvider), + instance(notebookManager), + instance(telemetry), + liveRefresher + ); + } + + test("links the picked integration, refreshes only this project's kernels and refreshes an open panel", async () => { + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'completed'); + + const expectedIntegrations: ProjectIntegration[] = [ + { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' } + ]; + assert.deepStrictEqual(writes.get(CURRENT_URI.fsPath)?.project.integrations, expectedIntegrations); + assert.deepStrictEqual(cacheUpdates, [[CURRENT_PROJECT_ID, CURRENT_NOTEBOOK_ID, expectedIntegrations]]); + assert.isUndefined(writes.get(OTHER_URI.fsPath), 'the other project must not be rewritten'); + + assert.isTrue(refreshSpy.calledOnce); + assert.deepStrictEqual(refreshSpy.firstCall.args, [[currentNotebook], 'integration_config']); + + verify(webviewProvider.refresh(CURRENT_PROJECT_ID, anything())).once(); + // Catches: the command opening, or re-targeting, the panel when run from the palette. + verify(webviewProvider.show(anything(), anything(), anything(), anything(), anything())).never(); + + verify( + telemetry.trackEvent( + deepEqual({ + eventName: 'add_existing_integration', + properties: { integrationType: 'pgsql', outcome: 'completed' } + }) + ) + ).once(); + + assert.strictEqual(quickPickItems?.length, 1); + assert.strictEqual(quickPickItems?.[0].label, 'Shared Postgres'); + assert.strictEqual(quickPickItems?.[0].description, 'PostgreSQL'); + assert.strictEqual(quickPickItems?.[0].detail, `Used in: ${OTHER_PROJECT_ID}`); + }); + + test('refreshes an open panel before the kernel env refresh settles', async () => { + // Catches: a busy kernel leaving the panel on the pre-link list until its running cell finishes. + const refreshGate = createDeferred(); + + refreshSpy = sinon.spy(() => refreshGate.promise); + + const command = buildManager().addExistingIntegration(CURRENT_URI.toString()); + + try { + await waitForCondition(() => refreshSpy.calledOnce, REFRESH_START_TIMEOUT_MS, 'the refresh never started'); + + verify(webviewProvider.refresh(CURRENT_PROJECT_ID, anything())).once(); + } finally { + refreshGate.resolve(); + } + + assert.strictEqual(await command, 'completed'); + }); + + test('still refreshes the kernels when refreshing the panel fails', async () => { + // Catches: the kernel env refresh depending on the panel refresh succeeding. + when(webviewProvider.refresh(anything(), anything())).thenReject(new Error('panel could not be refreshed')); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'completed'); + assert.deepStrictEqual(refreshSpy.args, [[[currentNotebook], 'integration_config']]); + }); + + test('shows an information message and writes nothing when no other project has a reusable integration', async () => { + onDiskOther = undefined; + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'completed'); + assert.strictEqual(writes.size, 0); + assert.deepStrictEqual(cacheUpdates, []); + assert.isTrue(refreshSpy.notCalled); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + verify(mockedVSCodeNamespaces.window.showInformationMessage(anything())).once(); + }); + + test('does not offer an integration the current project already has', async () => { + currentProject = projectFile(CURRENT_PROJECT_ID, CURRENT_NOTEBOOK_ID, [ + { id: SHARED_CONFIG.id, name: SHARED_CONFIG.name, type: SHARED_CONFIG.type } + ]); + + await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(writes.size, 0); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + }); + + test('warns and offers nothing when the only candidate is declared with a type the stored config does not match', async () => { + onDiskOther = projectFile(OTHER_PROJECT_ID, 'notebook-other', [ + { id: SHARED_CONFIG.id, name: SHARED_CONFIG.name, type: 'mysql' } + ]); + + await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(writes.size, 0); + verify( + mockedVSCodeNamespaces.window.showWarningMessage(Integrations.addExistingIntegrationConflictsSkipped(1)) + ).once(); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + }); + + test('returns cancelled and writes nothing when the picker is dismissed', async () => { + // Catches: drop-off in the picker never reaching analytics, being sent under another outcome, or being sent + // along with a stray second event. + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenResolve(undefined); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'cancelled'); + assert.strictEqual(writes.size, 0); + assert.isTrue(refreshSpy.notCalled); + verify(telemetry.trackEvent(anything())).once(); + verify( + telemetry.trackEvent( + deepEqual({ + eventName: 'add_existing_integration', + properties: { integrationType: 'unknown', outcome: 'cancelled' } + }) + ) + ).once(); + }); + + // Every branch below reports trouble to the user; without cover they can each regress into silent success, + // or into the wrong diagnosis: the message is what tells the user which one they hit. + const earlyFailures: { arrange: () => string | undefined; expectedMessage: string; name: string }[] = [ + { + arrange: () => { + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + + return undefined; + }, + expectedMessage: 'No active Deepnote notebook', + name: 'no Deepnote notebook is open' + }, + { + arrange: () => { + // The panel stayed open for the other project after its notebook closed; only this editor is left. + const editor = mock(); + + when(editor.notebook).thenReturn(currentNotebook); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([currentNotebook]); + when(mockedVSCodeNamespaces.window.visibleNotebookEditors).thenReturn([instance(editor)]); + + return OTHER_URI.toString(); + }, + expectedMessage: Integrations.commandNotebookClosed, + name: 'the panel names a notebook that is no longer open' + }, + { + arrange: () => { + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([ + createMockNotebook({ uri: CURRENT_URI, metadata: {} }) + ]); + + return CURRENT_URI.toString(); + }, + expectedMessage: 'Cannot determine project or notebook ID', + name: 'the notebook declares no project or notebook id' + }, + { + arrange: () => { + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([ + createMockNotebook({ + uri: SNAPSHOT_URI, + metadata: { + deepnoteProjectId: CURRENT_PROJECT_ID, + deepnoteNotebookId: CURRENT_NOTEBOOK_ID + } + }) + ]); + + return SNAPSHOT_URI.toString(); + }, + expectedMessage: Integrations.snapshotIntegrationsUnsupported, + name: 'the active file is a snapshot' + }, + { + arrange: () => { + currentProject = undefined; + + return CURRENT_URI.toString(); + }, + expectedMessage: Integrations.addExistingIntegrationFailed, + name: 'the project is not in the cache' + } + ]; + + for (const { arrange, expectedMessage, name } of earlyFailures) { + test(`fails without reading or writing any file when ${name}`, async () => { + const uri = arrange(); + + const outcome = await buildManager().addExistingIntegration(uri); + + assert.strictEqual(outcome, 'failed'); + assert.strictEqual(writes.size, 0); + assert.deepStrictEqual(cacheUpdates, [], 'the cache must not move before the file does'); + verify(mockedVSCodeNamespaces.window.showErrorMessage(expectedMessage)).once(); + verify(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).never(); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + verify(telemetry.trackEvent(anything())).never(); + }); + } + + const lateFailures: { arrange: () => void; checksCache?: boolean; name: string }[] = [ + { + arrange: () => writeFailures.add(CURRENT_URI.fsPath), + checksCache: true, + name: 'the active file cannot be written' + }, + // The cache double throws before recording, so `cacheUpdates` stays empty whatever the manager does. + { arrange: () => (cacheUpdateError = new Error('cache rejected the update')), name: 'the writer throws' } + ]; + + for (const { arrange, checksCache, name } of lateFailures) { + test(`reports failure and records the outcome in telemetry when ${name}`, async () => { + arrange(); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'failed'); + + if (checksCache) { + assert.deepStrictEqual(cacheUpdates, [], 'no failure path leaves the link in the cache'); + } + + assert.isTrue(refreshSpy.notCalled, 'the refresh reserved for a successful link never runs'); + verify(mockedVSCodeNamespaces.window.showErrorMessage(Integrations.addExistingIntegrationFailed)).once(); + verify( + telemetry.trackEvent( + deepEqual({ + eventName: 'add_existing_integration', + properties: { integrationType: 'pgsql', outcome: 'failed' } + }) + ) + ).once(); + }); + } + + test('completes with a warning when a sibling file of the same project cannot be updated', async () => { + onDiskSibling = projectFile(CURRENT_PROJECT_ID, 'notebook-sibling', []); + writeFailures.add(SIBLING_URI.fsPath); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'completed'); + assert.isDefined(writes.get(CURRENT_URI.fsPath), 'the active file is still persisted'); + verify(mockedVSCodeNamespaces.window.showWarningMessage(anything())).once(); + }); + + test('keeps an entry the file gained while the picker was open, though the cache never saw it', async () => { + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenCall((items: QuickPickItem[]) => { + // Only disk moves: the cached project stays on the roster the command read before the scan. + onDiskCurrent = projectFile(CURRENT_PROJECT_ID, CURRENT_NOTEBOOK_ID, [ + { id: 'added-meanwhile', name: 'Added meanwhile', type: 'mysql' } + ]); + + return Promise.resolve(items[0]); + }); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'completed'); + assert.deepStrictEqual(writes.get(CURRENT_URI.fsPath)?.project.integrations, [ + { id: 'added-meanwhile', name: 'Added meanwhile', type: 'mysql' }, + { id: 'pg-shared', name: 'Shared Postgres', type: 'pgsql' } + ]); + }); + + test('returns cancelled and writes nothing when the scan is cancelled', async () => { + // Catches: a cancelled scan counted as neither drop-off nor failure, or reported as 'failed'. + scanProgress.cancel(); + + const outcome = await buildManager().addExistingIntegration(CURRENT_URI.toString()); + + assert.strictEqual(outcome, 'cancelled'); + assert.strictEqual(writes.size, 0); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + verify(telemetry.trackEvent(anything())).once(); + verify( + telemetry.trackEvent( + deepEqual({ + eventName: 'add_existing_integration', + properties: { integrationType: 'unknown', outcome: 'cancelled' } + }) + ) + ).once(); + }); +}); + +suite('IntegrationManager: the Manage Integrations command', () => { + let currentNotebook: NotebookDocument; + let manageIntegrations: ((...args: unknown[]) => Promise) | undefined; + let webviewProvider: IIntegrationWebviewProvider; + + setup(() => { + resetVSCodeMocks(); + + currentNotebook = createMockNotebook({ + uri: CURRENT_URI, + metadata: { deepnoteProjectId: CURRENT_PROJECT_ID, deepnoteNotebookId: CURRENT_NOTEBOOK_ID } + }); + manageIntegrations = undefined; + when(mockedVSCodeNamespaces.commands.registerCommand(anything(), anything())).thenCall( + (command: string, handler: (...args: unknown[]) => Promise) => { + if (command === Commands.ManageIntegrations) { + manageIntegrations = handler; + } + + return { dispose: () => undefined }; + } + ); + + webviewProvider = mock(); + when(webviewProvider.show(anything(), anything(), anything(), anything(), anything())).thenResolve(); + + const detector = mock(); + when(detector.detectIntegrations(anything())).thenResolve(new Map()); + + const extensionContext = mock(); + when(extensionContext.subscriptions).thenReturn([]); + + new IntegrationManager( + instance(extensionContext), + instance(detector), + instance(mock()), + instance(webviewProvider), + instance(mock()), + instance(mock()) + ).activate(); + }); + + function focus(notebook: NotebookDocument): void { + const editor = mock(); + + when(editor.notebook).thenReturn(notebook); + when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(instance(editor)); + } + + test('refuses a notebook URI that is no longer open instead of opening the panel for another notebook', async () => { + // Catches: a URI that resolves to nothing falling back to the focused editor, which re-targets the panel + // at whatever project that editor belongs to. + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([currentNotebook]); + focus(currentNotebook); + + await manageIntegrations?.({ notebookUri: OTHER_URI.toString() }); + + verify(webviewProvider.show(anything(), anything(), anything(), anything(), anything())).never(); + verify(mockedVSCodeNamespaces.window.showErrorMessage(Integrations.commandNotebookClosed)).once(); + }); + + test('refuses a focused snapshot instead of opening the panel for it', async () => { + // Catches: a panel on a snapshot saving and deleting credentials while every project-file edit is skipped. + const snapshot = createMockNotebook({ + uri: SNAPSHOT_URI, + metadata: { deepnoteProjectId: CURRENT_PROJECT_ID, deepnoteNotebookId: CURRENT_NOTEBOOK_ID } + }); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([snapshot]); + focus(snapshot); + + await manageIntegrations?.(); + + verify(webviewProvider.show(anything(), anything(), anything(), anything(), anything())).never(); + verify(mockedVSCodeNamespaces.window.showErrorMessage(Integrations.snapshotIntegrationsUnsupported)).once(); + }); + + test('opens the panel for the focused notebook when no URI is given', async () => { + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([currentNotebook]); + focus(currentNotebook); + + await manageIntegrations?.(); + + verify(webviewProvider.show(CURRENT_PROJECT_ID, anything(), CURRENT_URI, undefined, anything())).once(); + }); +}); diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index e47249afc4..e103537a0c 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -1,5 +1,5 @@ import { inject, injectable, optional } from 'inversify'; -import { commands, Disposable, l10n, Uri, ViewColumn, WebviewPanel, window } from 'vscode'; +import { commands, Disposable, l10n, Uri, ViewColumn, WebviewPanel, window, workspace } from 'vscode'; import { BigQueryAuthMethods } from '@deepnote/database-integrations'; @@ -10,9 +10,20 @@ import * as localize from '../../../platform/common/utils/localize'; import { logger } from '../../../platform/logging'; import { LocalizedMessages, SharedMessages } from '../../../messageTypes'; import { ISqlIntegrationEnvVarsProvider } from '../../../platform/notebooks/deepnote/types'; -import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types'; -import { persistProjectIntegrations } from './projectIntegrationsWriter'; -import { IFederatedAuthTokenStorage, IIntegrationStorage, IIntegrationWebviewProvider } from './types'; +import { IDeepnoteNotebookManager } from '../../types'; +import { findOtherProjectsDeclaring } from './existingIntegrationPicker'; +import { + addProjectIntegration, + PersistIntegrationsResult, + ProjectFilesParams, + removeProjectIntegration +} from './projectIntegrationsWriter'; +import { + IFederatedAuthTokenStorage, + IIntegrationEnvLiveRefresher, + IIntegrationStorage, + IIntegrationWebviewProvider +} from './types'; import { ConfigurableDatabaseIntegrationConfig, FederatedAuthTokenStatus, @@ -49,7 +60,11 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { @inject(ISqlIntegrationEnvVarsProvider) private readonly sqlIntegrationEnvVars: ISqlIntegrationEnvVarsProvider, @inject(IFederatedAuthTokenStorage) @optional() - private readonly tokenStorage?: IFederatedAuthTokenStorage + private readonly tokenStorage?: IFederatedAuthTokenStorage, + // Node-only service: the web extension has no kernels to refresh. + @inject(IIntegrationEnvLiveRefresher) + @optional() + private readonly liveRefresher?: IIntegrationEnvLiveRefresher ) { // Refresh on token-storage change so the auth pill flips without panel reload. Pushed into the extension-lifetime registry to survive panel close/reopen. if (this.tokenStorage) { @@ -63,6 +78,16 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } } + public async refresh(projectId: string, integrations: Map): Promise { + if (!this.currentPanel || this.projectId !== projectId) { + return; + } + + this.integrations = integrations; + + await this.updateWebview(); + } + /** * Show the integration management webview * @param projectId The Deepnote project ID @@ -135,7 +160,6 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { this.disposables ); - await this.sendLocStrings(); await this.updateWebview(); // If a specific integration was requested, show its configuration form @@ -171,28 +195,11 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { integrationsConfirmDeleteDetails: localize.Integrations.confirmDeleteDetails, integrationsConfigureTitle: localize.Integrations.configureTitle, integrationsAddNewIntegration: localize.Integrations.addNewIntegration, + integrationsAddExistingIntegration: localize.Integrations.addExistingIntegration, integrationsDatabase: localize.Integrations.database, integrationsDataWarehousesLakes: localize.Integrations.dataWarehousesLakes, integrationsDatabases: localize.Integrations.databases, - integrationsPostgresTypeLabel: localize.Integrations.postgresTypeLabel, - integrationsBigQueryTypeLabel: localize.Integrations.bigQueryTypeLabel, - integrationsSnowflakeTypeLabel: localize.Integrations.snowflakeTypeLabel, - integrationsAlloyDBTypeLabel: localize.Integrations.alloyDBTypeLabel, - integrationsAthenaTypeLabel: localize.Integrations.athenaTypeLabel, - integrationsClickHouseTypeLabel: localize.Integrations.clickHouseTypeLabel, - integrationsCloudSqlTypeLabel: localize.Integrations.cloudSqlTypeLabel, - integrationsDatabricksTypeLabel: localize.Integrations.databricksTypeLabel, - integrationsDremioTypeLabel: localize.Integrations.dremioTypeLabel, - integrationsMariaDBTypeLabel: localize.Integrations.mariaDBTypeLabel, - integrationsMaterializeTypeLabel: localize.Integrations.materializeTypeLabel, - integrationsMindsDBTypeLabel: localize.Integrations.mindsDBTypeLabel, - integrationsMongoDBTypeLabel: localize.Integrations.mongoDBTypeLabel, - integrationsMySQLTypeLabel: localize.Integrations.mySQLTypeLabel, - integrationsDuckDBTypeLabel: localize.Integrations.duckDBTypeLabel, - integrationsRedshiftTypeLabel: localize.Integrations.redshiftTypeLabel, - integrationsSpannerTypeLabel: localize.Integrations.spannerTypeLabel, - integrationsSQLServerTypeLabel: localize.Integrations.sqlServerTypeLabel, - integrationsTrinoTypeLabel: localize.Integrations.trinoTypeLabel, + ...localize.Integrations.typeLabels, integrationsCancel: localize.Integrations.cancel, integrationsSave: localize.Integrations.save, integrationsRequiredField: localize.Integrations.requiredField, @@ -693,6 +700,12 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { config?: ConfigurableDatabaseIntegrationConfig; }): Promise { switch (message.type) { + // Posted by the webview once its listener is attached, on every mount: anything sent before that is lost, + // including after a reload. + case SharedMessages.Started: + await this.sendLocStrings(); + await this.updateWebview(); + break; case 'configure': if (message.integrationId) { await this.showConfigurationForm(message.integrationId); @@ -744,6 +757,16 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { await this.signOutIntegration(message.integrationId); } break; + case 'addExisting': + // The command owns the picker and the integrations write, and refreshes this panel when it is done. + try { + await commands.executeCommand(Commands.AddExistingIntegration, { + notebookUri: this.activeFileUri?.toString() + }); + } catch (error) { + logger.error('IntegrationWebviewProvider: AddExistingIntegration command failed', error); + } + break; case 'authenticate': if (message.integrationId) { const integrationType = this.integrations.get(message.integrationId)?.integrationType; @@ -815,6 +838,11 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } try { + // A new integration has a fresh id no other project can declare yet. + const otherProjectNames = this.integrations.has(integrationId) + ? await this.findOtherProjectsSharing(integrationId) + : []; + // Invalidate stale federated tokens before saving (fingerprint change or auth-method switch). await this.invalidateStaleFederatedToken(integrationId, config); @@ -837,15 +865,27 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { }); } - const persisted = await this.updateProjectIntegrationsList(); + const persisted = await this.updateProjectFiles((target) => + addProjectIntegration({ + ...target, + integration: { id: integrationId, name: config.name || integrationId, type: config.type } + }) + ); await this.updateWebview(); if (persisted) { await this.currentPanel?.webview.postMessage({ - message: l10n.t('Configuration saved successfully'), + message: + otherProjectNames.length > 0 + ? localize.Integrations.integrationSavedShared(otherProjectNames.join(', ')) + : l10n.t('Configuration saved successfully'), type: 'success' }); + + // The credential save above already refreshed the kernels, possibly before this project declared a + // new integration; refresh again now that it does. + await this.refreshProjectKernels(); } // The credential save above is the tracked operation; a skipped project-YAML sync is not a failure. @@ -872,8 +912,17 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { */ private async signOutIntegration(integrationId: string): Promise { try { + const otherProjectNames = await this.findOtherProjectsSharing(integrationId); + // `delete` fires onDidChangeTokens, which re-renders the panel; no explicit updateWebview needed. await this.tokenStorage?.delete(integrationId); + + if (otherProjectNames.length > 0) { + await this.currentPanel?.webview.postMessage({ + message: localize.Integrations.integrationSignedOutShared(otherProjectNames.join(', ')), + type: 'success' + }); + } } catch (error) { logger.error('Failed to sign out integration', error); await this.currentPanel?.webview.postMessage({ @@ -892,6 +941,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } try { + const otherProjectNames = await this.findOtherProjectsSharing(integrationId); + // Token first: a failure here has to abort before the config is committed, otherwise the token is // stranded with no integration left in the panel to retry from. await this.tokenStorage?.delete(integrationId); @@ -904,18 +955,16 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { this.integrations.set(integrationId, integration); } - const persisted = await this.updateProjectIntegrationsList(); - await this.updateWebview(); - if (persisted) { - await this.currentPanel?.webview.postMessage({ - message: l10n.t('Configuration reset successfully'), - type: 'success' - }); - } + await this.currentPanel?.webview.postMessage({ + message: + otherProjectNames.length > 0 + ? localize.Integrations.integrationResetShared(otherProjectNames.join(', ')) + : l10n.t('Configuration reset successfully'), + type: 'success' + }); - // The credential reset above is the tracked operation; a skipped project-YAML sync is not a failure. return true; } catch (error) { logger.error('Failed to reset integration configuration', error); @@ -932,7 +981,8 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } /** - * Delete the integration completely (removes credentials and integration entry) + * Removes the integration from the project, and its credentials too unless another workspace project still + * declares the id: credentials are keyed by id alone, so projects that linked one integration share them. */ private async deleteConfiguration(integrationId: string): Promise { if (await this.refuseEditIfFileConfigured(integrationId)) { @@ -940,26 +990,40 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } try { - // Token first: a failure here has to abort before the config is committed, otherwise the token is - // stranded with no integration left in the panel to retry from. - await this.tokenStorage?.delete(integrationId); - await this.integrationStorage.delete(integrationId); + const otherProjectNames = await this.findOtherProjectsSharing(integrationId); + + if (otherProjectNames.length === 0) { + // Token first: a failure here has to abort before the config is committed, otherwise the token is + // stranded with no integration left in the panel to retry from. + await this.tokenStorage?.delete(integrationId); + await this.integrationStorage.delete(integrationId); + } // Remove from local state this.integrations.delete(integrationId); - const persisted = await this.updateProjectIntegrationsList(); + const persisted = await this.updateProjectFiles((target) => + removeProjectIntegration({ ...target, integrationId }) + ); await this.updateWebview(); if (persisted) { await this.currentPanel?.webview.postMessage({ - message: l10n.t('Integration deleted successfully'), + message: + otherProjectNames.length > 0 + ? localize.Integrations.integrationUnlinked(otherProjectNames.join(', ')) + : l10n.t('Integration deleted successfully'), type: 'success' }); } - // The credential delete above is the tracked operation; a skipped project-YAML sync is not a failure. + if (otherProjectNames.length > 0) { + await this.refreshProjectKernels(); + } + + // Deleting the credentials, or taking a shared integration off this project, is the tracked operation; + // a skipped project-YAML sync is not a failure. return true; } catch (error) { logger.error('Failed to delete integration', error); @@ -976,41 +1040,46 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider { } /** - * Update the project's integrations list based on current integrations + * Other workspace projects declaring `integrationId`: credentials are keyed by id alone, so they share whatever + * this panel does to its stored config or token. + */ + private async findOtherProjectsSharing(integrationId: string): Promise { + return this.projectId ? findOtherProjectsDeclaring({ integrationId, projectId: this.projectId }) : []; + } + + /** + * Re-applies integration env in this project's running kernels. A SecretStorage change triggers + * `IntegrationEnvRefreshHandler` by itself, but an edit to the project's integrations list fires no event. */ - private async updateProjectIntegrationsList(): Promise { + private async refreshProjectKernels(): Promise { + const projectNotebooks = workspace.notebookDocuments.filter( + (notebook) => + notebook.notebookType === 'deepnote' && notebook.metadata?.deepnoteProjectId === this.projectId + ); + + try { + await this.liveRefresher?.refresh(projectNotebooks, 'integration_config'); + } catch (error) { + logger.error('IntegrationWebviewProvider: failed to refresh integration env', error); + } + } + + /** + * Applies one edit to the project's `.deepnote` files and reports how it went: an error when the active file did + * not take it, a warning for the siblings that did not. Returns whether the active file took it. + */ + private async updateProjectFiles( + edit: (target: ProjectFilesParams) => Promise + ): Promise { if (!this.projectId || !this.activeFileUri) { logger.warn('IntegrationWebviewProvider: No project ID / active file available, skipping project update'); return false; } - // Build the integrations list from current integrations - const projectIntegrations: ProjectIntegration[] = Array.from(this.integrations.entries()) - .map(([id, integration]): ProjectIntegration | null => { - // Get the integration type from config or integration metadata - const type = integration.config?.type || integration.integrationType; - if (!type) { - logger.warn(`IntegrationWebviewProvider: No type found for integration ${id}, skipping`); - return null; - } - - return { - id, - name: integration.config?.name || integration.integrationName || id, - type - }; - }) - .filter((integration): integration is ProjectIntegration => integration !== null); - - logger.debug( - `IntegrationWebviewProvider: Updating project ${this.projectId} with ${projectIntegrations.length} integrations` - ); - - const { activePersisted, siblingsFailed } = await persistProjectIntegrations({ + const { activePersisted, siblingsFailed } = await edit({ + activeFileUri: this.activeFileUri, notebookManager: this.notebookManager, - projectId: this.projectId, - integrations: projectIntegrations, - activeFileUri: this.activeFileUri + projectId: this.projectId }); if (!activePersisted) { diff --git a/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts b/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts index 903759ea99..53205f09dd 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.unit.test.ts @@ -1,3 +1,4 @@ +import type { DeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; import sinon from 'sinon'; import { EventEmitter, Uri } from 'vscode'; @@ -6,16 +7,29 @@ import { anyString, anything, deepEqual, instance, mock, reset, resetCalls, veri import { ITelemetryService } from '../../../platform/analytics/types'; import { IExtensionContext, IDisposable, Resource } from '../../../platform/common/types'; import { Commands } from '../../../platform/common/constants'; +import { Integrations } from '../../../platform/common/utils/localize'; import { ISqlIntegrationEnvVarsProvider } from '../../../platform/notebooks/deepnote/types'; -import { IDeepnoteNotebookManager } from '../../types'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../../types'; +import { + createDeepnoteFile, + createDeepnoteProject, + createMockNotebook, + createWorkspaceFolder +} from '../deepnoteTestHelpers'; import { IntegrationWebviewProvider } from './integrationWebview'; -import { FederatedAuthTokenEntry, IFederatedAuthTokenStorage, IIntegrationStorage } from './types'; +import { + FederatedAuthTokenEntry, + IFederatedAuthTokenStorage, + IIntegrationEnvLiveRefresher, + IIntegrationStorage +} from './types'; import { DatabaseIntegrationConfig } from '@deepnote/database-integrations'; import { computeMetadataFingerprint } from './federatedAuth/federatedAuthTokenStorage.node'; import { ConfigurableDatabaseIntegrationConfig, DetectedIntegration } from '../../../platform/notebooks/deepnote/integrationTypes'; +import { stubDeepnoteFiles } from '../../../test/mocks/vscodeFs'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; import { buildGoogleOauthIntegration, @@ -37,6 +51,8 @@ interface FakeWebviewPanel { setPostMessageImpl: (impl: (message: CapturedMessage) => Promise) => void; } +type RefreshFn = IIntegrationEnvLiveRefresher['refresh']; + function createFakeWebviewPanel(): FakeWebviewPanel { const posted: CapturedMessage[] = []; let messageHandler: ((message: unknown) => Promise | void) | undefined; @@ -186,6 +202,7 @@ suite('IntegrationWebviewProvider', () => { function buildProvider( opts: { + liveRefresher?: IIntegrationEnvLiveRefresher; sqlIntegrationEnvVars?: ISqlIntegrationEnvVarsProvider; tokenStorage?: IFederatedAuthTokenStorage; } = {} @@ -197,7 +214,8 @@ suite('IntegrationWebviewProvider', () => { instance(mockTelemetryService), extensionSubscriptions, opts.sqlIntegrationEnvVars ?? sqlIntegrationEnvVars, - opts.tokenStorage + opts.tokenStorage, + opts.liveRefresher ); } @@ -224,6 +242,29 @@ suite('IntegrationWebviewProvider', () => { }); } + function projectFile(projectId: string, name: string, integrations: RawProjectIntegration[]): DeepnoteFile { + return createDeepnoteFile({ project: createDeepnoteProject({ id: projectId, name, integrations }) }); + } + + /** Serves `files` through `workspace.fs` and `/ws` file discovery; returns what the panel writes, by path. */ + function stubProjectFiles(files: Array<{ file: DeepnoteFile; uri: Uri }>): Map { + const onDisk = new Map(files.map(({ file, uri }) => [uri.fsPath, file] as const)); + const discovered = files.map(({ uri }) => uri); + + when(mockedVSCodeNamespaces.workspace.workspaceFolders).thenReturn([createWorkspaceFolder(Uri.file('/ws'))]); + when(mockedVSCodeNamespaces.workspace.findFiles(anything(), anything(), anything(), anything())).thenReturn( + Promise.resolve(discovered) + ); + // The writer enumerates without a token; the scan passes one. + when(mockedVSCodeNamespaces.workspace.findFiles(anything())).thenReturn(Promise.resolve(discovered)); + + return stubDeepnoteFiles((uri) => onDisk.get(uri.fsPath)); + } + + function successMessages(): CapturedMessage[] { + return fakePanel.posted.filter((message) => message.type === 'success'); + } + suite('updateWebview tokenStatus', () => { // Eligibility now comes entirely from the candidate set; `config` no longer gates the status. test('candidate but no tokenStorage → unsupported', async () => { @@ -517,6 +558,86 @@ suite('IntegrationWebviewProvider', () => { ); }); + test('handleMessage: "addExisting" → executeCommand(AddExistingIntegration, { notebookUri }) for the active file', async () => { + const executeCommandStub = sinon.stub().resolves(undefined); + when(mockedVSCodeNamespaces.commands.executeCommand(anyString(), anything())).thenCall((command, arg) => + executeCommandStub(command, arg) + ); + + const provider = buildProvider(); + await show(provider, new Map()); + + await fakePanel.onDidReceiveMessage({ type: 'addExisting' }); + + assert.isTrue( + executeCommandStub.calledOnceWithExactly(Commands.AddExistingIntegration, { + notebookUri: ACTIVE_FILE_URI.toString() + }), + "expected the command to receive the panel's active notebook so the picker targets the same project" + ); + }); + + test('answers a webview that starts up, or reloads, with the loc strings and then the list', async () => { + // Catches: loc strings posted only once, at creation, which a reloaded webview, or one whose listener attached + // late, never receives, leaving it without the Add New and Add Existing controls. + const provider = buildProvider(); + await show(provider, singleIntegrationMap('pg-1', buildPostgresIntegration({ id: 'pg-1' }))); + fakePanel.posted.length = 0; + + await fakePanel.onDidReceiveMessage({ type: 'started' }); + + assert.deepStrictEqual( + fakePanel.posted.map((message) => message.type), + ['loc_init', 'update'] + ); + }); + + suite('refresh', () => { + const PG_CONFIG = buildPostgresIntegration({ id: 'pg-1', name: 'Team Postgres' }); + const LINKED_CONFIG = buildPostgresIntegration({ id: 'pg-linked', name: 'Linked Postgres' }); + + function linkedMap(): Map { + return new Map([ + ...singleIntegrationMap(PG_CONFIG.id, PG_CONFIG), + ...singleIntegrationMap(LINKED_CONFIG.id, LINKED_CONFIG) + ]); + } + + test('opens no panel when none is open', async () => { + // Catches: a refresh after a command-palette run opening the panel and taking focus from the notebook. + await buildProvider().refresh(PROJECT_ID, linkedMap()); + + verify( + mockedVSCodeNamespaces.window.createWebviewPanel(anything(), anything(), anything(), anything()) + ).never(); + }); + + test('re-renders an open panel of the project with the new list, without revealing it', async () => { + const provider = buildProvider(); + await show(provider, singleIntegrationMap(PG_CONFIG.id, PG_CONFIG)); + const reveal = sinon.spy(); + fakePanel.panel.reveal = reveal; + + await provider.refresh(PROJECT_ID, linkedMap()); + + assert.deepStrictEqual(lastUpdate().integrations?.map((integration) => integration.id), [ + 'pg-1', + 'pg-linked' + ]); + sinon.assert.notCalled(reveal); + }); + + test('leaves a panel that shows another project alone', async () => { + const provider = buildProvider(); + await show(provider, singleIntegrationMap(PG_CONFIG.id, PG_CONFIG)); + const postedBefore = fakePanel.posted.length; + + await provider.refresh('project-other', linkedMap()); + + assert.strictEqual(fakePanel.posted.length, postedBefore); + }); + }); + suite('handleMessage: "authenticate" telemetry outcome', () => { async function authenticate(commandResult: Promise): Promise { when(mockedVSCodeNamespaces.commands.executeCommand(anyString(), anything(), anything())).thenReturn( @@ -685,6 +806,247 @@ suite('IntegrationWebviewProvider', () => { }); }); + suite('deleteConfiguration', () => { + const ALPHA_URI = Uri.file('/ws/alpha.deepnote'); + const SHARED_CONFIG = buildPostgresIntegration({ id: 'pg-shared', name: 'Shared Postgres' }); + const SHARED_ENTRY: RawProjectIntegration = { + id: SHARED_CONFIG.id, + name: SHARED_CONFIG.name, + type: SHARED_CONFIG.type + }; + + let refreshSpy: sinon.SinonSpy, ReturnType>; + let writes: Map; + + setup(() => { + refreshSpy = sinon.spy(async () => undefined); + when(integrationStorage.delete(anyString())).thenResolve(); + preStoreToken(SHARED_CONFIG.id); + }); + + /** This project's active file declares `pg-shared`; `/ws/alpha.deepnote` holds project-alpha ("Alpha"). */ + function stubActiveAndAlphaFiles(alphaIntegrations: RawProjectIntegration[]): void { + writes = stubProjectFiles([ + { file: projectFile(PROJECT_ID, 'Active', [SHARED_ENTRY]), uri: ACTIVE_FILE_URI }, + { file: projectFile('project-alpha', 'Alpha', alphaIntegrations), uri: ALPHA_URI } + ]); + } + + async function deleteSharedIntegration(): Promise { + const provider = buildProvider({ liveRefresher: { refresh: refreshSpy }, tokenStorage }); + + await show(provider, singleIntegrationMap(SHARED_CONFIG.id, SHARED_CONFIG)); + + await fakePanel.onDidReceiveMessage({ type: 'delete', integrationId: SHARED_CONFIG.id }); + } + + test('takes an integration another project declares off this project and keeps its credentials', async () => { + // Catches: deleting a linked integration wipes the credentials another project still uses. + stubActiveAndAlphaFiles([SHARED_ENTRY]); + + await deleteSharedIntegration(); + + verify(integrationStorage.delete(anything())).never(); + sinon.assert.notCalled(tokenDeleteSpy); + assert.deepStrictEqual(writes.get(ACTIVE_FILE_URI.fsPath)?.project.integrations, []); + assert.isFalse(writes.has(ALPHA_URI.fsPath), "Alpha's file must not be rewritten"); + assert.deepStrictEqual(successMessages(), [ + { message: Integrations.integrationUnlinked('Alpha'), type: 'success' } + ]); + }); + + test("refreshes only this project's kernels after taking a shared integration off it", async () => { + // Catches: an unlink fires no storage event, so this project's running kernels keep the removed + // integration's env. + stubActiveAndAlphaFiles([SHARED_ENTRY]); + const activeNotebook = createMockNotebook({ + uri: ACTIVE_FILE_URI, + metadata: { deepnoteProjectId: PROJECT_ID } + }); + const alphaNotebook = createMockNotebook({ + uri: ALPHA_URI, + metadata: { deepnoteProjectId: 'project-alpha' } + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([activeNotebook, alphaNotebook]); + + await deleteSharedIntegration(); + + sinon.assert.calledOnceWithExactly(refreshSpy, [activeNotebook], 'integration_config'); + }); + + test('deletes the credentials of an integration no other project declares', async () => { + // Catches: an over-broad guard never deletes credentials, so they leak. + stubActiveAndAlphaFiles([{ id: 'pg-other', name: 'Other Postgres', type: 'pgsql' }]); + + await deleteSharedIntegration(); + + verify(integrationStorage.delete(SHARED_CONFIG.id)).once(); + sinon.assert.calledOnceWithExactly(tokenDeleteSpy, SHARED_CONFIG.id); + assert.deepStrictEqual(successMessages(), [ + { message: 'Integration deleted successfully', type: 'success' } + ]); + sinon.assert.notCalled(refreshSpy); + }); + }); + + suite('project file edits', () => { + const PG_CONFIG = buildPostgresIntegration({ id: 'pg-1', name: 'Team Postgres' }); + const PG_ENTRY: RawProjectIntegration = { id: PG_CONFIG.id, name: PG_CONFIG.name, type: PG_CONFIG.type }; + // The panel lists none of these: one was linked after it opened, and it cannot configure the other types. + const UNLISTED: RawProjectIntegration[] = [ + { id: 'pg-linked', name: 'Linked Postgres', type: 'pgsql' }, + { id: 'duckdb', name: 'DuckDB', type: 'pandas-dataframe' }, + { id: 'future', name: 'Unknown to this build', type: 'some-future-type' } + ]; + + let refreshSpy: sinon.SinonSpy, ReturnType>; + let writes: Map; + + setup(() => { + refreshSpy = sinon.spy(async () => undefined); + when(integrationStorage.save(anything())).thenResolve(); + when(integrationStorage.delete(anyString())).thenResolve(); + writes = stubProjectFiles([ + { file: projectFile(PROJECT_ID, 'Active', [PG_ENTRY, ...UNLISTED]), uri: ACTIVE_FILE_URI } + ]); + }); + + async function showPanelListingOnly(config: ConfigurableDatabaseIntegrationConfig): Promise { + const provider = buildProvider({ liveRefresher: { refresh: refreshSpy }, tokenStorage }); + + await show(provider, singleIntegrationMap(config.id, config)); + } + + test('a save rewrites the saved entry in place and keeps every entry the panel does not list', async () => { + // Catches: the panel's own list stamped over the file, deleting what it never listed. + await showPanelListingOnly(PG_CONFIG); + const renamed = { ...PG_CONFIG, name: 'Renamed Postgres' }; + + await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: renamed.id, config: renamed }); + + assert.deepStrictEqual(writes.get(ACTIVE_FILE_URI.fsPath)?.project.integrations, [ + { id: 'pg-1', name: 'Renamed Postgres', type: 'pgsql' }, + ...UNLISTED + ]); + }); + + test('a delete takes only the deleted entry off the file', async () => { + await showPanelListingOnly(PG_CONFIG); + + await fakePanel.onDidReceiveMessage({ type: 'delete', integrationId: PG_CONFIG.id }); + + assert.deepStrictEqual(writes.get(ACTIVE_FILE_URI.fsPath)?.project.integrations, UNLISTED); + }); + + test('a reset clears the credentials and leaves the project file alone', async () => { + await showPanelListingOnly(PG_CONFIG); + + await fakePanel.onDidReceiveMessage({ type: 'reset', integrationId: PG_CONFIG.id }); + + verify(integrationStorage.delete(PG_CONFIG.id)).once(); + assert.strictEqual(writes.size, 0); + assert.deepStrictEqual(successMessages(), [ + { message: 'Configuration reset successfully', type: 'success' } + ]); + }); + + test("a save refreshes this project's kernels once the file declares the saved integration", async () => { + // Catches: the refresh the credential save triggers reading the list before it holds a new integration, + // with nothing refreshing the kernels again once it does. + const activeNotebook = createMockNotebook({ + metadata: { deepnoteProjectId: PROJECT_ID }, + uri: ACTIVE_FILE_URI + }); + const added = buildPostgresIntegration({ id: 'pg-new', name: 'New Postgres' }); + let declaredAtRefresh: RawProjectIntegration[] | undefined; + + refreshSpy = sinon.spy(async () => { + declaredAtRefresh = writes.get(ACTIVE_FILE_URI.fsPath)?.project.integrations; + }); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([activeNotebook]); + await showPanelListingOnly(PG_CONFIG); + + await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: added.id, config: added }); + + sinon.assert.calledOnceWithExactly(refreshSpy, [activeNotebook], 'integration_config'); + assert.deepStrictEqual(declaredAtRefresh, [ + PG_ENTRY, + ...UNLISTED, + { id: 'pg-new', name: 'New Postgres', type: 'pgsql' } + ]); + }); + }); + + suite('credentials another project shares', () => { + const ALPHA_URI = Uri.file('/ws/alpha.deepnote'); + const SHARED_CONFIG = buildGoogleOauthIntegration({ id: 'bq-shared', name: 'Shared BigQuery' }); + const SHARED_ENTRY: RawProjectIntegration = { + id: SHARED_CONFIG.id, + name: SHARED_CONFIG.name, + type: SHARED_CONFIG.type + }; + + setup(() => { + when(integrationStorage.save(anything())).thenResolve(); + when(integrationStorage.delete(anyString())).thenResolve(); + preStoreToken(SHARED_CONFIG.id); + }); + + /** This project's active file declares `bq-shared`; `/ws/alpha.deepnote` holds project-alpha ("Alpha"). */ + async function showSharedIntegration(alphaIntegrations: RawProjectIntegration[]): Promise { + stubProjectFiles([ + { file: projectFile(PROJECT_ID, 'Active', [SHARED_ENTRY]), uri: ACTIVE_FILE_URI }, + { file: projectFile('project-alpha', 'Alpha', alphaIntegrations), uri: ALPHA_URI } + ]); + + await show(buildProvider({ tokenStorage }), singleIntegrationMap(SHARED_CONFIG.id, SHARED_CONFIG)); + } + + test('a reset still clears shared credentials, and names the projects that lose them', async () => { + await showSharedIntegration([SHARED_ENTRY]); + + await fakePanel.onDidReceiveMessage({ type: 'reset', integrationId: SHARED_CONFIG.id }); + + verify(integrationStorage.delete(SHARED_CONFIG.id)).once(); + sinon.assert.calledOnceWithExactly(tokenDeleteSpy, SHARED_CONFIG.id); + assert.deepStrictEqual(successMessages(), [ + { message: Integrations.integrationResetShared('Alpha'), type: 'success' } + ]); + }); + + test('a sign-out names the projects signed out along with this one', async () => { + await showSharedIntegration([SHARED_ENTRY]); + + await fakePanel.onDidReceiveMessage({ type: 'signOut', integrationId: SHARED_CONFIG.id }); + + sinon.assert.calledOnceWithExactly(tokenDeleteSpy, SHARED_CONFIG.id); + assert.deepStrictEqual(successMessages(), [ + { message: Integrations.integrationSignedOutShared('Alpha'), type: 'success' } + ]); + }); + + test('a sign-out no other project shares posts no message', async () => { + await showSharedIntegration([]); + + await fakePanel.onDidReceiveMessage({ type: 'signOut', integrationId: SHARED_CONFIG.id }); + + sinon.assert.calledOnceWithExactly(tokenDeleteSpy, SHARED_CONFIG.id); + assert.deepStrictEqual(successMessages(), []); + }); + + test('a save of shared credentials names the projects it also changes them for', async () => { + await showSharedIntegration([SHARED_ENTRY]); + const changed = { ...SHARED_CONFIG, name: 'Renamed BigQuery' }; + + await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: changed.id, config: changed }); + + assert.deepStrictEqual(successMessages(), [ + { message: Integrations.integrationSavedShared('Alpha'), type: 'success' } + ]); + }); + }); + test('saveConfiguration: deletes the token BEFORE save when fingerprint changes', async () => { const integrationId = 'bq-save-fp'; const integrationSaveSpy = sinon.spy(); @@ -877,49 +1239,24 @@ suite('IntegrationWebviewProvider', () => { assert.isEmpty(updateMessages, 'no `update` postMessage should be issued after the panel disposes mid-update'); }); - suite('project integrations list update (via save message)', () => { - async function callUpdateProjectIntegrationsList(provider: IntegrationWebviewProvider): Promise { - when(integrationStorage.save(anything())).thenResolve(); - - const pgConfig = buildPostgresIntegration({ id: 'pg-1' }); - // `show()` seeds projectId + the integrations map; the `save` message drives the cache update through the real handler. - await show(provider, singleIntegrationMap('pg-1', pgConfig)); - - await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: 'pg-1', config: pgConfig }); - } - - test('updates the cached project integrations via notebookManager.updateProjectIntegrations', async () => { - const updateProjectIntegrationsSpy = sinon.spy((_projectId: string, _integrations: unknown[]) => true); - when(notebookManager.updateProjectIntegrations(anyString(), anything())).thenCall( - updateProjectIntegrationsSpy - ); - - const provider = buildProvider({ tokenStorage }); - await callUpdateProjectIntegrationsList(provider); + test('a save that cannot update the notebook file says so instead of reporting success', async () => { + const errors: string[] = []; + when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenCall((msg: string) => { + errors.push(msg); - sinon.assert.calledOnce(updateProjectIntegrationsSpy); - sinon.assert.calledWith(updateProjectIntegrationsSpy, PROJECT_ID); + return Promise.resolve(undefined); }); + when(integrationStorage.save(anything())).thenResolve(); + // No file is served, so the active one cannot be read, let alone rewritten. + stubProjectFiles([]); - test('shows a "project not found" error when no cached entry was updated', async () => { - const errors: string[] = []; - when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenCall((msg: string) => { - errors.push(msg); - - return Promise.resolve(undefined); - }); + const provider = buildProvider({ tokenStorage }); + const pgConfig = buildPostgresIntegration({ id: 'pg-1' }); + await show(provider, singleIntegrationMap('pg-1', pgConfig)); - // updateProjectIntegrations returns false → no cached entry for the project → error. - when(notebookManager.updateProjectIntegrations(anyString(), anything())).thenReturn(false); + await fakePanel.onDidReceiveMessage({ type: 'save', integrationId: 'pg-1', config: pgConfig }); - const provider = buildProvider({ tokenStorage }); - await callUpdateProjectIntegrationsList(provider); - - assert.strictEqual( - errors.length, - 1, - 'project-not-found error should show when no cached entry was updated' - ); - }); + assert.deepStrictEqual(errors, ['Failed to save integrations to the notebook file. Please try again.']); + assert.deepStrictEqual(successMessages(), []); }); }); diff --git a/src/notebooks/deepnote/integrations/projectIntegrationsWriter.ts b/src/notebooks/deepnote/integrations/projectIntegrationsWriter.ts index 6822a22fa8..5485a5f8d0 100644 --- a/src/notebooks/deepnote/integrations/projectIntegrationsWriter.ts +++ b/src/notebooks/deepnote/integrations/projectIntegrationsWriter.ts @@ -1,10 +1,13 @@ import { serializeDeepnoteFile } from '@deepnote/blocks'; import { RelativePattern, Uri, workspace } from 'vscode'; -import { flushNotebookDocumentIfDirty } from '../../../platform/deepnote/deepnoteDocumentFlush'; +import { + findOpenNotebookDocument, + flushNotebookDocumentIfDirty +} from '../../../platform/deepnote/deepnoteDocumentFlush'; import { readDeepnoteProjectFile } from '../../../platform/deepnote/deepnoteProjectFileReader'; import { logger } from '../../../platform/logging'; -import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../../types'; import { isSnapshotFile } from '../snapshots/snapshotFiles'; export interface PersistIntegrationsResult { @@ -12,32 +15,83 @@ export interface PersistIntegrationsResult { siblingsFailed: number; } -export interface PersistProjectIntegrationsParams { +/** The project whose `.deepnote` files an edit goes to, starting with the one the user acted in. */ +export interface ProjectFilesParams { + activeFileUri: Uri; notebookManager: IDeepnoteNotebookManager; projectId: string; - integrations: ProjectIntegration[]; - activeFileUri: Uri; +} + +export interface AddProjectIntegrationParams extends ProjectFilesParams { + integration: RawProjectIntegration; +} + +export interface RemoveProjectIntegrationParams extends ProjectFilesParams { + integrationId: string; +} + +interface WriteSiblingFilesParams extends ProjectFilesParams { + resolve: ResolveIntegrations; } interface WriteIntegrationsToFileParams { fileUri: Uri; + notebookManager: IDeepnoteNotebookManager; projectId: string; - integrations: ProjectIntegration[]; + resolve: ResolveIntegrations; } -type IntegrationWriteOutcome = 'failed' | 'skipped' | 'written'; +/** Builds the array to write from the one the file holds, read a step earlier in the same function. */ +type ResolveIntegrations = (existing: RawProjectIntegration[]) => RawProjectIntegration[]; -/** Writes `integrations` to the active file and every on-disk sibling; `activePersisted` reflects disk truth, not the cache. */ -export async function persistProjectIntegrations( - params: PersistProjectIntegrationsParams -): Promise { - const { notebookManager, projectId, integrations, activeFileUri } = params; +type IntegrationWriteStatus = 'failed' | 'skipped' | 'written'; + +/** + * Adds one integration to the project; an id a file already declares is replaced where it stands. Like every edit + * here it merges into what each file holds instead of replacing the list, so a caller holding a list across a prompt + * cannot delete entries written while it waited, nor ones it never listed. + */ +export function addProjectIntegration(params: AddProjectIntegrationParams): Promise { + const { integration, ...target } = params; + + return editProjectFiles(target, (existing) => + existing.some((entry) => entry.id === integration.id) + ? existing.map((entry) => (entry.id === integration.id ? integration : entry)) + : [...existing, integration] + ); +} + +/** Takes one integration off the project, leaving every other entry each file holds as it is. */ +export function removeProjectIntegration(params: RemoveProjectIntegrationParams): Promise { + const { integrationId, ...target } = params; + + return editProjectFiles(target, (existing) => existing.filter((entry) => entry.id !== integrationId)); +} - // Refresh the cache first so live env/kernel behavior stays correct even if a disk write fails. - notebookManager.updateProjectIntegrations(projectId, integrations); +/** Applies `resolve` to the active file, then to every sibling; `activePersisted` reflects disk truth, not the cache. */ +async function editProjectFiles( + params: ProjectFilesParams, + resolve: ResolveIntegrations +): Promise { + const { activeFileUri, notebookManager, projectId } = params; // findFiles only covers open folders, so write the active file explicitly (no open folder / out-of-workspace). - const activeOutcome = await writeIntegrationsToFile({ fileUri: activeFileUri, projectId, integrations }); + const status = await writeIntegrationsToFile({ fileUri: activeFileUri, notebookManager, projectId, resolve }); + + // The active file is the mandate for the sweep. `skipped` means it is a snapshot or another project's file, and + // `failed` means the edit landed nowhere — stamping the project's other files on its behalf is data loss either way. + if (status !== 'written') { + return { activePersisted: false, siblingsFailed: 0 }; + } + + const siblingsFailed = await writeSiblingFiles({ activeFileUri, notebookManager, projectId, resolve }); + + return { activePersisted: true, siblingsFailed }; +} + +/** Writes every discovered `.deepnote` of the project except `activeFileUri`; returns how many failed. */ +async function writeSiblingFiles(params: WriteSiblingFilesParams): Promise { + const { activeFileUri, notebookManager, projectId, resolve } = params; const visited = new Set([activeFileUri.toString()]); let siblingsFailed = 0; @@ -48,7 +102,7 @@ export async function persistProjectIntegrations( try { files = await workspace.findFiles(new RelativePattern(workspaceFolder, '**/*.deepnote')); } catch (error) { - logger.error('persistProjectIntegrations: failed to enumerate .deepnote files', error); + logger.error('projectIntegrationsWriter: failed to enumerate .deepnote files', error); continue; } @@ -60,23 +114,29 @@ export async function persistProjectIntegrations( visited.add(fileUri.toString()); - if ((await writeIntegrationsToFile({ fileUri, projectId, integrations })) === 'failed') { + const status = await writeIntegrationsToFile({ fileUri, notebookManager, projectId, resolve }); + if (status === 'failed') { siblingsFailed++; } } } - return { activePersisted: activeOutcome === 'written', siblingsFailed }; + return siblingsFailed; } -/** Returns `'skipped'` (snapshot / other project), `'failed'`, or `'written'` for one `.deepnote` file. */ -async function writeIntegrationsToFile(params: WriteIntegrationsToFileParams): Promise { - const { fileUri, projectId, integrations } = params; +/** + * Writes one `.deepnote` file, and moves the cache entry of the notebook open from it to the same list: `skipped` + * for a snapshot or another project's file. + */ +async function writeIntegrationsToFile(params: WriteIntegrationsToFileParams): Promise { + const { fileUri, notebookManager, projectId, resolve } = params; if (isSnapshotFile(fileUri)) { return 'skipped'; } + let integrations: RawProjectIntegration[]; + try { let projectData = await readDeepnoteProjectFile(fileUri); @@ -86,7 +146,7 @@ async function writeIntegrationsToFile(params: WriteIntegrationsToFileParams): P // Flush an open dirty file and re-read, so live cell edits aren't clobbered by the watcher reload. if (!(await flushNotebookDocumentIfDirty(fileUri))) { - logger.warn(`persistProjectIntegrations: ${fileUri.path} — unsaved edits could not be saved`); + logger.warn(`projectIntegrationsWriter: ${fileUri.path} — unsaved edits could not be saved`); return 'failed'; } @@ -99,6 +159,8 @@ async function writeIntegrationsToFile(params: WriteIntegrationsToFileParams): P } // Rewrite ONLY integrations; every other field round-trips from disk, so saved cells are untouched. + integrations = resolve(projectData.project.integrations ?? []); + projectData.project.integrations = integrations; if (!projectData.metadata) { @@ -108,11 +170,19 @@ async function writeIntegrationsToFile(params: WriteIntegrationsToFileParams): P projectData.metadata.modifiedAt = new Date().toISOString(); await workspace.fs.writeFile(fileUri, new TextEncoder().encode(serializeDeepnoteFile(projectData))); - - return 'written'; } catch (error) { - logger.error(`persistProjectIntegrations: failed to update ${fileUri.path}`, error); + logger.error(`projectIntegrationsWriter: failed to update ${fileUri.path}`, error); return 'failed'; } + + // Saving the notebook rebuilds this whole file from its cache entry, so the entry has to hold this file's list: + // another file's list would overwrite the entries only this one declares on that save. + const notebookId = findOpenNotebookDocument(fileUri)?.metadata?.deepnoteNotebookId; + + if (notebookId) { + notebookManager.updateProjectIntegrationsForNotebook(projectId, notebookId, integrations); + } + + return 'written'; } diff --git a/src/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.ts b/src/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.ts index ff26585630..672cff748f 100644 --- a/src/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.ts +++ b/src/notebooks/deepnote/integrations/projectIntegrationsWriter.unit.test.ts @@ -1,28 +1,34 @@ -import { deserializeDeepnoteFile, serializeDeepnoteFile, type DeepnoteFile } from '@deepnote/blocks'; +import type { DeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; -import { anything, instance, mock, when } from 'ts-mockito'; -import { Uri, workspace, type NotebookDocument } from 'vscode'; +import { anything, when } from 'ts-mockito'; +import { Uri, type NotebookDocument } from 'vscode'; +import { stubDeepnoteFiles } from '../../../test/mocks/vscodeFs'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; -import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types'; +import { ProjectIntegration, RawProjectIntegration } from '../../types'; +import { DeepnoteNotebookManager } from '../deepnoteNotebookManager'; import { createDeepnoteBlock, createDeepnoteFile, createDeepnoteNotebook, createDeepnoteProject, + createMockNotebook, createWorkspaceFolder } from '../deepnoteTestHelpers'; -import { persistProjectIntegrations } from './projectIntegrationsWriter'; +import { addProjectIntegration, removeProjectIntegration } from './projectIntegrationsWriter'; const PROJECT_ID = 'project-1'; -const NEW_INTEGRATIONS: ProjectIntegration[] = [{ id: 'int-new', name: 'New BigQuery', type: 'big-query' }]; - -function projectFile(notebookId: string, projectId: string = PROJECT_ID): DeepnoteFile { +function projectFile( + notebookId: string, + projectId: string = PROJECT_ID, + integrations: RawProjectIntegration[] = [] +): DeepnoteFile { return createDeepnoteFile({ metadata: { createdAt: '2020-01-01T00:00:00Z', modifiedAt: '2021-01-01T00:00:00Z' }, project: createDeepnoteProject({ id: projectId, + integrations, name: 'Proj', notebooks: [ createDeepnoteNotebook({ @@ -34,13 +40,21 @@ function projectFile(notebookId: string, projectId: string = PROJECT_ID): Deepno }); } +/** A notebook open from `uri`; an `onSave` makes it dirty, and mutating the on-disk map models its save. */ +interface OpenNotebook { + notebookId: string; + onSave?: (onDisk: Map) => void; + uri: Uri; +} + /** Wires workspace.fs/findFiles; `discovered` is decoupled from the active file to model out-of-folder writes. */ function stubWorkspace(opts: { onDisk: Array<{ uri: Uri; file: DeepnoteFile }>; discovered: Uri[]; - dirtyDocuments?: Array<{ uri: Uri; onSave: (onDisk: Map) => void }>; + openNotebooks?: OpenNotebook[]; hasWorkspaceFolder?: boolean; failWriteFor?: Set; + onWrite?: (uri: Uri) => void; }): { writes: Map } { when(mockedVSCodeNamespaces.workspace.workspaceFolders).thenReturn( opts.hasWorkspaceFolder === false ? undefined : [createWorkspaceFolder(Uri.file('/ws'))] @@ -48,82 +62,126 @@ function stubWorkspace(opts: { when(mockedVSCodeNamespaces.workspace.findFiles(anything())).thenReturn(Promise.resolve(opts.discovered)); const byPath = new Map(opts.onDisk.map((entry) => [entry.uri.fsPath, entry.file] as const)); - const writes = new Map(); - - // A save() that mutates the on-disk map models a dirty open document being flushed before the re-read. - const documents = (opts.dirtyDocuments ?? []).map( - ({ uri, onSave }) => - ({ - uri, - isDirty: true, - notebookType: 'deepnote', - save: async () => { - onSave(byPath); - - return true; - } - }) as unknown as NotebookDocument - ); - when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn(documents); - const mockFs = mock(); - - when(mockFs.readFile(anything())).thenCall((uri: Uri) => { - const file = byPath.get(uri.fsPath); + const documents = (opts.openNotebooks ?? []).map(({ notebookId, onSave, uri }): NotebookDocument => { + const notebook = createMockNotebook({ + metadata: { deepnoteNotebookId: notebookId, deepnoteProjectId: PROJECT_ID }, + uri + }); - return file - ? Promise.resolve(new TextEncoder().encode(serializeDeepnoteFile(file))) - : Promise.reject(new Error(`no readFile stub for ${uri.fsPath}`)); + return onSave + ? { + ...notebook, + isDirty: true, + save: async () => { + onSave(byPath); + + return true; + } + } + : notebook; }); - when(mockFs.writeFile(anything(), anything())).thenCall((uri: Uri, bytes: Uint8Array) => { - if (opts.failWriteFor?.has(uri.fsPath)) { - return Promise.reject(new Error(`write failed for ${uri.fsPath}`)); - } - - writes.set(uri.fsPath, deserializeDeepnoteFile(new TextDecoder().decode(bytes))); - return Promise.resolve(); - }); - when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn(documents); - return { writes }; + return { + writes: stubDeepnoteFiles((uri) => byPath.get(uri.fsPath), { + failWriteFor: opts.failWriteFor, + onWrite: opts.onWrite + }) + }; } -suite('persistProjectIntegrations', () => { - let managerInstance: IDeepnoteNotebookManager; +suite('addProjectIntegration', () => { + const ADDED: ProjectIntegration = { id: 'int-added', name: 'Added Postgres', type: 'pgsql' }; + + let managerInstance: DeepnoteNotebookManager; setup(() => { resetVSCodeMocks(); - const mockManager = mock(); - when(mockManager.updateProjectIntegrations(anything(), anything())).thenReturn(true); - managerInstance = instance(mockManager); + managerInstance = new DeepnoteNotebookManager(); }); - test('writes the active file AND every discovered sibling of the project (disk-driven, not open-state driven)', async () => { + function cachedIntegrations(notebookId: string): RawProjectIntegration[] | undefined { + return managerInstance.getProjectForNotebook(PROJECT_ID, notebookId)?.project.integrations; + } + + test('merges into each file rather than stamping one roster over them all', async () => { const activeUri = Uri.file('/ws/active.deepnote'); const siblingUri = Uri.file('/ws/sibling.deepnote'); + const ownEntry: ProjectIntegration = { id: 'int-own', name: 'Own BigQuery', type: 'big-query' }; const { writes } = stubWorkspace({ onDisk: [ { uri: activeUri, file: projectFile('nb-active') }, - { uri: siblingUri, file: projectFile('nb-sibling') } + { uri: siblingUri, file: projectFile('nb-sibling', PROJECT_ID, [ownEntry]) } ], discovered: [activeUri, siblingUri] }); - const result = await persistProjectIntegrations({ + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); - assert.deepStrictEqual(writes.get(activeUri.fsPath)!.project.integrations, NEW_INTEGRATIONS); - assert.deepStrictEqual(writes.get(siblingUri.fsPath)!.project.integrations, NEW_INTEGRATIONS); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [ADDED]); + assert.deepStrictEqual( + writes.get(siblingUri.fsPath)?.project.integrations, + [ownEntry, ADDED], + "the sibling's own entry survives an add driven from another file" + ); + }); + + test('replaces an id the file already declares instead of duplicating it', async () => { + const activeUri = Uri.file('/ws/active.deepnote'); + const { writes } = stubWorkspace({ + onDisk: [ + { + uri: activeUri, + file: projectFile('nb-active', PROJECT_ID, [{ ...ADDED, name: 'Stale name' }]) + } + ], + discovered: [activeUri] + }); + + await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [ADDED]); + }); + + test('keeps a replaced entry where it was', async () => { + const activeUri = Uri.file('/ws/active.deepnote'); + const before: ProjectIntegration = { id: 'int-before', name: 'Before', type: 'mysql' }; + const after: ProjectIntegration = { id: 'int-after', name: 'After', type: 'mysql' }; + const { writes } = stubWorkspace({ + onDisk: [ + { + uri: activeUri, + file: projectFile('nb-active', PROJECT_ID, [before, { ...ADDED, name: 'Stale name' }, after]) + } + ], + discovered: [activeUri] + }); + + await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [before, ADDED, after]); }); - test('no workspace folder: still writes the ACTIVE file and reports activePersisted (regression: cache-only success wrote nothing to disk)', async () => { + test('writes the active file even with no workspace folder open', async () => { const activeUri = Uri.file('/loose/foo.deepnote'); const { writes } = stubWorkspace({ onDisk: [{ uri: activeUri, file: projectFile('nb-active') }], @@ -131,18 +189,18 @@ suite('persistProjectIntegrations', () => { hasWorkspaceFolder: false }); - const result = await persistProjectIntegrations({ + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); - assert.isTrue(result.activePersisted, 'the active file must be written even with no workspace folder'); - assert.deepStrictEqual(writes.get(activeUri.fsPath)!.project.integrations, NEW_INTEGRATIONS); + assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [ADDED]); }); - test('active file outside every workspace folder is still written (findFiles never returns it)', async () => { + test('writes an active file outside every workspace folder, which discovery never returns', async () => { const activeUri = Uri.file('/other/foo.deepnote'); const wsSibling = Uri.file('/ws/sibling.deepnote'); const { writes } = stubWorkspace({ @@ -153,18 +211,19 @@ suite('persistProjectIntegrations', () => { discovered: [wsSibling] }); - const result = await persistProjectIntegrations({ + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); - assert.isTrue(result.activePersisted); - assert.isTrue(writes.has(activeUri.fsPath), 'the active file outside the folder must be written'); + assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [ADDED]); + assert.deepStrictEqual(writes.get(wsSibling.fsPath)?.project.integrations, [ADDED]); }); - test('a sibling whose write rejects yields siblingsFailed=1 while activePersisted stays true', async () => { + test('counts a sibling whose write fails, while the active file still lands', async () => { const activeUri = Uri.file('/ws/active.deepnote'); const badSibling = Uri.file('/ws/bad.deepnote'); const { writes } = stubWorkspace({ @@ -176,11 +235,11 @@ suite('persistProjectIntegrations', () => { failWriteFor: new Set([badSibling.fsPath]) }); - const result = await persistProjectIntegrations({ + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 1 }); @@ -188,26 +247,38 @@ suite('persistProjectIntegrations', () => { assert.isFalse(writes.has(badSibling.fsPath)); }); - test('a failed ACTIVE-file write reports activePersisted=false (regression: no false success)', async () => { + test('skips a sibling whose flush swaps its on-disk project for another one', async () => { const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); const { writes } = stubWorkspace({ - onDisk: [{ uri: activeUri, file: projectFile('nb-active') }], - discovered: [activeUri], - failWriteFor: new Set([activeUri.fsPath]) + onDisk: [ + { uri: activeUri, file: projectFile('nb-active') }, + { uri: siblingUri, file: projectFile('nb-sibling') } + ], + discovered: [activeUri, siblingUri], + // The open document for the sibling is stale: saving it rewrites the file to a DIFFERENT project. + openNotebooks: [ + { + notebookId: 'nb-sibling', + onSave: (onDisk) => onDisk.set(siblingUri.fsPath, projectFile('nb-other', 'project-2')), + uri: siblingUri + } + ] }); - const result = await persistProjectIntegrations({ + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); - assert.isFalse(result.activePersisted, 'a failed active-file write must NOT report success'); - assert.isFalse(writes.has(activeUri.fsPath)); + assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, [ADDED]); + assert.isFalse(writes.has(siblingUri.fsPath), 'the integration must not be written into the other project'); }); - test('a sibling whose flush swaps its on-disk project is skipped, not written to the wrong project', async () => { + test('writes nothing and leaves the cache untouched when the active file cannot be written', async () => { const activeUri = Uri.file('/ws/active.deepnote'); const siblingUri = Uri.file('/ws/sibling.deepnote'); const { writes } = stubWorkspace({ @@ -216,24 +287,252 @@ suite('persistProjectIntegrations', () => { { uri: siblingUri, file: projectFile('nb-sibling') } ], discovered: [activeUri, siblingUri], - // The open document for the sibling is stale: saving it rewrites the file to a DIFFERENT project. - dirtyDocuments: [ + failWriteFor: new Set([activeUri.fsPath]), + openNotebooks: [ + { notebookId: 'nb-active', uri: activeUri }, + { notebookId: 'nb-sibling', uri: siblingUri } + ] + }); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-active', projectFile('nb-active')); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-sibling', projectFile('nb-sibling')); + + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(result, { activePersisted: false, siblingsFailed: 0 }); + assert.strictEqual(writes.size, 0, 'the add reaches no file once the one the user acted in refused it'); + assert.deepStrictEqual( + cachedIntegrations('nb-active'), + [], + 'nothing reaches the cache that did not reach the disk' + ); + assert.deepStrictEqual(cachedIntegrations('nb-sibling'), []); + }); + + test('sweeps no sibling when the active file is skipped rather than written', async () => { + // A snapshot records a past run, so its own write is skipped; sweeping from one stamps the integration + // across the project's real files on behalf of a file that never took it. + const snapshotUri = Uri.file('/ws/snapshots/proj_project-1_latest.snapshot.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + const { writes } = stubWorkspace({ + onDisk: [{ uri: siblingUri, file: projectFile('nb-sibling') }], + discovered: [siblingUri], + openNotebooks: [{ notebookId: 'nb-sibling', uri: siblingUri }] + }); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-sibling', projectFile('nb-sibling')); + + const result = await addProjectIntegration({ + activeFileUri: snapshotUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(result, { activePersisted: false, siblingsFailed: 0 }); + assert.strictEqual(writes.size, 0, 'the project files stay as they are'); + assert.deepStrictEqual(cachedIntegrations('nb-sibling'), []); + }); + + test('moves the cache before sweeping siblings, so a save mid-sweep cannot revert the add', async () => { + const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + // Saving a notebook rebuilds its file from the cached project, so anything written while the cache still + // holds the old list gets undone. Ordering is the only thing keeping that window shut. + let activeCacheAtSiblingWrite: RawProjectIntegration[] | undefined; + const { writes } = stubWorkspace({ + onDisk: [ + { uri: activeUri, file: projectFile('nb-active') }, + { uri: siblingUri, file: projectFile('nb-sibling') } + ], + discovered: [activeUri, siblingUri], + onWrite: (uri) => { + if (uri.fsPath === siblingUri.fsPath) { + activeCacheAtSiblingWrite = cachedIntegrations('nb-active'); + } + }, + openNotebooks: [{ notebookId: 'nb-active', uri: activeUri }] + }); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-active', projectFile('nb-active')); + + await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(activeCacheAtSiblingWrite, [ADDED]); + assert.deepStrictEqual(writes.get(siblingUri.fsPath)?.project.integrations, [ADDED]); + }); + + test("moves each open notebook's cache to what its own file now declares", async () => { + // Catches: every cached notebook of the project stamped with the active file's list, which a later save of + // a sibling then writes over the entries only that sibling declares. + const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + const ownEntry: ProjectIntegration = { id: 'int-own', name: 'Own BigQuery', type: 'big-query' }; + const siblingEntry: ProjectIntegration = { id: 'int-sibling', name: 'Sibling MySQL', type: 'mysql' }; + stubWorkspace({ + onDisk: [ + { uri: activeUri, file: projectFile('nb-active', PROJECT_ID, [ownEntry]) }, + { uri: siblingUri, file: projectFile('nb-sibling', PROJECT_ID, [siblingEntry]) } + ], + discovered: [activeUri, siblingUri], + openNotebooks: [ + { notebookId: 'nb-active', uri: activeUri }, + { notebookId: 'nb-sibling', uri: siblingUri } + ] + }); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-active', projectFile('nb-active', PROJECT_ID, [ownEntry])); + managerInstance.storeOriginalProject( + PROJECT_ID, + 'nb-sibling', + projectFile('nb-sibling', PROJECT_ID, [siblingEntry]) + ); + + await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(cachedIntegrations('nb-active'), [ownEntry, ADDED]); + assert.deepStrictEqual(cachedIntegrations('nb-sibling'), [siblingEntry, ADDED]); + }); + + test("keeps the entries only a dirty sibling declares when the sweep's flush saves it", async () => { + // Catches: the sibling's cache stamped with the active file's list before the sweep reaches it, so the flush + // that saves its unsaved edits writes that list over the sibling's own entries before the merge re-reads it. + const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + const siblingEntry: ProjectIntegration = { id: 'int-sibling', name: 'Sibling MySQL', type: 'mysql' }; + const { writes } = stubWorkspace({ + onDisk: [ + { uri: activeUri, file: projectFile('nb-active') }, + { uri: siblingUri, file: projectFile('nb-sibling', PROJECT_ID, [siblingEntry]) } + ], + discovered: [activeUri, siblingUri], + openNotebooks: [ + { notebookId: 'nb-active', uri: activeUri }, { - uri: siblingUri, - onSave: (onDisk) => onDisk.set(siblingUri.fsPath, projectFile('nb-other', 'project-2')) + notebookId: 'nb-sibling', + // Like the serializer: a save writes the whole file from the notebook's cache entry. + onSave: (onDisk) => { + const cached = managerInstance.getProjectForNotebook(PROJECT_ID, 'nb-sibling'); + + if (cached) { + onDisk.set(siblingUri.fsPath, cached); + } + }, + uri: siblingUri } ] }); + managerInstance.storeOriginalProject(PROJECT_ID, 'nb-active', projectFile('nb-active')); + managerInstance.storeOriginalProject( + PROJECT_ID, + 'nb-sibling', + projectFile('nb-sibling', PROJECT_ID, [siblingEntry]) + ); + + const result = await addProjectIntegration({ + activeFileUri: activeUri, + integration: ADDED, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); + assert.deepStrictEqual(writes.get(siblingUri.fsPath)?.project.integrations, [siblingEntry, ADDED]); + assert.deepStrictEqual(cachedIntegrations('nb-sibling'), [siblingEntry, ADDED]); + }); +}); + +suite('removeProjectIntegration', () => { + const REMOVED: ProjectIntegration = { id: 'int-removed', name: 'Removed Postgres', type: 'pgsql' }; + const KEPT: RawProjectIntegration[] = [ + { id: 'int-kept', name: 'Kept BigQuery', type: 'big-query' }, + { id: 'duckdb', name: 'DuckDB', type: 'pandas-dataframe' } + ]; + + let managerInstance: DeepnoteNotebookManager; - const result = await persistProjectIntegrations({ + setup(() => { + resetVSCodeMocks(); + + managerInstance = new DeepnoteNotebookManager(); + }); + + test("takes the id off the active file and every sibling, keeping each file's other entries and its cache in step", async () => { + const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + const siblingEntry: ProjectIntegration = { id: 'int-sibling', name: 'Sibling MySQL', type: 'mysql' }; + const { writes } = stubWorkspace({ + onDisk: [ + { uri: activeUri, file: projectFile('nb-active', PROJECT_ID, [...KEPT, REMOVED]) }, + { uri: siblingUri, file: projectFile('nb-sibling', PROJECT_ID, [REMOVED, siblingEntry]) } + ], + discovered: [activeUri, siblingUri], + openNotebooks: [ + { notebookId: 'nb-active', uri: activeUri }, + { notebookId: 'nb-sibling', uri: siblingUri } + ] + }); + managerInstance.storeOriginalProject( + PROJECT_ID, + 'nb-active', + projectFile('nb-active', PROJECT_ID, [...KEPT, REMOVED]) + ); + managerInstance.storeOriginalProject( + PROJECT_ID, + 'nb-sibling', + projectFile('nb-sibling', PROJECT_ID, [REMOVED, siblingEntry]) + ); + + const result = await removeProjectIntegration({ + activeFileUri: activeUri, + integrationId: REMOVED.id, notebookManager: managerInstance, - projectId: PROJECT_ID, - integrations: NEW_INTEGRATIONS, - activeFileUri: activeUri + projectId: PROJECT_ID }); assert.deepStrictEqual(result, { activePersisted: true, siblingsFailed: 0 }); - assert.deepStrictEqual(writes.get(activeUri.fsPath)!.project.integrations, NEW_INTEGRATIONS); - assert.isFalse(writes.has(siblingUri.fsPath), 'integrations must NOT be written into the swapped project'); + assert.deepStrictEqual(writes.get(activeUri.fsPath)?.project.integrations, KEPT); + assert.deepStrictEqual(writes.get(siblingUri.fsPath)?.project.integrations, [siblingEntry]); + assert.deepStrictEqual( + managerInstance.getProjectForNotebook(PROJECT_ID, 'nb-active')?.project.integrations, + KEPT + ); + assert.deepStrictEqual(managerInstance.getProjectForNotebook(PROJECT_ID, 'nb-sibling')?.project.integrations, [ + siblingEntry + ]); + }); + + test('sweeps no sibling when the active file cannot be written', async () => { + const activeUri = Uri.file('/ws/active.deepnote'); + const siblingUri = Uri.file('/ws/sibling.deepnote'); + const { writes } = stubWorkspace({ + onDisk: [ + { uri: activeUri, file: projectFile('nb-active', PROJECT_ID, [REMOVED]) }, + { uri: siblingUri, file: projectFile('nb-sibling', PROJECT_ID, [REMOVED]) } + ], + discovered: [activeUri, siblingUri], + failWriteFor: new Set([activeUri.fsPath]) + }); + + const result = await removeProjectIntegration({ + activeFileUri: activeUri, + integrationId: REMOVED.id, + notebookManager: managerInstance, + projectId: PROJECT_ID + }); + + assert.deepStrictEqual(result, { activePersisted: false, siblingsFailed: 0 }); + assert.strictEqual(writes.size, 0, 'the removal reaches no file once the one the user acted in refused it'); }); }); diff --git a/src/notebooks/deepnote/integrations/types.ts b/src/notebooks/deepnote/integrations/types.ts index 1065b35f57..9a82bc1fae 100644 --- a/src/notebooks/deepnote/integrations/types.ts +++ b/src/notebooks/deepnote/integrations/types.ts @@ -27,6 +27,12 @@ export interface IIntegrationDetector { export const IIntegrationWebviewProvider = Symbol('IIntegrationWebviewProvider'); export interface IIntegrationWebviewProvider { + /** + * Re-renders an open panel that shows `projectId` with `integrations`, without revealing it; does nothing when no + * panel is open or it shows another project. + */ + refresh(projectId: string, integrations: Map): Promise; + /** * Show the integration management webview * @param projectId The Deepnote project ID diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts index c5a32c96ac..bb2130edde 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts @@ -24,26 +24,23 @@ import { IDisposableRegistry } from '../../platform/common/types'; import { IIntegrationStorage } from './integrations/types'; import { Commands } from '../../platform/common/constants'; import { - ConfigurableDatabaseIntegrationType, DATAFRAME_SQL_INTEGRATION_ID, isConfigurableDatabaseIntegrationType, toTelemetryIntegrationType } from '../../platform/notebooks/deepnote/integrationTypes'; -import { persistProjectIntegrations } from './integrations/projectIntegrationsWriter'; -import { IDeepnoteNotebookManager, ProjectIntegration } from '../types'; +import { addProjectIntegration } from './integrations/projectIntegrationsWriter'; +import { isSnapshotFile } from './snapshots/snapshotFiles'; +import { IDeepnoteNotebookManager, RawProjectIntegration } from '../types'; import { logger } from '../../platform/logging'; import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; -import { SqlReturnVariableType } from '../../platform/common/utils/localize'; -import type { DeepnoteFile, SqlCellVariableType } from '@deepnote/blocks'; +import { Integrations, SqlReturnVariableType } from '../../platform/common/utils/localize'; +import type { SqlCellVariableType } from '@deepnote/blocks'; import { DatabaseIntegrationConfig, DatabaseIntegrationType, databaseIntegrationTypes } from '@deepnote/database-integrations'; -/** One entry of a project's `integrations` list as it appears in the file, where `type` is not yet validated. */ -type RawProjectIntegration = NonNullable[number]; - /** * QuickPick item with an integration ID */ @@ -69,27 +66,6 @@ const returnVariableTypeDetails: Record = { query_preview: SqlReturnVariableType.queryPreviewDetail }; -const integrationTypeLabels: Record = { - alloydb: l10n.t('Google AlloyDB'), - athena: l10n.t('Amazon Athena'), - 'big-query': l10n.t('Google BigQuery'), - clickhouse: l10n.t('ClickHouse'), - 'cloud-sql': l10n.t('Google Cloud SQL'), - databricks: l10n.t('Databricks'), - dremio: l10n.t('Dremio'), - mariadb: l10n.t('MariaDB'), - materialize: l10n.t('Materialize'), - mindsdb: l10n.t('MindsDB'), - mongodb: l10n.t('MongoDB'), - mysql: l10n.t('MySQL'), - pgsql: l10n.t('PostgreSQL'), - redshift: l10n.t('Amazon Redshift'), - snowflake: l10n.t('Snowflake'), - spanner: l10n.t('Google Spanner'), - 'sql-server': l10n.t('Microsoft SQL Server'), - trino: l10n.t('Trino') -}; - /** * Provides status bar items for SQL cells showing the integration name, variable name and return type */ @@ -252,7 +228,7 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid // Merged first: at execution time a `.deepnote.env.yaml` config wins on id conflict, so resolving // SecretStorage first would label the cell with a database it does not connect to. const mergedConfig = (await this.getMergedIntegrationConfigs(cell)).find((c) => c.id === integrationId); - // The merge only resolves ids the project roster or the file declares; SecretStorage still answers for the rest. + // The merge only resolves ids the project or the file declares; SecretStorage still answers for the rest. const config = mergedConfig ?? (await this.integrationStorage.getProjectIntegrationConfig(projectId, integrationId)); @@ -457,30 +433,23 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid this._onDidChangeCellStatusBarItems.fire(); } - /** - * Appends a picked integration to the project roster so the `.deepnote` file records what it uses. - * Additive only: existing entries pass through verbatim, never filtered, because a project's integrations - * are shared with sibling notebooks whose blocks this cannot see — dropping one would be data loss. - */ + /** Appends a picked integration to the project's integrations so the `.deepnote` file records what it uses. */ private async addToProjectIntegrations( cell: NotebookCell, projectId: string, - roster: RawProjectIntegration[], selected: RawProjectIntegration ): Promise { - // No usable `type` means no valid roster entry; leave it out rather than guessing one. + // No usable `type` means no valid entry; leave it out rather than guessing one. if (!isConfigurableDatabaseIntegrationType(selected.type)) { return; } try { - await persistProjectIntegrations({ + await addProjectIntegration({ + activeFileUri: cell.notebook.uri, + integration: selected, notebookManager: this.notebookManager, - projectId, - // Cast rather than narrow: validating the existing entries would silently drop any type this - // build does not know about, which is pruning by another name. - integrations: [...roster, selected] as ProjectIntegration[], - activeFileUri: cell.notebook.uri + projectId }); } catch (error) { // The cell metadata edit already succeeded, so the selection stands either way. @@ -503,25 +472,34 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid } /** - * The roster plus any `.deepnote.env.yaml` integrations it omits, so a file-only one can be picked here - * instead of only by hand-editing `sql_integration_id`. A failed lookup falls back to the roster alone. + * The project's integrations plus any `.deepnote.env.yaml` ones they omit, so a file-only integration can be + * picked here instead of only by hand-editing `sql_integration_id`. A failed lookup falls back to the + * project's integrations alone. */ private async getSelectableIntegrations( cell: NotebookCell, projectIntegrations: RawProjectIntegration[] ): Promise { const mergedIntegrationConfigs = await this.getMergedIntegrationConfigs(cell); - const rosterIds = new Set(projectIntegrations.map((integration) => integration.id)); + const projectIntegrationIds = new Set(projectIntegrations.map((integration) => integration.id)); - // Anything merged but absent from the roster came from the file alone — the merge resolves roster ids first. + // Anything merged but absent here came from the file alone — the merge resolves the project's ids first. const fileOnly = mergedIntegrationConfigs - .filter((config) => !rosterIds.has(config.id)) + .filter((config) => !projectIntegrationIds.has(config.id)) .map((config) => ({ id: config.id, name: config.name, type: config.type })); return fileOnly.length > 0 ? [...projectIntegrations, ...fileOnly] : projectIntegrations; } private async switchIntegration(cell: NotebookCell): Promise { + // A snapshot matches the notebook selector, but it records a past run: switching would edit that record + // and, through `addToProjectIntegrations`, rewrite the real project files behind it. + if (isSnapshotFile(cell.notebook.uri)) { + void window.showErrorMessage(Integrations.switchIntegrationSnapshotUnsupported); + + return; + } + const currentIntegrationId = this.getIntegrationId(cell); // Get the project ID from the notebook metadata @@ -542,14 +520,14 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid // Build quick pick items from project integrations const items: (QuickPickItem | LocalQuickPickItem)[] = []; - const roster = project.project.integrations || []; - const projectIntegrations = await this.getSelectableIntegrations(cell, roster); + const projectIntegrations = project.project.integrations || []; + const selectableIntegrations = await this.getSelectableIntegrations(cell, projectIntegrations); // Check if current integration is unknown (not in the project's list) const isCurrentIntegrationUnknown = currentIntegrationId && currentIntegrationId !== DATAFRAME_SQL_INTEGRATION_ID && - !projectIntegrations.some((i) => i.id === currentIntegrationId); + !selectableIntegrations.some((i) => i.id === currentIntegrationId); // Add current unknown integration first if it exists if (isCurrentIntegrationUnknown && currentIntegrationId) { @@ -563,7 +541,7 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid } // Add all project integrations - for (const projectIntegration of projectIntegrations) { + for (const projectIntegration of selectableIntegrations) { const integrationType = projectIntegration.type && (databaseIntegrationTypes as readonly string[]).includes(projectIntegration.type) @@ -575,10 +553,9 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid continue; } - const typeLabel = - integrationType && (databaseIntegrationTypes as readonly string[]).includes(integrationType) - ? integrationTypeLabels[integrationType] ?? integrationType - : projectIntegration.type; + const typeLabel = isConfigurableDatabaseIntegrationType(projectIntegration.type) + ? Integrations.typeLabel(projectIntegration.type) + : projectIntegration.type; const item: LocalQuickPickItem = { label: projectIntegration.name || projectIntegration.id, @@ -657,13 +634,15 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid return; } - // Picking a file-declared integration is the one place the roster drifts, so reconcile it here. - const selectedIntegration = projectIntegrations.find((integration) => integration.id === selectedId); + // Picking a file-declared integration is the one place the project's integrations drift, so reconcile + // it here. + const selectedIntegration = selectableIntegrations.find((integration) => integration.id === selectedId); // Shared with the telemetry below so the reconciliation and what it reports cannot drift apart. const fromEnvFile = - selectedIntegration !== undefined && !roster.some((integration) => integration.id === selectedId); + selectedIntegration !== undefined && + !projectIntegrations.some((integration) => integration.id === selectedId); if (selectedIntegration && fromEnvFile) { - await this.addToProjectIntegrations(cell, projectId, roster, selectedIntegration); + await this.addToProjectIntegrations(cell, projectId, selectedIntegration); } // Trigger status bar update diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts index 3197e91a69..147386fc67 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts @@ -1,18 +1,34 @@ +import type { DeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; import { anything, capture, deepEqual, instance, mock, verify, when } from 'ts-mockito'; -import { CancellationToken, CancellationTokenSource, EventEmitter, NotebookCell } from 'vscode'; +import { CancellationToken, CancellationTokenSource, EventEmitter, NotebookCell, NotebookDocument, Uri } from 'vscode'; import { IDisposableRegistry } from '../../platform/common/types'; import { IIntegrationStorage } from './integrations/types'; import { ITelemetryService } from '../../platform/analytics/types'; import { SqlCellStatusBarProvider } from './sqlCellStatusBarProvider'; import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes'; +import { stubDeepnoteFiles } from '../../test/mocks/vscodeFs'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { createEventHandler } from '../../test/common'; import { Commands } from '../../platform/common/constants'; import { IDeepnoteNotebookManager } from '../types'; import { createDeepnoteFile, createDeepnoteProject, createMockCell } from './deepnoteTestHelpers'; import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; +import { Integrations } from '../../platform/common/utils/localize'; + +/** + * Puts a readable project file behind the open `notebook`; the returned map captures what the integrations writer + * persists. + */ +function stubProjectFileOnDisk(notebook: NotebookDocument, projectId: string): Map { + const onDisk = createDeepnoteFile({ project: createDeepnoteProject({ id: projectId, name: projectId }) }); + + when(mockedVSCodeNamespaces.workspace.workspaceFolders).thenReturn(undefined); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + return stubDeepnoteFiles((target) => (target.fsPath === notebook.uri.fsPath ? onDisk : undefined)); +} /** * A merged-config source that yields nothing, so integrations resolve from SecretStorage alone. @@ -297,7 +313,7 @@ suite('SqlCellStatusBarProvider', () => { notebookMetadata: { deepnoteProjectId: 'project-1' } }); - // The merge only resolves roster and file ids; a bare merged-first rewrite would regress this to (configure). + // The merge only resolves project and file ids; a bare merged-first rewrite would regress this to (configure). when(integrationStorage.getProjectIntegrationConfig(anything(), anything())).thenResolve({ id: integrationId, name: 'Stored Only', @@ -713,7 +729,7 @@ suite('SqlCellStatusBarProvider', () => { verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).once(); }); - test('switchSqlIntegration offers `.deepnote.env.yaml` integrations the project roster omits', async () => { + test('switchSqlIntegration offers `.deepnote.env.yaml` integrations the project omits', async () => { let commandHandler: ((cell?: NotebookCell) => Promise) | undefined; when(mockedVSCodeNamespaces.commands.registerCommand('deepnote.switchSqlIntegration', anything())).thenCall( (_name, handler) => { @@ -1243,6 +1259,7 @@ suite('SqlCellStatusBarProvider', () => { const notebookMetadata = { deepnoteProjectId: 'project-1', deepnoteNotebookId: 'notebook-1' }; const cell = createMockCell({ languageId: 'sql', metadata: {}, notebookMetadata }); const fileOnlyId = 'file-only-bq'; + const writes = stubProjectFileOnDisk(cell.notebook, 'project-1'); const envVars = mock(); when(envVars.getMergedIntegrationConfigs(anything())).thenResolve([ @@ -1272,9 +1289,19 @@ suite('SqlCellStatusBarProvider', () => { await switchIntegrationHandler(cell); - const [projectId, integrations] = capture(commandNotebookManager.updateProjectIntegrations).last(); - assert.strictEqual(projectId, 'project-1'); - assert.deepStrictEqual(integrations, [{ id: fileOnlyId, name: 'BigQuery from file', type: 'big-query' }]); + const expected = [{ id: fileOnlyId, name: 'BigQuery from file', type: 'big-query' }]; + const [projectId, notebookId, integrations] = capture( + commandNotebookManager.updateProjectIntegrationsForNotebook + ).last(); + assert.deepStrictEqual( + { integrations, notebookId, projectId }, + { integrations: expected, notebookId: 'notebook-1', projectId: 'project-1' } + ); + assert.deepStrictEqual( + writes.get(cell.notebook.uri.fsPath)?.project.integrations, + expected, + 'the entry reaches the file, not just the cache' + ); verify( fileTelemetry.trackEvent( deepEqual({ @@ -1285,6 +1312,41 @@ suite('SqlCellStatusBarProvider', () => { ).once(); }); + test('refuses to switch the integration of a snapshot notebook', async () => { + // `*.snapshot.deepnote` matches the notebook selector, so the command is reachable there; switching + // would edit the record and rewrite the real project files behind it. Everything else is arranged so + // the switch would go through, leaving the snapshot check as the only thing that can stop it. + const newIntegrationId = 'new-integration'; + const cell = createMockCell({ + languageId: 'sql', + metadata: { sql_integration_id: 'old-integration' }, + notebookMetadata: { deepnoteProjectId: 'project-1', deepnoteNotebookId: 'notebook-1' }, + notebookUri: Uri.file('/ws/report_project-1_nb_main.snapshot.deepnote') + }); + + when(commandNotebookManager.getProjectForNotebook('project-1', 'notebook-1')).thenReturn( + createDeepnoteFile({ + project: createDeepnoteProject({ + integrations: [{ id: newIntegrationId, name: 'New Integration', type: 'pgsql' }] + }) + }) + ); + when(mockedVSCodeNamespaces.window.showErrorMessage(anything())).thenReturn(Promise.resolve(undefined)); + when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenReturn( + Promise.resolve({ id: newIntegrationId, label: 'New Integration' } as any) + ); + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenReturn(Promise.resolve(true)); + + await switchIntegrationHandler(cell); + + verify( + mockedVSCodeNamespaces.window.showErrorMessage(Integrations.switchIntegrationSnapshotUnsupported) + ).once(); + verify(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).never(); + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).never(); + verify(commandTelemetry.trackEvent(anything())).never(); + }); + test('does not update if user cancels quick pick', async () => { const notebookMetadata = { deepnoteProjectId: 'project-1', deepnoteNotebookId: 'notebook-1' }; const cell = createMockCell({ diff --git a/src/notebooks/types.ts b/src/notebooks/types.ts index d1a43b5b23..4bdaca0dff 100644 --- a/src/notebooks/types.ts +++ b/src/notebooks/types.ts @@ -36,6 +36,12 @@ export interface ProjectIntegration { type: ConfigurableDatabaseIntegrationType; } +/** + * An entry as recorded on disk: unlike `ProjectIntegration`, `type` is not narrowed to the known types, so it also + * covers the internal `pandas-dataframe` integration and anything a newer Deepnote release writes. + */ +export type RawProjectIntegration = NonNullable[number]; + export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager'); export interface IDeepnoteNotebookManager { /** @@ -46,14 +52,14 @@ export interface IDeepnoteNotebookManager { storeOriginalProject(projectId: string, notebookId: string, project: DeepnoteFile): void; /** - * Updates the integrations list in the cached project data (cache-only). - * Iterates every cached notebook entry under the project and updates each. - * - * @param projectId - Project identifier - * @param integrations - Array of integration metadata to store in the project - * @returns `true` if at least one cached entry was found and updated, `false` otherwise + * Replaces the integrations list of the one cached (projectId, notebookId) entry (cache-only); does nothing + * when that entry is not cached. Sibling entries keep their own lists: each mirrors its own file. */ - updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean; + updateProjectIntegrationsForNotebook( + projectId: string, + notebookId: string, + integrations: RawProjectIntegration[] + ): void; } export const IDeepnoteInitNotebookRunner = Symbol('IDeepnoteInitNotebookRunner'); diff --git a/src/platform/analytics/types.ts b/src/platform/analytics/types.ts index c36a69e7d1..632b710e5c 100644 --- a/src/platform/analytics/types.ts +++ b/src/platform/analytics/types.ts @@ -10,6 +10,7 @@ export type TelemetryEventName = | 'delete_integration' | 'delete_notebook' | 'duplicate_notebook' + | 'add_existing_integration' | 'execute_cell' | 'execute_notebook' | 'export_notebook' @@ -33,6 +34,7 @@ export type CommandOutcome = 'completed' | 'cancelled' | 'failed'; /** Caller-supplied properties per event; `undefined` means none beyond the common properties the service attaches. */ export interface TelemetryEventProperties { add_block: { blockType: string; isEphemeral: boolean }; + add_existing_integration: { integrationType: string; outcome: CommandOutcome }; authenticate_integration: { integrationType: string; outcome: CommandOutcome }; configure_integration: { integrationType: string }; copy_notebook_details: undefined; @@ -66,7 +68,7 @@ export interface TelemetryEventProperties { reset_integration: { integrationType: string }; save_integration: { integrationType: string; authMethod?: 'service-account' | 'google-oauth' }; split_notebook: { notebookCount: number; outcome: CommandOutcome }; - /** `fromEnvFile` is file-ONLY, not file-configured: false when the id is also in the project roster. */ + /** `fromEnvFile` is file-ONLY, not file-configured: false when the id is also in the project's integrations. */ switch_sql_integration: { fromEnvFile: boolean; integrationType: string }; switch_sql_return_variable_type: { returnVariableType: 'dataframe' | 'query_preview' }; toggle_snapshots: { enabled: boolean }; diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index 75027c68b6..dbbbb80601 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -239,6 +239,7 @@ export namespace Commands { export const CopyNotebookDetails = 'deepnote.copyNotebookDetails'; export const EnableSnapshots = 'deepnote.enableSnapshots'; export const DisableSnapshots = 'deepnote.disableSnapshots'; + export const AddExistingIntegration = 'deepnote.addExistingIntegration'; export const AuthenticateIntegration = 'deepnote.authenticateIntegration'; export const ManageIntegrations = 'deepnote.manageIntegrations'; export const SelectInterpreterForNotebook = 'deepnote.selectInterpreterForNotebook'; diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index 8fed4ae07d..6ce22bf72b 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -2,9 +2,11 @@ // Licensed under the MIT License. import { l10n } from 'vscode'; +import { getPythonEnvDisplayName } from '../../interpreter/helpers'; +import { integrationTypeLabelKey, type IntegrationTypeLabelKey } from '../../notebooks/deepnote/integrationTypeLabels'; +import type { ConfigurableDatabaseIntegrationType } from '../../notebooks/deepnote/integrationTypes'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { fromNow } from './date'; -import { getPythonEnvDisplayName } from '../../interpreter/helpers'; function getInterpreterDisplayName(interpreter: PythonEnvironment) { return getPythonEnvDisplayName(interpreter); @@ -861,6 +863,51 @@ export namespace Integrations { export const cancel = l10n.t('Cancel'); export const save = l10n.t('Save'); export const addNewIntegration = l10n.t('Add New Integration'); + export const addExistingIntegration = l10n.t('Add Existing Integration'); + export const addExistingIntegrationScanning = l10n.t('Looking for integrations in other projects...'); + export const snapshotIntegrationsUnsupported = l10n.t( + 'Integrations cannot be managed from a snapshot file. Open the project notebook and try again.' + ); + export const switchIntegrationSnapshotUnsupported = l10n.t( + 'The integration of a snapshot cannot be changed. Open the project notebook and try again.' + ); + export const commandNotebookClosed = l10n.t( + 'The notebook this was run for is no longer open. Open it again and retry.' + ); + export const addExistingIntegrationPlaceholder = l10n.t( + 'Select an integration configured in another project of this workspace' + ); + export const addExistingIntegrationNoneAvailable = l10n.t( + 'No integrations from other projects in this workspace are available to add. Integrations configured only in .deepnote.env.yaml already apply to every project under it.' + ); + export const addExistingIntegrationConflictsSkipped = (count: number) => + l10n.t( + '{0} integration(s) were skipped because another project declares them with a different type than the stored configuration.', + count + ); + export const addExistingIntegrationSucceeded = (integrationName: string) => + l10n.t('Added integration "{0}" to this project.', integrationName); + export const addExistingIntegrationFailed = l10n.t( + 'Failed to add the integration to the notebook file. Please try again.' + ); + export const addExistingIntegrationUsedIn = (projectNames: string) => l10n.t('Used in: {0}', projectNames); + export const integrationUnlinked = (projectNames: string) => + l10n.t( + 'Removed from this project. Its credentials were kept because other projects still use them: {0}', + projectNames + ); + export const integrationSavedShared = (projectNames: string) => + l10n.t( + 'Configuration saved successfully. These credentials are shared, so this also changed them for: {0}', + projectNames + ); + export const integrationResetShared = (projectNames: string) => + l10n.t( + 'Configuration reset successfully. These credentials were shared, so this also removed them from: {0}', + projectNames + ); + export const integrationSignedOutShared = (projectNames: string) => + l10n.t('Signed out. This sign-in was shared, so this also signed out: {0}', projectNames); export const database = l10n.t('Database'); export const dataWarehousesLakes = l10n.t('Data Warehouses & Lakes'); export const databases = l10n.t('Databases'); @@ -870,26 +917,32 @@ export namespace Integrations { export const defaultName = (type: string) => l10n.t('My {0} integration', type); export const unsupportedIntegrationType = (type: string) => l10n.t('Unsupported integration type: {0}', type); - // Integration type labels - export const postgresTypeLabel = l10n.t('PostgreSQL'); - export const bigQueryTypeLabel = l10n.t('Google BigQuery'); - export const snowflakeTypeLabel = l10n.t('Snowflake'); - export const alloyDBTypeLabel = l10n.t('Google AlloyDB'); - export const athenaTypeLabel = l10n.t('Amazon Athena'); - export const clickHouseTypeLabel = l10n.t('ClickHouse'); - export const cloudSqlTypeLabel = l10n.t('Google Cloud SQL'); - export const databricksTypeLabel = l10n.t('Databricks'); - export const dremioTypeLabel = l10n.t('Dremio'); - export const mariaDBTypeLabel = l10n.t('MariaDB'); - export const materializeTypeLabel = l10n.t('Materialize'); - export const mindsDBTypeLabel = l10n.t('MindsDB'); - export const mongoDBTypeLabel = l10n.t('MongoDB'); - export const mySQLTypeLabel = l10n.t('MySQL'); - export const duckDBTypeLabel = l10n.t('DuckDB'); - export const redshiftTypeLabel = l10n.t('Amazon Redshift'); - export const spannerTypeLabel = l10n.t('Google Spanner'); - export const sqlServerTypeLabel = l10n.t('Microsoft SQL Server'); - export const trinoTypeLabel = l10n.t('Trino'); + /** + * The one place integration type labels are written down. + * `l10n.t` extracts literal arguments only, so these stay literal. + */ + export const typeLabels = { + 'integrationType.alloydb': l10n.t('Google AlloyDB'), + 'integrationType.athena': l10n.t('Amazon Athena'), + 'integrationType.big-query': l10n.t('Google BigQuery'), + 'integrationType.clickhouse': l10n.t('ClickHouse'), + 'integrationType.cloud-sql': l10n.t('Google Cloud SQL'), + 'integrationType.databricks': l10n.t('Databricks'), + 'integrationType.dremio': l10n.t('Dremio'), + 'integrationType.mariadb': l10n.t('MariaDB'), + 'integrationType.materialize': l10n.t('Materialize'), + 'integrationType.mindsdb': l10n.t('MindsDB'), + 'integrationType.mongodb': l10n.t('MongoDB'), + 'integrationType.mysql': l10n.t('MySQL'), + 'integrationType.pgsql': l10n.t('PostgreSQL'), + 'integrationType.redshift': l10n.t('Amazon Redshift'), + 'integrationType.snowflake': l10n.t('Snowflake'), + 'integrationType.spanner': l10n.t('Google Spanner'), + 'integrationType.sql-server': l10n.t('Microsoft SQL Server'), + 'integrationType.trino': l10n.t('Trino') + } satisfies Record; + + export const typeLabel = (type: ConfigurableDatabaseIntegrationType) => typeLabels[integrationTypeLabelKey(type)]; // PostgreSQL form strings export const postgresNameLabel = l10n.t('Name (optional)'); diff --git a/src/platform/deepnote/deepnoteDocumentFlush.ts b/src/platform/deepnote/deepnoteDocumentFlush.ts index 8cbac88337..d82c2bfaaf 100644 --- a/src/platform/deepnote/deepnoteDocumentFlush.ts +++ b/src/platform/deepnote/deepnoteDocumentFlush.ts @@ -1,7 +1,14 @@ -import { Uri, workspace } from 'vscode'; +import { NotebookDocument, Uri, workspace } from 'vscode'; import { logger } from '../logging'; +/** The notebook document open from `fileUri`, if any; a query or fragment on the document's URI is ignored. */ +export function findOpenNotebookDocument(fileUri: Uri): NotebookDocument | undefined { + return workspace.notebookDocuments.find( + (doc) => doc.uri.with({ query: '', fragment: '' }).toString() === fileUri.toString() + ); +} + /** * Saves a dirty open notebook for `fileUri` before a disk read-modify-write so live edits are not * overwritten; callers that read the file first must re-read it when this returns `true`. @@ -9,9 +16,7 @@ import { logger } from '../logging'; * @returns `false` only when a dirty document could not be saved (the save was declined or threw). */ export async function flushNotebookDocumentIfDirty(fileUri: Uri): Promise { - const openDocument = workspace.notebookDocuments.find( - (doc) => doc.uri.with({ query: '', fragment: '' }).toString() === fileUri.toString() - ); + const openDocument = findOpenNotebookDocument(fileUri); if (!openDocument?.isDirty) { return true; diff --git a/src/platform/notebooks/deepnote/integrationTypeLabels.ts b/src/platform/notebooks/deepnote/integrationTypeLabels.ts new file mode 100644 index 0000000000..edc1625fa3 --- /dev/null +++ b/src/platform/notebooks/deepnote/integrationTypeLabels.ts @@ -0,0 +1,13 @@ +import type { ConfigurableDatabaseIntegrationType } from './integrationTypes'; + +/** + * Where an integration type's display label sits in the string bundle the extension host sends the webview. + * + * Derived from the type rather than listed, so `localize.Integrations.typeLabels` is the only place the labels + * themselves are written down — the webview is bundled separately and cannot read `localize.ts` directly. + */ +export type IntegrationTypeLabelKey = `integrationType.${ConfigurableDatabaseIntegrationType}`; + +export function integrationTypeLabelKey(type: ConfigurableDatabaseIntegrationType): IntegrationTypeLabelKey { + return `integrationType.${type}`; +} diff --git a/src/platform/notebooks/deepnote/integrationTypeLabels.unit.test.ts b/src/platform/notebooks/deepnote/integrationTypeLabels.unit.test.ts new file mode 100644 index 0000000000..a784f13026 --- /dev/null +++ b/src/platform/notebooks/deepnote/integrationTypeLabels.unit.test.ts @@ -0,0 +1,18 @@ +import { databaseIntegrationTypes } from '@deepnote/database-integrations'; +import { assert } from 'chai'; + +import * as localize from '../../common/utils/localize'; +import { isConfigurableDatabaseIntegrationType } from './integrationTypes'; + +suite('integrationTypeLabels', () => { + test('resolves a label for every configurable type', () => { + assert.strictEqual(localize.Integrations.typeLabel('pgsql'), 'PostgreSQL'); + assert.strictEqual(localize.Integrations.typeLabel('big-query'), 'Google BigQuery'); + + const configurable = databaseIntegrationTypes.filter(isConfigurableDatabaseIntegrationType); + const unresolved = configurable.filter((type) => !localize.Integrations.typeLabel(type)); + + assert.deepStrictEqual(unresolved, [], 'every configurable type needs a label the panel can show'); + assert.strictEqual(Object.keys(localize.Integrations.typeLabels).length, configurable.length); + }); +}); diff --git a/src/test/mocks/vscodeFs.ts b/src/test/mocks/vscodeFs.ts index 4341b60f06..871965ccc2 100644 --- a/src/test/mocks/vscodeFs.ts +++ b/src/test/mocks/vscodeFs.ts @@ -1,3 +1,7 @@ +// These stubs serve Node unit tests, which key files by fsPath. +/* eslint-disable local-rules/dont-use-fspath */ + +import { deserializeDeepnoteFile, serializeDeepnoteFile, type DeepnoteFile } from '@deepnote/blocks'; import { anything, instance, mock, when } from 'ts-mockito'; import { Uri } from 'vscode'; @@ -5,19 +9,60 @@ import { mockedVSCodeNamespaces } from '../vscode-mock'; /** * Stubs `workspace.fs.readFile` to yield `contents` as UTF-8 bytes, and points the mocked - * `workspace.fs` namespace at it. Pass a function to serve different bytes per URI. + * `workspace.fs` namespace at it. Pass a function to serve different bytes per URI; a URI it + * returns `undefined` for rejects, as a missing file would. * * Returns the ts-mockito mock so callers can `verify(mockFs.readFile(anything()))`. * Call after `resetVSCodeMocks()` — the reset replaces the namespace mocks this stubs. */ -export function stubReadFile(contents: string | ((uri: Uri) => string)): typeof import('vscode').workspace.fs { +export function stubReadFile( + contents: string | ((uri: Uri) => string | undefined) +): typeof import('vscode').workspace.fs { const resolveContents = typeof contents === 'function' ? contents : () => contents; const mockFs = mock(); - when(mockFs.readFile(anything())).thenCall((uri: Uri) => - Promise.resolve(new TextEncoder().encode(resolveContents(uri))) - ); + when(mockFs.readFile(anything())).thenCall((uri: Uri) => { + const resolved = resolveContents(uri); + + return resolved === undefined + ? Promise.reject(new Error(`no readFile stub for ${uri.fsPath}`)) + : Promise.resolve(new TextEncoder().encode(resolved)); + }); when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); return mockFs; } + +/** + * Serves `.deepnote` files through the mocked `workspace.fs`: a read yields `files(uri)` serialized, rejecting where + * it returns `undefined`, and each write lands in the returned map, parsed, by `fsPath`. Both `files` and + * `failWriteFor` are consulted on every call, so a test can change them after stubbing. + * + * Call after `resetVSCodeMocks()` — the reset replaces the namespace mocks this stubs. + */ +export function stubDeepnoteFiles( + files: (uri: Uri) => DeepnoteFile | undefined, + options: { failWriteFor?: ReadonlySet; onRead?: (uri: Uri) => void; onWrite?: (uri: Uri) => void } = {} +): Map { + const writes = new Map(); + const mockFs = stubReadFile((uri) => { + options.onRead?.(uri); + + const file = files(uri); + + return file ? serializeDeepnoteFile(file) : undefined; + }); + + when(mockFs.writeFile(anything(), anything())).thenCall((uri: Uri, bytes: Uint8Array) => { + if (options.failWriteFor?.has(uri.fsPath)) { + return Promise.reject(new Error(`write failed for ${uri.fsPath}`)); + } + + options.onWrite?.(uri); + writes.set(uri.fsPath, deserializeDeepnoteFile(new TextDecoder().decode(bytes))); + + return Promise.resolve(); + }); + + return writes; +} diff --git a/src/webviews/webview-side/integrations/ConfigurationForm.tsx b/src/webviews/webview-side/integrations/ConfigurationForm.tsx index 1799d71eb7..893451c18e 100644 --- a/src/webviews/webview-side/integrations/ConfigurationForm.tsx +++ b/src/webviews/webview-side/integrations/ConfigurationForm.tsx @@ -19,7 +19,7 @@ import { SpannerForm } from './SpannerForm'; import { SQLServerForm } from './SQLServerForm'; import { isTrinoPasswordConfig, TrinoForm } from './TrinoForm'; import { ConfigurableDatabaseIntegrationConfig, ConfigurableDatabaseIntegrationType } from './types'; -import { integrationTypeLabels } from './integrationUtils'; +import { integrationTypeLabel } from './integrationUtils'; export interface IConfigurationFormProps { integrationId: string; @@ -38,7 +38,7 @@ export const ConfigurationForm: React.FC = ({ onSave, onCancel }) => { - const typeLabel = integrationTypeLabels[integrationType] || integrationType; + const typeLabel = integrationTypeLabel(integrationType); const title = getLocString('integrationsConfigureTitle', '{0} integration').replace('{0}', typeLabel); return ( diff --git a/src/webviews/webview-side/integrations/IntegrationItem.tsx b/src/webviews/webview-side/integrations/IntegrationItem.tsx index 02607c1156..02d9a31d34 100644 --- a/src/webviews/webview-side/integrations/IntegrationItem.tsx +++ b/src/webviews/webview-side/integrations/IntegrationItem.tsx @@ -1,8 +1,8 @@ import * as React from 'react'; import { getLocString } from '../react-common/locReactSide'; -import { ConfigurableDatabaseIntegrationType, DetectedIntegration } from './types'; -import { integrationTypeIcons } from './integrationUtils'; +import { DetectedIntegration } from './types'; +import { integrationTypeIcons, integrationTypeLabel } from './integrationUtils'; export interface IIntegrationItemProps { integration: DetectedIntegration; @@ -13,49 +13,6 @@ export interface IIntegrationItemProps { onSignOut: (integrationId: string) => void; } -const getIntegrationTypeLabel = (type: ConfigurableDatabaseIntegrationType): string => { - switch (type) { - case 'alloydb': - return getLocString('integrationsAlloyDBTypeLabel', 'Google AlloyDB'); - case 'athena': - return getLocString('integrationsAthenaTypeLabel', 'Amazon Athena'); - case 'big-query': - return getLocString('integrationsBigQueryTypeLabel', 'Google BigQuery'); - case 'clickhouse': - return getLocString('integrationsClickHouseTypeLabel', 'ClickHouse'); - case 'cloud-sql': - return getLocString('integrationsCloudSqlTypeLabel', 'Google Cloud SQL'); - case 'databricks': - return getLocString('integrationsDatabricksTypeLabel', 'Databricks'); - case 'dremio': - return getLocString('integrationsDremioTypeLabel', 'Dremio'); - case 'mariadb': - return getLocString('integrationsMariaDBTypeLabel', 'MariaDB'); - case 'materialize': - return getLocString('integrationsMaterializeTypeLabel', 'Materialize'); - case 'mindsdb': - return getLocString('integrationsMindsDBTypeLabel', 'MindsDB'); - case 'mongodb': - return getLocString('integrationsMongoDBTypeLabel', 'MongoDB'); - case 'mysql': - return getLocString('integrationsMySQLTypeLabel', 'MySQL'); - case 'pgsql': - return getLocString('integrationsPostgresTypeLabel', 'PostgreSQL'); - case 'redshift': - return getLocString('integrationsRedshiftTypeLabel', 'Amazon Redshift'); - case 'snowflake': - return getLocString('integrationsSnowflakeTypeLabel', 'Snowflake'); - case 'spanner': - return getLocString('integrationsSpannerTypeLabel', 'Google Spanner'); - case 'sql-server': - return getLocString('integrationsSQLServerTypeLabel', 'Microsoft SQL Server'); - case 'trino': - return getLocString('integrationsTrinoTypeLabel', 'Trino'); - default: - return type; - } -}; - export const IntegrationItem: React.FC = ({ integration, onConfigure, @@ -86,7 +43,7 @@ export const IntegrationItem: React.FC = ({ const type = integration.config?.type || integration.integrationType; // Get the type label and icon - const typeLabel = type ? getIntegrationTypeLabel(type) : undefined; + const typeLabel = type ? integrationTypeLabel(type) : undefined; const typeIcon = type ? integrationTypeIcons[type] : undefined; // Federated-auth UI: `tokenStatus` alone decides. The extension gates on its candidate set, which also diff --git a/src/webviews/webview-side/integrations/IntegrationPanel.tsx b/src/webviews/webview-side/integrations/IntegrationPanel.tsx index 9c92ec19a6..2dbbcfec69 100644 --- a/src/webviews/webview-side/integrations/IntegrationPanel.tsx +++ b/src/webviews/webview-side/integrations/IntegrationPanel.tsx @@ -33,6 +33,7 @@ export const IntegrationPanel: React.FC = ({ baseTheme, const [message, setMessage] = React.useState<{ type: 'success' | 'error'; text: string } | null>(null); const [confirmReset, setConfirmReset] = React.useState(null); const [confirmDelete, setConfirmDelete] = React.useState(null); + const [locStringsLoaded, setLocStringsLoaded] = React.useState(false); const messageTimerRef = React.useRef(null); const confirmResetTimerRef = React.useRef(null); @@ -63,7 +64,9 @@ export const IntegrationPanel: React.FC = ({ baseTheme, switch (msg.type) { case 'loc_init': + // Store first: this state change re-renders synchronously, and the cards read the bundle. storeLocStrings(msg.locStrings); + setLocStringsLoaded(true); break; case 'update': @@ -97,6 +100,11 @@ export const IntegrationPanel: React.FC = ({ baseTheme, }; window.addEventListener('message', handleMessage); + + // The extension answers with the loc strings and the list, which it cannot deliver before this listener exists. + const started: WebviewOutboundMessage = { type: 'started' }; + vscodeApi.postMessage(started); + return () => window.removeEventListener('message', handleMessage); }, []); @@ -212,6 +220,10 @@ export const IntegrationPanel: React.FC = ({ baseTheme, setSelectedIntegrationType(undefined); }; + const handleAddExisting = () => { + postOutbound({ type: 'addExisting' }); + }; + const handleSelectIntegrationType = (type: ConfigurableDatabaseIntegrationType) => { // Generate a new UUID for the integration const newId = generateUuid(); @@ -239,7 +251,9 @@ export const IntegrationPanel: React.FC = ({ baseTheme, onSignOut={handleSignOut} /> - + {locStringsLoaded && ( + + )} {selectedIntegrationId && selectedIntegrationType && ( void; onSelectType: (type: ConfigurableDatabaseIntegrationType) => void; } -interface IntegrationTypeInfo { - type: ConfigurableDatabaseIntegrationType; - label: string; - icon: string; -} - -// Data Warehouses & Lakes -const WAREHOUSE_INTEGRATION_TYPES: IntegrationTypeInfo[] = [ - { - type: 'clickhouse', - label: integrationTypeLabels['clickhouse'], - icon: integrationTypeIcons['clickhouse'] - }, - { - type: 'redshift', - label: integrationTypeLabels['redshift'], - icon: integrationTypeIcons['redshift'] - }, - { - type: 'athena', - label: integrationTypeLabels['athena'], - icon: integrationTypeIcons['athena'] - }, - { - type: 'big-query', - label: integrationTypeLabels['big-query'], - icon: integrationTypeIcons['big-query'] - }, - { - type: 'snowflake', - label: integrationTypeLabels['snowflake'], - icon: integrationTypeIcons['snowflake'] - }, - { - type: 'databricks', - label: integrationTypeLabels['databricks'], - icon: integrationTypeIcons['databricks'] - }, - { - type: 'dremio', - label: integrationTypeLabels['dremio'], - icon: integrationTypeIcons['dremio'] - }, - { - type: 'trino', - label: integrationTypeLabels['trino'], - icon: integrationTypeIcons['trino'] - } +// Display order, not the map's alphabetical order. +const WAREHOUSE_INTEGRATION_TYPES: ConfigurableDatabaseIntegrationType[] = [ + 'clickhouse', + 'redshift', + 'athena', + 'big-query', + 'snowflake', + 'databricks', + 'dremio', + 'trino' ]; -// Databases -const DATABASE_INTEGRATION_TYPES: IntegrationTypeInfo[] = [ - { - type: 'mongodb', - label: integrationTypeLabels['mongodb'], - icon: integrationTypeIcons['mongodb'] - }, - { - type: 'pgsql', - label: integrationTypeLabels['pgsql'], - icon: integrationTypeIcons['pgsql'] - }, - { - type: 'mysql', - label: integrationTypeLabels['mysql'], - icon: integrationTypeIcons['mysql'] - }, - { - type: 'mariadb', - label: integrationTypeLabels['mariadb'], - icon: integrationTypeIcons['mariadb'] - }, - { - type: 'sql-server', - label: integrationTypeLabels['sql-server'], - icon: integrationTypeIcons['sql-server'] - }, - { - type: 'alloydb', - label: integrationTypeLabels['alloydb'], - icon: integrationTypeIcons['alloydb'] - }, - { - type: 'spanner', - label: integrationTypeLabels['spanner'], - icon: integrationTypeIcons['spanner'] - }, - { - type: 'cloud-sql', - label: integrationTypeLabels['cloud-sql'], - icon: integrationTypeIcons['cloud-sql'] - }, - { - type: 'materialize', - label: integrationTypeLabels['materialize'], - icon: integrationTypeIcons['materialize'] - }, - { - type: 'mindsdb', - label: integrationTypeLabels['mindsdb'], - icon: integrationTypeIcons['mindsdb'] - } +const DATABASE_INTEGRATION_TYPES: ConfigurableDatabaseIntegrationType[] = [ + 'mongodb', + 'pgsql', + 'mysql', + 'mariadb', + 'sql-server', + 'alloydb', + 'spanner', + 'cloud-sql', + 'materialize', + 'mindsdb' ]; -export const IntegrationTypeSelector: React.FC = ({ onSelectType }) => { +export const IntegrationTypeSelector: React.FC = ({ onAddExisting, onSelectType }) => { return (
-

{getLocString('integrationsAddNewIntegration', 'Add New Integration')}

+
+

{getLocString('integrationsAddNewIntegration', 'Add New Integration')}

+ +

{getLocString('integrationsDataWarehousesLakes', 'Data Warehouses & Lakes')}

- {WAREHOUSE_INTEGRATION_TYPES.map((integrationInfo) => ( - - ))} + {WAREHOUSE_INTEGRATION_TYPES.map((type) => { + const label = integrationTypeLabel(type); + + return ( + + ); + })}

{getLocString('integrationsDatabases', 'Databases')}

- {DATABASE_INTEGRATION_TYPES.map((integrationInfo) => ( - - ))} + {DATABASE_INTEGRATION_TYPES.map((type) => { + const label = integrationTypeLabel(type); + + return ( + + ); + })}
diff --git a/src/webviews/webview-side/integrations/integrationUtils.ts b/src/webviews/webview-side/integrations/integrationUtils.ts index cd329c8490..0e09a59266 100644 --- a/src/webviews/webview-side/integrations/integrationUtils.ts +++ b/src/webviews/webview-side/integrations/integrationUtils.ts @@ -1,3 +1,4 @@ +import { integrationTypeLabelKey } from '../../../platform/notebooks/deepnote/integrationTypeLabels'; import { getLocString } from '../react-common/locReactSide'; import { ConfigurableDatabaseIntegrationType } from './types'; @@ -23,27 +24,13 @@ const mindsdbLogo: string = require('./icons/mindsdb.svg'); const trinoLogo: string = require('./icons/trino.svg'); /* eslint-enable @typescript-eslint/no-require-imports */ -// Localized labels for integration types (duplicated from sqlCellStatusBarProvider.ts due to import restrictions) -export const integrationTypeLabels: Record = { - alloydb: 'Google AlloyDB', - athena: 'Amazon Athena', - 'big-query': 'Google BigQuery', - clickhouse: 'ClickHouse', - 'cloud-sql': 'Google Cloud SQL', - databricks: 'Databricks', - dremio: 'Dremio', - mariadb: 'MariaDB', - materialize: 'Materialize', - mindsdb: 'MindsDB', - mongodb: 'MongoDB', - mysql: 'MySQL', - pgsql: 'PostgreSQL', - redshift: 'Amazon Redshift', - snowflake: 'Snowflake', - spanner: 'Google Spanner', - 'sql-server': 'Microsoft SQL Server', - trino: 'Trino' -}; +/** + * The panel is bundled separately and cannot reach `localize.ts`, so labels resolve against the string bundle. + * `IntegrationPanel` renders the type cards only after that bundle arrives; the raw type covers an unknown key. + */ +export function integrationTypeLabel(type: ConfigurableDatabaseIntegrationType): string { + return getLocString(integrationTypeLabelKey(type), type); +} // Icon mapping for integration types export const integrationTypeIcons: Record = { @@ -73,6 +60,6 @@ export const integrationTypeIcons: Record extension); dispatched in `integrationWebview.ts:handleMessage`. Keep exhaustive. export type WebviewOutboundMessage = + | { type: 'started' } | { type: 'configure'; integrationId: string } | { type: 'save'; integrationId: string; config: ConfigurableDatabaseIntegrationConfig } | { type: 'reset'; integrationId: string } | { type: 'delete'; integrationId: string } | { type: 'signOut'; integrationId: string } + | { type: 'addExisting' } | AuthenticateMessage; diff --git a/test/e2e/suite/workspace/addExistingIntegration.e2e.test.ts b/test/e2e/suite/workspace/addExistingIntegration.e2e.test.ts new file mode 100644 index 0000000000..7ed7bc3042 --- /dev/null +++ b/test/e2e/suite/workspace/addExistingIntegration.e2e.test.ts @@ -0,0 +1,377 @@ +/** + * E2E (ExTester): "Add Existing Integration" links an integration ANOTHER project in the workspace + * has stored credentials for into the current project — a link, not a copy: the credentials stay in + * SecretStorage and only the project's integration list is rewritten. + * + * Candidates come from SecretStorage, which nothing outside the UI can seed, so the suite configures + * a PostgreSQL integration in one project through the integrations panel and reuses it from the + * other. Runs without a Python kernel. + */ + +import { deserializeDeepnoteFile } from '@deepnote/blocks'; +import { expect } from 'chai'; +import * as fs from 'fs'; +import { By, EditorView, InputBox, QuickPickItem, VSBrowser, WebView, Workbench } from 'vscode-extension-tester'; + +import { + QUICK_PICK_TIMEOUT, + SUITE_TIMEOUT, + WORKBENCH_TIMEOUT, + assertNotNull, + copyFixtureIntoDir, + copyFixtureToTempDir, + createScreenshotter, + dismissAllNotifications, + openFolderViaDialog, + openWorkspaceFile, + waitForNotification +} from '../../helpers'; + +/** Where the shared integration is configured. Its project declares none of its own to begin with. */ +const SOURCE_FILE = 'quick-notes.deepnote'; +const SOURCE_PROJECT_NAME = 'Quick Notes'; +/** Where it is reused. Its project already declares an integration, which the link must not drop. */ +const TARGET_FILE = 'sales-analytics-revenue.deepnote'; +const TARGET_OWN_INTEGRATION = { id: 'int-bq-sales', name: 'Sales BigQuery', type: 'big-query' }; + +const INTEGRATION_NAME = 'E2E Shared Postgres'; +const INTEGRATION_TYPE_LABEL = 'PostgreSQL'; +const INTEGRATION_HOST = 'e2e-postgres.invalid'; +const INTEGRATION_DATABASE = 'e2e_reuse_db'; +const INTEGRATION_USER = 'e2e_reuse_user'; +const INTEGRATION_PASSWORD = 'e2e-reuse-password'; + +const MANAGE_INTEGRATIONS = 'Deepnote: Manage Integrations'; +const ADD_EXISTING_INTEGRATION = 'Deepnote: Add Existing Integration'; +const PICKER_PLACEHOLDER = 'Select an integration configured in another project of this workspace'; +const INTEGRATIONS_PANEL_TITLE = 'Deepnote Integrations'; + +// Prior editors finish closing before the next notebook opens. +const EDITORS_CLOSE_DELAY = 500; +// Freshly opened notebook paints before we refocus it. +const NOTEBOOK_OPEN_SETTLE_DELAY = 1_500; +// Notebook editor becomes active so the command targets it. +const EDITOR_REFOCUS_DELAY = 500; +// The integrations panel opens and its React app renders. +const PANEL_RENDER_TIMEOUT = 45_000; +// Attach to the panel's webview frame per attempt. +const WEBVIEW_FRAME_SWITCH_TIMEOUT = 6_000; +// Pause between attach attempts. +const WEBVIEW_POLL_INTERVAL = 1_000; +// Saving writes SecretStorage and every `.deepnote` file of the project; poll the file for the result. +const FILE_WRITE_TIMEOUT = 30_000; +const FILE_POLL_INTERVAL = 500; +// How long the editors are watched for a panel the palette run must not open. The command used to open it within a +// few hundred milliseconds of its success message; every passing run spends the whole window. +const PANEL_STAYS_CLOSED_WINDOW = 3_000; +const EDITOR_POLL_INTERVAL = 500; + +type DeclaredIntegration = { id: string; name?: string; type: string }; + +function readDeclaredIntegrations(filePath: string): DeclaredIntegration[] { + return deserializeDeepnoteFile(fs.readFileSync(filePath, 'utf8')).project.integrations ?? []; +} + +/** Closes every editor and makes `fileName` the active one — both commands act on the active notebook. */ +async function focusNotebook(fileName: string): Promise { + const driver = VSBrowser.instance.driver; + + // The integrations panel is an editor too, and it would otherwise still be the active one. + await new EditorView().closeAllEditors().catch((error) => { + console.warn('[add-existing] close editors:', error); + }); + await driver.sleep(EDITORS_CLOSE_DELAY); + + await openWorkspaceFile(fileName); + await driver.sleep(NOTEBOOK_OPEN_SETTLE_DELAY); + await new EditorView().openEditor(fileName).catch((error) => { + console.warn(`[add-existing] refocus ${fileName}:`, error); + }); + await driver.sleep(EDITOR_REFOCUS_DELAY); +} + +/** Polls until the integrations panel has rendered, leaving the driver outside the webview frame. */ +async function awaitIntegrationsPanel(): Promise { + const driver = VSBrowser.instance.driver; + const deadline = Date.now() + PANEL_RENDER_TIMEOUT; + let lastError: unknown; + + while (Date.now() < deadline) { + const webview = new WebView(); + + try { + await webview.switchToFrame(WEBVIEW_FRAME_SWITCH_TIMEOUT); + + if ((await driver.findElements(By.css('.integration-type-selector'))).length > 0) { + return; + } + } catch (error) { + lastError = error; + } finally { + await webview.switchBack().catch((error) => { + console.warn('[add-existing] switch back while waiting for the panel:', error); + }); + } + + await driver.sleep(WEBVIEW_POLL_INTERVAL); + } + + throw new Error(`The integrations panel did not render within ${PANEL_RENDER_TIMEOUT}ms: ${lastError}`); +} + +/** Runs `act` inside the integrations panel's webview frame. */ +async function inIntegrationsPanel(act: () => Promise): Promise { + const webview = new WebView(); + await webview.switchToFrame(WEBVIEW_FRAME_SWITCH_TIMEOUT); + + try { + return await act(); + } finally { + await webview.switchBack().catch((error) => { + console.warn('[add-existing] switch back from the panel:', error); + }); + } +} + +/** + * Fills in and saves a new PostgreSQL integration in the panel of the currently open project, which + * is what puts its credentials in SecretStorage and declares it on that project. + */ +async function configurePostgresIntegration(): Promise { + const driver = VSBrowser.instance.driver; + + await focusNotebook(SOURCE_FILE); + await new Workbench().executeCommand(MANAGE_INTEGRATIONS); + await awaitIntegrationsPanel(); + + await inIntegrationsPanel(async () => { + // Locate AND click in one wait: the grid re-renders as the panel receives its first update. + await driver.wait( + async () => { + for (const card of await driver.findElements(By.css('.integration-type-card'))) { + if (!(await card.getText()).includes(INTEGRATION_TYPE_LABEL)) { + continue; + } + + await card.click(); + + return true; + } + + return false; + }, + PANEL_RENDER_TIMEOUT, + `the "${INTEGRATION_TYPE_LABEL}" integration type card could not be clicked` + ); + + await driver.wait( + async () => (await driver.findElements(By.css('.configuration-form-container #password'))).length > 0, + PANEL_RENDER_TIMEOUT, + 'the PostgreSQL configuration form did not open' + ); + + // Port keeps its 5432 default; the rest are required and start empty except the generated name. + for (const [selector, value] of [ + ['#name', INTEGRATION_NAME], + ['#host', INTEGRATION_HOST], + ['#database', INTEGRATION_DATABASE], + ['#username', INTEGRATION_USER], + ['#password', INTEGRATION_PASSWORD] + ]) { + const field = await driver.findElement(By.css(`.configuration-form-container ${selector}`)); + await field.clear(); + await field.sendKeys(value); + } + + await driver.findElement(By.css('.configuration-form-container .form-actions button.primary')).click(); + }); +} + +/** + * Every editor title seen over `windowMs`. Absence needs a window rather than one read: the panel used to open a beat + * after the command's success message. A failed read sees nothing, so callers also require a title they expect. + */ +async function watchOpenEditorTitles(windowMs: number): Promise { + const driver = VSBrowser.instance.driver; + const deadline = Date.now() + windowMs; + const seen = new Set(); + + while (Date.now() < deadline) { + const titles = await new EditorView().getOpenEditorTitles().catch((error) => { + console.warn('[add-existing] read editor titles:', error); + + return []; + }); + + titles.forEach((title) => seen.add(title)); + await driver.sleep(EDITOR_POLL_INTERVAL); + } + + return Array.from(seen); +} + +/** Polls `filePath` until its project declares an integration named `name`, and returns that entry's id. */ +async function awaitDeclaredIntegrationId(filePath: string, name: string): Promise { + const driver = VSBrowser.instance.driver; + const deadline = Date.now() + FILE_WRITE_TIMEOUT; + let declared: DeclaredIntegration[] = []; + let lastReadError: unknown; + + while (Date.now() < deadline) { + // The extension rewrites the file in place, so a poll can land mid-write and parse a torn file: not yet. + try { + declared = readDeclaredIntegrations(filePath); + lastReadError = undefined; + } catch (error) { + lastReadError = error; + } + + const match = declared.find((entry) => entry.name === name); + + if (match) { + return match.id; + } + + await driver.sleep(FILE_POLL_INTERVAL); + } + + throw new Error( + `${filePath} never declared an integration named "${name}" within ${FILE_WRITE_TIMEOUT}ms; ` + + `last saw ${JSON.stringify(declared)}` + + (lastReadError ? `, and the last read failed: ${lastReadError}` : '') + ); +} + +describe('Deepnote — adding an integration another project already configured', function () { + this.timeout(SUITE_TIMEOUT); + + let activeEditorAfterLink: string | undefined; + let cleanupTempDir: (() => void) | undefined; + let editorTitlesAfterLink: string[] = []; + let pickedDescription: string | undefined; + let pickedRowText = ''; + let sharedIntegrationId = ''; + let targetFileContents = ''; + let targetIntegrations: DeclaredIntegration[] = []; + + before(async function () { + const screenshot = createScreenshotter(this); + + const copy = copyFixtureToTempDir(SOURCE_FILE); + cleanupTempDir = copy.cleanup; + const sourceFilePath = copy.filePath; + const targetFilePath = copyFixtureIntoDir(copy.tempDir, TARGET_FILE); + + await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); + await openFolderViaDialog(copy.tempDir); + await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); + + // Nothing is reusable yet: the other project declares "Sales BigQuery" but has never stored + // credentials for it, and a declaration on its own carries nothing to reuse. + await focusNotebook(SOURCE_FILE); + await new Workbench().executeCommand(ADD_EXISTING_INTEGRATION); + await waitForNotification( + /No integrations from other projects in this workspace are available to add/i, + WORKBENCH_TIMEOUT, + true + ); + await screenshot('nothing-to-reuse'); + await dismissAllNotifications(); + + await configurePostgresIntegration(); + sharedIntegrationId = await awaitDeclaredIntegrationId(sourceFilePath, INTEGRATION_NAME); + await screenshot('configured-in-the-other-project'); + await dismissAllNotifications(); + + await focusNotebook(TARGET_FILE); + await new Workbench().executeCommand(ADD_EXISTING_INTEGRATION); + + const picker = await InputBox.create(QUICK_PICK_TIMEOUT); + let picked: QuickPickItem | undefined; + + // The command palette reuses this widget, so gate on OUR placeholder before reading the items. + await VSBrowser.instance.driver.wait( + async () => { + if ((await picker.getPlaceHolder().catch(() => '')) !== PICKER_PLACEHOLDER) { + return false; + } + + for (const candidate of await picker.getQuickPicks()) { + if ((await candidate.getLabel()) !== INTEGRATION_NAME) { + continue; + } + + picked = candidate; + + return true; + } + + return false; + }, + QUICK_PICK_TIMEOUT, + `the reuse picker never offered "${INTEGRATION_NAME}"` + ); + await screenshot('reuse-picker'); + + const item = assertNotNull(picked, `the reuse picker never offered "${INTEGRATION_NAME}"`); + + pickedDescription = await item.getDescription(); + // The detail line ("Used in: …") has no page object; it is part of the row's rendered text. + pickedRowText = await item.getText(); + await item.select(); + + await waitForNotification( + new RegExp(`Added integration "${INTEGRATION_NAME}" to this project`), + WORKBENCH_TIMEOUT, + true + ); + await screenshot('linked-into-this-project'); + + // No panel is open (focusing the target closed every editor), so a palette run must leave it that way. + editorTitlesAfterLink = await watchOpenEditorTitles(PANEL_STAYS_CLOSED_WINDOW); + activeEditorAfterLink = await new EditorView() + .getActiveTab() + .then((tab) => tab?.getTitle()) + .catch(() => undefined); + await screenshot('panel-stays-closed'); + + targetIntegrations = readDeclaredIntegrations(targetFilePath); + targetFileContents = fs.readFileSync(targetFilePath, 'utf8'); + }); + + after(async function () { + await new WebView().switchBack().catch(() => undefined); + await new EditorView().closeAllEditors().catch(() => undefined); + try { + cleanupTempDir?.(); + } catch (error) { + console.warn('[add-existing] remove temp workspace dir during cleanup:', error); + } + }); + + // Deliberately one test: every expectation reads state the `before` hook already captured, so + // splitting them buys separate mocha records and nothing else. + it('offers only what the other project configured, and links it without its credentials or the panel', function () { + // offers the integration the other project configured, and names that project + expect(pickedDescription, 'quick pick description').to.equal(INTEGRATION_TYPE_LABEL); + expect(pickedRowText, 'quick pick row').to.contain(`Used in: ${SOURCE_PROJECT_NAME}`); + + // writes the link into this project on disk, keeping the integration it already had + expect(targetIntegrations, 'integrations declared by the target project').to.deep.equal([ + TARGET_OWN_INTEGRATION, + { id: sharedIntegrationId, name: INTEGRATION_NAME, type: 'pgsql' } + ]); + + // links the integration without copying its credentials into the file + expect(targetFileContents, 'target project file').to.not.contain(INTEGRATION_HOST); + expect(targetFileContents, 'target project file').to.not.contain(INTEGRATION_PASSWORD); + + // leaves the integrations panel closed and the notebook focused when run from the palette + expect( + editorTitlesAfterLink.some((title) => title.includes(TARGET_FILE)), + `the target notebook among the editors seen after the link: ${JSON.stringify(editorTitlesAfterLink)}` + ).to.equal(true); + expect(editorTitlesAfterLink, 'editors seen after the link').to.not.include(INTEGRATIONS_PANEL_TITLE); + expect(activeEditorAfterLink, 'active editor after the link').to.contain(TARGET_FILE); + }); +}); diff --git a/test/e2e/suite/workspace/integrations.e2e.test.ts b/test/e2e/suite/workspace/integrations.e2e.test.ts index 22f903aae4..0256ae637e 100644 --- a/test/e2e/suite/workspace/integrations.e2e.test.ts +++ b/test/e2e/suite/workspace/integrations.e2e.test.ts @@ -24,6 +24,8 @@ const WEBVIEW_READ_TIMEOUT = 15_000; // Empty-state text asserted to prove the panel actually opened (else the negative `not.contain` // below passes trivially against a blank/failed `''` read). const NO_INTEGRATIONS_TEXT = 'No integrations found in this project.'; +// The reuse entry point must render even when this project has no integrations of its own. +const ADD_EXISTING_INTEGRATION_TEXT = 'Add Existing Integration'; // Prior editors finish closing before reopening the target notebook. const EDITORS_CLOSE_DELAY = 500; // Freshly opened notebook paints before we refocus it. @@ -126,5 +128,6 @@ describe('Deepnote — the integrations UI', function () { // Positive signal that the panel rendered (a blank read would make `not.contain` pass trivially). expect(text, 'integrations webview text').to.contain(NO_INTEGRATIONS_TEXT); expect(text, 'integrations webview text').to.not.contain(INTEGRATION_NAME); + expect(text, 'integrations webview text').to.contain(ADD_EXISTING_INTEGRATION_TEXT); }); });