Skip to content

Commit

Permalink
fix: performance optimization
Browse files Browse the repository at this point in the history
  • Loading branch information
marufrasully committed Aug 21, 2024
1 parent 232dfcc commit 3f472d5
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 40 deletions.
55 changes: 51 additions & 4 deletions packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
DidChangeConfigurationNotification,
FileEvent,
InitializeResult,
Diagnostic,
} from "vscode-languageserver/node";
import { URI } from "vscode-uri";
import { TextDocument } from "vscode-languageserver-textdocument";
Expand All @@ -27,7 +28,10 @@ import {
import { commands } from "@ui5-language-assistant/user-facing-text";
import { ServerInitializationOptions } from "../api";
import { getCompletionItems } from "./completion-items";
import { getXMLViewDiagnostics } from "./xml-view-diagnostics";
import {
getXMLViewDiagnostics,
getXMLViewIdDiagnostics,
} from "./xml-view-diagnostics";
import { getHoverResponse } from "./hover";
import {
initializeManifestData,
Expand Down Expand Up @@ -56,6 +60,8 @@ let ui5yamlStateInitialized: Promise<void[]> | undefined = undefined;
let initializationOptions: ServerInitializationOptions | undefined;
let hasConfigurationCapability = false;

const documentsDiagnostics = new Map<string, Diagnostic[]>();

connection.onInitialize(
async (params: InitializeParams): Promise<InitializeResult> => {
getLogger().info("`onInitialize` event", params);
Expand Down Expand Up @@ -227,7 +233,7 @@ connection.onHover(
);

/**
* Validate all `.view.xml` and `.fragment.xml` documents
* Validate all open `.view.xml` and `.fragment.xml` documents
*
* @returns void
*/
Expand All @@ -250,6 +256,40 @@ const validateOpenDocuments = async (): Promise<void> => {
document,
context,
});
documentsDiagnostics.set(document.uri, diagnostics);
getLogger().trace("computed diagnostics", {
diagnostics,
});
connection.sendDiagnostics({ uri: document.uri, diagnostics });
}
};
/**
* Validate ids for all open `.view.xml` and `.fragment.xml` documents
*
* @returns void
*/
const validateIdsOfOpenDocuments = async (): Promise<void> => {
const allDocuments = documents.all();
for (const document of allDocuments) {
const documentPath = URI.parse(document.uri).fsPath;
const context = await getContext(
documentPath,
initializationOptions?.modelCachePath
);
if (!isContext(context)) {
connection.sendNotification(
"UI5LanguageAssistant/context-error",
context
);
return;
}
const idDiagnostics = getXMLViewIdDiagnostics({
document,
context,
});
let diagnostics = documentsDiagnostics.get(document.uri) ?? [];
diagnostics = diagnostics.concat(idDiagnostics);

getLogger().trace("computed diagnostics", {
diagnostics,
});
Expand Down Expand Up @@ -277,6 +317,7 @@ async function validateOpenDocumentsOnDidChangeWatchedFiles(
return;
}
await validateOpenDocuments();
await validateIdsOfOpenDocuments();
}

connection.onDidChangeWatchedFiles(async (changeEvent): Promise<void> => {
Expand All @@ -294,7 +335,7 @@ connection.onDidChangeWatchedFiles(async (changeEvent): Promise<void> => {
cdsFileEvents.push(change);
} else if (uri.endsWith(".xml")) {
await reactOnXmlFileChange(uri, change.type);
await reactOnViewFileChange(uri, change.type, validateOpenDocuments);
await reactOnViewFileChange(uri, change.type, validateIdsOfOpenDocuments);
} else if (uri.endsWith("package.json")) {
await reactOnPackageJson(uri, change.type);
}
Expand Down Expand Up @@ -345,7 +386,12 @@ documents.onDidChangeContent(async (changeEvent): Promise<void> => {
isFallback,
isIncorrectVersion,
});
await validateOpenDocuments();
const diagnostics = getXMLViewDiagnostics({
document,
context,
});
documentsDiagnostics.set(document.uri, diagnostics);
await validateIdsOfOpenDocuments();
}
});

Expand Down Expand Up @@ -457,6 +503,7 @@ documents.onDidClose((textDocumentChangeEvent) => {
if (isXMLView(uri)) {
// clear diagnostics for a closed file
connection.sendDiagnostics({ uri, diagnostics: [] });
documentsDiagnostics.delete(uri);
}
clearDocumentSettings(uri);
});
Expand Down
11 changes: 11 additions & 0 deletions packages/language-server/src/xml-view-diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
defaultValidators,
validators,
UI5ValidatorsConfig,
validateNonUniqueID,
} from "@ui5-language-assistant/xml-views-validation";

import { defaultValidators as feValidators } from "@ui5-language-assistant/fe";
Expand Down Expand Up @@ -58,6 +59,16 @@ export function getXMLViewDiagnostics(opts: {
return diagnostics;
}

export function getXMLViewIdDiagnostics(opts: {
document: TextDocument;
context: Context;
}): Diagnostic[] {
const { context } = opts;
const issues = validateNonUniqueID(context);
const diagnostics = validationIssuesToLspDiagnostics(issues, opts.document);
return diagnostics;
}

function mergeValidators(
param: [
UI5ValidatorsConfig<UI5XMLViewIssue>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { resolve, sep, relative, dirname } from "path";
import { resolve, sep, relative, dirname, basename } from "path";
import { Diagnostic, Range } from "vscode-languageserver-types";
import { TextDocument } from "vscode-languageserver";
import { readJsonSync, readFileSync } from "fs-extra";
Expand All @@ -14,7 +14,10 @@ import {
DEFAULT_UI5_FRAMEWORK,
} from "@ui5-language-assistant/constant";
import { generate } from "@ui5-language-assistant/semantic-model";
import { getXMLViewDiagnostics } from "../../../../src/xml-view-diagnostics";
import {
getXMLViewDiagnostics,
getXMLViewIdDiagnostics,
} from "../../../../src/xml-view-diagnostics";
import { Context as AppContext } from "@ui5-language-assistant/context";
import { getDefaultContext } from "../../completion-items-utils";
import { DocumentCstNode, parse } from "@xml-tools/parser";
Expand Down Expand Up @@ -153,11 +156,14 @@ export async function computeNewDiagnosticLSPResponse(
0,
xmlTextSnippet
);

const actualDiagnostics = getXMLViewDiagnostics({
document: xmlTextDoc,
context: appContext,
});
const dirName = basename(testDir);
const actualDiagnostics =
dirName === "non-unique-id"
? getXMLViewIdDiagnostics({ document: xmlTextDoc, context: appContext })
: getXMLViewDiagnostics({
document: xmlTextDoc,
context: appContext,
});

const diagnosticsForAssertions =
cleanupLSPResponseForAssertions(actualDiagnostics);
Expand Down
3 changes: 2 additions & 1 deletion packages/xml-views-validation/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ export type Validators = {
validateBooleanValue: XMLAttributeValidator<InvalidBooleanValueIssue>;
validateUseOfDeprecatedAttribute: XMLAttributeValidator<UseOfDeprecatedAttributeIssue>;
validateUnknownAttributeKey: XMLAttributeValidator<UnknownAttributeKeyIssue>;
validateNonUniqueID: XMLDocumentValidator<NonUniqueIDIssue>;
validateUseOfDeprecatedAggregation: XMLElementValidator<UseOfDeprecatedAggregationIssue>;
validateUseOfDeprecatedClass: XMLElementValidator<UseOfDeprecatedClassIssue>;
validateUnknownTagName: XMLElementValidator<UnknownTagNameIssue>;
Expand All @@ -139,3 +138,5 @@ export type Validators = {
export const validators: Validators;

export function isPossibleBindingAttributeValue(value: string): boolean;

export { validateNonUniqueID } from "./src/api";
5 changes: 1 addition & 4 deletions packages/xml-views-validation/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
validateUseOfDeprecatedClass,
validateUseOfDeprecatedAggregation,
validateUseOfDeprecatedAttribute,
validateNonUniqueID,
validateUnknownAttributeKey,
validateUnknownTagName,
validateExplicitAggregationCardinality,
Expand All @@ -26,7 +25,6 @@ export const validators: Validators = {
validateUseOfDeprecatedClass,
validateUseOfDeprecatedAggregation,
validateUseOfDeprecatedAttribute,
validateNonUniqueID,
validateUnknownAttributeKey,
validateUnknownTagName,
validateExplicitAggregationCardinality,
Expand All @@ -43,11 +41,10 @@ export function validateXMLView<ExternalXMLViewIssue>(opts: {
accept(opts.xmlView, validatorVisitor);
const issues = validatorVisitor.collectedIssues;

const nonUniqueIdIssues = validateNonUniqueID(opts.xmlView, opts.context);
issues.push(...nonUniqueIdIssues);
return issues;
}

export type { UI5ValidatorsConfig } from "./validate-xml-views";

export { isPossibleBindingAttributeValue } from "../src/utils/is-binding-attribute-value";
export { validateNonUniqueID } from "./validators/non-unique-id";
2 changes: 1 addition & 1 deletion packages/xml-views-validation/src/validators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export { validateBooleanValue } from "./attributes/invalid-boolean-value";
export { validateUseOfDeprecatedClass } from "./elements/use-of-deprecated-class";
export { validateUseOfDeprecatedAggregation } from "./elements/use-of-depracated-aggregation";
export { validateUseOfDeprecatedAttribute } from "./attributes/use-of-depracated-attribute";
export { validateNonUniqueID } from "./document/non-unique-id";
export { validateNonUniqueID } from "./non-unique-id";
export { validateUnknownAttributeKey } from "./attributes/unknown-attribute-key";
export { validateUnknownTagName } from "./elements/unknown-tag-name";
export { validateExplicitAggregationCardinality } from "./elements/cardinality-of-aggregation";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { map } from "lodash";
import { XMLDocument } from "@xml-tools/ast";
import {
validations,
buildMessage,
} from "@ui5-language-assistant/user-facing-text";
import { NonUniqueIDIssue } from "../../../api";
import { NonUniqueIDIssue } from "../../api";
import { Context } from "@ui5-language-assistant/context";
import { pathToFileURL } from "url";

const { NON_UNIQUE_ID } = validations;

export function validateNonUniqueID(
xmlDoc: XMLDocument,
context: Context
): NonUniqueIDIssue[] {
export function validateNonUniqueID(context: Context): NonUniqueIDIssue[] {
const allIDsIssues: NonUniqueIDIssue[] = [];
const uri = pathToFileURL(context.documentPath).toString();
for (const [key, value] of context.controlIds) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "@ui5-language-assistant/test-framework";
import { Context, getContext, cache } from "@ui5-language-assistant/context";
import { join } from "path";
import { validateNonUniqueID } from "../../../../src/validators";
import { validateNonUniqueID } from "../../../src/validators";
import {
validations,
buildMessage,
Expand Down Expand Up @@ -135,12 +135,12 @@ describe("the use of non unique id validation", () => {
<Button id="DUPLICATE">
</Button>
</mvc:View>`;
const { xmlView, context, offsetRanges, documentPath } = await getParam(
const { context, offsetRanges, documentPath } = await getParam(
xmlSnippet
);
const offset = offsetRanges[documentPath];
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toEqual([
{
Expand Down Expand Up @@ -174,12 +174,12 @@ describe("the use of non unique id validation", () => {
</Button>
</custom:View>`;

const { xmlView, context, offsetRanges, documentPath } = await getParam(
const { context, offsetRanges, documentPath } = await getParam(
xmlSnippet
);
const offset = offsetRanges[documentPath];
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toEqual([
{
Expand Down Expand Up @@ -213,12 +213,12 @@ describe("the use of non unique id validation", () => {
<Button id="TRIPLICATE">
</Button>
</mvc:View>`;
const { xmlView, context, offsetRanges, documentPath } = await getParam(
const { context, offsetRanges, documentPath } = await getParam(
xmlSnippet
);
const offset = offsetRanges[documentPath];
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toEqual([
{
Expand Down Expand Up @@ -286,7 +286,7 @@ describe("the use of non unique id validation", () => {
...CustomSectionSegments
);

const { xmlView, context, offsetRanges, documentPath } = await getParam(
const { context, offsetRanges, documentPath } = await getParam(
xmlSnippet
);
const offset = offsetRanges[documentPath];
Expand All @@ -299,7 +299,7 @@ describe("the use of non unique id validation", () => {
range,
}));
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toEqual([
{
Expand Down Expand Up @@ -333,9 +333,9 @@ describe("the use of non unique id validation", () => {
<Button iddqd="DUPLICATE">
</Button>
</mvc:View>`;
const { xmlView, context } = await getParam(xmlSnippet);
const { context } = await getParam(xmlSnippet);
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toBeEmpty();
});
Expand All @@ -350,9 +350,9 @@ describe("the use of non unique id validation", () => {
<Button id="">
</Button>
</mvc:View>`;
const { xmlView, context } = await getParam(xmlSnippet);
const { context } = await getParam(xmlSnippet);
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toBeEmpty();
});
Expand All @@ -367,9 +367,9 @@ describe("the use of non unique id validation", () => {
<Button id="DUPLICATE">
</Button>
</mvc:view>`;
const { xmlView, context } = await getParam(xmlSnippet);
const { context } = await getParam(xmlSnippet);
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toBeEmpty();
});
Expand All @@ -384,9 +384,9 @@ describe("the use of non unique id validation", () => {
<svg:Circle id="DUPLICATE">
</svg:Circle>
</mvc:View>`;
const { xmlView, context } = await getParam(xmlSnippet);
const { context } = await getParam(xmlSnippet);
// act
const result = validateNonUniqueID(xmlView, context);
const result = validateNonUniqueID(context);
// assert
expect(result).toBeEmpty();
});
Expand Down

0 comments on commit 3f472d5

Please sign in to comment.