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

Changes x-pack action executor to return result. #39820

Merged
merged 4 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/actions/server/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function init(server: Legacy.Server) {
getBasePath: () => basePath,
};
return {
log: server.log,
log: server.log.bind(server),
callCluster: callWithInternalUser,
savedObjectsClient: server.savedObjects.getScopedSavedObjectsClient(fakeRequest),
...overwrites,
Expand Down
44 changes: 39 additions & 5 deletions x-pack/legacy/plugins/actions/server/lib/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,43 @@ export async function execute({
};
const validatedConfig = validateActionTypeConfig(actionType, mergedActionTypeConfig);
const validatedParams = validateActionTypeParams(actionType, params);
await actionType.executor({
services,
config: validatedConfig,
params: validatedParams,
});

let result;
let error;
try {
result = await actionType.executor({
services,
config: validatedConfig,
params: validatedParams,
});
} catch (err) {
error = err;
}

const { actionTypeId, description } = action.attributes;
const actionLabel = `${actionId} - ${actionTypeId} - ${description}`;

if (error != null) {
services.log(
['warning', 'x-pack', 'actions'],
`action executed unsuccessfully: ${actionLabel} - ${error.message}`
);
throw error;
}

services.log(['debug', 'x-pack', 'actions'], `action executed successfully: ${actionLabel}`);

// return result if it's JSONable, otherwise a simple success object
const simpleResult = { status: 'ok' };

if (result == null || typeof result !== 'object') {
return simpleResult;
}

try {
JSON.stringify(result);
return result;
} catch (err) {
return simpleResult;
}
}
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/actions/server/routes/fire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ it('fires an action with proper parameters', async () => {
execute.mockResolvedValueOnce({ success: true });

const { payload, statusCode } = await server.inject(request);
expect(statusCode).toBe(204);
expect(payload).toBe('');
expect(statusCode).toBe(200);
expect(payload).toBe('{"success":true}');

expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.get.mock.calls[0]).toMatchInlineSnapshot(`
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/actions/server/routes/fire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ export function fireRoute({ server, actionTypeRegistry, getServices }: FireRoute
const savedObjectsClient = request.getSavedObjectsClient();
// Ensure user can read the action and has access to it
await savedObjectsClient.get('action', id);
await execute({
const result = await execute({
params,
actionTypeRegistry,
actionId: id,
namespace: namespace === 'default' ? undefined : namespace,
services: getServices(request.getBasePath(), { savedObjectsClient }),
encryptedSavedObjectsPlugin: server.plugins.encrypted_saved_objects!,
});
return h.response();
return result;
},
});
}
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/alerting/server/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function init(server: Legacy.Server) {
getBasePath: () => basePath,
};
return {
log: server.log,
log: server.log.bind(server),
callCluster: callWithInternalUser,
savedObjectsClient: server.savedObjects.getScopedSavedObjectsClient(fakeRequest),
};
Expand All @@ -56,7 +56,7 @@ export function init(server: Legacy.Server) {
const request = this;
const savedObjectsClient = request.getSavedObjectsClient();
const alertsClient = new AlertsClient({
log: server.log,
log: server.log.bind(server),
savedObjectsClient,
alertTypeRegistry,
taskManager: taskManager!,
Expand Down
8 changes: 4 additions & 4 deletions x-pack/test/api_integration/apis/actions/fire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) {
message: 'Testing 123',
},
})
.expect(204)
.expect(200)
.then((resp: any) => {
expect(resp.body).to.eql({});
expect(resp.body).to.be.an('object');
});
const indexedRecord = await retry.tryForTime(5000, async () => {
const searchResult = await es.search({
Expand Down Expand Up @@ -146,9 +146,9 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) {
message: 'Testing 123',
},
})
.expect(204)
.expect(200)
.then((resp: any) => {
expect(resp.body).to.eql({});
expect(resp.body).to.be.an('object');
});
const indexedRecord = await retry.tryForTime(5000, async () => {
const searchResult = await es.search({
Expand Down