Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,72 @@ import type { Document, Scalar } from 'yaml';
import { isPair, visit } from 'yaml';

/**
* Gets the scalar value node at a specific position in the YAML document
* Returns the node if found, null otherwise
* This finds the scalar VALUE node (not the key) at the given position
* Cache of scalar value nodes per Document. Uses WeakMap so entries are
* garbage-collected when the Document is no longer referenced.
* Nodes are sorted by range start for O(log n) offset lookups.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👑

*/
export function getScalarValueAtOffset(document: Document, offset: number): Scalar | null {
let scalarNode: Scalar | null = null;
const scalarValueCache = new WeakMap<Document, Scalar[]>();

function collectScalarValues(document: Document): Scalar[] {
const cached = scalarValueCache.get(document);
if (cached) {
return cached;
}

const scalars: Scalar[] = [];

if (!document.contents) {
return null;
scalarValueCache.set(document, scalars);
return scalars;
}

visit(document, {
Scalar(key, node, ancestors) {
Scalar(_key, node, ancestors) {
if (!node.range) {
return;
}
if (offset >= node.range[0] && offset <= node.range[2]) {
// Check if this is a value (not a key)
// The value will be the value of a Pair
const lastAncestor = ancestors?.[ancestors.length - 1];
if (isPair(lastAncestor) && lastAncestor.value === node) {
scalarNode = node;
return visit.BREAK;
}
// Also handle cases where the scalar might be a top-level value or in a sequence
// If it's not a key, treat it as a value
if (lastAncestor && !isPair(lastAncestor)) {
// This is a top-level value or sequence item
scalarNode = node;
return visit.BREAK;
}
const lastAncestor = ancestors?.[ancestors.length - 1];
const isValue = isPair(lastAncestor) ? lastAncestor.value === node : lastAncestor != null;
if (isValue) {
scalars.push(node);
}
},
});

return scalarNode;
scalars.sort((a, b) => (a.range?.[0] ?? 0) - (b.range?.[0] ?? 0));
scalarValueCache.set(document, scalars);
return scalars;
}

/**
* Gets the scalar value node at a specific position in the YAML document
* Returns the node if found, null otherwise
* This finds the scalar VALUE node (not the key) at the given position
*
* Caches all scalar value nodes per Document (via WeakMap) so that repeated
* lookups for different offsets in the same document avoid re-traversing
* the AST -- O(log n) binary search instead of O(n) per call.
*/
export function getScalarValueAtOffset(document: Document, offset: number): Scalar | null {
const scalars = collectScalarValues(document);
// Binary search for the scalar whose range contains the offset
let lo = 0;
let hi = scalars.length - 1;
while (lo <= hi) {
const mid = Math.floor((lo + hi) / 2);
const node = scalars[mid];
const { range } = node;
if (!range) {
break;
}
const [start, , end] = range;
if (offset < start) {
hi = mid - 1;
} else if (offset > end) {
lo = mid + 1;
} else {
return node;
}
}
return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function collectAllVariables(
key: match.groups?.key ?? null,
type,
yamlPath,
offset: startOffset,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ export function useYamlValidation(
variableItems,
workflowGraph,
workflowDefinition,
yamlDocument
yamlDocument,
model
),
...validateJsonSchemaDefaults(yamlDocument, workflowDefinition, model)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@ import { WorkflowGraph } from '@kbn/workflows/graph';
// Mock the imports
jest.mock('../../workflow_context/lib/get_context_for_path');
jest.mock('./validate_variable');
jest.mock('../../../../common/lib/yaml/get_scalar_value_at_offset');

import { validateVariable } from './validate_variable';
import { validateVariables } from './validate_variables';
import { getScalarValueAtOffset } from '../../../../common/lib/yaml/get_scalar_value_at_offset';
import { getContextSchemaForPath } from '../../workflow_context/lib/get_context_for_path';
import type { VariableItem, YamlValidationResult } from '../model/types';

const mockGetScalarValueAtOffset = getScalarValueAtOffset as jest.MockedFunction<
typeof getScalarValueAtOffset
>;

const mockGetContextSchemaForPath = getContextSchemaForPath as jest.MockedFunction<
typeof getContextSchemaForPath
>;
Expand All @@ -38,6 +44,7 @@ describe('validateVariables', () => {
endLineNumber: 1,
endColumn: 10,
yamlPath: ['steps', 0, 'params', 'value'],
offset: 0,
...overrides,
});

Expand Down Expand Up @@ -254,7 +261,8 @@ describe('validateVariables', () => {
mockWorkflowDefinition,
mockWorkflowGraph,
['steps', 0, 'params', 'value'],
undefined
undefined,
0
);
expect(mockValidateVariable).toHaveBeenCalledWith(variable, mockContext);
});
Expand Down Expand Up @@ -286,13 +294,68 @@ describe('validateVariables', () => {
expect(result[0].message).toContain('Foreach parameter');
});

it('should extend context with template-local assign so x is valid', () => {
const templateString = '{% assign x = 1 %}{{ x }}';
const scalarStart = 100;
const variableOffsetInDoc = scalarStart + templateString.indexOf('{{ x }}') + 4;
const variableItem = createVariableItem({
key: 'x',
startLineNumber: 1,
startColumn: 1,
endLineNumber: 1,
endColumn: 10,
yamlPath: ['steps', 0, 'with', 'message'],
offset: variableOffsetInDoc,
});
const mockModel = {
getOffsetAt: jest.fn((pos: { lineNumber: number; column: number }) => {
if (pos.lineNumber === 1 && pos.column === 1) return variableOffsetInDoc;
return 0;
}),
} as any;
const mockYamlDocument = {} as any;
mockGetScalarValueAtOffset.mockReturnValue({
value: templateString,
range: [
scalarStart,
scalarStart + templateString.length,
scalarStart + templateString.length,
],
} as any);
// Use real getContextSchemaForPath so it applies template locals via getContextSchemaWithTemplateLocals
const { getContextSchemaForPath: realGetContextSchemaForPath } = jest.requireActual<
typeof import('../../workflow_context/lib/get_context_for_path')
>('../../workflow_context/lib/get_context_for_path');
mockGetContextSchemaForPath.mockImplementation(realGetContextSchemaForPath);
mockValidateVariable.mockReturnValue({
...variableItem,
message: null,
severity: null,
owner: 'variable-validation',
hoverMessage: null,
});

const result = validateVariables(
[variableItem],
mockWorkflowGraph,
mockWorkflowDefinition,
mockYamlDocument,
mockModel
);

expect(mockGetScalarValueAtOffset).toHaveBeenCalledWith(mockYamlDocument, variableOffsetInDoc);
expect(result).toHaveLength(1);
expect(result[0].message).toBe(null);
});

it('should preserve all properties from validation errors', () => {
const variable = createVariableItem({
key: 'test.var',
startLineNumber: 5,
startColumn: 10,
endLineNumber: 5,
endColumn: 20,
offset: 0,
});

mockGetContextSchemaForPath.mockReturnValue({} as any);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,53 @@
*/

import type { Document } from 'yaml';
import type { monaco } from '@kbn/monaco';
import type { DynamicStepContextSchema, WorkflowYaml } from '@kbn/workflows';
import type { WorkflowGraph } from '@kbn/workflows/graph';
import { validateVariable } from './validate_variable';
import { getContextSchemaForPath } from '../../workflow_context/lib/get_context_for_path';
import type { VariableItem, YamlValidationResult } from '../model/types';

/**
* If the variable item has no offset, try to compute it on the fly from the editor model.
*/
function fallbackForOffsetValue(
variableItem: VariableItem,
yamlDocument?: Document | null,
model?: monaco.editor.ITextModel
) {
if (yamlDocument && model) {
return model?.getOffsetAt({
lineNumber: variableItem.startLineNumber,
column: variableItem.startColumn,
});
}
return undefined;
}

export function validateVariables(
variableItems: VariableItem[],
workflowGraph: WorkflowGraph,
workflowDefinition: WorkflowYaml,
yamlDocument?: Document | null
yamlDocument?: Document | null,
model?: monaco.editor.ITextModel
): YamlValidationResult[] {
const errors: YamlValidationResult[] = [];

for (const variableItem of variableItems) {
const { yamlPath: path } = variableItem;
const { yamlPath: path, offset } = variableItem;

let context: typeof DynamicStepContextSchema;
try {
context = getContextSchemaForPath(workflowDefinition, workflowGraph, path, yamlDocument);
const variableOffset = offset ?? fallbackForOffsetValue(variableItem, yamlDocument, model);
context = getContextSchemaForPath(
workflowDefinition,
workflowGraph,
path,
yamlDocument,
variableOffset
);

const error = validateVariable(variableItem, context);
if (error) {
errors.push(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface ConnectorIdItem extends BaseItem {

export interface VariableItem extends BaseItem {
type: 'regexp' | 'foreach';
offset?: number;
}

export interface CustomPropertyItem extends BaseItem {
Expand Down
Loading
Loading