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(orchestrator): support pagination for /instances and /overview #1313

Merged
merged 2 commits into from
Mar 7, 2024
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
34 changes: 34 additions & 0 deletions plugins/orchestrator-backend/src/helpers/queryBuilder.ts
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;
}
58 changes: 58 additions & 0 deletions plugins/orchestrator-backend/src/pagination.test.ts
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,
});
});
});
49 changes: 49 additions & 0 deletions plugins/orchestrator-backend/src/queryBuilder.test.ts
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);
});
});
65 changes: 45 additions & 20 deletions plugins/orchestrator-backend/src/service/DataIndexService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)}`,
Expand All @@ -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> {
pagination.sortField ??= FETCH_PROCESS_INSTANCES_SORT_FIELD;

const graphQlQuery = buildGraphQlQuery({
type: 'ProcessInstances',
queryBody:
Copy link
Contributor

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

Copy link
Contributor

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

'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(
Expand All @@ -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[];
Copy link
Contributor

Choose a reason for hiding this comment

The 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?
I'm concerned this is making things slower... the whole idea of pagination is to make things faster

Copy link
Contributor

Choose a reason for hiding this comment

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

@caponetto
WDYT

Copy link
Contributor Author

@JudeNiroshan JudeNiroshan Mar 6, 2024

Choose a reason for hiding this comment

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

why not just do the pagination logic here?

The only reason I thought was the data payload retrieved from the graphQL is smaller compared to retrieving the full query with attributes.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you please check what the difference in payload is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 totalCount as a result output[1] in the graphQL, we can remove this method.

[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.

  • GraphQL query without pagination params(returns all instance data) - 2.76KB
  • GraphQL query only to fetch all IDs - 450B


return Promise.resolve(idArr.length);
}

private async getWorkflowDefinitionFromInstance(instance: ProcessInstance) {
const workflowInfo = await this.getWorkflowDefinition(instance.processId);
if (!workflowInfo?.source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { spawn } from 'child_process';
import { join, resolve } from 'path';

import { Pagination } from '../types/pagination';
import { DataIndexService } from './DataIndexService';
import { executeWithRetry } from './Helper';

Expand Down Expand Up @@ -143,11 +144,11 @@ export class SonataFlowService {
return undefined;
}

public async fetchWorkflowOverviews(): Promise<
WorkflowOverview[] | undefined
> {
public async fetchWorkflowOverviews(
pagination: Pagination,
): Promise<WorkflowOverview[] | undefined> {
try {
const workflowInfos = await this.dataIndex.getWorkflowInfos();
const workflowInfos = await this.dataIndex.getWorkflowInfos(pagination);
if (!workflowInfos?.length) {
return [];
}
Expand Down
8 changes: 6 additions & 2 deletions plugins/orchestrator-backend/src/service/api/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
WorkflowOverviewListResult,
} from '@janus-idp/backstage-plugin-orchestrator-common';

import { Pagination } from '../../types/pagination';
import { DataIndexService } from '../DataIndexService';
import { retryAsyncFunction } from '../Helper';
import { SonataFlowService } from '../SonataFlowService';
Expand All @@ -20,8 +21,10 @@ const FETCH_INSTANCE_RETRY_DELAY_MS = 1000;
export namespace V1 {
export async function getWorkflowsOverview(
sonataFlowService: SonataFlowService,
pagination: Pagination,
): Promise<WorkflowOverviewListResult> {
const overviews = await sonataFlowService.fetchWorkflowOverviews();
const overviews =
await sonataFlowService.fetchWorkflowOverviews(pagination);
if (!overviews) {
throw new Error("Couldn't fetch workflow overviews");
}
Expand Down Expand Up @@ -76,8 +79,9 @@ export namespace V1 {

export async function getInstances(
dataIndexService: DataIndexService,
pagination: Pagination,
): Promise<ProcessInstance[]> {
const instances = await dataIndexService.fetchProcessInstances();
const instances = await dataIndexService.fetchProcessInstances(pagination);

if (!instances) {
throw new Error("Couldn't fetch process instances");
Expand Down
Loading
Loading