Skip to content

Commit

Permalink
Expose only pullConfiguration as API
Browse files Browse the repository at this point in the history 
Changes the SettingsHandler and exposes only pullConfiguration as
API. This makes it easy to keep a consistent Settings list which would
otherwise be affected from multiple events causing race conditions.

Also modifies the tests to test through pullConfiguration.
  • Loading branch information
@gorkem
gorkem committed Mar 26, 2022
1 parent 01d4e9e commit fd33d05
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 84 deletions.
4 changes: 2 additions & 2 deletions src/languageserver/handlers/notificationHandlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ export class NotificationHandlers {
* Update the associations in the server
*/
private schemaAssociationNotificationHandler(associations: Record<string, string[]> | SchemaConfiguration[]): void {
console.trace('schemaAssociationsHandler : ' + JSON.stringify(associations));
this.yamlSettings.schemaAssociations = associations;
this.yamlSettings.specificValidatorPaths = [];
this.settingsHandler.setSchemaStoreSettingsIfNotSet();
this.settingsHandler.updateConfiguration();
this.settingsHandler.pullConfiguration().catch((error) => console.log(error));
}

/**
Expand Down
72 changes: 34 additions & 38 deletions src/languageserver/handlers/settingsHandlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
* Copyright (c) Red Hat, Inc. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { xhr, configure as configureHttpRequests } from 'request-light';
import { DocumentFormattingRequest, Connection, DidChangeConfigurationNotification } from 'vscode-languageserver';
import { configure as configureHttpRequests, xhr } from 'request-light';
import { Connection, DidChangeConfigurationNotification, DocumentFormattingRequest } from 'vscode-languageserver';
import { convertErrorToTelemetryMsg } from '../../languageservice/utils/objects';
import { isRelativePath, relativeToAbsolutePath } from '../../languageservice/utils/paths';
import { checkSchemaURI, JSON_SCHEMASTORE_URL, KUBERNETES_SCHEMA_URL } from '../../languageservice/utils/schemaUrls';
Expand All @@ -26,7 +26,7 @@ export class SettingsHandler {
if (this.yamlSettings.hasConfigurationCapability && this.yamlSettings.clientDynamicRegisterSupport) {
try {
// Register for all configuration changes.
await this.connection.client.register(DidChangeConfigurationNotification.type, undefined);
await this.connection.client.register(DidChangeConfigurationNotification.type);
} catch (err) {
this.telemetry.sendError('yaml.settings.error', { error: convertErrorToTelemetryMsg(err) });
}
Expand All @@ -53,10 +53,10 @@ export class SettingsHandler {
yamlEditor: config[2],
vscodeEditor: config[3],
};
this.setConfiguration(settings);
await this.setConfiguration(settings);
}

async setConfiguration(settings: Settings): Promise<void> {
private async setConfiguration(settings: Settings): Promise<void> {
configureHttpRequests(settings.http && settings.http.proxy, settings.http && settings.http.proxyStrictSSL);

this.yamlSettings.specificValidatorPaths = [];
Expand All @@ -83,7 +83,6 @@ export class SettingsHandler {
this.yamlSettings.schemaStoreUrl = settings.yaml.schemaStore.url;
}
}

this.yamlSettings.yamlVersion = settings.yaml.yamlVersion ?? '1.2';

if (settings.yaml.format) {
Expand Down Expand Up @@ -160,7 +159,7 @@ export class SettingsHandler {
* AND the schema store setting is enabled. If the schema store setting
* is not enabled we need to clear the schemas.
*/
public async setSchemaStoreSettingsIfNotSet(): Promise<void> {
private async setSchemaStoreSettingsIfNotSet(): Promise<void> {
const schemaStoreIsSet = this.yamlSettings.schemaStoreSettings.length !== 0;
let schemaStoreUrl = '';
if (this.yamlSettings.schemaStoreUrl.length !== 0) {
Expand All @@ -173,59 +172,56 @@ export class SettingsHandler {
try {
const schemaStore = await this.getSchemaStoreMatchingSchemas(schemaStoreUrl);
this.yamlSettings.schemaStoreSettings = schemaStore.schemas;
this.updateConfiguration();
} catch (err) {
// ignore
}
} else if (!this.yamlSettings.schemaStoreEnabled) {
this.yamlSettings.schemaStoreSettings = [];
this.updateConfiguration();
}
}

/**
* When the schema store is enabled, download and store YAML schema associations
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private getSchemaStoreMatchingSchemas(schemaStoreUrl: string): Promise<{ schemas: any[] }> {
return xhr({ url: schemaStoreUrl }).then((response) => {
const languageSettings = {
schemas: [],
};
private async getSchemaStoreMatchingSchemas(schemaStoreUrl: string): Promise<{ schemas: any[] }> {
const response = await xhr({ url: schemaStoreUrl });

const languageSettings = {
schemas: [],
};

// Parse the schema store catalog as JSON
const schemas = JSON.parse(response.responseText);

for (const schemaIndex in schemas.schemas) {
const schema = schemas.schemas[schemaIndex];

if (schema && schema.fileMatch) {
for (const fileMatch in schema.fileMatch) {
const currFileMatch: string = schema.fileMatch[fileMatch];
// If the schema is for files with a YAML extension, save the schema association
if (currFileMatch.indexOf('.yml') !== -1 || currFileMatch.indexOf('.yaml') !== -1) {
languageSettings.schemas.push({
uri: schema.url,
fileMatch: [currFileMatch],
priority: SchemaPriority.SchemaStore,
name: schema.name,
description: schema.description,
versions: schema.versions,
});
}
// Parse the schema store catalog as JSON
const schemas = JSON.parse(response.responseText);

for (const schemaIndex in schemas.schemas) {
const schema = schemas.schemas[schemaIndex];

if (schema && schema.fileMatch) {
for (const fileMatch in schema.fileMatch) {
const currFileMatch: string = schema.fileMatch[fileMatch];
// If the schema is for files with a YAML extension, save the schema association
if (currFileMatch.indexOf('.yml') !== -1 || currFileMatch.indexOf('.yaml') !== -1) {
languageSettings.schemas.push({
uri: schema.url,
fileMatch: [currFileMatch],
priority: SchemaPriority.SchemaStore,
name: schema.name,
description: schema.description,
versions: schema.versions,
});
}
}
}

return languageSettings;
});
}
return languageSettings;
}

/**
* Called when server settings or schema associations are changed
* Re-creates schema associations and re-validates any open YAML files
*/
public updateConfiguration(): void {
private updateConfiguration(): void {
let languageSettings: LanguageSettings = {
validate: this.yamlSettings.yamlShouldValidate,
hover: this.yamlSettings.yamlShouldHover,
Expand Down
114 changes: 70 additions & 44 deletions test/settingsHandlers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { SettingsHandler } from '../src/languageserver/handlers/settingsHandlers';
import * as sinon from 'sinon';
import * as chai from 'chai';
import * as request from 'request-light';
import * as sinon from 'sinon';
import * as sinonChai from 'sinon-chai';
import { Connection, RemoteClient, RemoteWorkspace } from 'vscode-languageserver';
import { SettingsState } from '../src/yamlSettings';
import { ValidationHandler } from '../src/languageserver/handlers/validationHandlers';
import { LanguageService, LanguageSettings, SchemaConfiguration, SchemaPriority } from '../src';
import * as request from 'request-light';
import { setupLanguageService } from './utils/testHelper';
import { SettingsHandler } from '../src/languageserver/handlers/settingsHandlers';
import { ValidationHandler } from '../src/languageserver/handlers/validationHandlers';
import { Telemetry } from '../src/languageserver/telemetry';
import { SettingsState } from '../src/yamlSettings';
import { setupLanguageService } from './utils/testHelper';
import { TestWorkspace } from './utils/testsTypes';

const expect = chai.expect;
Expand All @@ -23,7 +23,7 @@ describe('Settings Handlers Tests', () => {
const sandbox = sinon.createSandbox();
const connection: Connection = {} as Connection;
let workspaceStub: sinon.SinonStubbedInstance<RemoteWorkspace>;
let languageService: sinon.SinonMockStatic;
let languageService: LanguageService;
let settingsState: SettingsState;
let validationHandler: sinon.SinonMock;
let xhrStub: sinon.SinonStub;
Expand All @@ -34,7 +34,8 @@ describe('Settings Handlers Tests', () => {
connection.onDidChangeConfiguration = sandbox.mock();
connection.client = {} as RemoteClient;
connection.client.register = sandbox.mock();
languageService = sandbox.mock();
const languageServerSetup = setupLanguageService({});
languageService = languageServerSetup.languageService;
settingsState = new SettingsState();
validationHandler = sandbox.mock(ValidationHandler);
xhrStub = sandbox.stub(request, 'xhr');
Expand Down Expand Up @@ -77,6 +78,10 @@ describe('Settings Handlers Tests', () => {
});

it('SettingsHandler should not modify file match patterns', async () => {
const languageServerSetup = setupLanguageService({});

const languageService = languageServerSetup.languageService;

xhrStub.resolves({
responseText: `{"schemas": [
{
Expand All @@ -95,11 +100,10 @@ describe('Settings Handlers Tests', () => {
(validationHandler as unknown) as ValidationHandler,
{} as Telemetry
);

sandbox.stub(settingsHandler, 'updateConfiguration').returns();

await settingsHandler.setSchemaStoreSettingsIfNotSet();

workspaceStub.getConfiguration.resolves([{}, {}, {}, {}]);
const configureSpy = sinon.stub(languageService, 'configure');
await settingsHandler.pullConfiguration();
configureSpy.restore();
expect(settingsState.schemaStoreSettings).deep.include({
uri: 'https://raw.githubusercontent.com/adonisjs/application/master/adonisrc.schema.json',
fileMatch: ['.adonisrc.yaml'],
Expand All @@ -110,39 +114,40 @@ describe('Settings Handlers Tests', () => {
});
});

describe('Test that schema priorities are available', () => {
describe('Test that schema priorities are available', async () => {
const testSchemaFileMatch = ['foo/*.yml'];
const testSchemaURI = 'file://foo.json';

function configureSchemaPriorityTest(): LanguageSettings {
const languageServerSetup = setupLanguageService({});

const languageService = languageServerSetup.languageService;
async function configureSchemaPriorityTest(): Promise<LanguageSettings> {
const settingsHandler = new SettingsHandler(
connection,
languageService,
settingsState,
(validationHandler as unknown) as ValidationHandler,
{} as Telemetry
);

const configureSpy = sinon.spy(languageService, 'configure');
settingsHandler.updateConfiguration();

// Check things here
await settingsHandler.pullConfiguration();
configureSpy.restore();
return configureSpy.args[0][0];
}

it('Schema Settings should have a priority', async () => {
settingsState.schemaConfigurationSettings = [
{
fileMatch: testSchemaFileMatch,
uri: testSchemaURI,
},
];

const configureSpy = configureSchemaPriorityTest();
xhrStub.resolves({
responseText: `{"schemas": [
{
"name": ".adonisrc.json",
"description": "AdonisJS configuration file",
"fileMatch": [
".adonisrc.yaml"
],
"url": "https://raw.githubusercontent.com/adonisjs/application/master/adonisrc.schema.json"
}]}`,
});
const schemas = {};
schemas[testSchemaURI] = testSchemaFileMatch;
workspaceStub.getConfiguration.resolves([{ schemas: schemas }, {}, {}, {}]);
const configureSpy = await configureSchemaPriorityTest();

expect(configureSpy.schemas).deep.include({
uri: testSchemaURI,
Expand All @@ -153,14 +158,26 @@ describe('Settings Handlers Tests', () => {
});

it('Schema Associations should have a priority when schema association is an array', async () => {
xhrStub.resolves({
responseText: `{"schemas": [
{
"name": ".adonisrc.json",
"description": "AdonisJS configuration file",
"fileMatch": [
".adonisrc.yaml"
],
"url": "https://raw.githubusercontent.com/adonisjs/application/master/adonisrc.schema.json"
}]}`,
});
settingsState.schemaAssociations = [
{
fileMatch: testSchemaFileMatch,
uri: testSchemaURI,
},
] as SchemaConfiguration[];

const configureSpy = configureSchemaPriorityTest();
workspaceStub.getConfiguration.resolves([{}, {}, {}, {}]);
const configureSpy = await configureSchemaPriorityTest();

expect(configureSpy.schemas).deep.include({
uri: testSchemaURI,
Expand All @@ -174,8 +191,8 @@ describe('Settings Handlers Tests', () => {
settingsState.schemaAssociations = {
[testSchemaURI]: testSchemaFileMatch,
} as Record<string, string[]>;

const configureSpy = configureSchemaPriorityTest();
workspaceStub.getConfiguration.resolves([{}, {}, {}, {}]);
const configureSpy = await configureSchemaPriorityTest();

expect(configureSpy.schemas).deep.include({
uri: testSchemaURI,
Expand All @@ -195,7 +212,6 @@ describe('Settings Handlers Tests', () => {
{} as Telemetry
);
workspaceStub.getConfiguration.resolves([{}, {}, {}, {}]);
const setConfigurationStub = sandbox.stub(settingsHandler, 'setConfiguration');

await settingsHandler.pullConfiguration();

Expand All @@ -205,18 +221,28 @@ describe('Settings Handlers Tests', () => {
{ section: '[yaml]' },
{ section: 'editor' },
]);

expect(setConfigurationStub).calledOnceWith({
yaml: {},
http: {
proxy: '',
proxyStrictSSL: false,
},
yamlEditor: {},
vscodeEditor: {},
});
});
it('should set schemaStoreSettings to empty when schemaStore is disabled', async () => {
const languageServerSetup = setupLanguageService({});

const languageService = languageServerSetup.languageService;
settingsState.schemaStoreEnabled = true;
const settingsHandler = new SettingsHandler(
connection,
(languageService as unknown) as LanguageService,
settingsState,
(validationHandler as unknown) as ValidationHandler,
{} as Telemetry
);

workspaceStub.getConfiguration.resolves([{ schemaStore: { enable: false, url: 'http://shouldnot.activate' } }, {}, {}, {}]);

// const configureSpy = sinon.spy(languageService, 'configure');
await settingsHandler.pullConfiguration();
// configureSpy.restore();
expect(settingsState.schemaStoreEnabled).to.be.false;
expect(settingsState.schemaStoreSettings).to.be.empty;
});
it('detect indentation settings change', async () => {
const settingsHandler = new SettingsHandler(
connection,
Expand Down

0 comments on commit fd33d05

Please sign in to comment.