Skip to content

Commit c13964c

Browse files
felix-oqfelix-oq
authored
Fix faulty mocking in provider-side CDC-Test of Pipeline (#387)
* Fix update and delete interactions to handle absent ids Previously, the provider-side mocking did not mimic the actual behaviour for update and delete correctly. * Fix mocking in unit test of pipeline config manager * Fix integration test to handle changed interactions * Fix system test to handle changed interactions * Change status code to 404 for deleting absent pipelines
1 parent 86736d2 commit c13964c

16 files changed

+222
-101
lines changed

‎pipeline/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ Use `npm test` to run the unit tests. There is also `nrm run watch-test` availab
3737
| *base_url*/configs | GET | - | PipelineConfig[] | Get all pipeline configs |
3838
| *base_url*/configs/:id | GET | - | PipelineConfig | Get pipeline config by id |
3939
| *base_url*/configs | POST | PipelineConfigDTO | PipelineConfig | Create a pipeline config |
40-
| *base_url*/configs/:id | PUT | PipelineConfigDTO | text | Update a pipeline config |
41-
| *base_url*/configs/:id | DELETE | - | text | Delete a pipeline config by id |
40+
| *base_url*/configs/:id | PUT | PipelineConfigDTO | PipelineConfig | Update a pipeline config |
41+
| *base_url*/configs/:id | DELETE | - | PipelineConfig | Delete a pipeline config by id |
4242
| *base_url*/configs | DELETE | - | text | Delete all pipeline configs |
4343

4444
### PipelineExecutionRequest

‎pipeline/integration-test/src/pipeline-config.test.js

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,14 @@ describe('Pipeline Config Test', () => {
7979
.delete('/configs/' + configId)
8080
.send()
8181

82-
expect(delResponse.status).toEqual(204)
82+
expect(delResponse.status).toEqual(200)
83+
expect(delResponse.body).toEqual(
84+
expect.objectContaining({
85+
...pipelineConfig,
86+
id: configId,
87+
metadata: expect.objectContaining(pipelineConfig.metadata)
88+
})
89+
)
8390

8491
await sleep(PUBLICATION_WAIT_TIME_MS)
8592
expect(getPublishedEvent(AMQP_PIPELINE_CONFIG_CREATED_TOPIC)).toContainEqual({
@@ -108,7 +115,14 @@ describe('Pipeline Config Test', () => {
108115
.put('/configs/' + configId)
109116
.send(updatedConfig)
110117

111-
expect(putResponse.status).toEqual(204)
118+
expect(putResponse.status).toEqual(200)
119+
expect(putResponse.body).toEqual(
120+
expect.objectContaining({
121+
...updatedConfig,
122+
id: configId,
123+
metadata: expect.objectContaining(updatedConfig.metadata)
124+
})
125+
)
112126

113127
const updatedGetResponse = await request(URL)
114128
.get('/configs/' + configId)
@@ -122,7 +136,14 @@ describe('Pipeline Config Test', () => {
122136
.delete('/configs/' + configId)
123137
.send()
124138

125-
expect(delResponse.status).toEqual(204)
139+
expect(delResponse.status).toEqual(200)
140+
expect(delResponse.body).toEqual(
141+
expect.objectContaining({
142+
...updatedConfig,
143+
id: configId,
144+
metadata: expect.objectContaining(updatedConfig.metadata)
145+
})
146+
)
126147

127148
await sleep(PUBLICATION_WAIT_TIME_MS)
128149
expect(getPublishedEvent(AMQP_PIPELINE_CONFIG_CREATED_TOPIC)).toContainEqual({

‎pipeline/src/api/rest/pipelineConfigEndpoint.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@ export class PipelineConfigEndpoint {
2929
res.status(400).send('Path parameter id is missing or is incorrect');
3030
return;
3131
}
32-
await this.pipelineConfigManager.delete(configId);
33-
res.status(204).send();
32+
const deletedPipeline = await this.pipelineConfigManager.delete(configId);
33+
if (deletedPipeline === undefined) {
34+
res.status(404).send(`Could not find config with id ${configId}`);
35+
return;
36+
}
37+
res.status(200).json(deletedPipeline);
3438
};
3539

3640
deleteAll = async (
@@ -56,15 +60,16 @@ export class PipelineConfigEndpoint {
5660
return;
5761
}
5862
const config = req.body;
59-
try {
60-
await this.pipelineConfigManager.update(configId, config);
61-
} catch (e) {
62-
res
63-
.status(404)
64-
.send(`Could not find config with id ${configId}: ${String(e)}`);
63+
64+
const updatedPipeline = await this.pipelineConfigManager.update(
65+
configId,
66+
config,
67+
);
68+
if (updatedPipeline === undefined) {
69+
res.status(404).send(`Could not find config with id ${configId}`);
6570
return;
6671
}
67-
res.status(204).send();
72+
res.status(200).json(updatedPipeline);
6873
};
6974

7075
create = async (

‎pipeline/src/pipeline-config/pipelineConfigManager.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jest.mock('./pipelineConfigRepository', () => {
3737
get: jest.fn(),
3838
getAll: jest.fn().mockResolvedValue([generateConfig(), generateConfig()]),
3939
getByDatasourceId: jest.fn(),
40-
update: jest.fn(),
40+
update: jest.fn().mockResolvedValue(generateConfig()),
4141
deleteById: jest.fn().mockResolvedValue(generateConfig()),
4242
deleteAll: jest
4343
.fn()

‎pipeline/src/pipeline-config/pipelineConfigManager.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,28 +47,41 @@ export class PipelineConfigManager {
4747
);
4848
}
4949

50-
async update(id: number, config: PipelineConfigDTO): Promise<void> {
50+
async update(
51+
id: number,
52+
config: PipelineConfigDTO,
53+
): Promise<PipelineConfig | undefined> {
5154
return await this.pgClient.transaction(async (client) => {
52-
await PipelineConfigRepository.update(client, id, config);
53-
await EventPublisher.publishUpdate(
55+
const updatedPipeline = await PipelineConfigRepository.update(
5456
client,
5557
id,
56-
config.metadata.displayName,
58+
config,
5759
);
60+
if (updatedPipeline !== undefined) {
61+
await EventPublisher.publishUpdate(
62+
client,
63+
id,
64+
config.metadata.displayName,
65+
);
66+
}
67+
return updatedPipeline;
5868
});
5969
}
6070

61-
async delete(id: number): Promise<void> {
71+
async delete(id: number): Promise<PipelineConfig | undefined> {
6272
return await this.pgClient.transaction(async (client) => {
6373
const deletedPipeline = await PipelineConfigRepository.deleteById(
6474
client,
6575
id,
6676
);
67-
await EventPublisher.publishDeletion(
68-
client,
69-
id,
70-
deletedPipeline.metadata.displayName,
71-
);
77+
if (deletedPipeline !== undefined) {
78+
await EventPublisher.publishDeletion(
79+
client,
80+
id,
81+
deletedPipeline.metadata.displayName,
82+
);
83+
}
84+
return deletedPipeline;
7285
});
7386
}
7487

‎pipeline/src/pipeline-config/pipelineConfigRepository.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const INSERT_CONFIG_STATEMENT = `
2727
const UPDATE_CONFIG_STATEMENT = `
2828
UPDATE "${POSTGRES_SCHEMA}"."${CONFIG_TABLE_NAME}"
2929
SET "datasourceId"=$2, "func"=$3, "author"=$4, "displayName"=$5, "license"=$6, "description"=$7, "schema"=$8
30-
WHERE id=$1`;
30+
WHERE id=$1 RETURNING *`;
3131
const GET_CONFIG_STATEMENT = `
3232
SELECT * FROM "${POSTGRES_SCHEMA}"."${CONFIG_TABLE_NAME}" WHERE "id" = $1`;
3333
const GET_ALL_CONFIGS_STATEMENT = `
@@ -107,7 +107,7 @@ export async function update(
107107
client: ClientBase,
108108
id: number,
109109
config: PipelineConfigDTO,
110-
): Promise<void> {
110+
): Promise<PipelineConfig | undefined> {
111111
const values = [
112112
id,
113113
config.datasourceId,
@@ -119,20 +119,18 @@ export async function update(
119119
config.schema,
120120
];
121121
const result = await client.query(UPDATE_CONFIG_STATEMENT, values);
122-
if (result.rowCount === 0) {
123-
throw new Error(`Could not find config with ${id} to update`);
124-
}
122+
const content = toPipelineConfigs(result);
123+
// Returns undefined if the array is empty
124+
return content[0];
125125
}
126126

127127
export async function deleteById(
128128
client: ClientBase,
129129
id: number,
130-
): Promise<PipelineConfig> {
130+
): Promise<PipelineConfig | undefined> {
131131
const result = await client.query(DELETE_CONFIG_STATEMENT, [id]);
132-
if (result.rowCount === 0) {
133-
throw new Error(`Could not find config with ${id} to delete`);
134-
}
135132
const content = toPipelineConfigs(result);
133+
// Returns undefined if the array is empty
136134
return content[0];
137135
}
138136

‎pipeline/src/ui.provider.pact.test.ts

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,49 +20,72 @@ jest.mock('./pipeline-config/pipelineConfigManager', () => {
2020
return {
2121
getAll: jest.fn().mockResolvedValue(pipelineConfigs),
2222

23-
get: jest.fn().mockImplementation(async (id: number) => {
24-
const result = pipelineConfigs.find((config) => config.id === id);
25-
return Promise.resolve(result);
26-
}),
23+
get: jest
24+
.fn()
25+
.mockImplementation(
26+
async (id: number): Promise<PipelineConfig | undefined> => {
27+
const result = pipelineConfigs.find((config) => config.id === id);
28+
return Promise.resolve(result);
29+
},
30+
),
2731

2832
getByDatasourceId: jest
2933
.fn()
30-
.mockImplementation(async (datasourceId: number) => {
31-
const result = pipelineConfigs.filter(
32-
(config) => config.datasourceId === datasourceId,
33-
);
34-
return Promise.resolve(result);
35-
}),
34+
.mockImplementation(
35+
async (datasourceId: number): Promise<PipelineConfig[]> => {
36+
const result = pipelineConfigs.filter(
37+
(config) => config.datasourceId === datasourceId,
38+
);
39+
return Promise.resolve(result);
40+
},
41+
),
3642

3743
create: jest
3844
.fn()
39-
.mockImplementation(async (config: PipelineConfigDTO) => {
40-
const result: PipelineConfig = {
41-
...config,
42-
metadata: {
43-
...config.metadata,
44-
creationTimestamp: new Date(2022, 1),
45-
},
46-
id: ++nextPipelineConfigId,
47-
};
48-
pipelineConfigs.push(result);
49-
return await Promise.resolve(result);
50-
}),
51-
52-
update: jest.fn((id: number, config: PipelineConfigDTO) => {
53-
const configToUpdate = pipelineConfigs.find(
54-
(config) => config.id === id,
55-
);
56-
Object.assign(configToUpdate, config);
57-
}),
45+
.mockImplementation(
46+
async (config: PipelineConfigDTO): Promise<PipelineConfig> => {
47+
const result: PipelineConfig = {
48+
...config,
49+
metadata: {
50+
...config.metadata,
51+
creationTimestamp: new Date(2022, 1),
52+
},
53+
id: ++nextPipelineConfigId,
54+
};
55+
pipelineConfigs.push(result);
56+
return Promise.resolve(result);
57+
},
58+
),
59+
60+
update: jest.fn(
61+
(
62+
id: number,
63+
config: PipelineConfigDTO,
64+
): Promise<PipelineConfig | undefined> => {
65+
const configToUpdate = pipelineConfigs.find(
66+
(config) => config.id === id,
67+
);
68+
69+
if (configToUpdate === undefined) {
70+
return Promise.resolve(undefined);
71+
}
72+
Object.assign(configToUpdate, config);
73+
return Promise.resolve(configToUpdate);
74+
},
75+
),
5876

59-
delete: jest.fn((id: number) => {
77+
delete: jest.fn((id: number): Promise<PipelineConfig | undefined> => {
6078
const indexOfConfigToDelete = pipelineConfigs.findIndex(
6179
(config) => config.id === id,
6280
);
63-
if (indexOfConfigToDelete !== -1) {
64-
pipelineConfigs.splice(indexOfConfigToDelete, 1);
81+
82+
if (indexOfConfigToDelete === -1) {
83+
return Promise.resolve(undefined);
6584
}
85+
86+
const configToDelete = pipelineConfigs[indexOfConfigToDelete];
87+
pipelineConfigs.splice(indexOfConfigToDelete, 1);
88+
return Promise.resolve(configToDelete);
6689
}),
6790
};
6891
}),

‎system-test/src/scenario1.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ describe('Test 1: Create non-periodic pipeline without transformation', () => {
9595

9696
test('Delete pipeline config', async () => {
9797
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
98-
expect(response.status).toEqual(204)
98+
expect(response.status).toEqual(200)
99+
expect(response.type).toEqual('application/json')
100+
expect(response.body.id).toEqual(pipelineId)
99101
}, TEST_TIMEOUT)
100102

101103
test('Delete adapter config', async () => {

‎system-test/src/scenario2.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ describe('Test 2: Create periodic pipeline without transformation', () => {
108108

109109
test('Delete pipeline config', async () => {
110110
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
111-
expect(response.status).toEqual(204)
111+
expect(response.status).toEqual(200)
112+
expect(response.type).toEqual('application/json')
113+
expect(response.body.id).toEqual(pipelineId)
112114
}, TEST_TIMEOUT)
113115

114116
test('Delete adapter config', async () => {

‎system-test/src/scenario3.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@ describe('Test 3: Create non-periodic pipeline with transformation', () => {
9999

100100
test('Delete pipeline config', async () => {
101101
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
102-
expect(response.status).toEqual(204)
102+
expect(response.status).toEqual(200)
103+
expect(response.type).toEqual('application/json')
104+
expect(response.body.id).toEqual(pipelineId)
103105
}, TEST_TIMEOUT)
104106

105107
test('Delete adapter config', async () => {

‎system-test/src/scenario4.test.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,15 @@ describe('Test 4: Update periodic datasource with pipeline', () => {
112112
pipelineConfig.id = pipelineId
113113

114114
const updateResponse = await request(PIPELINE_URL).put(`/configs/${pipelineId}`).send(pipelineConfig)
115-
expect(updateResponse.status).toEqual(204)
115+
expect(updateResponse.status).toEqual(200)
116+
expect(updateResponse.type).toEqual('application/json')
117+
expect(updateResponse.body).toEqual(
118+
expect.objectContaining({
119+
id: pipelineId,
120+
datasourceId: pipelineConfig.datasourceId,
121+
metadata: expect.objectContaining(pipelineConfig.metadata)
122+
})
123+
)
116124
})
117125

118126
test('Add second notification', async () => {
@@ -138,7 +146,9 @@ describe('Test 4: Update periodic datasource with pipeline', () => {
138146

139147
test('Delete pipeline config', async () => {
140148
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
141-
expect(response.status).toEqual(204)
149+
expect(response.status).toEqual(200)
150+
expect(response.type).toEqual('application/json')
151+
expect(response.body.id).toEqual(pipelineId)
142152
}, TEST_TIMEOUT)
143153

144154
test('Delete adapter config', async () => {

‎system-test/src/scenario5.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ describe('Test 5: Create pipeline with multiple notifications', () => {
135135

136136
test('Delete pipeline config', async () => {
137137
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
138-
expect(response.status).toEqual(204)
138+
expect(response.status).toEqual(200)
139+
expect(response.type).toEqual('application/json')
140+
expect(response.body.id).toEqual(pipelineId)
139141
}, TEST_TIMEOUT)
140142

141143
test('Delete adapter config', async () => {

‎system-test/src/scenario6.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ describe('Test 6: Delete periodic pipeline', () => {
8686

8787
test('Delete pipeline config', async () => {
8888
const response = await request(PIPELINE_URL).delete(`/configs/${pipelineId}`).send()
89-
expect(response.status).toEqual(204)
89+
expect(response.status).toEqual(200)
90+
expect(response.type).toEqual('application/json')
91+
expect(response.body.id).toEqual(pipelineId)
9092
}, TEST_TIMEOUT)
9193

9294
test('Notification webhook does not return new data', async () => {

0 commit comments

Comments
 (0)