Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ajv validation errors markers #182

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useServerSideValidation } from "../validation/useServerSideValidation";

export interface IAjvError {
message: string;
line?: number | null;
}

export interface IJsYamlError {
Expand All @@ -26,7 +27,6 @@ export interface IError {

export default function ValidationErrorConsole({ errors, font }: { errors?: IError; font: NextFont }) {
const serverSideValidationResult = useServerSideValidation();

const errorCount =
(errors?.ajvErrors?.length ?? 0) +
(errors?.jsYamlError != null ? 1 : 0) +
Expand Down Expand Up @@ -123,7 +123,7 @@ export function ErrorMessage({
)}
{ajvError && (
<div className={`${font.className} ${errorsStyle}`}>
<p>{`${ajvError.message}`}</p>
<p>{`${ajvError.message} ${(ajvError.line ?? 0) > 1 ? `(Line ${ajvError.line})` : ""}`}</p>
</div>
)}
{customErrors && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@

import { describe, expect, test, it } from "@jest/globals";
import {
type IValidateItem,
type IItem,
type IYamlElement,
findLineAndColumn,
extractMainItemsData,
extractServiceItems,
findLeafs,
getParsedValue,
type IValidateItem,
type IItem,
getYamlDocument,
parseYaml,
} from "./parseYaml";
import { capitalize, customValidate } from "./otelCollectorConfigValidation";
import { capitalize, customValidate, findErrorElement } from "./otelCollectorConfigValidation";
import type { editor } from "monaco-editor";

const editorBinding = {
Expand Down Expand Up @@ -71,7 +73,7 @@ test("find Line And Column of the given offset in a string", () => {
describe("extractMainItemsData", () => {
it("should correctly extract level 1 and leve2 key value pairs with level2 offset", () => {
const yaml = editorBinding.fallback;
const docElements = getParsedValue(yaml);
const docElements = getYamlDocument(yaml);
const result = extractMainItemsData(docElements);

const expectedOutput: IValidateItem = {
Expand All @@ -95,7 +97,7 @@ describe("extractMainItemsData", () => {
describe("findLeafs", () => {
it("should return leaf level and the parent of the leaf with offsets for the given yaml item", () => {
const yaml = editorBinding.fallback;
const docElements = getParsedValue(yaml);
const docElements = getYamlDocument(yaml);
const yamlItems = extractServiceItems(docElements);

const result = findLeafs(yamlItems, docElements.filter((item: IItem) => item.key?.source === "service")[0], {});
Expand Down Expand Up @@ -185,3 +187,45 @@ describe("customValidate", () => {
});
});
});

// Tested with brief editorBinding.fallback
describe("findErrorElement", () => {
const yaml = editorBinding.fallback;
const docElements = getYamlDocument(yaml);
const parsedYaml = parseYaml(docElements);
const exampleAjvErrorPath = ["service", "pipelines", "traces", "exporters"];

it("should correctly find last element of ajv validation errorPath from a yaml file that parsed with parseYaml function", () => {
const result = findErrorElement(exampleAjvErrorPath, parsedYaml);

const expectedOutput: IYamlElement = {
key: "exporters",
offset: 174,
value: [
{
key: "otlp",
offset: 186,
value: "otlp",
},
],
};

expect(result).toEqual(expectedOutput);
});

it("with empty error path should return undefined", () => {
const result = findErrorElement([], parsedYaml);

const expectedOutput = undefined;

expect(result).toEqual(expectedOutput);
});

it("with empty parsed yaml doc should return undefined", () => {
const result = findErrorElement(exampleAjvErrorPath, []);

const expectedOutput = undefined;

expect(result).toEqual(expectedOutput);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import type { editor } from "monaco-editor";
import { type Monaco } from "@monaco-editor/react";
import {
type IItem,
getParsedValue,
type IYamlElement,
type IValidateItem,
getYamlDocument,
extractMainItemsData,
extractServiceItems,
findLeafs,
findLineAndColumn,
parseYaml,
} from "./parseYaml";

type EditorRefType = RefObject<editor.IStandaloneCodeEditor | null>;
Expand All @@ -35,7 +37,8 @@ export function validateOtelCollectorConfigurationAndSetMarkers(
const ajvError: IAjvError[] = [];
const totalErrors: IError = { ajvErrors: ajvError, customErrors: [], customWarnings: [] };
const errorMarkers: editor.IMarkerData[] = [];
const docElements = getParsedValue(configData);
const docElements = getYamlDocument(configData);
const parsedYamlConfig = parseYaml(docElements);
const mainItemsData: IValidateItem = extractMainItemsData(docElements);
const serviceItems: IItem[] | undefined = extractServiceItems(docElements);
serviceItemsData = {};
Expand All @@ -53,12 +56,22 @@ export function validateOtelCollectorConfigurationAndSetMarkers(

if (errors) {
const validationErrors = errors.map((error: ErrorObject) => {
const errorPath = error.instancePath.split("/").slice(1);
const errorElement = findErrorElement(errorPath, parsedYamlConfig);
const { line, column } = findLineAndColumn(configData, errorElement?.offset);
const errorInfo = {
line: null as number | null,
column: null as number | null,
line: line as number | null,
column: column as number | null,
message: error.message || "Unknown error",
};

errorMarkers.push({
startLineNumber: errorInfo.line ?? 0,
endLineNumber: 0,
startColumn: errorInfo.column ?? 0,
endColumn: errorInfo.column ?? 0,
severity: 8,
message: errorInfo.message,
});
if (error instanceof JsYaml.YAMLException) {
errorInfo.line = error.mark.line + 1;
errorInfo.column = error.mark.column + 1;
Expand Down Expand Up @@ -160,3 +173,22 @@ export function capitalize(input: string): string {

return capitalized;
}

export const findErrorElement = (path: string[], data?: IYamlElement[]): IYamlElement | undefined => {
if (!path.length || !data) {
return undefined;
}

const [head, ...tail] = path;

for (const item of data) {
if (item.key === head) {
if (tail.length === 0 || !Array.isArray(item.value)) {
return item;
}

return findErrorElement(tail, item.value);
}
}
return undefined;
};
85 changes: 81 additions & 4 deletions packages/otelbin/src/components/monaco-editor/parseYaml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

import { describe, expect, it } from "@jest/globals";
import type { IItem } from "./parseYaml";
import { getParsedValue, extractServiceItems, findPipelinesKeyValues } from "./parseYaml";
import type { IItem, IYamlElement } from "./parseYaml";
import { getYamlDocument, extractServiceItems, findPipelinesKeyValues, parseYaml } from "./parseYaml";

//The example contains pipelines with duplicated names (otlp and batch)
const editorBinding = {
Expand Down Expand Up @@ -48,11 +48,88 @@ testItem2:
.replaceAll(/\t/g, " ") as string,
} as const;

// Tested with brief serviceTest.fallback
describe("parseYaml", () => {
it("should return a minimal version of npm yaml Document consists all of the key values of yaml string with related offsets", () => {
const yaml = serviceTest.fallback;
const docElements = getYamlDocument(yaml);
const result: IYamlElement[] | undefined = parseYaml(docElements);

expect(result).toEqual([
{
key: "receivers",
offset: 0,
value: [
{
key: "otlp",
offset: 13,
value: undefined,
},
],
},
{
key: "processors",
offset: 19,
value: [
{
key: "batch",
offset: 33,
value: undefined,
},
],
},
{
key: "service",
offset: 40,
value: [
{
key: "extensions",
offset: 51,
value: [
{
key: "health_check",
offset: 64,
value: "health_check",
},
{
key: "pprof",
offset: 78,
value: "pprof",
},
{
key: "zpages",
offset: 85,
value: "zpages",
},
],
},
],
},
{
key: "testItem1",
offset: 93,
value: undefined,
},
{
key: "testItem2",
offset: 104,
value: undefined,
},
]);
});

it("should return an empty array if docElements is empty", () => {
const result = parseYaml([]);

expect(result).toEqual([]);
});
});

// Tested with brief serviceTest.fallback
describe("extractServiceItems", () => {
it("should return service item in the doc object of the yaml parser", () => {
const yaml = serviceTest.fallback;
const docElements = getParsedValue(yaml);
const docElements = getYamlDocument(yaml);
const result: IItem[] | undefined = extractServiceItems(docElements);

expect(result).toEqual([
Expand Down Expand Up @@ -105,7 +182,7 @@ describe("extractServiceItems", () => {
describe("findPipelinesKeyValues", () => {
it("should return return main key values (also with duplicated names) under service.pipelines with their offset in the config", () => {
const yaml = editorBinding.fallback;
const docElements = getParsedValue(yaml);
const docElements = getYamlDocument(yaml);
const serviceItems: IItem[] | undefined = extractServiceItems(docElements);
const pipeLineItems: IItem[] | undefined = serviceItems?.filter((item: IItem) => item.key?.source === "pipelines");

Expand Down
24 changes: 23 additions & 1 deletion packages/otelbin/src/components/monaco-editor/parseYaml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export interface Document {
end?: SourceToken[];
}

export interface IYamlElement {
key: string;
offset: number;
value?: IYamlElement | IYamlElement[] | string;
}

export interface ILeaf {
source?: string;
offset: number;
Expand All @@ -70,14 +76,30 @@ export interface IValidateItem {
[key: string]: ILeaf[];
}

export const getParsedValue = (editorValue: string) => {
export const getYamlDocument = (editorValue: string) => {
const value = editorValue;
const parsedYaml = Array.from(new Parser().parse(value));
const doc = parsedYaml.find((token) => token.type === "document") as Document;
const docElements: IItem[] = doc?.value?.items ?? [];
return docElements;
};

export function parseYaml(yamlItems: IItem[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseYaml function is more efficient and accessible function to make the big yaml Document smaller, through the cleanups, will adjust other parsing functions with this one. @bripkens

const parsedYamlConfig: IYamlElement[] = [];
if (!yamlItems) return;
else if (Array.isArray(yamlItems)) {
for (const item of yamlItems) {
if (item) {
const key = item.key?.source ?? item.value?.source;
const keyOffset = item.key?.offset ?? item.value?.offset;
const value = parseYaml(item.value?.items) ?? item.value?.source;
parsedYamlConfig.push({ key: key, offset: keyOffset, value: value });
}
}
}
return parsedYamlConfig;
}

export function extractMainItemsData(docElements: IItem[]) {
const mainItemsData: IValidateItem = {};

Expand Down
4 changes: 2 additions & 2 deletions packages/otelbin/src/components/react-flow/FlowClick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
extractServiceItems,
findLineAndColumn,
findPipelinesKeyValues,
getParsedValue,
getYamlDocument,
} from "../monaco-editor/parseYaml";
type EditorRefType = RefObject<editor.IStandaloneCodeEditor | null>;

Expand All @@ -30,7 +30,7 @@ export function FlowClick(
) {
event.stopPropagation();
const config = editorRef?.current?.getModel()?.getValue() || "";
const docElements = getParsedValue(config);
const docElements = getYamlDocument(config);
const mainItemsData: IValidateItem = extractMainItemsData(docElements);
let pipelinesKeyValues: IValidateItem | undefined = {};
const serviceItems: IItem[] | undefined = extractServiceItems(docElements);
Expand Down
6 changes: 3 additions & 3 deletions packages/otelbin/src/contexts/EditorContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import schema from "../components/monaco-editor/schema.json";
import { fromPosition, toCompletionList } from "monaco-languageserver-types";
import { type languages } from "monaco-editor/esm/vs/editor/editor.api.js";
import type { IItem } from "../components/monaco-editor/parseYaml";
import { getParsedValue } from "../components/monaco-editor/parseYaml";
import { getYamlDocument } from "../components/monaco-editor/parseYaml";
import { type WorkerGetter, createWorkerManager } from "monaco-worker-manager";
import { type CompletionList, type Position } from "vscode-languageserver-types";
import { validateOtelCollectorConfigurationAndSetMarkers } from "~/components/monaco-editor/otelCollectorConfigValidation";
Expand Down Expand Up @@ -172,10 +172,10 @@ export const EditorProvider = ({ children }: { children: ReactNode }) => {
);

let value = editorRef.current?.getValue() ?? "";
let docElements = getParsedValue(value);
let docElements = getYamlDocument(value);
editorRef.current?.onDidChangeModelContent(() => {
value = editorRef.current?.getValue() ?? "";
docElements = getParsedValue(value);
docElements = getYamlDocument(value);
});

function correctKey(value: string, key?: string, key2?: string) {
Expand Down