diff --git a/workspaces/orchestrator/.changeset/five-meals-cover.md b/workspaces/orchestrator/.changeset/five-meals-cover.md new file mode 100644 index 0000000000..9dcd8eb7ee --- /dev/null +++ b/workspaces/orchestrator/.changeset/five-meals-cover.md @@ -0,0 +1,6 @@ +--- +'@red-hat-developer-hub/backstage-plugin-orchestrator-backend': patch +--- + +- Update dependecy @urql/core to fix CVE-2026-3118 +- Reworks the filter and query builder code to use query variables diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/package.json b/workspaces/orchestrator/plugins/orchestrator-backend/package.json index ef47ac9600..c18a5e2f3a 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/package.json +++ b/workspaces/orchestrator/plugins/orchestrator-backend/package.json @@ -76,7 +76,7 @@ "@backstage/plugin-scaffolder-node": "^0.12.1", "@red-hat-developer-hub/backstage-plugin-orchestrator-common": "workspace:^", "@red-hat-developer-hub/backstage-plugin-orchestrator-node": "workspace:^", - "@urql/core": "^4.1.4", + "@urql/core": "^6.0.1", "ajv-formats": "^2.1.1", "cloudevents": "^8.0.0", "express": "^4.21.2", diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts index f07fc77717..584cafe714 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilder.ts @@ -24,6 +24,10 @@ import { TypeName, } from '@red-hat-developer-hub/backstage-plugin-orchestrator-common'; +import { randomBytes } from 'node:crypto'; + +import { FilterClause, FilterClauseVariable } from '../types/filterClause'; + type ProcessType = 'ProcessDefinition' | 'ProcessInstance'; const supportedOperators = [ @@ -73,21 +77,25 @@ function handleLogicalFilter( introspection: IntrospectionField[], type: ProcessType, filter: LogicalFilter, -): string { - if (!filter.operator) return ''; +): FilterClause { + if (!filter.operator) return {} as FilterClause; const subClauses = filter.filters.map(f => buildFilterCondition(introspection, type, f), ); - return `${filter.operator.toLowerCase()}: {${subClauses.join(', ')}}`; + const filterClause: FilterClause = { + clause: `${filter.operator.toLowerCase()}: {${subClauses.map(cl => cl.clause).join(', ')}}`, + clauseVariable: subClauses.flatMap(cl => cl.clauseVariable), + }; + return filterClause; } function handleNestedFilter( introspection: IntrospectionField[], type: ProcessType, filter: NestedFilter, -): string { +): FilterClause { const subClauses = buildFilterCondition( introspection, type, @@ -95,22 +103,62 @@ function handleNestedFilter( true, ); - return `${filter.field}: {${subClauses}}`; + const filterClause: FilterClause = { + clauseVariable: subClauses.clauseVariable, + clause: `${filter.field}: {${subClauses.clause}}`, + }; + + return filterClause; } -function handleBetweenOperator(filter: FieldFilter): string { +function handleBetweenOperator(filter: FieldFilter): FilterClause { if (!Array.isArray(filter.value) || filter.value.length !== 2) { throw new Error('Between operator requires an array of two elements'); } - return `${filter.field}: {${getGraphQLOperator( + const filterClauseVariableArray: FilterClauseVariable[] = []; + const clauseVariableName1 = `clauseVariable${nonSecureRandomAlphaNumeric()}`; + const filterClauseVariable1: FilterClauseVariable = { + clauseVariableName: clauseVariableName1, + formattedValue: filter.value[0], + clauseVariableType: 'String', + }; + + const clauseVariableName2 = `clauseVariable${nonSecureRandomAlphaNumeric()}`; + const filterClauseVariable2: FilterClauseVariable = { + clauseVariableName: clauseVariableName2, + formattedValue: filter.value[1], + clauseVariableType: 'String', + }; + + const clause = `${filter.field}: {${getGraphQLOperator( FieldFilterOperatorEnum.Between, - )}: {from: "${filter.value[0]}", to: "${filter.value[1]}"}}`; + )}: {from: $${clauseVariableName1}, to: $${clauseVariableName2}}}`; + filterClauseVariableArray.push(filterClauseVariable1, filterClauseVariable2); + const filterClause: FilterClause = { + clause: clause, + clauseVariable: filterClauseVariableArray, + }; + + return filterClause; } -function handleIsNullOperator(filter: FieldFilter): string { - return `${filter.field}: {${getGraphQLOperator( - FieldFilterOperatorEnum.IsNull, - )}: ${convertToBoolean(filter.value)}}`; +function handleIsNullOperator(filter: FieldFilter): FilterClause { + const clauseVariableName = `clauseVariable${nonSecureRandomAlphaNumeric()}`; + const clause = `${filter.field}: {${getGraphQLOperator(FieldFilterOperatorEnum.IsNull)}: $${clauseVariableName}}`; + + const filterClauseVariable: FilterClauseVariable = { + clauseVariableName: clauseVariableName, + formattedValue: convertToBoolean(filter.value), + clauseVariableType: 'Boolean', + }; + const filterClauseVariableArray: FilterClauseVariable[] = []; + filterClauseVariableArray.push(filterClauseVariable); + const clauseObject: FilterClause = { + clauseVariable: filterClauseVariableArray, + clause, + }; + + return clauseObject; } function isEnumFilter( @@ -136,7 +184,7 @@ function handleBinaryOperator( binaryFilter: FieldFilter, fieldDef: IntrospectionField | undefined, type: 'ProcessDefinition' | 'ProcessInstance', -): string { +): FilterClause { if (isEnumFilter(binaryFilter.field, type)) { if (!isValidEnumOperator(binaryFilter.operator)) { throw new Error( @@ -144,14 +192,40 @@ function handleBinaryOperator( ); } } - const formattedValue = Array.isArray(binaryFilter.value) - ? `[${binaryFilter.value - .map(v => formatValue(binaryFilter.field, v, fieldDef, type)) - .join(', ')}]` - : formatValue(binaryFilter.field, binaryFilter.value, fieldDef, type); - return `${binaryFilter.field}: {${getGraphQLOperator( - binaryFilter.operator, - )}: ${formattedValue}}`; + let formattedValue: any; + let paramType: string; + if (Array.isArray(binaryFilter.value)) { + formattedValue = binaryFilter.value.map(v => + formatValue(binaryFilter.field, v, fieldDef, type), + ); + paramType = isEnumFilter(binaryFilter.field, type) + ? '[ProcessInstanceState!]' + : '[String!]'; + } else { + formattedValue = formatValue( + binaryFilter.field, + binaryFilter.value, + fieldDef, + type, + ); + paramType = 'String'; + } + + const clauseVariableName = `clauseVariable${nonSecureRandomAlphaNumeric()}`; + const clause = `${binaryFilter.field}: {${getGraphQLOperator(binaryFilter.operator)}: $${clauseVariableName}}`; + const filterClauseVariable: FilterClauseVariable = { + clauseVariableName: clauseVariableName, + formattedValue: formattedValue, + clauseVariableType: paramType, + }; + const filterClauseVariableArray: FilterClauseVariable[] = []; + filterClauseVariableArray.push(filterClauseVariable); + const clauseObject: FilterClause = { + clauseVariable: filterClauseVariableArray, + clause, + }; + + return clauseObject; } export function buildFilterCondition( @@ -159,9 +233,9 @@ export function buildFilterCondition( type: ProcessType, filters?: Filter, isNested?: boolean, -): string { +): FilterClause { if (!filters) { - return ''; + return {} as FilterClause; } if (isNestedFilter(filters)) { @@ -255,7 +329,7 @@ function formatValue( type: ProcessType, ): string { if (!fieldDef) { - return `"${fieldValue}"`; + return `${fieldValue}`; } if (!isFieldFilterSupported) { @@ -270,7 +344,7 @@ function formatValue( fieldDef.type.name === TypeName.Id || fieldDef.type.name === TypeName.Date ) { - return `"${fieldValue}"`; + return `${fieldValue}`; } throw new Error( `Failed to format value for ${fieldName} ${fieldValue} with type ${fieldDef.type.name}`, @@ -301,3 +375,9 @@ function getGraphQLOperator(operator: FieldFilterOperatorEnum): string { throw new Error(`Operation "${operator}" not supported`); } } + +// Function for getting 4 random digits to append to the clause variable name. +// Not used for any secrets or anything +function nonSecureRandomAlphaNumeric() { + return randomBytes(8).toString('hex'); +} diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilders.test.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilders.test.ts index b4e9279228..6befa390ed 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilders.test.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/filterBuilders.test.ts @@ -18,12 +18,12 @@ import { FieldFilterOperatorEnum, Filter, IntrospectionField, - ProcessInstanceState, ProcessInstanceStatusDTO, TypeKind, TypeName, } from '@red-hat-developer-hub/backstage-plugin-orchestrator-common'; +import { FilterClause } from '../types/filterClause'; import { buildFilterCondition, isOperatorAllowedForField, @@ -126,15 +126,17 @@ describe('column filters', () => { name: string; introspectionFields: IntrospectionField[]; filter: Filter | undefined; - expectedResult: string; + expectedResult: string | FilterClause; + expectedFormattedValue: Array; }; describe('empty filter testcases', () => { const emptyFilterTestCases: FilterTestCase[] = [ { - name: 'returns empty string when filters are null or undefined', + name: 'returns empty object when filters are null or undefined', introspectionFields: [], filter: undefined, - expectedResult: '', + expectedResult: {} as FilterClause, + expectedFormattedValue: [''], }, ]; emptyFilterTestCases.forEach( @@ -145,7 +147,7 @@ describe('column filters', () => { 'ProcessInstance', filter, ); - expect(result).toBe(expectedResult); + expect(result).toEqual(expectedResult); }); }, ); @@ -162,7 +164,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Eq, 'Hello World Workflow', ), - expectedResult: 'name: {equal: "Hello World Workflow"}', + expectedResult: 'name: {equal: $variable1}', + expectedFormattedValue: ['Hello World Workflow'], }, { name: 'returns correct filter for single string field with like operator', @@ -174,7 +177,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Like, 'Hello%', ), - expectedResult: 'name: {like: "Hello%"}', + expectedResult: 'name: {like: $variable1}', + expectedFormattedValue: ['Hello%'], }, { name: 'returns correct filter for string field with isNull operator (true)', @@ -182,7 +186,8 @@ describe('column filters', () => { createIntrospectionField('name', TypeName.String), ], filter: createFieldFilter('name', FieldFilterOperatorEnum.IsNull, true), - expectedResult: 'name: {isNull: true}', + expectedResult: 'name: {isNull: $variable1}', + expectedFormattedValue: [true], }, { name: 'returns correct filter for string field with isNull operator (false)', @@ -194,7 +199,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, false, ), - expectedResult: 'name: {isNull: false}', + expectedResult: 'name: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for string field with isNull operator ("true" as string)', @@ -206,7 +212,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, 'True', ), - expectedResult: 'name: {isNull: true}', + expectedResult: 'name: {isNull: $variable1}', + expectedFormattedValue: [true], }, { name: 'returns correct filter for string field with isNull operator ("false" as string)', @@ -218,7 +225,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, 'FALSE', ), - expectedResult: 'name: {isNull: false}', + expectedResult: 'name: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for string field with in operator (single value)', @@ -228,7 +236,8 @@ describe('column filters', () => { filter: createFieldFilter('name', FieldFilterOperatorEnum.In, [ 'Test String', ]), - expectedResult: 'name: {in: ["Test String"]}', + expectedResult: 'name: {in: $variable1}', + expectedFormattedValue: [['Test String']], }, { name: 'returns correct filter for string field with in operator (multiple values)', @@ -240,8 +249,10 @@ describe('column filters', () => { 'Test String 2', 'Test String 3', ]), - expectedResult: - 'name: {in: ["Test String 1", "Test String 2", "Test String 3"]}', + expectedResult: 'name: {in: $variable1}', + expectedFormattedValue: [ + ['Test String 1', 'Test String 2', 'Test String 3'], + ], }, { name: 'returns correct OR filter for two string fields with equal operator', @@ -265,7 +276,8 @@ describe('column filters', () => { ], }, expectedResult: - 'or: {name: {equal: "Hello World Workflow"}, processName: {equal: "Greeting workflow"}}', + 'or: {name: {equal: $variable1}, processName: {equal: $variable2}}', + expectedFormattedValue: ['Hello World Workflow', 'Greeting workflow'], }, { name: 'returns correct filter for string field with like and isNull operators', @@ -288,7 +300,8 @@ describe('column filters', () => { ], }, expectedResult: - 'or: {description: {like: "%Test%"}, description: {isNull: true}}', + 'or: {description: {like: $variable1}, description: {isNull: $variable2}}', + expectedFormattedValue: ['%Test%', true], }, { name: 'returns correct filter for string field with in, like, equal, and isNull operators', @@ -312,7 +325,13 @@ describe('column filters', () => { ], }, expectedResult: - 'or: {name: {in: ["Test String 1", "Test String 2"]}, name: {like: "%Test%"}, name: {equal: "Exact Match"}, name: {isNull: false}}', + 'or: {name: {in: $variable1}, name: {like: $variable2}, name: {equal: $variable3}, name: {isNull: $variable4}}', + expectedFormattedValue: [ + ['Test String 1', 'Test String 2'], + '%Test%', + 'Exact Match', + false, + ], }, { name: 'returns correct filter for string field with in, like, equal, and isNull operators', @@ -336,18 +355,39 @@ describe('column filters', () => { ], }, expectedResult: - 'and: {name: {in: ["Test String 1", "Test String 2"]}, name: {like: "%Test%"}, name: {equal: "Exact Match"}, name: {isNull: false}}', + 'and: {name: {in: $variable1}, name: {like: $variable2}, name: {equal: $variable3}, name: {isNull: $variable4}}', + expectedFormattedValue: [ + ['Test String 1', 'Test String 2'], + '%Test%', + 'Exact Match', + false, + ], }, ]; stringTestCases.forEach( - ({ name, introspectionFields, filter, expectedResult }) => { + ({ + name, + introspectionFields, + filter, + expectedResult, + expectedFormattedValue, + }) => { it(`${name}`, () => { - const result = buildFilterCondition( + const result: FilterClause = buildFilterCondition( introspectionFields, 'ProcessInstance', filter, ); - expect(result).toBe(expectedResult); + expect(result).toBeDefined(); + let formattedClause = expectedResult as string; + result.clauseVariable.forEach((item, index) => { + formattedClause = formattedClause.replace( + `$variable${index + 1}`, + `$${item.clauseVariableName}`, + ); + expect(item.formattedValue).toEqual(expectedFormattedValue[index]); + }); + expect(formattedClause).toBe(result.clause); }); }, ); @@ -358,13 +398,15 @@ describe('column filters', () => { name: 'returns correct filter for single id field with equal operator', introspectionFields: [createIntrospectionField('id', TypeName.Id)], filter: createFieldFilter('id', FieldFilterOperatorEnum.Eq, 'idA'), - expectedResult: 'id: {equal: "idA"}', + expectedResult: 'id: {equal: $variable1}', + expectedFormattedValue: ['idA'], }, { name: 'returns correct filter for single id field with isNull operator (false as boolean)', introspectionFields: [createIntrospectionField('id', TypeName.Id)], filter: createFieldFilter('id', FieldFilterOperatorEnum.IsNull, false), - expectedResult: 'id: {isNull: false}', + expectedResult: 'id: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for single id field with isNull operator (false as string)', @@ -374,7 +416,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, 'false', ), - expectedResult: 'id: {isNull: false}', + expectedResult: 'id: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for single id field with IN operator', @@ -384,7 +427,8 @@ describe('column filters', () => { 'idB', 'idC', ]), - expectedResult: 'id: {in: ["idA", "idB", "idC"]}', + expectedResult: 'id: {in: $variable1}', + expectedFormattedValue: [['idA', 'idB', 'idC']], }, { name: 'returns correct OR filter for multiple id fields with equal, isNull, and IN operators', @@ -405,7 +449,8 @@ describe('column filters', () => { ], }, expectedResult: - 'or: {id: {equal: "idA"}, processId: {isNull: true}, id: {in: ["idA", "idB", "idC"]}}', + 'or: {id: {equal: $variable1}, processId: {isNull: $variable2}, id: {in: $variable3}}', + expectedFormattedValue: ['idA', true, ['idA', 'idB', 'idC']], }, { name: 'returns correct AND filter for multiple id fields with equal, isNull, and IN operators', @@ -426,19 +471,35 @@ describe('column filters', () => { ], }, expectedResult: - 'and: {id: {equal: "idA"}, processId: {isNull: true}, id: {in: ["idA", "idB", "idC"]}}', + 'and: {id: {equal: $variable1}, processId: {isNull: $variable2}, id: {in: $variable3}}', + expectedFormattedValue: ['idA', true, ['idA', 'idB', 'idC']], }, ]; idTestCases.forEach( - ({ name, introspectionFields, filter, expectedResult }) => { + ({ + name, + introspectionFields, + filter, + expectedResult, + expectedFormattedValue, + }) => { it(`${name}`, () => { const result = buildFilterCondition( introspectionFields, 'ProcessInstance', filter, ); - expect(result).toBe(expectedResult); + expect(result).toBeDefined(); + let formattedClause = expectedResult as string; + result.clauseVariable.forEach((item, index) => { + formattedClause = formattedClause.replace( + `$variable${index + 1}`, + `$${item.clauseVariableName}`, + ); + expect(item.formattedValue).toEqual(expectedFormattedValue[index]); + }); + expect(formattedClause).toBe(result.clause); }); }, ); @@ -456,7 +517,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Eq, testDate1, ), - expectedResult: `start: {equal: "${testDate1}"}`, + expectedResult: `start: {equal: $variable1}`, + expectedFormattedValue: [testDate1], }, { name: 'returns correct filter for single date field with isNull operator (false as boolean)', @@ -466,7 +528,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, false, ), - expectedResult: 'start: {isNull: false}', + expectedResult: 'start: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for single date field with isNull operator (false as string)', @@ -476,7 +539,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.IsNull, 'false', ), - expectedResult: 'start: {isNull: false}', + expectedResult: 'start: {isNull: $variable1}', + expectedFormattedValue: [false], }, { name: 'returns correct filter for single date field with GT operator', @@ -486,7 +550,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Gt, testDate1, ), - expectedResult: `start: {greaterThan: "${testDate1}"}`, + expectedResult: `start: {greaterThan: $variable1}`, + expectedFormattedValue: [testDate1], }, { name: 'returns correct filter for single date field with GTE operator', @@ -496,7 +561,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Gte, testDate1, ), - expectedResult: `start: {greaterThanEqual: "${testDate1}"}`, + expectedResult: `start: {greaterThanEqual: $variable1}`, + expectedFormattedValue: [testDate1], }, { name: 'returns correct filter for single date field with LT operator', @@ -506,7 +572,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Lt, testDate1, ), - expectedResult: `start: {lessThan: "${testDate1}"}`, + expectedResult: `start: {lessThan: $variable1}`, + expectedFormattedValue: [testDate1], }, { name: 'returns correct filter for single date field with LTE operator', @@ -516,7 +583,8 @@ describe('column filters', () => { FieldFilterOperatorEnum.Lte, testDate1, ), - expectedResult: `start: {lessThanEqual: "${testDate1}"}`, + expectedResult: `start: {lessThanEqual: $variable1}`, + expectedFormattedValue: [testDate1], }, { name: 'returns correct filter for single date field with BETWEEN operator', @@ -525,7 +593,8 @@ describe('column filters', () => { testDate1, testDate2, ]), - expectedResult: `start: {between: {from: "${testDate1}", to: "${testDate2}"}}`, + expectedResult: `start: {between: {from: $variable1, to: $variable2}}`, + expectedFormattedValue: [testDate1, testDate2], }, { name: 'returns correct OR filter for multiple id fields with equal, isNull, and GT operators', @@ -541,7 +610,8 @@ describe('column filters', () => { createFieldFilter('end', FieldFilterOperatorEnum.Gt, testDate1), ], }, - expectedResult: `or: {start: {equal: "${testDate1}"}, end: {isNull: false}, end: {greaterThan: "${testDate1}"}}`, + expectedResult: `or: {start: {equal: $variable1}, end: {isNull: $variable2}, end: {greaterThan: $variable3}}`, + expectedFormattedValue: [testDate1, false, testDate1], }, { name: 'returns correct OR filter for multiple id fields with equal, isNull, and GTE operators', @@ -557,7 +627,8 @@ describe('column filters', () => { createFieldFilter('end', FieldFilterOperatorEnum.Gte, testDate1), ], }, - expectedResult: `or: {start: {equal: "${testDate1}"}, end: {isNull: false}, end: {greaterThanEqual: "${testDate1}"}}`, + expectedResult: `or: {start: {equal: $variable1}, end: {isNull: $variable2}, end: {greaterThanEqual: $variable3}}`, + expectedFormattedValue: [testDate1, false, testDate1], }, { name: 'returns correct AND filter for multiple id fields with equal, isNull, and LTE operators', @@ -573,7 +644,8 @@ describe('column filters', () => { createFieldFilter('end', FieldFilterOperatorEnum.Lte, testDate1), ], }, - expectedResult: `and: {start: {equal: "${testDate1}"}, end: {isNull: false}, end: {lessThanEqual: "${testDate1}"}}`, + expectedResult: `and: {start: {equal: $variable1}, end: {isNull: $variable2}, end: {lessThanEqual: $variable3}}`, + expectedFormattedValue: [testDate1, false, testDate1], }, { name: 'returns correct AND filter for multiple id fields with equal, isNull, LTE, and between operators', @@ -593,19 +665,41 @@ describe('column filters', () => { ]), ], }, - expectedResult: `and: {start: {equal: "${testDate1}"}, end: {isNull: false}, end: {lessThanEqual: "${testDate1}"}, start: {between: {from: "${testDate1}", to: "${testDate2}"}}}`, + expectedResult: `and: {start: {equal: $variable1}, end: {isNull: $variable2}, end: {lessThanEqual: $variable3}, start: {between: {from: $variable4, to: $variable5}}}`, + expectedFormattedValue: [ + testDate1, + false, + testDate1, + testDate1, + testDate2, + ], }, ]; idTestCases.forEach( - ({ name, introspectionFields, filter, expectedResult }) => { + ({ + name, + introspectionFields, + filter, + expectedResult, + expectedFormattedValue, + }) => { it(`${name}`, () => { const result = buildFilterCondition( introspectionFields, 'ProcessInstance', filter, ); - expect(result).toBe(expectedResult); + expect(result).toBeDefined(); + let formattedClause = expectedResult as string; + result.clauseVariable.forEach((item, index) => { + formattedClause = formattedClause.replace( + `$variable${index + 1}`, + `$${item.clauseVariableName}`, + ); + expect(item.formattedValue).toEqual(expectedFormattedValue[index]); + }); + expect(formattedClause).toBe(result.clause); }); }, ); @@ -622,19 +716,35 @@ describe('column filters', () => { FieldFilterOperatorEnum.Eq, ProcessInstanceStatusDTO.Completed, ), - expectedResult: `state: {equal: ${ProcessInstanceState.Completed}}`, + expectedResult: `state: {equal: $variable1}`, + expectedFormattedValue: ['COMPLETED'], }, ]; idTestCases.forEach( - ({ name, introspectionFields, filter, expectedResult }) => { + ({ + name, + introspectionFields, + filter, + expectedResult, + expectedFormattedValue, + }) => { it(`${name}`, () => { const result = buildFilterCondition( introspectionFields, 'ProcessInstance', filter, ); - expect(result).toBe(expectedResult); + expect(result).toBeDefined(); + let formattedClause = expectedResult as string; + result.clauseVariable.forEach((item, index) => { + formattedClause = formattedClause.replace( + `$variable${index + 1}`, + `$${item.clauseVariableName}`, + ); + expect(item.formattedValue).toEqual(expectedFormattedValue[index]); + }); + expect(formattedClause).toBe(result.clause); }); }, ); diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.test.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.test.ts index ab77d65abb..c36727601b 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.test.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.test.ts @@ -1,5 +1,5 @@ /* - * Copyright 2024 The Backstage Authors + * Copyright Red Hat, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Pagination } from '../types/pagination'; import { buildGraphQlQuery } from './queryBuilder'; @@ -32,16 +33,6 @@ describe('buildGraphQlQuery', () => { whereClause: 'version: "1.0"', }; - const getPaginationString = (pagination: Pagination | undefined) => { - const paginationOrder = pagination?.order - ? pagination.order.toUpperCase() - : 'ASC'; - if (pagination) { - return `orderBy: {${pagination.sortField}: ${paginationOrder}}, pagination: {limit: ${pagination.limit}, offset: ${pagination.offset}})`; - } - return undefined; - }; - type TestCase = { name: string; params: typeof defaultTestParams; @@ -57,7 +48,7 @@ describe('buildGraphQlQuery', () => { whereClause: '', pagination: {}, }, - expectedResult: `{${defaultTestParams.type} {${defaultTestParams.queryBody} } }`, + expectedResult: `query ($paginationInfo: Pagination, $orderByInfo: ${defaultTestParams.type.slice(0, -1)}OrderBy){${defaultTestParams.type} (orderBy: $orderByInfo, pagination: $paginationInfo) {${defaultTestParams.queryBody} } }`, }, { name: 'should build a query with a where clause', @@ -67,7 +58,7 @@ describe('buildGraphQlQuery', () => { whereClause: defaultTestParams.whereClause, pagination: {}, }, - expectedResult: `{${defaultTestParams.type} (where: {${defaultTestParams.whereClause}}) {${defaultTestParams.queryBody} } }`, + expectedResult: `query ($paginationInfo: Pagination, $orderByInfo: ${defaultTestParams.type.slice(0, -1)}OrderBy){${defaultTestParams.type} (where: {${defaultTestParams.whereClause}}, orderBy: $orderByInfo, pagination: $paginationInfo) {${defaultTestParams.queryBody} } }`, }, { name: 'should build a query with pagination', @@ -77,18 +68,16 @@ describe('buildGraphQlQuery', () => { whereClause: '', pagination: defaultTestParams.pagination, }, - expectedResult: `{${defaultTestParams.type} (${getPaginationString( - defaultTestParams.pagination, - )} {${defaultTestParams.queryBody} } }`, + expectedResult: `query ($paginationInfo: Pagination, $orderByInfo: ${defaultTestParams.type.slice(0, -1)}OrderBy){${defaultTestParams.type} (orderBy: $orderByInfo, pagination: $paginationInfo) {${defaultTestParams.queryBody} } }`, }, { name: 'should build a query with both where clause and pagination', params: { ...defaultTestParams, }, - expectedResult: `{${defaultTestParams.type} (where: {${ + expectedResult: `query ($paginationInfo: Pagination, $orderByInfo: ${defaultTestParams.type.slice(0, -1)}OrderBy){${defaultTestParams.type} (where: {${ defaultTestParams.whereClause - }}, ${getPaginationString(defaultTestParams.pagination)} {${ + }}, orderBy: $orderByInfo, pagination: $paginationInfo) {${ defaultTestParams.queryBody } } }`, }, diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.ts index e33d20db3f..5a4e4ea7d1 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/helpers/queryBuilder.ts @@ -1,5 +1,5 @@ /* - * Copyright 2024 The Backstage Authors + * Copyright Red Hat, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,22 +13,43 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Pagination } from '../types/pagination'; + +import { FilterClause } from '../types/filterClause'; +import { Pagination, PaginationQueryVariable } from '../types/pagination'; export function buildGraphQlQuery(args: { type: 'ProcessDefinitions' | 'ProcessInstances' | 'Jobs'; queryBody: string; whereClause?: string; pagination?: Pagination; + filterCondition?: FilterClause; }): string { - let query = `{${args.type}`; + const queryHeaderStart = 'query ('; + const queryHeaderEnd = ')'; + + const queryHeaderPaginationOrderByParams = `$paginationInfo: Pagination, $orderByInfo: ${args.type.slice(0, -1)}OrderBy`; + + const filterParams = args.filterCondition?.clauseVariable + ?.map(cl => { + return `$${cl.clauseVariableName}: ${cl.clauseVariableType}`; + }) + .join(', '); + + const params = [queryHeaderPaginationOrderByParams, filterParams] + .filter(Boolean) + .join(', '); + let query = `${queryHeaderStart}${params}${queryHeaderEnd}{${args.type}`; const whereClause = buildWhereClause(args.whereClause); - const paginationClause = buildPaginationClause(args.pagination); - if (whereClause || paginationClause) { + const paginationClause = 'pagination: $paginationInfo'; + const orderByClause = 'orderBy: $orderByInfo'; + + if (whereClause || paginationClause || orderByClause) { query += ' ('; - query += [whereClause, paginationClause].filter(Boolean).join(', '); + query += [whereClause, orderByClause, paginationClause] + .filter(Boolean) + .join(', '); query += ') '; } @@ -41,29 +62,46 @@ function buildWhereClause(whereClause?: string): string { return whereClause ? `where: {${whereClause}}` : ''; } -function buildPaginationClause(pagination?: Pagination): string { - if (!pagination) return ''; - - const parts = []; +export function buildOrderByVariables(pagination?: Pagination): { + [key: string]: string; +} { + const orderByVariable: { [key: string]: string } = {}; - if (pagination.sortField !== undefined) { - parts.push( - `orderBy: {${pagination.sortField}: ${ - pagination.order !== undefined ? pagination.order?.toUpperCase() : 'ASC' - }}`, - ); + if (pagination?.sortField !== undefined) { + orderByVariable[pagination.sortField] = + pagination.order?.toUpperCase() ?? 'ASC'; } - const paginationParts = []; - if (pagination.limit !== undefined) { - paginationParts.push(`limit: ${pagination.limit}`); - } - if (pagination.offset !== undefined) { - paginationParts.push(`offset: ${pagination.offset}`); + return orderByVariable; +} + +export function buildPaginationVariables( + pagination?: Pagination, +): PaginationQueryVariable { + const paginationVariable: PaginationQueryVariable = {}; + + if (pagination?.limit !== undefined) { + paginationVariable.limit = pagination.limit; } - if (paginationParts.length) { - parts.push(`pagination: {${paginationParts.join(', ')}}`); + + if (pagination?.offset !== undefined) { + paginationVariable.offset = pagination.offset; } + return paginationVariable; +} + +export function buildQueryParamVariable( + pagination?: Pagination, + filterCondition?: FilterClause, +) { + const paramVariables: any = { + paginationInfo: buildPaginationVariables(pagination), + orderByInfo: buildOrderByVariables(pagination), + }; + + filterCondition?.clauseVariable?.forEach(p => { + paramVariables[p.clauseVariableName] = p.formattedValue; + }); - return parts.join(', '); + return paramVariables; } diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.test.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.test.ts index c5ec7ff490..1b2cc32752 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.test.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.test.ts @@ -31,6 +31,7 @@ import { import * as buildGrahQLFilterUtils from '../helpers/filterBuilder'; import * as buildGrahQLQueryUtils from '../helpers/queryBuilder'; +import { FilterClause } from '../types/filterClause'; import { Pagination } from '../types/pagination'; import { mockProcessDefinitionArguments, @@ -85,11 +86,13 @@ const createQueryArgs = ( queryBody: string, whereClause?: string, pagination?: Pagination, + filterCondition?: FilterClause, ) => ({ type, queryBody, whereClause, pagination, + filterCondition, }); describe('initInputArgs', () => { @@ -180,9 +183,6 @@ describe('fetchWorkflowInfos', () => { 'id, name, version, type, endpoint, serviceUrl, source, metadata'; const pagination = { limit: 10, offset: 0, order: 'ASC', sortField: 'name' }; - const filterString = - 'or: {name: {equal: "Hello World Workflow"}, id: {equal: "yamlgreet"}}'; - const helloWorldFilter = { field: 'name', operator: FieldFilterOperatorEnum.Eq, @@ -252,7 +252,10 @@ describe('fetchWorkflowInfos', () => { expect(mockClient.query).toHaveBeenCalled(); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: {}, + paginationInfo: {}, + }, ); }); @@ -289,7 +292,10 @@ describe('fetchWorkflowInfos', () => { expect(mockClient.query).toHaveBeenCalled(); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: {}, + paginationInfo: {}, + }, ); }); @@ -327,7 +333,15 @@ describe('fetchWorkflowInfos', () => { expect(mockClient.query).toHaveBeenCalledTimes(1); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: { + name: 'ASC', + }, + paginationInfo: { + limit: 10, + offset: 0, + }, + }, ); expect(buildFilterConditionSpy).not.toHaveBeenCalled(); }); @@ -343,12 +357,6 @@ describe('fetchWorkflowInfos', () => { ) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); - const expectedQueryArgs = createQueryArgs( - 'ProcessDefinitions', - queryBody, - filterString, - ); - // When const result = await dataIndexService.fetchWorkflowInfos({ filter: logicalFilter, @@ -358,30 +366,44 @@ describe('fetchWorkflowInfos', () => { expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessDefinitions); - expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(1); - expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ - type: 'ProcessDefinitions', - queryBody, - whereClause: filterString, - }); expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); expect(buildFilterConditionSpy).toHaveBeenCalledWith( mockProcessDefinitionIntrospection, 'ProcessDefinition', logicalFilter, ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(1); + expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ + type: 'ProcessDefinitions', + queryBody, + whereClause: createdFilter.clause, + filterCondition: createdFilter, + }); + + const expectedQueryArgs = createQueryArgs( + 'ProcessDefinitions', + queryBody, + createdFilter.clause, + undefined, + createdFilter, + ); + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + undefined, + createdFilter, + ); + expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); }); it('should fetch workflow infos with definitionIds and filter', async () => { - // Given - const whereClause = `and: [{id: {in: ${JSON.stringify( - definitionIds, - )}}}, {${filterString}}]`; // Given const mockQueryResult = { ProcessDefinitions: mockWfInfos, @@ -392,12 +414,6 @@ describe('fetchWorkflowInfos', () => { ) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); - const expectedQueryArgs = createQueryArgs( - 'ProcessDefinitions', - queryBody, - whereClause, - ); - // When const result = await dataIndexService.fetchWorkflowInfos({ definitionIds, @@ -405,6 +421,26 @@ describe('fetchWorkflowInfos', () => { }); // Then + expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); + expect(buildFilterConditionSpy).toHaveBeenCalledWith( + mockProcessDefinitionIntrospection, + 'ProcessDefinition', + logicalFilter, + ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + const whereClause = `and: [{id: {in: ${JSON.stringify( + definitionIds, + )}}}, {${createdFilter.clause}}]`; + + const expectedQueryArgs = createQueryArgs( + 'ProcessDefinitions', + queryBody, + whereClause, + undefined, + createdFilter, + ); expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(1); expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ @@ -412,17 +448,18 @@ describe('fetchWorkflowInfos', () => { queryBody: 'id, name, version, type, endpoint, serviceUrl, source, metadata', whereClause, + filterCondition: createdFilter, }); - expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); - expect(buildFilterConditionSpy).toHaveBeenCalledWith( - mockProcessDefinitionIntrospection, - 'ProcessDefinition', - logicalFilter, + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + undefined, + createdFilter, ); + expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessDefinitions); @@ -430,9 +467,7 @@ describe('fetchWorkflowInfos', () => { it('should fetch workflow infos with definitionIds, pagination, and filter', async () => { // Given - const whereClause = `and: [{id: {in: ${JSON.stringify( - definitionIds, - )}}}, {${filterString}}]`; + // Given const mockQueryResult = { ProcessDefinitions: mockWfInfos, @@ -443,12 +478,6 @@ describe('fetchWorkflowInfos', () => { ) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); - const expectedQueryArgs = createQueryArgs( - 'ProcessDefinitions', - queryBody, - whereClause, - pagination, - ); // When const result = await dataIndexService.fetchWorkflowInfos({ definitionIds, @@ -457,11 +486,36 @@ describe('fetchWorkflowInfos', () => { }); // Then + expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); + expect(buildFilterConditionSpy).toHaveBeenCalledWith( + mockProcessDefinitionIntrospection, + 'ProcessDefinition', + logicalFilter, + ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + const whereClause = `and: [{id: {in: ${JSON.stringify( + definitionIds, + )}}}, {${createdFilter.clause}}]`; + + const expectedQueryArgs = createQueryArgs( + 'ProcessDefinitions', + queryBody, + whereClause, + pagination, + createdFilter, + ); + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + pagination, + createdFilter, + ); expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(2); expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ @@ -469,13 +523,9 @@ describe('fetchWorkflowInfos', () => { queryBody, whereClause, pagination, + filterCondition: createdFilter, }); - expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); - expect(buildFilterConditionSpy).toHaveBeenCalledWith( - mockProcessDefinitionIntrospection, - 'ProcessDefinition', - logicalFilter, - ); + expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessDefinitions); }); @@ -513,9 +563,6 @@ describe('fetchInstances', () => { }, ]; - const filterString = - 'or: {processId: {equal: "processId1"}, processName: {like: "processName%"}}'; - const procName1Filter: FieldFilter = { field: 'processName', operator: FieldFilterOperatorEnum.Like, @@ -598,7 +645,10 @@ describe('fetchInstances', () => { expect(mockClient.query).toHaveBeenCalled(); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: {}, + paginationInfo: {}, + }, ); }); @@ -632,7 +682,10 @@ describe('fetchInstances', () => { expect(mockClient.query).toHaveBeenCalled(); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: {}, + paginationInfo: {}, + }, ); expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessInstances); @@ -673,26 +726,48 @@ describe('fetchInstances', () => { expect(mockClient.query).toHaveBeenCalledTimes(1); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + { + orderByInfo: { + name: 'ASC', + }, + paginationInfo: { + limit: 10, + offset: 0, + }, + }, ); }); it('should fetch instances with only filter', async () => { // Given - const whereClause = `and: [{${processIdNotNullCondition}}, {${filterString}}]`; + mockClient.query .mockResolvedValueOnce(mockOperationResult(mockProcessInstanceArguments)) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); + // When + const result = await dataIndexService.fetchInstances({ + filter: logicalFilter, + }); + + expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); + expect(buildFilterConditionSpy).toHaveBeenCalledWith( + mockProcessInstanceIntrospection, + 'ProcessInstance', + logicalFilter, + ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + const whereClause = `and: [{${processIdNotNullCondition}}, {${createdFilter.clause}}]`; + const expectedQueryArgs = createQueryArgs( 'ProcessInstances', queryBody, whereClause, + undefined, + createdFilter, ); - // When - const result = await dataIndexService.fetchInstances({ - filter: logicalFilter, - }); // Then expect(result).toBeDefined(); @@ -702,31 +777,26 @@ describe('fetchInstances', () => { type: 'ProcessInstances', queryBody, whereClause, + filterCondition: createdFilter, }); - expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); - expect(buildFilterConditionSpy).toHaveBeenCalledWith( - mockProcessInstanceIntrospection, - 'ProcessInstance', - logicalFilter, + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + undefined, + createdFilter, ); + expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); }); it('should fetch instances with definitionIds and filter', async () => { // Given - const whereClause = `and: [{${processIdNotNullCondition}}, {${processIdDefinitions}}}, {${filterString}}]`; mockClient.query .mockResolvedValueOnce(mockOperationResult(mockProcessInstanceArguments)) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); - const expectedQueryArgs = createQueryArgs( - 'ProcessInstances', - queryBody, - whereClause, - ); // When const result = await dataIndexService.fetchInstances({ definitionIds, @@ -734,22 +804,42 @@ describe('fetchInstances', () => { }); // Then + expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); + expect(buildFilterConditionSpy).toHaveBeenCalledWith( + mockProcessInstanceIntrospection, + 'ProcessInstance', + logicalFilter, + ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + const whereClause = `and: [{${processIdNotNullCondition}}, {${processIdDefinitions}}}, {${createdFilter.clause}}]`; + + const expectedQueryArgs = createQueryArgs( + 'ProcessInstances', + queryBody, + whereClause, + undefined, + createdFilter, + ); + expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(1); expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ type: 'ProcessInstances', queryBody, whereClause, + filterCondition: createdFilter, }); - expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); - expect(buildFilterConditionSpy).toHaveBeenCalledWith( - mockProcessInstanceIntrospection, - 'ProcessInstance', - logicalFilter, + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + undefined, + createdFilter, ); + expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessInstances); @@ -757,16 +847,10 @@ describe('fetchInstances', () => { it('should fetch instances with definitionIds, pagination, and filter', async () => { // Given - const whereClause = `and: [{${processIdNotNullCondition}}, {${processIdDefinitions}}}, {${filterString}}]`; mockClient.query .mockResolvedValueOnce(mockOperationResult(mockProcessInstanceArguments)) .mockResolvedValueOnce(mockOperationResult(mockQueryResult)); - const expectedQueryArgs = createQueryArgs( - 'ProcessInstances', - queryBody, - whereClause, - pagination, - ); + // When const result = await dataIndexService.fetchInstances({ definitionIds, @@ -775,23 +859,43 @@ describe('fetchInstances', () => { }); // Then + expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); + expect(buildFilterConditionSpy).toHaveBeenCalledWith( + mockProcessInstanceIntrospection, + 'ProcessInstance', + logicalFilter, + ); + + const createdFilter = buildFilterConditionSpy.mock.results[0].value; + + const whereClause = `and: [{${processIdNotNullCondition}}, {${processIdDefinitions}}}, {${createdFilter.clause}}]`; + + const expectedQueryArgs = createQueryArgs( + 'ProcessInstances', + queryBody, + whereClause, + pagination, + createdFilter, + ); + expect(buildGraphQlQuerySpy).toHaveBeenCalledTimes(1); expect(buildGraphQlQuerySpy).toHaveBeenCalledWith({ type: 'ProcessInstances', queryBody, whereClause, pagination, + filterCondition: createdFilter, }); - expect(buildFilterConditionSpy).toHaveBeenCalledTimes(1); - expect(buildFilterConditionSpy).toHaveBeenCalledWith( - mockProcessInstanceIntrospection, - 'ProcessInstance', - logicalFilter, + + const params = buildGrahQLQueryUtils.buildQueryParamVariable( + pagination, + createdFilter, ); + expect(mockClient.query).toHaveBeenCalledTimes(2); expect(mockClient.query).toHaveBeenCalledWith( buildGrahQLQueryUtils.buildGraphQlQuery(expectedQueryArgs), - {}, + params, ); expect(result).toBeDefined(); expect(result).toStrictEqual(mockQueryResult.ProcessInstances); diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts index 22231c52a5..c935bc4ce2 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/service/DataIndexService.ts @@ -31,7 +31,10 @@ import { import { ErrorBuilder } from '../helpers/errorBuilder'; import { buildFilterCondition } from '../helpers/filterBuilder'; -import { buildGraphQlQuery } from '../helpers/queryBuilder'; +import { + buildGraphQlQuery, + buildQueryParamVariable, +} from '../helpers/queryBuilder'; import { Pagination } from '../types/pagination'; import { FETCH_PROCESS_INSTANCES_SORT_FIELD } from './constants'; @@ -132,9 +135,23 @@ export class DataIndexService { public async fetchWorkflowInfo( definitionId: string, ): Promise { - const graphQlQuery = `{ ProcessDefinitions ( where: {id: {equal: "${definitionId}" } } ) { id, name, version, type, endpoint, serviceUrl, source } }`; + const graphQlQuery = gql` + query FindProcessInstanceQuery($definitionId: String!) { + ProcessDefinitions(where: { id: { equal: $definitionId } }) { + id + name + version + type + endpoint + serviceUrl + source + } + } + `; - const result = await this.client.query(graphQlQuery, {}); + const result = await this.client.query(graphQlQuery, { + definitionId, + }); this.logger.debug( `Get workflow definition result: ${JSON.stringify(result)}`, @@ -225,9 +242,9 @@ export class DataIndexService { let whereClause: string | undefined; if (definitionIdsCondition && filterCondition) { - whereClause = `and: [{${definitionIdsCondition}}, {${filterCondition}}]`; + whereClause = `and: [{${definitionIdsCondition}}, {${filterCondition?.clause}}]`; } else if (definitionIdsCondition || filterCondition) { - whereClause = definitionIdsCondition ?? filterCondition; + whereClause = definitionIdsCondition ?? filterCondition?.clause; } else { whereClause = undefined; } @@ -238,9 +255,15 @@ export class DataIndexService { 'id, name, version, type, endpoint, serviceUrl, source, metadata', whereClause, pagination, + filterCondition: filterCondition, }); this.logger.debug(`GraphQL query: ${graphQlQuery}`); - const result = await this.client.query(graphQlQuery, {}); + + const paramVariables = buildQueryParamVariable(pagination, filterCondition); + + this.logger.debug(`GraphQL query params:`, paramVariables); + + const result = await this.client.query(graphQlQuery, paramVariables); this.logger.debug( `Get workflow definitions result: ${JSON.stringify(result)}`, ); @@ -273,7 +296,7 @@ export class DataIndexService { type, filter, ) - : ''; + : undefined; let whereClause = ''; const conditions = []; @@ -286,8 +309,8 @@ export class DataIndexService { conditions.push(`{${definitionIdsCondition}}`); } - if (filter) { - conditions.push(`{${filterCondition}}`); + if (filterCondition) { + conditions.push(`{${filterCondition?.clause}}`); } if (conditions.length === 0) { @@ -304,13 +327,18 @@ export class DataIndexService { 'id, processName, processId, state, start, end, nodes { id }, variables, executionSummary, parentProcessInstance {id, processName, businessKey}', whereClause, pagination, + filterCondition: filterCondition, }); this.logger.debug(`GraphQL query: ${graphQlQuery}`); + const paramVariables = buildQueryParamVariable(pagination, filterCondition); + + this.logger.debug(`GraphQL query params:`, paramVariables); + const result = await this.client.query<{ ProcessInstances: ProcessInstance[]; - }>(graphQlQuery, {}); + }>(graphQlQuery, paramVariables); this.logger.debug( `Fetch process instances result: ${JSON.stringify(result)}`, ); @@ -350,7 +378,7 @@ export class DataIndexService { targetEntityFilter, ); - const whereClause = `and: [{${processIdNotNullCondition}}, {${filterCondition}}]`; + const whereClause = `and: [{${processIdNotNullCondition}}, {${filterCondition.clause}}]`; // Apply a limit to prevent memory exhaustion and network timeouts when entities // have thousands of process instances. Entities with more instances than this limit @@ -365,13 +393,18 @@ export class DataIndexService { queryBody: 'processId', whereClause, pagination, + filterCondition, }); this.logger.debug(`GraphQL query: ${graphQlQuery}`); + const paramVariables = buildQueryParamVariable(pagination, filterCondition); + + this.logger.debug(`GraphQL query params:`, paramVariables); + const result = await this.client.query<{ ProcessInstances: ProcessInstance[]; - }>(graphQlQuery, {}); + }>(graphQlQuery, paramVariables); this.logger.debug( `Fetch definition ids from instances history result: ${JSON.stringify(result)}`, ); @@ -404,9 +437,18 @@ export class DataIndexService { public async fetchWorkflowSource( definitionId: string, ): Promise { - const graphQlQuery = `{ ProcessDefinitions ( where: {id: {equal: "${definitionId}" } } ) { id, source } }`; + const graphQlQuery = gql` + query FindProcessInstanceQuery($definitionId: String!) { + ProcessDefinitions(where: { id: { equal: $definitionId } }) { + id + source + } + } + `; - const result = await this.client.query(graphQlQuery, {}); + const result = await this.client.query(graphQlQuery, { + definitionId, + }); this.logger.debug( `Fetch workflow source result: ${JSON.stringify(result)}`, @@ -434,12 +476,34 @@ export class DataIndexService { targetEntity?: string; }): Promise { const targetEntityWhereCondition = args.targetEntity - ? `, variables: {targetEntity: {equal: "${args.targetEntity}" } }` + ? `, variables: {targetEntity: {equal: $targetEntity } }` : ''; - const graphQlQuery = `{ ProcessInstances( where: {processId: {equal: "${args.definitionId}" } ${targetEntityWhereCondition} }, orderBy: {start:DESC}, pagination: {limit: ${args.limit}, offset: ${args.offset}}) { id, processName, state, start, end } }`; - - const result = await this.client.query(graphQlQuery, {}); + const graphQlQuery = gql` + query FindProcessInstanceQuery($definitionId: String!, $limit: Int!, $offset: Int! ${args.targetEntity ? `, $targetEntity: String` : ''}) { + ProcessInstances( + where: {processId: {equal: $definitionId } ${targetEntityWhereCondition} }, + orderBy: {start:DESC}, + pagination: {limit: $limit, offset: $offset} + ) { id, processName, state, start, end } + } + `; + + const result = await this.client.query( + graphQlQuery, + args.targetEntity + ? { + definitionId: args.definitionId, + limit: args.limit, + offset: args.offset, + targetEntity: args.targetEntity, + } + : { + definitionId: args.definitionId, + limit: args.limit, + offset: args.offset, + }, + ); this.logger.debug( `Fetch workflow instances result: ${JSON.stringify(result)}`, @@ -456,9 +520,17 @@ export class DataIndexService { public async fetchInstanceVariables( instanceId: string, ): Promise { - const graphQlQuery = `{ ProcessInstances (where: { id: {equal: "${instanceId}" } } ) { variables } }`; + const graphQlQuery = gql` + query FindProcessInstanceQuery($instanceId: String!) { + ProcessInstances(where: { id: { equal: $instanceId } }) { + variables + } + } + `; - const result = await this.client.query(graphQlQuery, {}); + const result = await this.client.query(graphQlQuery, { + instanceId, + }); this.logger.debug( `Fetch process instance variables result: ${JSON.stringify(result)}`, @@ -481,9 +553,17 @@ export class DataIndexService { public async fetchDefinitionIdByInstanceId( instanceId: string, ): Promise { - const graphQlQuery = `{ ProcessInstances (where: { id: {equal: "${instanceId}" } } ) { processId } }`; + const graphQlQuery = gql` + query FindProcessInstanceQuery($instanceId: String!) { + ProcessInstances(where: { id: { equal: $instanceId } }) { + processId + } + } + `; - const result = await this.client.query(graphQlQuery, {}); + const result = await this.client.query(graphQlQuery, { + instanceId, + }); this.logger.debug( `Fetch process id from instance result: ${JSON.stringify(result)}`, diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/types/filterClause.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/types/filterClause.ts new file mode 100644 index 0000000000..a06183ebaa --- /dev/null +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/types/filterClause.ts @@ -0,0 +1,26 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export interface FilterClauseVariable { + clauseVariableName: string; + formattedValue: string | boolean | string[]; + clauseVariableType: string; +} + +export interface FilterClause { + clauseVariable: Array; + clause: string; +} diff --git a/workspaces/orchestrator/plugins/orchestrator-backend/src/types/pagination.ts b/workspaces/orchestrator/plugins/orchestrator-backend/src/types/pagination.ts index b60ab673cf..f77358bd05 100644 --- a/workspaces/orchestrator/plugins/orchestrator-backend/src/types/pagination.ts +++ b/workspaces/orchestrator/plugins/orchestrator-backend/src/types/pagination.ts @@ -1,5 +1,5 @@ /* - * Copyright 2024 The Backstage Authors + * Copyright Red Hat, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Request } from 'express-serve-static-core'; import { PaginationInfoDTO } from '@red-hat-developer-hub/backstage-plugin-orchestrator-common'; @@ -24,6 +25,11 @@ export interface Pagination { sortField?: string; } +export interface PaginationQueryVariable { + offset?: number; + limit?: number; +} + export function buildPagination(req: Request): Pagination { const pagination: Pagination = { limit: undefined, diff --git a/workspaces/orchestrator/yarn.lock b/workspaces/orchestrator/yarn.lock index ebd60e383e..424f0a3343 100644 --- a/workspaces/orchestrator/yarn.lock +++ b/workspaces/orchestrator/yarn.lock @@ -5,15 +5,15 @@ __metadata: version: 6 cacheKey: 8 -"@0no-co/graphql.web@npm:^1.0.1": - version: 1.0.9 - resolution: "@0no-co/graphql.web@npm:1.0.9" +"@0no-co/graphql.web@npm:^1.0.13": + version: 1.2.0 + resolution: "@0no-co/graphql.web@npm:1.2.0" peerDependencies: graphql: ^14.0.0 || ^15.0.0 || ^16.0.0 peerDependenciesMeta: graphql: optional: true - checksum: a7580304d3b545b69e11e4a7b95d0c81b5fee3d071f92b1386f0f2e4e4f69703cbe19e0a4f5c489cbb70e7b5ac1ccec3a51f529c85dfbbccf8ebd6370fd6b0e2 + checksum: 4d5a54b93e6024b7d476e94b991e4e4ebc4ecb97e4ce886f76889741f5e419b587bedc6a00488753069534d8ae3e4de2e901ad58506ba2f74eeb8642edccc4ca languageName: node linkType: hard @@ -12457,7 +12457,7 @@ __metadata: "@types/fs-extra": 11.0.4 "@types/json-schema": 7.0.15 "@types/luxon": ^3.7.1 - "@urql/core": ^4.1.4 + "@urql/core": ^6.0.1 ajv-formats: ^2.1.1 cloudevents: ^8.0.0 express: ^4.21.2 @@ -16563,13 +16563,13 @@ __metadata: languageName: node linkType: hard -"@urql/core@npm:^4.1.4": - version: 4.3.0 - resolution: "@urql/core@npm:4.3.0" +"@urql/core@npm:^6.0.1": + version: 6.0.1 + resolution: "@urql/core@npm:6.0.1" dependencies: - "@0no-co/graphql.web": ^1.0.1 - wonka: ^6.3.2 - checksum: 706e4f28390a1ee441588bfb2ab1517e620c2b63ed26756e8e5dd808acb6c871b1d0d760d569b162af773bff05df3d90fed753963ce70806fa8ca1f7a37bb7e9 + "@0no-co/graphql.web": "npm:^1.0.13" + wonka: "npm:^6.3.2" + checksum: 6cedd18dc9f386d361991c6cbde61dc45c1cac9dfa9db60274fdd9acec91185d176a5c63bb39593152490f539441e226db32bdf88cfba0af4488d90399e58482 languageName: node linkType: hard