Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 9 additions & 8 deletions frontend/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,26 @@ export type HistoryEntry = {
details?: string | null;
};

export type ExpressionDiagnostic = {
export type FormError = {
path: string[];
message: string;
start_line: number;
start_column: number;
end_line: number;
end_column: number;
};

export async function validateExpression(
expression: string,
): Promise<ExpressionDiagnostic[]> {
const r = await fetch("/api/expressions/validate", {
export async function validateTaskForm(
taskId: string,
formData: Record<string, unknown>,
): Promise<FormError[]> {
const r = await fetch(`/api/tasks/${taskId}/validate`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ expression }),
body: JSON.stringify(formData),
});
if (!r.ok) throw new Error(`validate: ${r.status}`);
const body = (await r.json()) as { diagnostics: ExpressionDiagnostic[] };
return body.diagnostics;
return r.json();
}

export async function fetchHistory(): Promise<HistoryEntry[]> {
Expand Down
24 changes: 16 additions & 8 deletions frontend/src/components/ExpressionWidget.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,6 @@ import type { WidgetProps } from "@rjsf/utils";
import { describe, expect, it, vi } from "vitest";
import { ExpressionWidget, findToken } from "./ExpressionWidget";

vi.mock("../api", async () => {
const actual = await vi.importActual<typeof import("../api")>("../api");
return {
...actual,
validateExpression: vi.fn().mockResolvedValue([]),
};
});

const tokens = [
{
label: "col",
Expand All @@ -35,6 +27,22 @@ function renderWidget(onChange = vi.fn(), value = "") {
onChange,
options: { tokens },
schema: { type: "string" },
registry: {
formContext: {
expressionErrors: {
root_expression: [
{
path: ["expression"],
message: "unknown name 'foo'",
start_line: 1,
start_column: 1,
end_line: 1,
end_column: 4,
},
],
},
},
},
} as unknown as WidgetProps;
return render(<ExpressionWidget {...props} />);
}
Expand Down
92 changes: 41 additions & 51 deletions frontend/src/components/ExpressionWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTheme } from "@mui/material/styles";
import type { WidgetProps } from "@rjsf/utils";
import type { editor, Position } from "monaco-editor";
import { useEffect, useRef } from "react";
import { type ExpressionDiagnostic, validateExpression } from "../api";
import type { FormError } from "../api";

const LANGUAGE_ID = "hyperleda-expression";
const MARKER_OWNER = "hyperleda-expression";
Expand All @@ -16,6 +16,10 @@ export type ExpressionToken = {
detail: string;
};

export type ExpressionFormContext = {
expressionErrors?: Record<string, FormError[]>;
};

let languageRegistered = false;
let currentTokens: ExpressionToken[] = [];

Expand Down Expand Up @@ -47,20 +51,29 @@ export function findToken(
return tokens.find((token) => token.label === word);
}

function toMarkers(
monaco: Monaco,
diagnostics: ExpressionDiagnostic[],
): editor.IMarkerData[] {
return diagnostics.map((diagnostic) => ({
function toMarkers(monaco: Monaco, errors: FormError[]): editor.IMarkerData[] {
return errors.map((error) => ({
severity: monaco.MarkerSeverity.Error,
message: diagnostic.message,
startLineNumber: diagnostic.start_line,
startColumn: diagnostic.start_column,
endLineNumber: diagnostic.end_line,
endColumn: diagnostic.end_column,
message: error.message,
startLineNumber: error.start_line,
startColumn: error.start_column,
endLineNumber: error.end_line,
endColumn: error.end_column,
}));
}

function readFieldErrors(formContext: unknown, fieldId: string): FormError[] {
if (typeof formContext !== "object" || formContext === null) {
return [];
}
const errors = (formContext as ExpressionFormContext).expressionErrors;
if (!errors || typeof errors !== "object") {
return [];
}
const fieldErrors = errors[fieldId];
return Array.isArray(fieldErrors) ? fieldErrors : [];
}

function registerExpressionLanguage(
monaco: Monaco,
tokens: ExpressionToken[],
Expand Down Expand Up @@ -148,54 +161,26 @@ export function ExpressionWidget(props: WidgetProps) {
const isDark = theme.palette.mode === "dark";
const editorRef = useRef<editor.IStandaloneCodeEditor | null>(null);
const monacoRef = useRef<Monaco | null>(null);
const requestIdRef = useRef(0);
const timeoutRef = useRef<number | null>(null);
const formContext =
props.registry?.formContext ?? props.formContext ?? undefined;
const fieldErrors = readFieldErrors(formContext, props.id);

useEffect(
() => () => {
if (timeoutRef.current !== null) {
window.clearTimeout(timeoutRef.current);
}
requestIdRef.current += 1;
},
[],
);

function applyMarkers(diagnostics: ExpressionDiagnostic[]) {
useEffect(() => {
const editorInstance = editorRef.current;
const monaco = monacoRef.current;
const model = editorInstance?.getModel();
if (!editorInstance || !monaco || !model) {
return;
}
const context =
props.registry?.formContext ?? props.formContext ?? undefined;
const errors = readFieldErrors(context, props.id);
monaco.editor.setModelMarkers(
model,
MARKER_OWNER,
toMarkers(monaco, diagnostics),
toMarkers(monaco, errors),
);
}

function scheduleValidation(text: string) {
if (timeoutRef.current !== null) {
window.clearTimeout(timeoutRef.current);
}
timeoutRef.current = window.setTimeout(() => {
const requestId = ++requestIdRef.current;
validateExpression(text)
.then((diagnostics) => {
if (requestId !== requestIdRef.current) {
return;
}
applyMarkers(diagnostics);
})
.catch(() => {
if (requestId !== requestIdRef.current) {
return;
}
applyMarkers([]);
});
}, 200);
}
}, [props.registry?.formContext, props.formContext, props.id]);

function handleBeforeMount(monaco: Monaco) {
registerExpressionLanguage(monaco, tokens);
Expand All @@ -213,8 +198,14 @@ export function ExpressionWidget(props: WidgetProps) {
e.stopPropagation();
}
});
applyMarkers([]);
scheduleValidation(value.replace(/\r?\n/g, ""));
const model = editorInstance.getModel();
if (model) {
monaco.editor.setModelMarkers(
model,
MARKER_OWNER,
toMarkers(monaco, fieldErrors),
);
}
}

return (
Expand All @@ -241,7 +232,6 @@ export function ExpressionWidget(props: WidgetProps) {
onChange={(next) => {
const singleLine = (next ?? "").replace(/\r?\n/g, "");
props.onChange(singleLine);
scheduleValidation(singleLine);
}}
beforeMount={handleBeforeMount}
onMount={handleMount}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/TaskPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const fakeTaskSchema = {
vi.mock("../api", () => ({
fetchTaskSchema: vi.fn(() => Promise.resolve(fakeTaskSchema)),
submitTask: vi.fn(),
validateTaskForm: vi.fn(() => Promise.resolve([])),
}));

function renderTaskPage(taskId: string, formData?: Record<string, unknown>) {
Expand Down
71 changes: 64 additions & 7 deletions frontend/src/components/TaskPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from "react";
import { useEffect, useRef, useState } from "react";
import { useLocation, useParams } from "react-router-dom";
import Form from "@rjsf/mui";
import type { RegistryWidgetsType } from "@rjsf/utils";
Expand All @@ -10,7 +10,12 @@ import CircularProgress from "@mui/material/CircularProgress";
import IconButton from "@mui/material/IconButton";
import Popover from "@mui/material/Popover";
import Typography from "@mui/material/Typography";
import { fetchTaskSchema, submitTask } from "../api";
import {
type FormError,
fetchTaskSchema,
submitTask,
validateTaskForm,
} from "../api";
import { extractUiSchema } from "../extractUiSchema";
import { ExpressionWidget } from "./ExpressionWidget";
import { FoldableObjectFieldTemplate } from "./FoldableObjectFieldTemplate";
Expand All @@ -21,6 +26,19 @@ const widgets: RegistryWidgetsType = {
expression: ExpressionWidget,
};

function pathToFieldId(path: string[]): string {
return ["root", ...path].join("_");
}

function toExpressionErrors(errors: FormError[]): Record<string, FormError[]> {
const byId: Record<string, FormError[]> = {};
for (const error of errors) {
const fieldId = pathToFieldId(error.path);
(byId[fieldId] ??= []).push(error);
}
return byId;
}

export function TaskPage() {
const { taskId } = useParams<{ taskId: string }>();
const location = useLocation();
Expand All @@ -34,13 +52,17 @@ export function TaskPage() {
const [loadError, setLoadError] = useState<string | null>(null);
const [submitError, setSubmitError] = useState<string | null>(null);
const [runId, setRunId] = useState<string | null>(null);
const [prefillData, setPrefillData] = useState<Record<string, unknown>>({});
const [formData, setFormData] = useState<Record<string, unknown>>({});
const [formErrors, setFormErrors] = useState<FormError[]>([]);
const requestIdRef = useRef(0);
const timeoutRef = useRef<number | null>(null);

useEffect(() => {
const state = location.state as {
formData?: Record<string, unknown>;
} | null;
setPrefillData(state?.formData ?? {});
setFormData(state?.formData ?? {});
setFormErrors([]);
}, [location.state, taskId]);

useEffect(() => {
Expand Down Expand Up @@ -72,6 +94,37 @@ export function TaskPage() {
};
}, [taskId]);

useEffect(() => {
if (!taskId || !schema) {
return () => {};
}
if (timeoutRef.current !== null) {
window.clearTimeout(timeoutRef.current);
}
timeoutRef.current = window.setTimeout(() => {
const requestId = ++requestIdRef.current;
validateTaskForm(taskId, formData)
.then((errors) => {
if (requestId !== requestIdRef.current) {
return;
}
setFormErrors(errors);
})
.catch(() => {
if (requestId !== requestIdRef.current) {
return;
}
setFormErrors([]);
});
}, 200);
return () => {
if (timeoutRef.current !== null) {
window.clearTimeout(timeoutRef.current);
}
requestIdRef.current += 1;
};
}, [taskId, schema, formData]);

if (!taskId) return null;

if (runId) {
Expand Down Expand Up @@ -157,16 +210,20 @@ export function TaskPage() {
<Form
schema={schema}
validator={validator}
formData={prefillData}
formData={formData}
widgets={widgets}
uiSchema={uiSchema}
templates={{ ObjectFieldTemplate: FoldableObjectFieldTemplate }}
onSubmit={async ({ formData }) => {
formContext={{ expressionErrors: toExpressionErrors(formErrors) }}
onChange={({ formData: next }) => {
setFormData((next ?? {}) as Record<string, unknown>);
}}
onSubmit={async ({ formData: submittedData }) => {
setSubmitError(null);
try {
const submitted = await submitTask(
taskId,
formData as Record<string, unknown>,
submittedData as Record<string, unknown>,
);
setRunId(submitted.run_id);
} catch (e: unknown) {
Expand Down
24 changes: 23 additions & 1 deletion tests/test_formula_validate.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from dataclasses import asdict

from uploader.app.lib.formula import validate_expression
from pydantic import BaseModel, ValidationError

from uploader.app.lib.formula import ExpressionStr, expression_form_errors, validate_expression


def test_validate_expression_accepts_valid() -> None:
Expand Down Expand Up @@ -37,3 +39,23 @@ def test_validate_expression_reports_unknown_names() -> None:

def test_validate_expression_ignores_names_inside_strings() -> None:
assert validate_expression('col("foo_bar")') == []


def test_expression_str_puts_diagnostics_in_validation_error_ctx() -> None:
class SampleForm(BaseModel):
name: str
expression: ExpressionStr

try:
SampleForm.model_validate({"expression": 'foo + col("a")'})
except ValidationError as e:
errors = expression_form_errors(e)
else:
raise AssertionError("expected ValidationError")

assert len(errors) == 1
assert errors[0].path == ("expression",)
assert "foo" in errors[0].message
assert errors[0].start_line == 1
assert errors[0].start_column == 1
assert errors[0].end_column == 4
Loading
Loading