Skip to content

Commit

Permalink
feat(orchestrator): use v2 endpoint fot getWorkflowSource (#2031)
Browse files Browse the repository at this point in the history
* rm v1 endpoint for get workflow source by id

Signed-off-by: Gloria Ciavarrini <gciavarrini@redhat.com>

* use v2 endpoint for getWorkflowSource

Signed-off-by: Gloria Ciavarrini <gciavarrini@redhat.com>

* fix axios response type

Signed-off-by: Gloria Ciavarrini <gciavarrini@redhat.com>

---------

Signed-off-by: Gloria Ciavarrini <gciavarrini@redhat.com>
  • Loading branch information
gciavarrini authored Sep 3, 2024
1 parent cc29934 commit 46db5ff
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 89 deletions.
13 changes: 0 additions & 13 deletions plugins/orchestrator-backend/src/service/api/v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,6 @@ export class V1 {
return definition;
}

public async getWorkflowSourceById(definitionId: string): Promise<string> {
const source = await this.orchestratorService.fetchWorkflowSource({
definitionId,
cacheHandler: 'throw',
});

if (!source) {
throw new Error(`Couldn't fetch workflow source for ${definitionId}`);
}

return source;
}

public async retriggerInstanceInError(
instanceId: string,
inputData: ProcessInstanceVariables,
Expand Down
3 changes: 1 addition & 2 deletions plugins/orchestrator-backend/src/service/api/v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
generateTestWorkflowOverviewList,
generateWorkflowDefinition,
} from './test-utils';
import { V1 } from './v1';
import { V2 } from './v2';

jest.mock('../Helper.ts', () => ({
Expand Down Expand Up @@ -56,7 +55,7 @@ const createMockOrchestratorService = (): OrchestratorService => {
return mockOrchestratorService;
};
const mockOrchestratorService = createMockOrchestratorService();
const v2 = new V2(mockOrchestratorService, new V1(mockOrchestratorService));
const v2 = new V2(mockOrchestratorService);

describe('getWorkflowOverview', () => {
beforeEach(() => {
Expand Down
20 changes: 12 additions & 8 deletions plugins/orchestrator-backend/src/service/api/v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,12 @@ import {
mapToWorkflowOverviewDTO,
mapToWorkflowRunStatusDTO,
} from './mapping/V2Mappings';
import { V1 } from './v1';

const FETCH_INSTANCE_MAX_ATTEMPTS = 10;
const FETCH_INSTANCE_RETRY_DELAY_MS = 1000;

export class V2 {
constructor(
private readonly orchestratorService: OrchestratorService,
private readonly v1: V1,
) {}
constructor(private readonly orchestratorService: OrchestratorService) {}

public async getWorkflowsOverview(
pagination: Pagination,
Expand Down Expand Up @@ -74,13 +70,21 @@ export class V2 {
}

public async getWorkflowById(workflowId: string): Promise<WorkflowDTO> {
const resultV1 = await this.v1.getWorkflowSourceById(workflowId);
const resultV1 = await this.getWorkflowSourceById(workflowId);
return mapToWorkflowDTO(resultV1);
}

public async getWorkflowSourceById(workflowId: string): Promise<string> {
const resultV1 = await this.v1.getWorkflowSourceById(workflowId);
return resultV1;
const source = await this.orchestratorService.fetchWorkflowSource({
definitionId: workflowId,
cacheHandler: 'throw',
});

if (!source) {
throw new Error(`Couldn't fetch workflow source for ${workflowId}`);
}

return source;
}

public async getInstances(
Expand Down
44 changes: 3 additions & 41 deletions plugins/orchestrator-backend/src/service/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ async function initRouterApi(
});
await openApiBackend.init();
const v1 = new V1(orchestratorService);
const v2 = new V2(orchestratorService, v1);
const v2 = new V2(orchestratorService);
return { v1, v2, openApiBackend };
}

Expand Down Expand Up @@ -324,44 +324,6 @@ function setupInternalRoutes(
},
);

// v1
router.get('/workflows/:workflowId/source', async (req, res) => {
const {
params: { workflowId },
} = req;
const endpointName = 'WorkflowsWorkflowIdSource';
const endpoint = `/v1/workflows/${workflowId}/source`;

auditLogger.auditLog({
eventName: endpointName,
stage: 'start',
status: 'succeeded',
level: 'debug',
request: req,
message: `Received request to '${endpoint}' endpoint`,
});

const decision = await authorize(
req,
orchestratorWorkflowReadPermission,
permissions,
httpAuth,
);
if (decision.result === AuthorizeResult.DENY) {
manageDenyAuthorization(endpointName, endpoint, req);
}

try {
const result = await routerApi.v1.getWorkflowSourceById(workflowId);
res.status(200).contentType('text/plain').send(result);
} catch (error) {
res
.status(500)
.contentType('text/plain')
.send((error as Error)?.message || INTERNAL_SERVER_ERROR_MESSAGE);
}
});

// v2
routerApi.openApiBackend.register(
'getWorkflowSourceById',
Expand Down Expand Up @@ -391,12 +353,12 @@ function setupInternalRoutes(

try {
const result = await routerApi.v2.getWorkflowSourceById(workflowId);
res.status(200).contentType('plain/text').send(result);
res.status(200).contentType('text/plain').send(result);
} catch (error) {
auditLogRequestError(error, endpointName, endpoint, _req);
res
.status(500)
.contentType('plain/text')
.contentType('text/plain')
.send((error as Error)?.message || INTERNAL_SERVER_ERROR_MESSAGE);
next();
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/orchestrator/src/api/MockOrchestratorClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class MockOrchestratorClient implements OrchestratorApi {
return Promise.resolve(this._mockData.listInstancesResponse);
}

getWorkflowSource(_workflowId: string): Promise<string> {
getWorkflowSource(_workflowId: string): Promise<AxiosResponse<string>> {
if (
!hasOwnProp(this._mockData, 'getWorkflowSourceResponse') ||
!isNonNullable(this._mockData.getWorkflowSourceResponse)
Expand Down
51 changes: 35 additions & 16 deletions plugins/orchestrator/src/api/OrchestratorClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,35 +270,54 @@ describe('OrchestratorClient', () => {
// Given
const workflowId = 'workflow123';
const mockWorkflowSource = 'test workflow source';
const responseConfigOptions = getDefaultTestRequestConfig();
responseConfigOptions.responseType = 'text';
const mockResponse: AxiosResponse<string> = {
data: mockWorkflowSource,
status: 200,
statusText: 'OK',
headers: {} as RawAxiosResponseHeaders,
config: {} as InternalAxiosRequestConfig,
};
// Mock axios request to simulate a successful response
jest.spyOn(axios, 'request').mockResolvedValueOnce(mockResponse);

// Mock fetch to simulate a successful response
(global.fetch as jest.Mock).mockResolvedValueOnce({
ok: true,
text: jest.fn().mockResolvedValue(mockWorkflowSource),
});
// Spy DefaultApi
const getSourceSpy = jest.spyOn(
DefaultApi.prototype,
'getWorkflowSourceById',
);

// When
const result = await orchestratorClient.getWorkflowSource(workflowId);

// Then
expect(fetch).toHaveBeenCalledWith(
`${baseUrl}/workflows/${workflowId}/source`,
{ headers: defaultAuthHeaders },
expect(result).toBeDefined();
expect(result.data).toEqual(mockWorkflowSource);
expect(axios.request).toHaveBeenCalledTimes(1);
expect(axios.request).toHaveBeenCalledWith({
...getAxiosTestRequest(`/v2/workflows/${workflowId}/source`),
method: 'GET',
headers: {
...defaultAuthHeaders,
},
responseType: 'text',
});
expect(getSourceSpy).toHaveBeenCalledTimes(1);
expect(getSourceSpy).toHaveBeenCalledWith(
workflowId,
responseConfigOptions,
);
expect(result).toEqual(mockWorkflowSource);
});

it('should throw a ResponseError when fetching the workflow source fails', async () => {
// Given
const workflowId = 'workflow123';

// Mock fetch to simulate a failed response
(global.fetch as jest.Mock).mockResolvedValueOnce({
ok: false,
status: 404,
statusText: 'Not Found',
});

// Mock fetch to simulate a failure
axios.request = jest
.fn()
.mockRejectedValueOnce(new Error('Simulated error'));
// When
const promise = orchestratorClient.getWorkflowSource(workflowId);

Expand Down
11 changes: 6 additions & 5 deletions plugins/orchestrator/src/api/OrchestratorClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,12 @@ export class OrchestratorClient implements OrchestratorApi {
);
}

async getWorkflowSource(workflowId: string): Promise<string> {
const baseUrl = await this.getBaseUrl();
return await this.fetcher(`${baseUrl}/workflows/${workflowId}/source`).then(
r => r.text(),
);
async getWorkflowSource(workflowId: string): Promise<AxiosResponse<string>> {
const defaultApi = await this.getDefaultAPI();
const reqConfigOption: AxiosRequestConfig =
await this.getDefaultReqConfig();
reqConfigOption.responseType = 'text';
return await defaultApi.getWorkflowSourceById(workflowId, reqConfigOption);
}

async listWorkflowOverviews(
Expand Down
2 changes: 1 addition & 1 deletion plugins/orchestrator/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface OrchestratorApi {

getWorkflowDefinition(workflowId: string): Promise<WorkflowDefinition>;

getWorkflowSource(workflowId: string): Promise<string>;
getWorkflowSource(workflowId: string): Promise<AxiosResponse<string>>;

getInstance(
instanceId: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,10 @@ const RefForwardingWorkflowEditor: ForwardRefRenderFunction<
if (canceled.get()) {
return;
}
const definition = fromWorkflowSource(source);
const definition = fromWorkflowSource(source.data);
setWorkflowDefinitionPromise({ data: definition });

const workflowFormat = extractWorkflowFormat(source);
const workflowFormat = extractWorkflowFormat(source.data);

if (format && workflowId && format !== workflowFormat) {
const link = viewWorkflowLink({
Expand Down

0 comments on commit 46db5ff

Please sign in to comment.