diff --git a/CHANGELOG.md b/CHANGELOG.md index 58941856f3..663ebfeaa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ All notable changes to this extension will be documented in this file. - Submitted Flink statements now allow you to choose your own statement name prefix (or to leave generic). Set your prefix using preference: [`confluent.flink.statementPrefix`](vscode://settings/confluent.flink.statementPrefix). +- Clicking a statement from the Flink Statements view will now open an editable document instead of + a read-only preview document. This allows users to more easily edit and resubmit statements. ### Fixed diff --git a/src/commands/flinkStatements.test.ts b/src/commands/flinkStatements.test.ts index 2dd1e2d167..e48d5758db 100644 --- a/src/commands/flinkStatements.test.ts +++ b/src/commands/flinkStatements.test.ts @@ -1,8 +1,7 @@ import assert from "assert"; import sinon from "sinon"; import * as vscode from "vscode"; - -import { TextDocument } from "vscode-json-languageservice"; +import { TextDocument } from "vscode"; import { eventEmitterStubs } from "../../tests/stubs/emitters"; import { getStubbedCCloudResourceLoader } from "../../tests/stubs/resourceLoaders"; import { @@ -13,7 +12,7 @@ import { import { TEST_CCLOUD_FLINK_COMPUTE_POOL } from "../../tests/unit/testResources/flinkComputePool"; import { createFlinkStatement } from "../../tests/unit/testResources/flinkStatement"; import * as flinkCodeLens from "../codelens/flinkSqlProvider"; -import { FlinkStatementDocumentProvider } from "../documentProviders/flinkStatement"; +import { FLINK_SQL_LANGUAGE_ID } from "../flinkSql/constants"; import * as statementUtils from "../flinkSql/statementUtils"; import { CCloudResourceLoader } from "../loaders"; import { CCloudEnvironment } from "../models/environment"; @@ -45,14 +44,42 @@ describe("commands/flinkStatements.ts", () => { describe("viewStatementSqlCommand", () => { let getCatalogDatabaseFromMetadataStub: sinon.SinonStub; + let workspaceTextDocumentsStub: sinon.SinonStub; + let openTextDocumentStub: sinon.SinonStub; let showTextDocumentStub: sinon.SinonStub; let setUriMetadataStub: sinon.SinonStub; + const testPool = TEST_CCLOUD_FLINK_COMPUTE_POOL; + const testEnv = new CCloudEnvironment({ + ...TEST_CCLOUD_ENVIRONMENT, + flinkComputePools: [testPool], + }); + + const testSqlStatement = "SELECT * FROM my_test_flink_statement_table"; + const testStatement: FlinkStatement = createFlinkStatement({ + sqlStatement: testSqlStatement, + }); + const testDoc = { + languageId: FLINK_SQL_LANGUAGE_ID, + content: testSqlStatement, + uri: vscode.Uri.parse("untitled:SELECT1"), + getText: () => testSqlStatement, + } as unknown as TextDocument; + beforeEach(() => { - getCatalogDatabaseFromMetadataStub = sandbox.stub( - flinkCodeLens, - "getCatalogDatabaseFromMetadata", - ); + stubbedLoader.getEnvironments.resolves([testEnv]); + stubbedLoader.getFlinkComputePool.resolves(testPool); + + getCatalogDatabaseFromMetadataStub = sandbox + .stub(flinkCodeLens, "getCatalogDatabaseFromMetadata") + .resolves({ + catalog: testEnv, + database: TEST_CCLOUD_KAFKA_CLUSTER, + }); + + // no open documents in the workspace by default + workspaceTextDocumentsStub = sandbox.stub(vscode.workspace, "textDocuments").get(() => []); + openTextDocumentStub = sandbox.stub(vscode.workspace, "openTextDocument").resolves(testDoc); showTextDocumentStub = sandbox.stub(vscode.window, "showTextDocument"); setUriMetadataStub = sandbox.stub(ResourceManager.getInstance(), "setUriMetadata"); }); @@ -67,77 +94,72 @@ describe("commands/flinkStatements.ts", () => { assert.strictEqual(result, undefined); }); - it("should open a read-only document for a FlinkStatement", async () => { - const testPool = TEST_CCLOUD_FLINK_COMPUTE_POOL; - const testEnv = new CCloudEnvironment({ - ...TEST_CCLOUD_ENVIRONMENT, - flinkComputePools: [testPool], - }); - stubbedLoader.getEnvironments.resolves([testEnv]); - stubbedLoader.getFlinkComputePool.resolves(testPool); - getCatalogDatabaseFromMetadataStub.returns({ - catalog: testEnv, - database: TEST_CCLOUD_KAFKA_CLUSTER, - }); - - const statement = createFlinkStatement({ - sqlStatement: "SELECT * FROM my_test_flink_statement_table", - }); - const uri = FlinkStatementDocumentProvider.getStatementDocumentUri(statement); + it("should create and open an untitled document for a FlinkStatement", async () => { + // no existing open documents by default - await viewStatementSqlCommand(statement); + await viewStatementSqlCommand(testStatement); sinon.assert.calledOnce(stubbedLoader.getFlinkComputePool); - sinon.assert.calledWithExactly(stubbedLoader.getFlinkComputePool, statement.computePoolId!); + sinon.assert.calledWithExactly( + stubbedLoader.getFlinkComputePool, + testStatement.computePoolId!, + ); sinon.assert.calledOnce(getCatalogDatabaseFromMetadataStub); sinon.assert.calledWithExactly( getCatalogDatabaseFromMetadataStub, { - [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: statement.computePoolId, - [UriMetadataKeys.FLINK_CATALOG_NAME]: statement.catalog, - [UriMetadataKeys.FLINK_DATABASE_NAME]: statement.database, + [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: testStatement.computePoolId, + [UriMetadataKeys.FLINK_CATALOG_NAME]: testStatement.catalog, + [UriMetadataKeys.FLINK_DATABASE_NAME]: testStatement.database, }, testPool, ); + sinon.assert.calledOnce(openTextDocumentStub); + sinon.assert.calledOnceWithExactly(openTextDocumentStub, { + language: FLINK_SQL_LANGUAGE_ID, + content: testStatement.sqlStatement, + }); + + sinon.assert.calledOnce(setUriMetadataStub); // params tested in separate test + sinon.assert.calledOnce(showTextDocumentStub); const document: TextDocument = showTextDocumentStub.firstCall.args[0]; - assert.strictEqual(document.uri.toString(), uri.toString()); + assert.strictEqual(document.uri.scheme, "untitled"); sinon.assert.calledWithExactly(showTextDocumentStub, document, { preview: false }); }); - it("should set Uri metadata before opening the document", async () => { - const testPool = TEST_CCLOUD_FLINK_COMPUTE_POOL; - const testEnv = new CCloudEnvironment({ - ...TEST_CCLOUD_ENVIRONMENT, - flinkComputePools: [testPool], - }); - stubbedLoader.getEnvironments.resolves([testEnv]); - stubbedLoader.getFlinkComputePool.resolves(testPool); - getCatalogDatabaseFromMetadataStub.returns({ - catalog: testEnv, - database: TEST_CCLOUD_KAFKA_CLUSTER, - }); + it("should show an existing untitled document for a FlinkStatement", async () => { + // simulate an existing open document with the same SQL as the statement + workspaceTextDocumentsStub.get(() => [testDoc]); - const statement = createFlinkStatement({ - sqlStatement: "SELECT * FROM my_test_flink_statement_table", - }); - const uri = FlinkStatementDocumentProvider.getStatementDocumentUri(statement); + await viewStatementSqlCommand(testStatement); + + // same assertions as the previous test, except we don't create a new document or set metadata + sinon.assert.notCalled(openTextDocumentStub); + sinon.assert.notCalled(setUriMetadataStub); + sinon.assert.calledOnce(showTextDocumentStub); + const document: TextDocument = showTextDocumentStub.firstCall.args[0]; + assert.strictEqual(document.uri.scheme, "untitled"); + sinon.assert.calledWithExactly(showTextDocumentStub, document, { preview: false }); + }); - await viewStatementSqlCommand(statement); + it("should set Uri metadata before showing the document", async () => { + await viewStatementSqlCommand(testStatement); sinon.assert.calledOnce(setUriMetadataStub); const callArgs = setUriMetadataStub.firstCall.args; assert.strictEqual(callArgs.length, 2); - assert.strictEqual(callArgs[0].toString(), uri.toString()); + assert.strictEqual(callArgs[0].toString(), testDoc.uri.toString()); assert.deepStrictEqual(callArgs[1], { - [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: statement.computePoolId, + [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: testStatement.computePoolId, [UriMetadataKeys.FLINK_CATALOG_ID]: TEST_CCLOUD_ENVIRONMENT.id, - [UriMetadataKeys.FLINK_CATALOG_NAME]: statement.catalog, + [UriMetadataKeys.FLINK_CATALOG_NAME]: testStatement.catalog, [UriMetadataKeys.FLINK_DATABASE_ID]: TEST_CCLOUD_KAFKA_CLUSTER.id, - [UriMetadataKeys.FLINK_DATABASE_NAME]: statement.database, + [UriMetadataKeys.FLINK_DATABASE_NAME]: testStatement.database, }); + sinon.assert.callOrder(openTextDocumentStub, setUriMetadataStub, showTextDocumentStub); }); }); diff --git a/src/commands/flinkStatements.ts b/src/commands/flinkStatements.ts index cbefb595a6..5e09f25bb8 100644 --- a/src/commands/flinkStatements.ts +++ b/src/commands/flinkStatements.ts @@ -1,10 +1,6 @@ import * as vscode from "vscode"; import { registerCommandWithLogging } from "."; import { getCatalogDatabaseFromMetadata } from "../codelens/flinkSqlProvider"; -import { - FLINKSTATEMENT_URI_SCHEME, - FlinkStatementDocumentProvider, -} from "../documentProviders/flinkStatement"; import { udfsChanged } from "../emitters"; import { extractResponseBody, isResponseError, logError } from "../errors"; import { FLINK_SQL_FILE_EXTENSIONS, FLINK_SQL_LANGUAGE_ID } from "../flinkSql/constants"; @@ -36,25 +32,15 @@ const logger = new Logger("commands.flinkStatements"); /** View the SQL statement portion of a FlinkStatement in a read-only document. */ export async function viewStatementSqlCommand(statement: FlinkStatement): Promise { - if (!statement) { - logger.error("viewStatementSqlCommand", "statement is undefined"); - return; - } - if (!(statement instanceof FlinkStatement)) { - logger.error("viewStatementSqlCommand", "statement is not an instance of FlinkStatement"); - return; - } - - if (!statement.computePoolId) { - logger.error("viewStatementSqlCommand", "statement has no computePoolId"); + logger.error("statement is not an instance of FlinkStatement"); return; } - const uri = FlinkStatementDocumentProvider.getStatementDocumentUri(statement); - const loader = CCloudResourceLoader.getInstance(); - const pool = await loader.getFlinkComputePool(statement.computePoolId); + // if we're opening a statement from the Flink Statements view, it was submitted to a pool and + // we'll have the computePoolId + const pool = await loader.getFlinkComputePool(statement.computePoolId!); if (!pool) { logger.error( "viewStatementSqlCommand", @@ -78,10 +64,22 @@ export async function viewStatementSqlCommand(statement: FlinkStatement): Promis updatedMetadata[UriMetadataKeys.FLINK_DATABASE_ID] = database.id; } const rm = ResourceManager.getInstance(); - await rm.setUriMetadata(uri, updatedMetadata); - const doc = await vscode.workspace.openTextDocument(uri); - vscode.languages.setTextDocumentLanguage(doc, "flinksql"); + // check if any existing document is already open with this statement's SQL content + // (exact match for now, but if we wanted to add header comments to new untitled documents and/or + // check for "document contains statement SQL" we'll need to update this logic) + let doc: vscode.TextDocument | undefined = vscode.workspace.textDocuments.find( + (doc) => doc.languageId === FLINK_SQL_LANGUAGE_ID && doc.getText() === statement.sqlStatement, + ); + if (!doc) { + // create a new untitled doc, set its language and content, then update its metadata before + // showing it to the user + doc = await vscode.workspace.openTextDocument({ + language: FLINK_SQL_LANGUAGE_ID, + content: statement.sqlStatement, + }); + await rm.setUriMetadata(doc.uri, updatedMetadata); + } await vscode.window.showTextDocument(doc, { preview: false }); } /** @@ -107,7 +105,7 @@ export async function submitFlinkStatementCommand( const funcLogger = logger.withCallpoint("submitFlinkStatementCommand"); // 1. Choose the document with the SQL to submit - const uriSchemes = ["file", "untitled", FLINKSTATEMENT_URI_SCHEME]; + const uriSchemes = ["file", "untitled"]; const languageIds = ["plaintext", FLINK_SQL_LANGUAGE_ID, "sql"]; const fileFilters = { "FlinkSQL files": [...FLINK_SQL_FILE_EXTENSIONS, ".sql"], diff --git a/src/documentMetadataManager.test.ts b/src/documentMetadataManager.test.ts index 653d63b8cb..68a8ebea48 100644 --- a/src/documentMetadataManager.test.ts +++ b/src/documentMetadataManager.test.ts @@ -2,156 +2,155 @@ import * as assert from "assert"; import * as sinon from "sinon"; import { TextDocument, Uri, workspace } from "vscode"; import { DocumentMetadataManager } from "./documentMetadataManager"; -import { FLINKSTATEMENT_URI_SCHEME } from "./documentProviders/flinkStatement"; import { UriMetadataKeys } from "./storage/constants"; import { ResourceManager } from "./storage/resourceManager"; import { UriMetadataMap } from "./storage/types"; describe("documentMetadataManager.ts", () => { let sandbox: sinon.SinonSandbox; - let stubResourceManager: sinon.SinonStubbedInstance; - let manager: DocumentMetadataManager; beforeEach(() => { sandbox = sinon.createSandbox(); - - stubResourceManager = sandbox.createStubInstance(ResourceManager); - sandbox.stub(ResourceManager, "getInstance").returns(stubResourceManager); - - manager = DocumentMetadataManager.getInstance(); }); afterEach(() => { - manager.dispose(); - DocumentMetadataManager["instance"] = null; sandbox.restore(); }); - it("should create only one instance of DocumentMetadataManager", () => { - const manager2 = DocumentMetadataManager.getInstance(); - try { - assert.strictEqual(manager, manager2); - } finally { - manager2.dispose(); - } - }); - - it("should register document lifecycle event handlers when instantiated", () => { - const onDidOpenTextDocumentSpy = sandbox.spy(workspace, "onDidOpenTextDocument"); - const onDidSaveTextDocumentSpy = sandbox.spy(workspace, "onDidSaveTextDocument"); - const onDidCloseTextDocumentSpy = sandbox.spy(workspace, "onDidCloseTextDocument"); - - // ensure we start fresh with a new constructor call - manager.dispose(); - DocumentMetadataManager["instance"] = null; - DocumentMetadataManager.getInstance(); - - sinon.assert.calledOnce(onDidOpenTextDocumentSpy); - sinon.assert.calledOnce(onDidSaveTextDocumentSpy); - sinon.assert.calledOnce(onDidCloseTextDocumentSpy); - }); - - it("handleDocumentSave() should exit early for 'untitled' documents", async () => { - // NOTE: setting up fake TextDocuments is tricky since we can't create them directly, so we're - // only populating the fields needed for the test and associated codebase logic, then using the - // `as unknown as TextDocument` pattern to appease TypeScript - const fakeUntitledDoc: TextDocument = { - uri: Uri.parse("untitled:test.sql"), - } as unknown as TextDocument; - - await manager["handleDocumentSave"](fakeUntitledDoc); - - sinon.assert.notCalled(stubResourceManager.getAllUriMetadata); - }); - - for (const scheme of ["untitled", FLINKSTATEMENT_URI_SCHEME]) { - it(`handleDocumentSave() should migrate metadata to newly-saved 'file' document when content matches exactly (unsaved scheme='${scheme}')`, async () => { - // set up the unsaved document - const fakeUnsavedDoc: TextDocument = { - uri: Uri.parse(`${scheme}:test.sql`), - getText: () => "SELECT * FROM test", - } as unknown as TextDocument; - sandbox.stub(workspace, "textDocuments").get(() => [fakeUnsavedDoc]); - sandbox.stub(workspace, "openTextDocument").resolves(fakeUnsavedDoc); - // set up the "after save" file-scheme document - const fakeFileDoc: TextDocument = { - uri: Uri.parse("file:///test.sql"), - getText: () => "SELECT * FROM test", - } as unknown as TextDocument; + describe("DocumentMetadataManager", () => { + let stubResourceManager: sinon.SinonStubbedInstance; + let manager: DocumentMetadataManager; - // set some initial metadata for the untitled document to be migrated - const metadata = { - [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", - }; - const metadataMap: UriMetadataMap = new Map(); - metadataMap.set(fakeUnsavedDoc.uri.toString(), metadata); - stubResourceManager.getAllUriMetadata.resolves(metadataMap); + beforeEach(() => { + stubResourceManager = sandbox.createStubInstance(ResourceManager); + sandbox.stub(ResourceManager, "getInstance").returns(stubResourceManager); - await manager["handleDocumentSave"](fakeFileDoc); + manager = DocumentMetadataManager.getInstance(); + }); - // migration should have happened by setting the metadata for the file document and deleting the - // metadata for the untitled document - sinon.assert.calledWith(stubResourceManager.setUriMetadata, fakeFileDoc.uri, metadata); - sinon.assert.calledWith(stubResourceManager.deleteUriMetadata, fakeUnsavedDoc.uri); + afterEach(() => { + manager.dispose(); + DocumentMetadataManager["instance"] = null; }); - it(`handleDocumentSave() should migrate metadata to newly-saved 'file' document when content matches aside from whitespace/newlines (unsaved scheme='${scheme}')`, async () => { - // set up the unsaved document - const fakeUnsavedDoc: TextDocument = { - uri: Uri.parse(`${scheme}:test.sql`), - getText: () => " SELECT * FROM test ", - } as unknown as TextDocument; - sandbox.stub(workspace, "textDocuments").get(() => [fakeUnsavedDoc]); - sandbox.stub(workspace, "openTextDocument").resolves(fakeUnsavedDoc); - // set up the "after save" file document - const fakeFileDoc: TextDocument = { - uri: Uri.parse("file:///test.sql"), - getText: () => "SELECT * FROM test\n", - } as unknown as TextDocument; + it("should create only one instance of DocumentMetadataManager", () => { + const manager2 = DocumentMetadataManager.getInstance(); + try { + assert.strictEqual(manager, manager2); + } finally { + manager2.dispose(); + } + }); - // set some initial metadata for the untitled document to be migrated - const metadata = { - [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", - }; - const metadataMap: UriMetadataMap = new Map(); - metadataMap.set(fakeUnsavedDoc.uri.toString(), metadata); - stubResourceManager.getAllUriMetadata.resolves(metadataMap); + it("should register document lifecycle event handlers when instantiated", () => { + const onDidOpenTextDocumentSpy = sandbox.spy(workspace, "onDidOpenTextDocument"); + const onDidSaveTextDocumentSpy = sandbox.spy(workspace, "onDidSaveTextDocument"); + const onDidCloseTextDocumentSpy = sandbox.spy(workspace, "onDidCloseTextDocument"); - await manager["handleDocumentSave"](fakeFileDoc); + // ensure we start fresh with a new constructor call + manager.dispose(); + DocumentMetadataManager["instance"] = null; + DocumentMetadataManager.getInstance(); - // migration should have happened by setting the metadata for the file document and deleting the - // metadata for the untitled document - sinon.assert.calledWith(stubResourceManager.setUriMetadata, fakeFileDoc.uri, metadata); - sinon.assert.calledWith(stubResourceManager.deleteUriMetadata, fakeUnsavedDoc.uri); + sinon.assert.calledOnce(onDidOpenTextDocumentSpy); + sinon.assert.calledOnce(onDidSaveTextDocumentSpy); + sinon.assert.calledOnce(onDidCloseTextDocumentSpy); }); - it(`handleDocumentSave() should not migrate metadata when content does not match (unsaved scheme='${scheme}')`, async () => { - // set up the unsaved document - const fakeUnsavedDoc: TextDocument = { - uri: Uri.parse(`${scheme}:test.sql`), + describe("handleDocumentSave()", () => { + // NOTE: setting up fake TextDocuments is tricky since we can't create them directly, so we're + // only populating the fields needed for the test and associated codebase logic, then using the + // `as unknown as TextDocument` pattern to appease TypeScript. + + // first, the unsaved "untitled" document + const fakeUntitledDoc: TextDocument = { + uri: Uri.parse("untitled:test.sql"), getText: () => "SELECT * FROM test", } as unknown as TextDocument; - sandbox.stub(workspace, "textDocuments").get(() => [fakeUnsavedDoc]); - sandbox.stub(workspace, "openTextDocument").resolves(fakeUnsavedDoc); - // set up the "after save" file document + // and the after-save "file" document const fakeFileDoc: TextDocument = { - uri: Uri.parse("file:///test.sql"), - getText: () => "SELECT * FROM some_other_table", + ...fakeUntitledDoc, + uri: fakeUntitledDoc.uri.with({ scheme: "file" }), } as unknown as TextDocument; - // set some initial metadata for the untitled document to (hopefully) not migrate - const metadata = { - [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", - }; - const metadataMap: UriMetadataMap = new Map(); - metadataMap.set(fakeUnsavedDoc.uri.toString(), metadata); - stubResourceManager.getAllUriMetadata.resolves(metadataMap); - - await manager["handleDocumentSave"](fakeFileDoc); - - // migration should not have happened - sinon.assert.notCalled(stubResourceManager.setUriMetadata); - sinon.assert.notCalled(stubResourceManager.deleteUriMetadata); + it("should exit early for 'untitled' documents", async () => { + await manager["handleDocumentSave"](fakeUntitledDoc); + + sinon.assert.notCalled(stubResourceManager.getAllUriMetadata); + }); + + it("should migrate 'untitled' doc metadata to newly-saved 'file' doc when content matches exactly", async () => { + // set up the unsaved document + sandbox.stub(workspace, "textDocuments").get(() => [fakeUntitledDoc]); + sandbox.stub(workspace, "openTextDocument").resolves(fakeUntitledDoc); + + // set some initial metadata for the untitled document to be migrated + const metadata = { + [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", + }; + const metadataMap: UriMetadataMap = new Map(); + metadataMap.set(fakeUntitledDoc.uri.toString(), metadata); + stubResourceManager.getAllUriMetadata.resolves(metadataMap); + + await manager["handleDocumentSave"](fakeFileDoc); + + // migration should have happened by setting the metadata for the file document and deleting the + // metadata for the untitled document + sinon.assert.calledWith(stubResourceManager.setUriMetadata, fakeFileDoc.uri, metadata); + sinon.assert.calledWith(stubResourceManager.deleteUriMetadata, fakeUntitledDoc.uri); + }); + + it("should migrate 'untitled' doc metadata to newly-saved 'file' doc when content matches aside from whitespace/newlines", async () => { + // set up the unsaved document + const fakeUntitledDocWithWhitespaceContent: TextDocument = { + ...fakeUntitledDoc, + getText: () => " SELECT * FROM test ", + } as unknown as TextDocument; + sandbox.stub(workspace, "textDocuments").get(() => [fakeUntitledDocWithWhitespaceContent]); + sandbox.stub(workspace, "openTextDocument").resolves(fakeUntitledDocWithWhitespaceContent); + + // set some initial metadata for the untitled document to be migrated + const metadata = { + [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", + }; + const metadataMap: UriMetadataMap = new Map(); + metadataMap.set(fakeUntitledDocWithWhitespaceContent.uri.toString(), metadata); + stubResourceManager.getAllUriMetadata.resolves(metadataMap); + + await manager["handleDocumentSave"](fakeFileDoc); + + // migration should have happened by setting the metadata for the file document and deleting the + // metadata for the untitled document + sinon.assert.calledWith(stubResourceManager.setUriMetadata, fakeFileDoc.uri, metadata); + sinon.assert.calledWith( + stubResourceManager.deleteUriMetadata, + fakeUntitledDocWithWhitespaceContent.uri, + ); + }); + + it("should not migrate metadata when content does not match", async () => { + sandbox.stub(workspace, "textDocuments").get(() => [fakeUntitledDoc]); + sandbox.stub(workspace, "openTextDocument").resolves(fakeUntitledDoc); + // set up the "after save" file document from some other source + const fakeFileDocWithOtherContents: TextDocument = { + uri: Uri.parse("file:///test.sql"), + getText: () => "SELECT * FROM some_other_table", + } as unknown as TextDocument; + + // set some initial metadata for the untitled document to (hopefully) not migrate + const metadata = { + [UriMetadataKeys.FLINK_COMPUTE_POOL_ID]: "test-compute-pool", + }; + const metadataMap: UriMetadataMap = new Map(); + metadataMap.set(fakeUntitledDoc.uri.toString(), metadata); + stubResourceManager.getAllUriMetadata.resolves(metadataMap); + + await manager["handleDocumentSave"](fakeFileDocWithOtherContents); + + // migration should not have happened + sinon.assert.notCalled(stubResourceManager.setUriMetadata); + sinon.assert.notCalled(stubResourceManager.deleteUriMetadata); + }); }); - } + }); }); diff --git a/src/documentMetadataManager.ts b/src/documentMetadataManager.ts index 6b6b7de387..e266681c44 100644 --- a/src/documentMetadataManager.ts +++ b/src/documentMetadataManager.ts @@ -1,5 +1,4 @@ import { TextDocument, Uri, workspace } from "vscode"; -import { FLINKSTATEMENT_URI_SCHEME } from "./documentProviders/flinkStatement"; import { Logger } from "./logging"; import { getResourceManager } from "./storage/resourceManager"; import { UriMetadataMap } from "./storage/types"; @@ -8,7 +7,7 @@ import { DisposableCollection } from "./utils/disposables"; const logger = new Logger("documentMetadataManager"); // Central list of supported URI schemes -const SUPPORTED_URI_SCHEMES = ["file", "untitled", FLINKSTATEMENT_URI_SCHEME]; +const SUPPORTED_URI_SCHEMES = ["file", "untitled"]; /** Manager for VS Code {@link TextDocument}s that tracks metadata across document lifecycle events */ export class DocumentMetadataManager extends DisposableCollection { @@ -72,7 +71,7 @@ export class DocumentMetadataManager extends DisposableCollection { const uri: Uri = Uri.parse(uriString); const scheme = uri.scheme; - if (!["untitled", FLINKSTATEMENT_URI_SCHEME].includes(scheme)) continue; + if (scheme !== "untitled") continue; const unsavedDoc: TextDocument | undefined = workspace.textDocuments.find( (doc) => doc.uri.toString() === uriString, diff --git a/src/documentProviders/flinkStatement.test.ts b/src/documentProviders/flinkStatement.test.ts deleted file mode 100644 index b2fc150293..0000000000 --- a/src/documentProviders/flinkStatement.test.ts +++ /dev/null @@ -1,22 +0,0 @@ -import assert from "assert"; -import { createFlinkStatement } from "../../tests/unit/testResources/flinkStatement"; -import { FlinkStatementDocumentProvider } from "./flinkStatement"; - -describe("FlinkStatementDocumentProvider", () => { - const provider: FlinkStatementDocumentProvider = new FlinkStatementDocumentProvider(); - - it("getStatementDocumentUri() / provideTextDocumentContent round trip", async () => { - const statementSQL = "SELECT * FROM my_test_flink_statement_table"; - const testStatement = createFlinkStatement({ sqlStatement: statementSQL }); - - // Should be able to round-trip the statement SQL through the URI and provideTextDocumentContent(). - const uri = FlinkStatementDocumentProvider.getStatementDocumentUri(testStatement); - const content = await provider.provideTextDocumentContent(uri); - - assert.strictEqual( - content, - statementSQL, - "The content of the document should match the SQL statement in the FlinkStatement", - ); - }); -}); diff --git a/src/documentProviders/flinkStatement.ts b/src/documentProviders/flinkStatement.ts deleted file mode 100644 index f6e741e82d..0000000000 --- a/src/documentProviders/flinkStatement.ts +++ /dev/null @@ -1,40 +0,0 @@ -import * as vscode from "vscode"; -import { ResourceDocumentProvider } from "."; -import { FlinkStatement } from "../models/flinkStatement"; - -/** The URI scheme for read-only flink statements, used by FlinkStatementDocumentProvider. */ -export const FLINKSTATEMENT_URI_SCHEME = "confluent.flinkstatement"; - -/** - * Minimal interface for the URI query string portion of a FlinkStatement. - * The resulting URIs are then durably stored in the workspace's open file list. - */ -interface FlinkStatementSQL { - sqlStatement: string; -} - -/** Makes a read-only editor buffer holding a flink SQL statement */ -export class FlinkStatementDocumentProvider extends ResourceDocumentProvider { - scheme = FLINKSTATEMENT_URI_SCHEME; - - /** - * Provide the text contents given a URI for this document provider scheme. - * Simply extracts the SQL statement from the URI query string and returns it. - */ - public async provideTextDocumentContent(uri: vscode.Uri): Promise { - // parse the URI query string into a FlinkStatementSQL instance, given that all - // our document URIs should have been created with the getStatementDocumentUri() method. - const fromUriQuery: FlinkStatementSQL = this.parseUriQueryBody(uri.query) as FlinkStatementSQL; - - return fromUriQuery.sqlStatement; - } - - /** Encode the SQL statement portion of the FlinkStatment into URI's query string. */ - static getStatementDocumentUri(statement: FlinkStatement): vscode.Uri { - return ResourceDocumentProvider.baseResourceToUri( - FLINKSTATEMENT_URI_SCHEME, - { sqlStatement: statement.sqlStatement } as FlinkStatementSQL, - statement.name, - ); - } -} diff --git a/src/extension.ts b/src/extension.ts index f42aa883fe..65d4d8e754 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -54,7 +54,6 @@ import { DirectConnectionManager } from "./directConnectManager"; import { EventListener } from "./docker/eventListener"; import { registerLocalResourceWorkflows } from "./docker/workflows/workflowInitialization"; import { DocumentMetadataManager } from "./documentMetadataManager"; -import { FlinkStatementDocumentProvider } from "./documentProviders/flinkStatement"; import { MESSAGE_URI_SCHEME, MessageDocumentProvider } from "./documentProviders/message"; import { SCHEMA_URI_SCHEME, SchemaDocumentProvider } from "./documentProviders/schema"; import { logError } from "./errors"; @@ -587,11 +586,7 @@ async function setupAuthProvider(): Promise { function setupDocumentProviders(): vscode.Disposable[] { const disposables: vscode.Disposable[] = []; // any document providers set here must provide their own `scheme` to register with - const providerClasses = [ - SchemaDocumentProvider, - MessageDocumentProvider, - FlinkStatementDocumentProvider, - ]; + const providerClasses = [SchemaDocumentProvider, MessageDocumentProvider]; for (const providerClass of providerClasses) { const provider = new providerClass(); disposables.push( diff --git a/src/flinkSql/flinkLanguageClientManager.test.ts b/src/flinkSql/flinkLanguageClientManager.test.ts index 9e58d8da83..70f23a7abd 100644 --- a/src/flinkSql/flinkLanguageClientManager.test.ts +++ b/src/flinkSql/flinkLanguageClientManager.test.ts @@ -20,7 +20,6 @@ import { import { TEST_CCLOUD_ORGANIZATION } from "../../tests/unit/testResources/organization"; import * as flinkSqlProvider from "../codelens/flinkSqlProvider"; import { CCLOUD_CONNECTION_ID } from "../constants"; -import { FLINKSTATEMENT_URI_SCHEME } from "../documentProviders/flinkStatement"; import { FLINK_CONFIG_COMPUTE_POOL, FLINK_CONFIG_DATABASE } from "../extensionSettings/constants"; import { CCloudResourceLoader } from "../loaders"; import { CCloudEnvironment } from "../models/environment"; @@ -149,12 +148,6 @@ describe("FlinkLanguageClientManager", () => { const document = { languageId: "plaintext", uri } as vscode.TextDocument; assert.strictEqual(flinkManager.isAppropriateDocument(document), false); }); - - it("should return false for read-only FlinkStatement URIs", () => { - const uri = vscode.Uri.parse(`${FLINKSTATEMENT_URI_SCHEME}://test-statement`); - const document = { languageId: FLINKSQL_LANGUAGE_ID, uri } as vscode.TextDocument; - assert.strictEqual(flinkManager.isAppropriateDocument(document), false); - }); }); describe("isAppropriateUri", () => { @@ -164,11 +157,6 @@ describe("FlinkLanguageClientManager", () => { assert.strictEqual(flinkManager.isAppropriateUri(uri), true); }); } - - it("should return false for read-only FlinkStatement URIs", () => { - const uri = vscode.Uri.parse(`${FLINKSTATEMENT_URI_SCHEME}://test-statement`); - assert.strictEqual(flinkManager.isAppropriateUri(uri), false); - }); }); describe("lookupComputePoolInfo", () => { @@ -1160,8 +1148,8 @@ describe("FlinkLanguageClientManager", () => { sinon.assert.notCalled(maybeStartLanguageClientStub); }); - it("should not call maybeStartLanguageClient if new document is flinksql-y but not an appropriate uri", async () => { - const documentUri = vscode.Uri.parse(`${FLINKSTATEMENT_URI_SCHEME}:///fake/path/test.txt`); + it("should not call maybeStartLanguageClient if new document is flinksql-y but not a file/untitled uri", async () => { + const documentUri = vscode.Uri.parse("some-other-scheme:///fake/path/test.txt"); flinkManager["lastDocUri"] = null; // No last document openTextDocumentStub.resolves({ @@ -1204,8 +1192,8 @@ describe("FlinkLanguageClientManager", () => { sinon.assert.notCalled(maybeStartLanguageClientStub); }); - it("Should not call maybeStartLanguageClient when active editor is flinksql but not a valid uri", async () => { - const fakeUri = vscode.Uri.parse(`${FLINKSTATEMENT_URI_SCHEME}:///fake/path/test.flinksql`); + it("Should not call maybeStartLanguageClient when active editor is flinksql but not a file/untitled uri", async () => { + const fakeUri = vscode.Uri.parse("some-other-scheme:///fake/path/test.flinksql"); const fakeDocument = { languageId: FLINKSQL_LANGUAGE_ID, uri: fakeUri, diff --git a/src/flinkSql/flinkLanguageClientManager.ts b/src/flinkSql/flinkLanguageClientManager.ts index df2f8c02e1..7438466bcf 100644 --- a/src/flinkSql/flinkLanguageClientManager.ts +++ b/src/flinkSql/flinkLanguageClientManager.ts @@ -12,7 +12,6 @@ import { LanguageClient } from "vscode-languageclient/node"; import { CloseEvent, ErrorEvent, MessageEvent, WebSocket } from "ws"; import { getCatalogDatabaseFromMetadata } from "../codelens/flinkSqlProvider"; import { CCLOUD_CONNECTION_ID } from "../constants"; -import { FLINKSTATEMENT_URI_SCHEME } from "../documentProviders/flinkStatement"; import { ccloudConnected, uriMetadataSet } from "../emitters"; import { logError } from "../errors"; import { FLINK_CONFIG_COMPUTE_POOL, FLINK_CONFIG_DATABASE } from "../extensionSettings/constants"; @@ -114,10 +113,9 @@ export class FlinkLanguageClientManager extends DisposableCollection { /** Should we consider language serving for this sort of document URI? */ isAppropriateUri(uri: Uri): boolean { - // Just not FLINKSTATEMENT_URI_SCHEME, the one used for readonly statements - // downloaded from the Flink Statements view. Be happy with, say, - // "file" or "untitled" schemes. - return uri.scheme !== FLINKSTATEMENT_URI_SCHEME; + // only basic "untitled" (unsaved) documents or "file" (saved) documents, + // not any unrelated ("output") and/or read-only schemes + return ["file", "untitled"].includes(uri.scheme); } /** Set up to track the appropriate documents open at instance construction (extension startup) time. */