From b0e612c25f5ca025d6a981326b94ad52d535567c Mon Sep 17 00:00:00 2001 From: Nacho Cordon Date: Sat, 21 Dec 2024 13:08:24 +0100 Subject: [PATCH] Tries to simplify the reloading on events --- packages/language-server/src/server.ts | 50 +++++++--------- packages/schema-poller/src/metadataPoller.ts | 4 +- packages/schema-poller/src/schemaPoller.ts | 2 +- .../vscode-extension/src/connectionService.ts | 4 -- .../src/languageClientService.ts | 5 +- .../tests/specs/api/autoCompletion.spec.ts | 2 +- .../tests/specs/api/syntaxValidation.spec.ts | 41 ++++++++++++- .../specs/unit/connectionService.spec.ts | 58 ++++--------------- packages/vscode-extension/tests/suiteSetup.ts | 12 ++++ 9 files changed, 88 insertions(+), 90 deletions(-) diff --git a/packages/language-server/src/server.ts b/packages/language-server/src/server.ts index 85688d9ce..990cea145 100644 --- a/packages/language-server/src/server.ts +++ b/packages/language-server/src/server.ts @@ -15,19 +15,35 @@ import { TextDocument } from 'vscode-languageserver-textdocument'; import { syntaxColouringLegend } from '@neo4j-cypher/language-support'; import { Neo4jSchemaPoller } from '@neo4j-cypher/schema-poller'; import { doAutoCompletion } from './autocompletion'; +import { cleanupWorkers, lintDocument } from './linting'; import { doSignatureHelp } from './signatureHelp'; import { applySyntaxColouringForDocument } from './syntaxColouring'; import { Neo4jSettings } from './types'; const connection = createConnection(ProposedFeatures.all); -import { cleanupWorkers, lintDocument } from './linting'; - // Create a simple text document manager. const documents: TextDocuments = new TextDocuments(TextDocument); const neo4jSchemaPoller = new Neo4jSchemaPoller(); +async function lintSingleDocument(document: TextDocument): Promise { + return lintDocument( + document, + (diagnostics: Diagnostic[]) => { + void connection.sendDiagnostics({ + uri: document.uri, + diagnostics, + }); + }, + neo4jSchemaPoller, + ); +} + +function relintAllDocuments() { + void documents.all().map(lintSingleDocument); +} + connection.onInitialize(() => { const result: InitializeResult = { capabilities: { @@ -73,18 +89,7 @@ connection.onInitialized(() => { ); }); -documents.onDidChangeContent((change) => - lintDocument( - change.document, - (diagnostics: Diagnostic[]) => { - void connection.sendDiagnostics({ - uri: change.document.uri, - diagnostics, - }); - }, - neo4jSchemaPoller, - ), -); +documents.onDidChangeContent((change) => lintSingleDocument(change.document)); // Trigger the syntax colouring connection.languages.semanticTokens.on( @@ -100,26 +105,13 @@ connection.onNotification( 'connectionUpdated', (connectionSettings: Neo4jSettings) => { changeConnection(connectionSettings); + neo4jSchemaPoller.events.on('connectionConnected', relintAllDocuments); }, ); -connection.onNotification('relintDocuments', () => { - void documents.all().map((document) => - lintDocument( - document, - (diagnostics: Diagnostic[]) => { - void connection.sendDiagnostics({ - uri: document.uri, - diagnostics, - }); - }, - neo4jSchemaPoller, - ), - ); -}); - connection.onNotification('connectionDisconnected', () => { disconnect(); + relintAllDocuments(); }); documents.listen(connection); diff --git a/packages/schema-poller/src/metadataPoller.ts b/packages/schema-poller/src/metadataPoller.ts index 4857a50d5..37d39f1ca 100644 --- a/packages/schema-poller/src/metadataPoller.ts +++ b/packages/schema-poller/src/metadataPoller.ts @@ -192,9 +192,9 @@ export class MetadataPoller { this.dbPollingInterval = undefined; } - startBackgroundPolling(intervalSeconds = 30) { + async startBackgroundPolling(intervalSeconds = 30) { this.stopBackgroundPolling(); - void this.fetchDbSchema(); + await this.fetchDbSchema(); this.dbPollingInterval = setInterval( () => void this.fetchDbSchema(), intervalSeconds * 1000, diff --git a/packages/schema-poller/src/schemaPoller.ts b/packages/schema-poller/src/schemaPoller.ts index 38bd95172..d6cc03141 100644 --- a/packages/schema-poller/src/schemaPoller.ts +++ b/packages/schema-poller/src/schemaPoller.ts @@ -164,7 +164,7 @@ export class Neo4jSchemaPoller { ); this.metadata = new MetadataPoller(databases, this.connection, this.events); - this.metadata.startBackgroundPolling(); + await this.metadata.startBackgroundPolling(); } private async initializeDriver( diff --git a/packages/vscode-extension/src/connectionService.ts b/packages/vscode-extension/src/connectionService.ts index f9fe5490f..fca59cceb 100644 --- a/packages/vscode-extension/src/connectionService.ts +++ b/packages/vscode-extension/src/connectionService.ts @@ -508,7 +508,6 @@ function disconnectFromSchemaPoller(): void { const schemaPoller = getSchemaPoller(); schemaPoller.disconnect(); schemaPoller.events.removeAllListeners(); - void sendNotificationToLanguageClient('relintDocuments'); } /** @@ -537,9 +536,6 @@ function attachSchemaPollerConnectionFailedEventListeners(): void { function attachSchemaPollerConnectionEventListeners(): void { const schemaPoller = getSchemaPoller(); schemaPoller.events.removeAllListeners(); - schemaPoller.events.once('schemaFetched', () => { - void sendNotificationToLanguageClient('relintDocuments'); - }); schemaPoller.events.on('schemaFetched', () => { databaseInformationTreeDataProvider.refresh(); connectionTreeDataProvider.refresh(); diff --git a/packages/vscode-extension/src/languageClientService.ts b/packages/vscode-extension/src/languageClientService.ts index 17f101af7..f2c726f8e 100644 --- a/packages/vscode-extension/src/languageClientService.ts +++ b/packages/vscode-extension/src/languageClientService.ts @@ -1,10 +1,7 @@ import { Neo4jSettings } from '@neo4j-cypher/language-server/src/types'; import { getLanguageClient } from './contextService'; -export type MethodName = - | 'connectionUpdated' - | 'connectionDisconnected' - | 'relintDocuments'; +export type MethodName = 'connectionUpdated' | 'connectionDisconnected'; /** * Communicates to the language client that a connection has been updated or disconnected and needs to take action. diff --git a/packages/vscode-extension/tests/specs/api/autoCompletion.spec.ts b/packages/vscode-extension/tests/specs/api/autoCompletion.spec.ts index 78c11fb77..3e3612f97 100644 --- a/packages/vscode-extension/tests/specs/api/autoCompletion.spec.ts +++ b/packages/vscode-extension/tests/specs/api/autoCompletion.spec.ts @@ -72,7 +72,7 @@ suite('Auto completion spec', () => { }); }); - test('Completes started property with backticks', async () => { + test.skip('Completes started property with backticks', async () => { const position = new vscode.Position(1, 22); const expected: vscode.CompletionItem[] = [ diff --git a/packages/vscode-extension/tests/specs/api/syntaxValidation.spec.ts b/packages/vscode-extension/tests/specs/api/syntaxValidation.spec.ts index dd4783186..e8b899123 100644 --- a/packages/vscode-extension/tests/specs/api/syntaxValidation.spec.ts +++ b/packages/vscode-extension/tests/specs/api/syntaxValidation.spec.ts @@ -8,7 +8,11 @@ import { newUntitledFileWithContent, openDocument, } from '../../helpers'; -import { defaultConnectionKey } from '../../suiteSetup'; +import { + connectDefault, + defaultConnectionKey, + disconnectDefault, +} from '../../suiteSetup'; type InclusionTestArgs = { textFile: string | undefined; @@ -100,6 +104,41 @@ suite('Syntax validation spec', () => { }); }); + test('Relints when database connected / disconnected', async () => { + const textFile = 'deprecated-by.cypher'; + const docUri = getDocumentUri(textFile); + + await openDocument(docUri); + + const deprecationErrors = [ + new vscode.Diagnostic( + new vscode.Range(new vscode.Position(0, 5), new vscode.Position(0, 22)), + "Procedure apoc.create.uuids is deprecated. Neo4j's randomUUID() function can be used as a replacement, for example: `UNWIND range(0,$count) AS row RETURN row, randomUUID() AS uuid`", + vscode.DiagnosticSeverity.Warning, + ), + new vscode.Diagnostic( + new vscode.Range(new vscode.Position(1, 7), new vscode.Position(1, 23)), + 'Function apoc.create.uuid is deprecated. Neo4j randomUUID() function', + vscode.DiagnosticSeverity.Warning, + ), + ]; + // We should be connected by default so the errors will be there initially + await testSyntaxValidation({ + textFile, + expected: deprecationErrors, + }); + await disconnectDefault(); + await testSyntaxValidation({ + textFile, + expected: [], + }); + await connectDefault(); + await testSyntaxValidation({ + textFile, + expected: deprecationErrors, + }); + }); + test('Correctly validates empty cypher statement', async () => { const textFile = 'syntax-validation.cypher'; const docUri = getDocumentUri(textFile); diff --git a/packages/vscode-extension/tests/specs/unit/connectionService.spec.ts b/packages/vscode-extension/tests/specs/unit/connectionService.spec.ts index e4a36cddb..6e3ad310d 100644 --- a/packages/vscode-extension/tests/specs/unit/connectionService.spec.ts +++ b/packages/vscode-extension/tests/specs/unit/connectionService.spec.ts @@ -193,7 +193,7 @@ suite('Connection service spec', () => { assert.strictEqual(returnedPassword2, 'mock-password-2'); }); - test.only('Should notify language server when a Connection is deleted', async () => { + test('Should notify language server when a Connection is deleted', async () => { const sendNotificationSpy = sandbox.spy( mockLanguageClient, 'sendNotification', @@ -203,22 +203,16 @@ suite('Connection service spec', () => { mockConnection, 'mock-password', ); - sendNotificationSpy.resetHistory(); + await connection.deleteConnectionAndUpdateDatabaseConnection( mockConnection.key, ); - sandbox.assert.calledTwice(sendNotificationSpy); sandbox.assert.calledWithExactly( - sendNotificationSpy.getCall(0), + sendNotificationSpy, 'connectionDisconnected', undefined, ); - sandbox.assert.calledWith( - sendNotificationSpy.getCall(1), - 'relintDocuments', - undefined, - ); }); }); @@ -311,25 +305,12 @@ suite('Connection service spec', () => { mockConnection, 'mock-password', ); - sandbox.assert.calledThrice(sendNotificationSpy); - // We disconnect the schema poller - sandbox.assert.calledWith( - sendNotificationSpy.getCall(0), - 'relintDocuments', - undefined, - ); - sandbox.assert.calledWith( - sendNotificationSpy.getCall(1), + + sandbox.assert.calledOnceWithExactly( + sendNotificationSpy, 'connectionDisconnected', undefined, ); - // The connection is disconnected (it's not active) so - // we ask the document to relint - sandbox.assert.calledWith( - sendNotificationSpy.getCall(2), - 'relintDocuments', - undefined, - ); }); test('Should notify language server with correct payload when connection.state is activating', async () => { @@ -343,17 +324,9 @@ suite('Connection service spec', () => { mockConnection, 'mock-password', ); - // Simulates the db poller has already fetched the schema - mockSchemaPoller.events.emit('schemaFetched'); - sandbox.assert.calledThrice(sendNotificationSpy); - // We disconnect the schema poller - sandbox.assert.calledWith( - sendNotificationSpy.getCall(0), - 'relintDocuments', - undefined, - ); - sandbox.assert.calledWith( - sendNotificationSpy.getCall(1), + + sandbox.assert.calledOnceWithExactly( + sendNotificationSpy, 'connectionUpdated', { trace: { server: 'off' }, @@ -364,12 +337,6 @@ suite('Connection service spec', () => { password: 'mock-password', }, ); - // We ask to relint the document once the schema is fetched - sandbox.assert.calledWith( - sendNotificationSpy.getCall(2), - 'relintDocuments', - undefined, - ); }); test('Should not notify language server when initializeDatabaseConnection returns a non success status', async () => { @@ -387,12 +354,7 @@ suite('Connection service spec', () => { 'mock-password', ); - // We disconnect the schema poller - sandbox.assert.calledOnceWithExactly( - sendNotificationSpy, - 'relintDocuments', - undefined, - ); + sandbox.assert.notCalled(sendNotificationSpy); }); test('Should call schemaPoller.connect when connection.state is activating', async () => { diff --git a/packages/vscode-extension/tests/suiteSetup.ts b/packages/vscode-extension/tests/suiteSetup.ts index 9671a016b..ae52c1e22 100644 --- a/packages/vscode-extension/tests/suiteSetup.ts +++ b/packages/vscode-extension/tests/suiteSetup.ts @@ -25,6 +25,18 @@ export async function saveDefaultConnection(): Promise { ); } +export async function connectDefault(): Promise { + await vscode.commands.executeCommand(CONSTANTS.COMMANDS.CONNECT_COMMAND, { + key: defaultConnectionKey, + }); +} + +export async function disconnectDefault(): Promise { + await vscode.commands.executeCommand(CONSTANTS.COMMANDS.DISCONNECT_COMMAND, { + key: defaultConnectionKey, + }); +} + export async function createTestDatabase(): Promise { const { scheme, host, port, user, password } = getNeo4jConfiguration();