From 36a2f8a6e0c19f728ddd8e88ccd45fa2a7aea283 Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Sun, 31 Oct 2021 15:02:16 +0100 Subject: chore(web): refactor xtext client --- language-web/src/main/js/editor/EditorStore.ts | 11 +- .../src/main/js/xtext/ContentAssistService.ts | 4 +- .../src/main/js/xtext/HighlightingService.ts | 4 +- .../src/main/js/xtext/OccurrencesService.ts | 10 +- language-web/src/main/js/xtext/UpdateService.ts | 116 ++++++++++++++++----- .../src/main/js/xtext/ValidationService.ts | 4 +- language-web/src/main/js/xtext/XtextClient.ts | 22 ++-- .../src/main/js/xtext/XtextWebSocketClient.ts | 34 +++--- 8 files changed, 130 insertions(+), 75 deletions(-) (limited to 'language-web/src/main/js') diff --git a/language-web/src/main/js/editor/EditorStore.ts b/language-web/src/main/js/editor/EditorStore.ts index 8788e00f..ba31efcb 100644 --- a/language-web/src/main/js/editor/EditorStore.ts +++ b/language-web/src/main/js/editor/EditorStore.ts @@ -56,11 +56,11 @@ import { XtextClient } from '../xtext/XtextClient'; const log = getLogger('editor.EditorStore'); export class EditorStore { - themeStore; + private readonly themeStore; state: EditorState; - client: XtextClient; + private readonly client: XtextClient; showLineNumbers = false; @@ -74,11 +74,11 @@ export class EditorStore { infoCount = 0; - readonly defaultDispatcher = (tr: Transaction): void => { + private readonly defaultDispatcher = (tr: Transaction): void => { this.onTransaction(tr); }; - dispatcher = this.defaultDispatcher; + private dispatcher = this.defaultDispatcher; constructor(initialValue: string, themeStore: ThemeStore) { this.themeStore = themeStore; @@ -148,10 +148,7 @@ export class EditorStore { }, ); makeAutoObservable(this, { - themeStore: false, state: observable.ref, - defaultDispatcher: false, - dispatcher: false, }); } diff --git a/language-web/src/main/js/xtext/ContentAssistService.ts b/language-web/src/main/js/xtext/ContentAssistService.ts index 8461ec7f..f085c5b1 100644 --- a/language-web/src/main/js/xtext/ContentAssistService.ts +++ b/language-web/src/main/js/xtext/ContentAssistService.ts @@ -61,9 +61,9 @@ function computeSpan(prefix: string, entryCount: number) { } export class ContentAssistService { - updateService: UpdateService; + private readonly updateService: UpdateService; - lastCompletion: CompletionResult | null = null; + private lastCompletion: CompletionResult | null = null; constructor(updateService: UpdateService) { this.updateService = updateService; diff --git a/language-web/src/main/js/xtext/HighlightingService.ts b/language-web/src/main/js/xtext/HighlightingService.ts index 451a3a52..fc3e9e53 100644 --- a/language-web/src/main/js/xtext/HighlightingService.ts +++ b/language-web/src/main/js/xtext/HighlightingService.ts @@ -7,9 +7,9 @@ import { isHighlightingResult } from './xtextServiceResults'; const log = getLogger('xtext.ValidationService'); export class HighlightingService { - private store: EditorStore; + private readonly store: EditorStore; - private updateService: UpdateService; + private readonly updateService: UpdateService; constructor(store: EditorStore, updateService: UpdateService) { this.store = store; diff --git a/language-web/src/main/js/xtext/OccurrencesService.ts b/language-web/src/main/js/xtext/OccurrencesService.ts index 804f5ba2..d1dec9e9 100644 --- a/language-web/src/main/js/xtext/OccurrencesService.ts +++ b/language-web/src/main/js/xtext/OccurrencesService.ts @@ -34,19 +34,19 @@ function transformOccurrences(regions: ITextRegion[]): IOccurrence[] { } export class OccurrencesService { - private store: EditorStore; + private readonly store: EditorStore; - private webSocketClient: XtextWebSocketClient; + private readonly webSocketClient: XtextWebSocketClient; - private updateService: UpdateService; + private readonly updateService: UpdateService; private hasOccurrences = false; - private findOccurrencesTimer = new Timer(() => { + private readonly findOccurrencesTimer = new Timer(() => { this.handleFindOccurrences(); }, FIND_OCCURRENCES_TIMEOUT_MS); - private clearOccurrencesTimer = new Timer(() => { + private readonly clearOccurrencesTimer = new Timer(() => { this.clearOccurrences(); }, CLEAR_OCCURRENCES_TIMEOUT_MS); diff --git a/language-web/src/main/js/xtext/UpdateService.ts b/language-web/src/main/js/xtext/UpdateService.ts index 838f9d5b..9b672e79 100644 --- a/language-web/src/main/js/xtext/UpdateService.ts +++ b/language-web/src/main/js/xtext/UpdateService.ts @@ -32,20 +32,27 @@ export class UpdateService { xtextStateId: string | null = null; - private store: EditorStore; + private readonly store: EditorStore; + /** + * The changes being synchronized to the server if a full or delta text update is running, + * `null` otherwise. + */ private pendingUpdate: ChangeDesc | null = null; + /** + * Local changes not yet sychronized to the server and not part of the running update, if any. + */ private dirtyChanges: ChangeDesc; - private webSocketClient: XtextWebSocketClient; + private readonly webSocketClient: XtextWebSocketClient; - private updatedCondition = new ConditionVariable( + private readonly updatedCondition = new ConditionVariable( () => this.pendingUpdate === null && this.xtextStateId !== null, WAIT_FOR_UPDATE_TIMEOUT_MS, ); - private idleUpdateTimer = new Timer(() => { + private readonly idleUpdateTimer = new Timer(() => { this.handleIdleUpdate(); }, UPDATE_TIMEOUT_MS); @@ -56,9 +63,11 @@ export class UpdateService { this.webSocketClient = webSocketClient; } - onConnect(): Promise { + onReconnect(): void { this.xtextStateId = null; - return this.updateFullText(); + this.updateFullText().catch((error) => { + log.error('Unexpected error during initial update', error); + }); } onTransaction(transaction: Transaction): void { @@ -68,6 +77,14 @@ export class UpdateService { } } + /** + * Computes the summary of any changes happened since the last complete update. + * + * The result reflects any changes that happened since the `xtextStateId` + * version was uploaded to the server. + * + * @return the summary of changes since the last update + */ computeChangesSinceLastUpdate(): ChangeDesc { return this.pendingUpdate?.composeDesc(this.dirtyChanges) || this.dirtyChanges; } @@ -106,6 +123,15 @@ export class UpdateService { throw new Error('Full text update failed'); } + /** + * Makes sure that the document state on the server reflects recent + * local changes. + * + * Performs either an update with delta text or a full text update if needed. + * If there are not local dirty changes, the promise resolves immediately. + * + * @return a promise resolving when the update is completed + */ async update(): Promise { await this.prepareForDeltaUpdate(); const delta = this.computeDelta(); @@ -151,31 +177,36 @@ export class UpdateService { return []; } const delta = this.computeDelta(); - if (delta === null) { - // Poscondition of `prepareForDeltaUpdate`: `xtextStateId !== null` - return this.doFetchContentAssist(params, this.xtextStateId as string); - } - log.trace('Editor delta', delta); - return this.withUpdate(async () => { - const result = await this.webSocketClient.send({ - ...params, - requiredStateId: this.xtextStateId, - ...delta, + if (delta !== null) { + log.trace('Editor delta', delta); + const entries = await this.withUpdate(async () => { + const result = await this.webSocketClient.send({ + ...params, + requiredStateId: this.xtextStateId, + ...delta, + }); + if (isContentAssistResult(result)) { + return [result.stateId, result.entries]; + } + if (isInvalidStateIdConflictResult(result)) { + const [newStateId] = await this.doFallbackToUpdateFullText(); + // We must finish this state update transaction to prepare for any push events + // before querying for content assist, so we just return `null` and will query + // the content assist service later. + return [newStateId, null]; + } + log.error('Unextpected content assist result with delta update', result); + throw new Error('Unexpexted content assist result with delta update'); }); - if (isContentAssistResult(result)) { - return [result.stateId, result.entries]; + if (entries !== null) { + return entries; } - if (isInvalidStateIdConflictResult(result)) { - const [newStateId] = await this.doFallbackToUpdateFullText(); - if (signal.aborted) { - return [newStateId, []]; - } - const entries = await this.doFetchContentAssist(params, newStateId); - return [newStateId, entries]; + if (signal.aborted) { + return []; } - log.error('Unextpected content assist result with delta update', result); - throw new Error('Unexpexted content assist result with delta update'); - }); + } + // Poscondition of `prepareForDeltaUpdate`: `xtextStateId !== null` + return this.doFetchContentAssist(params, this.xtextStateId as string); } private async doFetchContentAssist(params: Record, expectedStateId: string) { @@ -211,6 +242,27 @@ export class UpdateService { }; } + /** + * Executes an asynchronous callback that updates the state on the server. + * + * Ensures that updates happen sequentially and manages `pendingUpdate` + * and `dirtyChanges` to reflect changes being synchronized to the server + * and not yet synchronized to the server, respectively. + * + * Optionally, `callback` may return a second value that is retured by this function. + * + * Once the remote procedure call to update the server state finishes + * and returns the new `stateId`, `callback` must return _immediately_ + * to ensure that the local `stateId` is updated likewise to be able to handle + * push messages referring to the new `stateId` from the server. + * If additional work is needed to compute the second value in some cases, + * use `T | null` instead of `T` as a return type and signal the need for additional + * computations by returning `null`. Thus additional computations can be performed + * outside of the critical section. + * + * @param callback the asynchronous callback that updates the server state + * @return a promise resolving to the second value returned by `callback` + */ private async withUpdate(callback: () => Promise<[string, T]>): Promise { if (this.pendingUpdate !== null) { throw new Error('Another update is pending, will not perform update'); @@ -239,6 +291,14 @@ export class UpdateService { } } + /** + * Ensures that there is some state available on the server (`xtextStateId`) + * and that there is not pending update. + * + * After this function resolves, a delta text update is possible. + * + * @return a promise resolving when there is a valid state id but no pending update + */ private async prepareForDeltaUpdate() { // If no update is pending, but the full text hasn't been uploaded to the server yet, // we must start a full text upload. diff --git a/language-web/src/main/js/xtext/ValidationService.ts b/language-web/src/main/js/xtext/ValidationService.ts index 31c8f716..8e4934ac 100644 --- a/language-web/src/main/js/xtext/ValidationService.ts +++ b/language-web/src/main/js/xtext/ValidationService.ts @@ -8,9 +8,9 @@ import { isValidationResult } from './xtextServiceResults'; const log = getLogger('xtext.ValidationService'); export class ValidationService { - private store: EditorStore; + private readonly store: EditorStore; - private updateService: UpdateService; + private readonly updateService: UpdateService; constructor(store: EditorStore, updateService: UpdateService) { this.store = store; diff --git a/language-web/src/main/js/xtext/XtextClient.ts b/language-web/src/main/js/xtext/XtextClient.ts index 03b81b1c..28f3d0cc 100644 --- a/language-web/src/main/js/xtext/XtextClient.ts +++ b/language-web/src/main/js/xtext/XtextClient.ts @@ -16,21 +16,21 @@ import { XtextWebSocketClient } from './XtextWebSocketClient'; const log = getLogger('xtext.XtextClient'); export class XtextClient { - private webSocketClient: XtextWebSocketClient; + private readonly webSocketClient: XtextWebSocketClient; - private updateService: UpdateService; + private readonly updateService: UpdateService; - private contentAssistService: ContentAssistService; + private readonly contentAssistService: ContentAssistService; - private highlightingService: HighlightingService; + private readonly highlightingService: HighlightingService; - private validationService: ValidationService; + private readonly validationService: ValidationService; - private occurrencesService: OccurrencesService; + private readonly occurrencesService: OccurrencesService; constructor(store: EditorStore) { this.webSocketClient = new XtextWebSocketClient( - () => this.updateService.onConnect(), + () => this.updateService.onReconnect(), (resource, stateId, service, push) => this.onPush(resource, stateId, service, push), ); this.updateService = new UpdateService(store, this.webSocketClient); @@ -52,15 +52,17 @@ export class XtextClient { this.occurrencesService.onTransaction(transaction); } - private async onPush(resource: string, stateId: string, service: string, push: unknown) { + private onPush(resource: string, stateId: string, service: string, push: unknown) { const { resourceName, xtextStateId } = this.updateService; if (resource !== resourceName) { log.error('Unknown resource name: expected:', resourceName, 'got:', resource); return; } if (stateId !== xtextStateId) { - log.error('Unexpected xtext state id: expected:', xtextStateId, 'got:', resource); - await this.updateService.updateFullText(); + log.error('Unexpected xtext state id: expected:', xtextStateId, 'got:', stateId); + // The current push message might be stale (referring to a previous state), + // so this is not neccessarily an error and there is no need to force-reconnect. + return; } switch (service) { case 'highlight': diff --git a/language-web/src/main/js/xtext/XtextWebSocketClient.ts b/language-web/src/main/js/xtext/XtextWebSocketClient.ts index 839d6518..488e4b3b 100644 --- a/language-web/src/main/js/xtext/XtextWebSocketClient.ts +++ b/language-web/src/main/js/xtext/XtextWebSocketClient.ts @@ -27,14 +27,14 @@ const REQUEST_TIMEOUT_MS = 1000; const log = getLogger('xtext.XtextWebSocketClient'); -type ReconnectHandler = () => Promise; +export type ReconnectHandler = () => void; -type PushHandler = ( +export type PushHandler = ( resourceId: string, stateId: string, service: string, data: unknown, -) => Promise; +) => void; enum State { Initial, @@ -47,29 +47,29 @@ enum State { } export class XtextWebSocketClient { - nextMessageId = 0; + private nextMessageId = 0; - connection!: WebSocket; + private connection!: WebSocket; - pendingRequests = new Map>(); + private readonly pendingRequests = new Map>(); - onReconnect: ReconnectHandler; + private readonly onReconnect: ReconnectHandler; - onPush: PushHandler; + private readonly onPush: PushHandler; - state = State.Initial; + private state = State.Initial; - reconnectTryCount = 0; + private reconnectTryCount = 0; - idleTimer = new Timer(() => { + private readonly idleTimer = new Timer(() => { this.handleIdleTimeout(); }, BACKGROUND_IDLE_TIMEOUT_MS); - pingTimer = new Timer(() => { + private readonly pingTimer = new Timer(() => { this.sendPing(); }, PING_TIMEOUT_MS); - reconnectTimer = new Timer(() => { + private readonly reconnectTimer = new Timer(() => { this.handleReconnect(); }); @@ -115,9 +115,7 @@ export class XtextWebSocketClient { this.nextMessageId = 0; this.reconnectTryCount = 0; this.pingTimer.schedule(); - this.onReconnect().catch((error) => { - log.error('Unexpected error in onReconnect handler', error); - }); + this.onReconnect(); }); this.connection.addEventListener('error', (event) => { log.error('Unexpected websocket error', event); @@ -264,9 +262,7 @@ export class XtextWebSocketClient { message.stateId, message.service, message.push, - ).catch((error) => { - log.error('Unexpected error in onPush handler', error); - }); + ); } else { log.error('Unexpected websocket message', message); this.forceReconnectOnError(); -- cgit v1.2.3-54-g00ecf