-
Notifications
You must be signed in to change notification settings - Fork 147
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(orchestrator): support pagination for /instances and /overview #1313
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { Pagination } from '../types/pagination'; | ||
|
||
export function buildGraphQlQuery(args: { | ||
type: 'ProcessDefinitions' | 'ProcessInstances' | 'Jobs'; | ||
queryBody: string; | ||
whereClause?: string; | ||
pagination?: Pagination; | ||
}): string { | ||
let query = `{${args.type}`; | ||
|
||
if (args.whereClause || args.pagination) { | ||
query += ` (`; | ||
|
||
if (args.whereClause) { | ||
query += `where: {${args.whereClause}}`; | ||
if (args.pagination) { | ||
query += `, `; | ||
} | ||
} | ||
if (args.pagination) { | ||
if (args.pagination.sortField) { | ||
query += `orderBy: {${ | ||
args.pagination.sortField | ||
}: ${args.pagination.order?.toUpperCase()}}, `; | ||
} | ||
query += `pagination: {limit: ${args.pagination.limit} , offset: ${args.pagination.offset}}`; | ||
} | ||
|
||
query += `) `; | ||
} | ||
query += ` {${args.queryBody} } }`; | ||
|
||
return query; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import { buildPagination } from './types/pagination'; | ||
|
||
describe('buildPagination()', () => { | ||
it('should build the correct pagination obj when no query parameters are passed', () => { | ||
const mockRequest: any = { | ||
query: {}, | ||
}; | ||
expect(buildPagination(mockRequest)).toEqual({ | ||
limit: 10, | ||
offset: 0, | ||
order: 'ASC', | ||
sortField: undefined, | ||
}); | ||
}); | ||
it('should build the correct pagination obj when partial query parameters are passed', () => { | ||
const mockRequest: any = { | ||
query: { | ||
orderBy: 'lastUpdated', | ||
}, | ||
}; | ||
expect(buildPagination(mockRequest)).toEqual({ | ||
limit: 10, | ||
offset: 0, | ||
order: 'ASC', | ||
sortField: 'lastUpdated', | ||
}); | ||
}); | ||
it('should build the correct pagination obj when all query parameters are passed', () => { | ||
const mockRequest: any = { | ||
query: { | ||
page: 1, | ||
pageSize: 50, | ||
orderBy: 'lastUpdated', | ||
orderDirection: 'DESC', | ||
}, | ||
}; | ||
expect(buildPagination(mockRequest)).toEqual({ | ||
limit: 50, | ||
offset: 1, | ||
order: 'DESC', | ||
sortField: 'lastUpdated', | ||
}); | ||
}); | ||
it('should build the correct pagination obj when non numeric value passed to number fields', () => { | ||
const mockRequest: any = { | ||
query: { | ||
page: 'abc', | ||
pageSize: 'cde', | ||
}, | ||
}; | ||
expect(buildPagination(mockRequest)).toEqual({ | ||
limit: 10, | ||
offset: 0, | ||
order: 'ASC', | ||
sortField: undefined, | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { buildGraphQlQuery } from './helpers/queryBuilder'; | ||
import { Pagination } from './types/pagination'; | ||
|
||
describe('GraphQL query builder', () => { | ||
it('should return properly formatted graphQL query when where clause and pagination are present', () => { | ||
const expectedQuery: string = | ||
'{ProcessInstances (where: {processId: {isNull: false}}, orderBy: {lastUpdate: DESC}, pagination: {limit: 5 , offset: 2}) {id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey} } }'; | ||
const pagination: Pagination = { | ||
offset: 2, | ||
limit: 5, | ||
order: 'DESC', | ||
sortField: 'lastUpdate', | ||
}; | ||
expect( | ||
buildGraphQlQuery({ | ||
type: 'ProcessInstances', | ||
queryBody: | ||
'id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey}', | ||
whereClause: 'processId: {isNull: false}', | ||
pagination, | ||
}), | ||
).toEqual(expectedQuery); | ||
}); | ||
|
||
it('should return properly formatted graphQL query when where clause is present', () => { | ||
const expectedQuery: string = | ||
'{ProcessInstances (where: {processId: {isNull: false}}) {id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey} } }'; | ||
expect( | ||
buildGraphQlQuery({ | ||
type: 'ProcessInstances', | ||
queryBody: | ||
'id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey}', | ||
whereClause: 'processId: {isNull: false}', | ||
}), | ||
).toEqual(expectedQuery); | ||
}); | ||
|
||
it('should return properly formatted graphQL query when where clause is NOT present', () => { | ||
const expectedQuery: string = | ||
'{ProcessInstances {id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey} } }'; | ||
expect( | ||
buildGraphQlQuery({ | ||
type: 'ProcessInstances', | ||
queryBody: | ||
'id, processName, processId, state, start, lastUpdate, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey}', | ||
}), | ||
).toEqual(expectedQuery); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ import { | |
} from '@janus-idp/backstage-plugin-orchestrator-common'; | ||
|
||
import { ErrorBuilder } from '../helpers/errorBuilder'; | ||
import { buildGraphQlQuery } from '../helpers/queryBuilder'; | ||
import { Pagination } from '../types/pagination'; | ||
import { FETCH_PROCESS_INSTANCES_SORT_FIELD } from './constants'; | ||
|
||
export class DataIndexService { | ||
private client: Client; | ||
|
@@ -89,23 +92,18 @@ export class DataIndexService { | |
return processDefinitions[0]; | ||
} | ||
|
||
public async getWorkflowInfos(): Promise<WorkflowInfo[]> { | ||
const QUERY = ` | ||
query ProcessDefinitions { | ||
ProcessDefinitions { | ||
id | ||
name | ||
version | ||
type | ||
endpoint | ||
serviceUrl | ||
source | ||
} | ||
} | ||
`; | ||
|
||
public async getWorkflowInfos( | ||
pagination?: Pagination, | ||
): Promise<WorkflowInfo[]> { | ||
this.logger.info(`getWorkflowInfos() called: ${this.dataIndexUrl}`); | ||
const result = await this.client.query(QUERY, {}); | ||
|
||
const graphQlQuery = buildGraphQlQuery({ | ||
type: 'ProcessDefinitions', | ||
queryBody: 'id, name, version, type, endpoint, serviceUrl, source', | ||
pagination, | ||
}); | ||
this.logger.debug(`GraphQL query: ${graphQlQuery}`); | ||
const result = await this.client.query(graphQlQuery, {}); | ||
|
||
this.logger.debug( | ||
`Get workflow definitions result: ${JSON.stringify(result)}`, | ||
|
@@ -121,10 +119,19 @@ export class DataIndexService { | |
return result.data.ProcessDefinitions; | ||
} | ||
|
||
public async fetchProcessInstances(): Promise<ProcessInstance[] | undefined> { | ||
const graphQlQuery = | ||
'{ ProcessInstances ( orderBy: { start: ASC }, where: {processId: {isNull: false} } ) { id, processName, processId, businessKey, state, start, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey} } }'; | ||
|
||
public async fetchProcessInstances( | ||
pagination?: Pagination, | ||
): Promise<ProcessInstance[] | undefined> { | ||
if (pagination) pagination.sortField ??= FETCH_PROCESS_INSTANCES_SORT_FIELD; | ||
|
||
const graphQlQuery = buildGraphQlQuery({ | ||
type: 'ProcessInstances', | ||
queryBody: | ||
'id, processName, processId, businessKey, state, start, end, nodes { id }, variables, parentProcessInstance {id, processName, businessKey}', | ||
whereClause: 'processId: {isNull: false}', | ||
pagination, | ||
}); | ||
this.logger.debug(`GraphQL query: ${graphQlQuery}`); | ||
const result = await this.client.query(graphQlQuery, {}); | ||
|
||
this.logger.debug( | ||
|
@@ -147,6 +154,24 @@ export class DataIndexService { | |
return processInstances; | ||
} | ||
|
||
public async getProcessInstancesTotalCount(): Promise<number> { | ||
const graphQlQuery = buildGraphQlQuery({ | ||
type: 'ProcessInstances', | ||
queryBody: 'id', | ||
}); | ||
this.logger.debug(`GraphQL query: ${graphQlQuery}`); | ||
const result = await this.client.query(graphQlQuery, {}); | ||
|
||
if (result.error) { | ||
this.logger.error(`Error when fetching instances: ${result.error}`); | ||
throw result.error; | ||
} | ||
|
||
const idArr = result.data.ProcessInstances as ProcessInstance[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're already getting all of the instances why not just do the pagination logic here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @caponetto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The only reason I thought was the data payload retrieved from the graphQL is smaller compared to retrieving the full query with attributes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you please check what the difference in payload is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It depends on the number of all instances and the process definition structure. When the number of nodes is high, the instance will have a larger JSON output. When the result payload is larger, it can lead to graphQL query complexity. It will result in a runtime error. However, if we have the [1] apache/incubator-kie-kogito-apps#2012 Here is a simple comparison which I did with the default orchestrator container we use in our dev setup. I have mostly executed the Hello world workflow. I have 12 workflow instances.
|
||
|
||
return Promise.resolve(idArr.length); | ||
} | ||
|
||
private async getWorkflowDefinitionFromInstance(instance: ProcessInstance) { | ||
const workflowInfo = await this.getWorkflowDefinition(instance.processId); | ||
if (!workflowInfo?.source) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC the behavior in v1 in which by default it's sorted asc by start won't be here anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it's fine because the frontend overrides it anyhow to be desc