From c5834eae5a0b495d6d2b87543fe3d5a4583f2804 Mon Sep 17 00:00:00 2001 From: Colin Grant Date: Thu, 15 Apr 2021 20:33:41 +0200 Subject: [PATCH] Use file service in preference provider initialization Signed-off-by: Colin Grant --- ...tract-resource-preference-provider.spec.ts | 108 ++++++++++++++++++ .../abstract-resource-preference-provider.ts | 38 +++--- 2 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 packages/preferences/src/browser/abstract-resource-preference-provider.spec.ts diff --git a/packages/preferences/src/browser/abstract-resource-preference-provider.spec.ts b/packages/preferences/src/browser/abstract-resource-preference-provider.spec.ts new file mode 100644 index 0000000000000..002085a10b1a1 --- /dev/null +++ b/packages/preferences/src/browser/abstract-resource-preference-provider.spec.ts @@ -0,0 +1,108 @@ +/******************************************************************************** + * Copyright (C) 2021 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +/* eslint-disable @typescript-eslint/no-explicit-any,no-unused-expressions */ + +import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom'; +const disableJSDOM = enableJSDOM(); + +import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider'; +import { ApplicationProps } from '@theia/application-package/lib/application-props'; +FrontendApplicationConfigProvider.set({ + ...ApplicationProps.DEFAULT.frontend.config +}); + +import { expect } from 'chai'; +import { Container } from '@theia/core/shared/inversify'; +import { AbstractResourcePreferenceProvider } from './abstract-resource-preference-provider'; +import { FileService } from '@theia/filesystem/lib/browser/file-service'; +import { bindPreferenceService } from '@theia/core/lib/browser/frontend-application-bindings'; +import { bindMockPreferenceProviders } from '@theia/core/lib/browser/preferences/test'; +import { Deferred } from '@theia/core/lib/common/promise-util'; +import { MonacoTextModelService } from '@theia/monaco/lib/browser/monaco-text-model-service'; +import { Disposable, MessageService } from '@theia/core/lib/common'; +import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace'; +import { PreferenceSchemaProvider } from '@theia/core/lib/browser'; + +disableJSDOM(); + +class MockFileService { + releaseContent = new Deferred(); + async read(): Promise<{ value: string }> { + await this.releaseContent.promise; + return { value: JSON.stringify({ 'editor.fontSize': 20 }) }; + } +} + +const DO_NOTHING = () => { }; +const RETURN_DISPOSABLE = () => Disposable.NULL; + +class MockTextModelService { + createModelReference(): any { + return { + dispose: DO_NOTHING, + object: { + onDidChangeContent: RETURN_DISPOSABLE, + onDirtyChanged: RETURN_DISPOSABLE, + onDidChangeValid: RETURN_DISPOSABLE, + } + }; + } +} + +const mockSchemaProvider = { getCombinedSchema: () => ({ properties: {} }) }; + +class LessAbstractPreferenceProvider extends AbstractResourcePreferenceProvider { + getUri(): any { } + getScope(): any { } +} + +describe('AbstractResourcePreferenceProvider', () => { + let provider: AbstractResourcePreferenceProvider; + let fileService: MockFileService; + + beforeEach(() => { + fileService = new MockFileService(); + const testContainer = new Container(); + bindPreferenceService(testContainer.bind.bind(testContainer)); + bindMockPreferenceProviders(testContainer.bind.bind(testContainer), testContainer.unbind.bind(testContainer)); + testContainer.rebind(PreferenceSchemaProvider).toConstantValue(mockSchemaProvider); + testContainer.bind(FileService).toConstantValue(fileService); + testContainer.bind(MonacoTextModelService).toConstantValue(new MockTextModelService); + testContainer.bind(MessageService).toConstantValue(undefined); + testContainer.bind(MonacoWorkspace).toConstantValue(undefined); + provider = testContainer.resolve(LessAbstractPreferenceProvider); + }); + + it('should not store any preferences before it is ready.', async () => { + const resolveWhenFinished = new Deferred(); + const errorIfReadyFirst = provider.ready.then(() => Promise.reject()); + + expect(provider.get('editor.fontSize')).to.be.undefined; + + resolveWhenFinished.resolve(); + fileService.releaseContent.resolve(); // Allow the initialization to run + + // This promise would reject if the provider had declared itself ready before we resolve `resolveWhenFinished` + await Promise.race([resolveWhenFinished.promise, errorIfReadyFirst]); + }); + + it('should have processed file content when it is ready.', async () => { + fileService.releaseContent.resolve(); + await provider.ready; + expect(provider.get('editor.fontSize')).to.equal(20); // The value provided by the mock FileService implementation. + }); +}); diff --git a/packages/preferences/src/browser/abstract-resource-preference-provider.ts b/packages/preferences/src/browser/abstract-resource-preference-provider.ts index 82be52478495a..6bd5ea780eb8a 100644 --- a/packages/preferences/src/browser/abstract-resource-preference-provider.ts +++ b/packages/preferences/src/browser/abstract-resource-preference-provider.ts @@ -22,13 +22,14 @@ import { JSONExt } from '@theia/core/shared/@phosphor/coreutils'; import { inject, injectable, postConstruct } from '@theia/core/shared/inversify'; import { MessageService } from '@theia/core/lib/common/message-service'; import { Disposable } from '@theia/core/lib/common/disposable'; -import { PreferenceProvider, PreferenceSchemaProvider, PreferenceScope, PreferenceProviderDataChange, PreferenceService } from '@theia/core/lib/browser'; +import { PreferenceProvider, PreferenceSchemaProvider, PreferenceScope, PreferenceProviderDataChange } from '@theia/core/lib/browser'; import URI from '@theia/core/lib/common/uri'; import { PreferenceConfigurations } from '@theia/core/lib/browser/preferences/preference-configurations'; import { MonacoTextModelService } from '@theia/monaco/lib/browser/monaco-text-model-service'; import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model'; import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace'; import { Deferred } from '@theia/core/lib/common/promise-util'; +import { FileService } from '@theia/filesystem/lib/browser/file-service'; @injectable() export abstract class AbstractResourcePreferenceProvider extends PreferenceProvider { @@ -36,10 +37,11 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi protected preferences: { [key: string]: any } = {}; protected model: MonacoEditorModel | undefined; protected readonly loading = new Deferred(); + protected modelInitialized = false; - @inject(PreferenceService) protected readonly preferenceService: PreferenceService; @inject(MessageService) protected readonly messageService: MessageService; @inject(PreferenceSchemaProvider) protected readonly schemaProvider: PreferenceSchemaProvider; + @inject(FileService) protected readonly fileService: FileService; @inject(PreferenceConfigurations) protected readonly configurations: PreferenceConfigurations; @@ -54,6 +56,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi protected async init(): Promise { const uri = this.getUri(); this.toDispose.push(Disposable.create(() => this.loading.reject(new Error(`preference provider for '${uri}' was disposed`)))); + await this.readPreferencesFromFile(); this._ready.resolve(); const reference = await this.textModelService.createModelReference(uri); @@ -64,11 +67,11 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi this.model = reference.object; this.loading.resolve(); + this.modelInitialized = true; this.toDispose.push(reference); this.toDispose.push(Disposable.create(() => this.model = undefined)); - this.readPreferences(); this.toDispose.push(this.model.onDidChangeContent(() => this.readPreferences())); this.toDispose.push(this.model.onDirtyChanged(() => this.readPreferences())); this.toDispose.push(this.model.onDidChangeValid(() => this.readPreferences())); @@ -80,7 +83,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi protected abstract getScope(): PreferenceScope; protected get valid(): boolean { - return this.model && this.model.valid || false; + return this.modelInitialized ? !!this.model?.valid : Object.keys(this.preferences).length > 0; } getConfigUri(): URI; @@ -165,6 +168,11 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi return [preferenceName]; } + protected async readPreferencesFromFile(): Promise { + const content = await this.fileService.read(this.getUri()).catch(() => ({ value: '' })); + this.readPreferencesFromContent(content.value); + } + /** * It HAS to be sync to ensure that `setPreference` returns only when values are updated * or any other operation modifying the monaco model content. @@ -175,20 +183,24 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi return; } try { - let preferences; - if (model.valid) { - const content = model.getText(); - const jsonContent = this.parse(content); - preferences = this.getParsedContent(jsonContent); - } else { - preferences = {}; - } - this.handlePreferenceChanges(preferences); + const content = model.valid ? model.getText() : ''; + this.readPreferencesFromContent(content); } catch (e) { console.error(`Failed to load preferences from '${this.getUri()}'.`, e); } } + protected readPreferencesFromContent(content: string): void { + let preferencesInJson; + try { + preferencesInJson = this.parse(content); + } catch { + preferencesInJson = {}; + } + const parsedPreferences = this.getParsedContent(preferencesInJson); + this.handlePreferenceChanges(parsedPreferences); + } + protected parse(content: string): any { content = content.trim(); if (!content) {