From 090e0942faf6d278e3d52f30c03c7e46921c74e4 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 11 Nov 2019 18:14:27 -0500 Subject: [PATCH 1/9] feat(task-manager): added scheduleIfNotExists api --- .../plugins/task_manager/plugin.test.ts | 1 + x-pack/legacy/plugins/task_manager/plugin.ts | 2 + .../plugins/task_manager/task_manager.test.ts | 79 +++++++++++++++++++ .../plugins/task_manager/task_manager.ts | 23 ++++++ .../plugins/task_manager/init_routes.js | 31 +++++--- .../task_manager/task_manager_integration.js | 28 ++++++- 6 files changed, 154 insertions(+), 10 deletions(-) diff --git a/x-pack/legacy/plugins/task_manager/plugin.test.ts b/x-pack/legacy/plugins/task_manager/plugin.test.ts index f8ca6bd7a9ab3..bb77dc2fe6e29 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.test.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.test.ts @@ -46,6 +46,7 @@ describe('Task Manager Plugin', () => { "registerTaskDefinitions": [Function], "remove": [Function], "schedule": [Function], + "scheduleIfNotExists": [Function], } `); }); diff --git a/x-pack/legacy/plugins/task_manager/plugin.ts b/x-pack/legacy/plugins/task_manager/plugin.ts index f8d95f4880c6e..61566b1de5181 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.ts @@ -11,6 +11,7 @@ export interface PluginSetupContract { fetch: TaskManager['fetch']; remove: TaskManager['remove']; schedule: TaskManager['schedule']; + scheduleIfNotExists: TaskManager['scheduleIfNotExists']; addMiddleware: TaskManager['addMiddleware']; registerTaskDefinitions: TaskManager['registerTaskDefinitions']; } @@ -59,6 +60,7 @@ export class Plugin { fetch: (...args) => taskManager.fetch(...args), remove: (...args) => taskManager.remove(...args), schedule: (...args) => taskManager.schedule(...args), + scheduleIfNotExists: (...args) => taskManager.scheduleIfNotExists(...args), addMiddleware: (...args) => taskManager.addMiddleware(...args), registerTaskDefinitions: (...args) => taskManager.registerTaskDefinitions(...args), }; diff --git a/x-pack/legacy/plugins/task_manager/task_manager.test.ts b/x-pack/legacy/plugins/task_manager/task_manager.test.ts index 9ae2f5e1e027b..80e64a16b2975 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.test.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.test.ts @@ -121,6 +121,85 @@ describe('TaskManager', () => { expect(savedObjectsClient.create).toHaveBeenCalled(); }); + test('allows scheduling existing tasks that may have already been scheduled', async () => { + const client = new TaskManager(taskManagerOpts); + client.registerTaskDefinitions({ + foo: { + type: 'foo', + title: 'Foo', + createTaskRunner: jest.fn(), + }, + }); + savedObjectsClient.create.mockRejectedValueOnce({ + statusCode: 409, + }); + + client.start(); + + const result = await client.scheduleIfNotExists({ + id: 'my-foo-id', + taskType: 'foo', + params: {}, + state: {}, + }); + + expect(result.id).toEqual('my-foo-id'); + }); + + test('doesnt ignore failure to scheduling existing tasks for reasons other than already being scheduled', async () => { + const client = new TaskManager(taskManagerOpts); + client.registerTaskDefinitions({ + foo: { + type: 'foo', + title: 'Foo', + createTaskRunner: jest.fn(), + }, + }); + savedObjectsClient.create.mockRejectedValueOnce({ + statusCode: 500, + }); + + client.start(); + + return expect( + client.scheduleIfNotExists({ + id: 'my-foo-id', + taskType: 'foo', + params: {}, + state: {}, + }) + ).rejects.toMatchObject({ + statusCode: 500, + }); + }); + + test('doesnt allow naively rescheduling existing tasks that have already been scheduled', async () => { + const client = new TaskManager(taskManagerOpts); + client.registerTaskDefinitions({ + foo: { + type: 'foo', + title: 'Foo', + createTaskRunner: jest.fn(), + }, + }); + savedObjectsClient.create.mockRejectedValueOnce({ + statusCode: 409, + }); + + client.start(); + + return expect( + client.schedule({ + id: 'my-foo-id', + taskType: 'foo', + params: {}, + state: {}, + }) + ).rejects.toMatchObject({ + statusCode: 409, + }); + }); + test('allows and queues removing tasks before starting', async () => { const client = new TaskManager(taskManagerOpts); savedObjectsClient.delete.mockResolvedValueOnce({}); diff --git a/x-pack/legacy/plugins/task_manager/task_manager.ts b/x-pack/legacy/plugins/task_manager/task_manager.ts index 4ddb18c7cfe74..2f2becd202d74 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.ts @@ -14,6 +14,7 @@ import { TaskDefinition, TaskDictionary, ConcreteTaskInstance, + ExistingTaskInstance, RunContext, TaskInstance, } from './task'; @@ -29,6 +30,8 @@ import { } from './task_store'; import { identifyEsError } from './lib/identify_es_error'; +const VERSION_CONFLICT_STATUS = 409; + export interface TaskManagerOpts { logger: Logger; config: any; @@ -219,6 +222,26 @@ export class TaskManager { return result; } + /** + * Schedules a task with an Id + * + * @param task - The task being scheduled. + * @returns {Promise} + */ + public async scheduleIfNotExists( + taskInstance: ExistingTaskInstance, + options?: any + ): Promise { + try { + return await this.schedule(taskInstance, options); + } catch (err) { + if (err.statusCode === VERSION_CONFLICT_STATUS) { + return taskInstance; + } + throw err; + } + } + /** * Fetches a paginatable list of scheduled tasks. * diff --git a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js index ad06fb15fd9ae..07d0c8391fce6 100644 --- a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js +++ b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js @@ -32,21 +32,34 @@ export function initRoutes(server) { config: { validate: { payload: Joi.object({ - taskType: Joi.string().required(), - interval: Joi.string().optional(), - params: Joi.object().required(), - state: Joi.object().optional(), - id: Joi.string().optional(), + task: Joi.object({ + taskType: Joi.string().required(), + interval: Joi.string().optional(), + params: Joi.object().required(), + state: Joi.object().optional(), + id: Joi.string().optional() + }), + scheduleIfNotExists: Joi.boolean() + .default(false) + .optional(), }), }, }, async handler(request) { try { - const task = await taskManager.schedule({ - ...request.payload, + const { scheduleIfNotExists = false, task: taskFields } = request.payload; + const task = { + ...taskFields, scope: [scope], - }, { request }); - return task; + }; + + const taskResult = await ( + scheduleIfNotExists + ? taskManager.scheduleIfNotExists(task, { request }) + : taskManager.schedule(task, { request }) + ); + + return taskResult; } catch (err) { return err; } diff --git a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js index 30d830cd6c919..ef21dd2786294 100644 --- a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js +++ b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js @@ -60,7 +60,15 @@ export default function ({ getService }) { function scheduleTask(task) { return supertest.post('/api/sample_tasks') .set('kbn-xsrf', 'xxx') - .send(task) + .send({ task }) + .expect(200) + .then((response) => response.body); + } + + function scheduleTaskIfNotExists(task) { + return supertest.post('/api/sample_tasks') + .set('kbn-xsrf', 'xxx') + .send({ task, scheduleIfNotExists: true }) .expect(200) .then((response) => response.body); } @@ -116,6 +124,24 @@ export default function ({ getService }) { expect(result.id).to.be('test-task-for-sample-task-plugin-to-test-task-manager'); }); + it('should allow a task with a given ID to be scheduled multiple times', async () => { + const result = await scheduleTaskIfNotExists({ + id: 'test-task-to-reschedule-in-task-manager', + taskType: 'sampleTask', + params: { }, + }); + + expect(result.id).to.be('test-task-to-reschedule-in-task-manager'); + + const rescheduleResult = await scheduleTaskIfNotExists({ + id: 'test-task-to-reschedule-in-task-manager', + taskType: 'sampleTask', + params: { }, + }); + + expect(rescheduleResult.id).to.be('test-task-to-reschedule-in-task-manager'); + }); + it('should reschedule if task errors', async () => { const task = await scheduleTask({ taskType: 'sampleTask', From 1d7e751cf3c3b859111aebb9cc6ae0e34bb695d5 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 11 Nov 2019 18:20:27 -0500 Subject: [PATCH 2/9] refactor(task-manager): renamed scheduleIfNotExists api to clarify what it does --- x-pack/legacy/plugins/task_manager/plugin.test.ts | 2 +- x-pack/legacy/plugins/task_manager/plugin.ts | 4 ++-- x-pack/legacy/plugins/task_manager/task_manager.test.ts | 4 ++-- x-pack/legacy/plugins/task_manager/task_manager.ts | 2 +- .../plugins/task_manager/init_routes.js | 8 ++++---- .../test_suites/task_manager/task_manager_integration.js | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/x-pack/legacy/plugins/task_manager/plugin.test.ts b/x-pack/legacy/plugins/task_manager/plugin.test.ts index bb77dc2fe6e29..49945ca500315 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.test.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.test.ts @@ -42,11 +42,11 @@ describe('Task Manager Plugin', () => { expect(setupResult).toMatchInlineSnapshot(` Object { "addMiddleware": [Function], + "ensureScheduling": [Function], "fetch": [Function], "registerTaskDefinitions": [Function], "remove": [Function], "schedule": [Function], - "scheduleIfNotExists": [Function], } `); }); diff --git a/x-pack/legacy/plugins/task_manager/plugin.ts b/x-pack/legacy/plugins/task_manager/plugin.ts index 61566b1de5181..1db6c37a7d995 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.ts @@ -11,7 +11,7 @@ export interface PluginSetupContract { fetch: TaskManager['fetch']; remove: TaskManager['remove']; schedule: TaskManager['schedule']; - scheduleIfNotExists: TaskManager['scheduleIfNotExists']; + ensureScheduling: TaskManager['ensureScheduling']; addMiddleware: TaskManager['addMiddleware']; registerTaskDefinitions: TaskManager['registerTaskDefinitions']; } @@ -60,7 +60,7 @@ export class Plugin { fetch: (...args) => taskManager.fetch(...args), remove: (...args) => taskManager.remove(...args), schedule: (...args) => taskManager.schedule(...args), - scheduleIfNotExists: (...args) => taskManager.scheduleIfNotExists(...args), + ensureScheduling: (...args) => taskManager.ensureScheduling(...args), addMiddleware: (...args) => taskManager.addMiddleware(...args), registerTaskDefinitions: (...args) => taskManager.registerTaskDefinitions(...args), }; diff --git a/x-pack/legacy/plugins/task_manager/task_manager.test.ts b/x-pack/legacy/plugins/task_manager/task_manager.test.ts index 80e64a16b2975..5682bdd4aa6c2 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.test.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.test.ts @@ -136,7 +136,7 @@ describe('TaskManager', () => { client.start(); - const result = await client.scheduleIfNotExists({ + const result = await client.ensureScheduling({ id: 'my-foo-id', taskType: 'foo', params: {}, @@ -162,7 +162,7 @@ describe('TaskManager', () => { client.start(); return expect( - client.scheduleIfNotExists({ + client.ensureScheduling({ id: 'my-foo-id', taskType: 'foo', params: {}, diff --git a/x-pack/legacy/plugins/task_manager/task_manager.ts b/x-pack/legacy/plugins/task_manager/task_manager.ts index 2f2becd202d74..0bd57aa9a9d52 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.ts @@ -228,7 +228,7 @@ export class TaskManager { * @param task - The task being scheduled. * @returns {Promise} */ - public async scheduleIfNotExists( + public async ensureScheduling( taskInstance: ExistingTaskInstance, options?: any ): Promise { diff --git a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js index 07d0c8391fce6..74d6c17debba8 100644 --- a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js +++ b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js @@ -39,7 +39,7 @@ export function initRoutes(server) { state: Joi.object().optional(), id: Joi.string().optional() }), - scheduleIfNotExists: Joi.boolean() + ensureScheduling: Joi.boolean() .default(false) .optional(), }), @@ -47,15 +47,15 @@ export function initRoutes(server) { }, async handler(request) { try { - const { scheduleIfNotExists = false, task: taskFields } = request.payload; + const { ensureScheduling = false, task: taskFields } = request.payload; const task = { ...taskFields, scope: [scope], }; const taskResult = await ( - scheduleIfNotExists - ? taskManager.scheduleIfNotExists(task, { request }) + ensureScheduling + ? taskManager.ensureScheduling(task, { request }) : taskManager.schedule(task, { request }) ); diff --git a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js index ef21dd2786294..172ac1265beff 100644 --- a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js +++ b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js @@ -68,7 +68,7 @@ export default function ({ getService }) { function scheduleTaskIfNotExists(task) { return supertest.post('/api/sample_tasks') .set('kbn-xsrf', 'xxx') - .send({ task, scheduleIfNotExists: true }) + .send({ task, ensureScheduling: true }) .expect(200) .then((response) => response.body); } From aacff4dae63ca93fa655a6800a55cdba39279af4 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Tue, 12 Nov 2019 11:04:46 -0500 Subject: [PATCH 3/9] fix(task-manager): fixed typing --- x-pack/legacy/plugins/task_manager/task.ts | 11 +++++++++++ .../legacy/plugins/task_manager/task_manager.mock.ts | 1 + 2 files changed, 12 insertions(+) diff --git a/x-pack/legacy/plugins/task_manager/task.ts b/x-pack/legacy/plugins/task_manager/task.ts index dd74acc2636e9..2004c350be75b 100644 --- a/x-pack/legacy/plugins/task_manager/task.ts +++ b/x-pack/legacy/plugins/task_manager/task.ts @@ -216,6 +216,17 @@ export interface TaskInstance { ownerId?: string | null; } +/** + * A task instance that has an id. + */ +export interface ExistingTaskInstance extends TaskInstance { + /** + * The id of the Elastic document that stores this instance's data. This can + * be passed by the caller when scheduling the task. + */ + id: string; +} + /** * A task instance that has an id and is ready for storage. */ diff --git a/x-pack/legacy/plugins/task_manager/task_manager.mock.ts b/x-pack/legacy/plugins/task_manager/task_manager.mock.ts index 2737e83f0ba4a..2a880475e1469 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.mock.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.mock.ts @@ -10,6 +10,7 @@ const createTaskManagerMock = () => { const mocked: jest.Mocked = { registerTaskDefinitions: jest.fn(), addMiddleware: jest.fn(), + ensureScheduling: jest.fn(), schedule: jest.fn(), fetch: jest.fn(), remove: jest.fn(), From d90a48306cca84225b0e173eb06fe4c1c5d78791 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Tue, 12 Nov 2019 11:24:43 -0500 Subject: [PATCH 4/9] feat(task-manager): use ensureScheduling whenever the ID of the task is included in lens, maps and telemetry --- x-pack/legacy/plugins/lens/server/usage/task.ts | 2 +- .../legacy/plugins/maps/server/maps_telemetry/telemetry_task.js | 2 +- x-pack/legacy/plugins/oss_telemetry/index.d.ts | 2 +- x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts | 2 +- x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/lens/server/usage/task.ts b/x-pack/legacy/plugins/lens/server/usage/task.ts index 3cb857a453e1d..0a07c828cf8c9 100644 --- a/x-pack/legacy/plugins/lens/server/usage/task.ts +++ b/x-pack/legacy/plugins/lens/server/usage/task.ts @@ -82,7 +82,7 @@ function scheduleTasks(server: Server) { // function block. (async () => { try { - await taskManager.schedule({ + await taskManager.ensureScheduling({ id: TASK_ID, taskType: TELEMETRY_TASK_TYPE, state: { byDate: {}, suggestionsByDate: {}, saved: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js b/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js index 3702bc8e29539..00eca827c4a1a 100644 --- a/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js +++ b/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js @@ -29,7 +29,7 @@ export function scheduleTask(server) { // function block. (async () => { try { - await taskManager.schedule({ + await taskManager.ensureScheduling({ id: TASK_ID, taskType: TELEMETRY_TASK_TYPE, state: { stats: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/oss_telemetry/index.d.ts b/x-pack/legacy/plugins/oss_telemetry/index.d.ts index 9f735c676fe6d..ef55a66e68fcb 100644 --- a/x-pack/legacy/plugins/oss_telemetry/index.d.ts +++ b/x-pack/legacy/plugins/oss_telemetry/index.d.ts @@ -46,7 +46,7 @@ export interface HapiServer { }; task_manager: { registerTaskDefinitions: (opts: any) => void; - schedule: (opts: any) => Promise; + ensureScheduling: (opts: any) => Promise; fetch: ( opts: any ) => Promise<{ diff --git a/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts b/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts index eaa8cc7405821..9ea783486f58e 100644 --- a/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts +++ b/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts @@ -42,7 +42,7 @@ export function scheduleTasks(server: HapiServer) { // function block. (async () => { try { - await taskManager.schedule({ + await taskManager.ensureScheduling({ id: `${PLUGIN_ID}-${VIS_TELEMETRY_TASK}`, taskType: VIS_TELEMETRY_TASK, state: { stats: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts b/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts index 7168f598dca23..67b331f4d4d5c 100644 --- a/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts +++ b/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts @@ -50,7 +50,7 @@ export const getMockKbnServer = ( xpack_main: {}, task_manager: { registerTaskDefinitions: (opts: any) => undefined, - schedule: (opts: any) => Promise.resolve(), + ensureScheduling: (opts: any) => Promise.resolve(), fetch: mockTaskFetch, }, }, From cdf9ec52a2d59e8a9ea4842c52e2c49f94bc0f02 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Wed, 13 Nov 2019 11:33:34 -0500 Subject: [PATCH 5/9] doc(task-manager): document ensureShceduling api --- x-pack/legacy/plugins/task_manager/README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/x-pack/legacy/plugins/task_manager/README.md b/x-pack/legacy/plugins/task_manager/README.md index 63c92102af251..744643458e136 100644 --- a/x-pack/legacy/plugins/task_manager/README.md +++ b/x-pack/legacy/plugins/task_manager/README.md @@ -222,6 +222,9 @@ The data stored for a task instance looks something like this: The task manager mixin exposes a taskManager object on the Kibana server which plugins can use to manage scheduled tasks. Each method takes an optional `scope` argument and ensures that only tasks with the specified scope(s) will be affected. +### schedule +Using `schedule` you can instruct TaskManger to schedule an instance of a TaskType at some point in the future. + ```js const taskManager = server.plugins.task_manager; // Schedules a task. All properties are as documented in the previous @@ -256,6 +259,14 @@ const results = await manager.find({ scope: 'my-fanci-app', searchAfter: ['ids'] } ``` +### ensureScheduling +When using the `schedule` api to schedule a Task you can provide a hard coded `id` on the Task. This tells TaskManager to use this `id` to identify the Task Instance rather than generate an `id` on its own. +The danger is that in such a situation, a Task with that same `id` might already have been scheduled at some earlier point, and this would result in an error. In some cases, this is the expected behavior, but often you only care about ensuring the task has been _scheduled_ and don't need it to be scheduled a fresh. + +To achieve this you should use the `ensureScheduling` api which has the exact same behavior as `schedule`, except it allows the scheduling of a Task with an `id` that's already in assigned to another Task and it will assume that the existing Task is the one you wished to `schedule`, treating this as a successful operation. + +### more options + More custom access to the tasks can be done directly via Elasticsearch, though that won't be officially supported, as we can change the document structure at any time. ## Middleware From a7d12c5c4ade8606e3a58c85d44a4526ba4c7c50 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 14 Nov 2019 10:11:39 -0500 Subject: [PATCH 6/9] refactor(task-manager): introduced a more explicit type for Task with ID --- x-pack/legacy/plugins/task_manager/task.ts | 10 +++------- x-pack/legacy/plugins/task_manager/task_manager.ts | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/x-pack/legacy/plugins/task_manager/task.ts b/x-pack/legacy/plugins/task_manager/task.ts index 2004c350be75b..0ab2beda0c86f 100644 --- a/x-pack/legacy/plugins/task_manager/task.ts +++ b/x-pack/legacy/plugins/task_manager/task.ts @@ -10,6 +10,8 @@ import Joi from 'joi'; * Type definitions and validations for tasks. */ +type Require = Omit & Required>; + /** * A loosely typed definition of the elasticjs wrapper. It's beyond the scope * of this work to try to make a comprehensive type definition of this. @@ -219,13 +221,7 @@ export interface TaskInstance { /** * A task instance that has an id. */ -export interface ExistingTaskInstance extends TaskInstance { - /** - * The id of the Elastic document that stores this instance's data. This can - * be passed by the caller when scheduling the task. - */ - id: string; -} +export type TaskInstanceWithId = Require; /** * A task instance that has an id and is ready for storage. diff --git a/x-pack/legacy/plugins/task_manager/task_manager.ts b/x-pack/legacy/plugins/task_manager/task_manager.ts index 0bd57aa9a9d52..9abcc3cfe7f7d 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.ts @@ -14,8 +14,8 @@ import { TaskDefinition, TaskDictionary, ConcreteTaskInstance, - ExistingTaskInstance, RunContext, + TaskInstanceWithId, TaskInstance, } from './task'; import { TaskPoller } from './task_poller'; @@ -229,9 +229,9 @@ export class TaskManager { * @returns {Promise} */ public async ensureScheduling( - taskInstance: ExistingTaskInstance, + taskInstance: TaskInstanceWithId, options?: any - ): Promise { + ): Promise { try { return await this.schedule(taskInstance, options); } catch (err) { From 7198e4c257515c8b87fe96eec981ac19b810a36b Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 14 Nov 2019 10:31:24 -0500 Subject: [PATCH 7/9] refactor(task-manager): renamed ensureScheduling to ensureScheduled --- x-pack/legacy/plugins/lens/server/usage/task.ts | 2 +- .../plugins/maps/server/maps_telemetry/telemetry_task.js | 2 +- x-pack/legacy/plugins/oss_telemetry/index.d.ts | 2 +- .../plugins/oss_telemetry/server/lib/tasks/index.ts | 2 +- x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts | 2 +- x-pack/legacy/plugins/task_manager/plugin.test.ts | 2 +- x-pack/legacy/plugins/task_manager/plugin.ts | 4 ++-- x-pack/legacy/plugins/task_manager/task_manager.mock.ts | 2 +- x-pack/legacy/plugins/task_manager/task_manager.test.ts | 4 ++-- x-pack/legacy/plugins/task_manager/task_manager.ts | 4 ++-- .../plugins/task_manager/init_routes.js | 8 ++++---- .../test_suites/task_manager/task_manager_integration.js | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/x-pack/legacy/plugins/lens/server/usage/task.ts b/x-pack/legacy/plugins/lens/server/usage/task.ts index 0a07c828cf8c9..03e085cc9e669 100644 --- a/x-pack/legacy/plugins/lens/server/usage/task.ts +++ b/x-pack/legacy/plugins/lens/server/usage/task.ts @@ -82,7 +82,7 @@ function scheduleTasks(server: Server) { // function block. (async () => { try { - await taskManager.ensureScheduling({ + await taskManager.ensureScheduled({ id: TASK_ID, taskType: TELEMETRY_TASK_TYPE, state: { byDate: {}, suggestionsByDate: {}, saved: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js b/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js index 00eca827c4a1a..78b04543e72f2 100644 --- a/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js +++ b/x-pack/legacy/plugins/maps/server/maps_telemetry/telemetry_task.js @@ -29,7 +29,7 @@ export function scheduleTask(server) { // function block. (async () => { try { - await taskManager.ensureScheduling({ + await taskManager.ensureScheduled({ id: TASK_ID, taskType: TELEMETRY_TASK_TYPE, state: { stats: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/oss_telemetry/index.d.ts b/x-pack/legacy/plugins/oss_telemetry/index.d.ts index ef55a66e68fcb..012f987627369 100644 --- a/x-pack/legacy/plugins/oss_telemetry/index.d.ts +++ b/x-pack/legacy/plugins/oss_telemetry/index.d.ts @@ -46,7 +46,7 @@ export interface HapiServer { }; task_manager: { registerTaskDefinitions: (opts: any) => void; - ensureScheduling: (opts: any) => Promise; + ensureScheduled: (opts: any) => Promise; fetch: ( opts: any ) => Promise<{ diff --git a/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts b/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts index 9ea783486f58e..16e83a7938e60 100644 --- a/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts +++ b/x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts @@ -42,7 +42,7 @@ export function scheduleTasks(server: HapiServer) { // function block. (async () => { try { - await taskManager.ensureScheduling({ + await taskManager.ensureScheduled({ id: `${PLUGIN_ID}-${VIS_TELEMETRY_TASK}`, taskType: VIS_TELEMETRY_TASK, state: { stats: {}, runs: 0 }, diff --git a/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts b/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts index 67b331f4d4d5c..998a1d2beeab1 100644 --- a/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts +++ b/x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts @@ -50,7 +50,7 @@ export const getMockKbnServer = ( xpack_main: {}, task_manager: { registerTaskDefinitions: (opts: any) => undefined, - ensureScheduling: (opts: any) => Promise.resolve(), + ensureScheduled: (opts: any) => Promise.resolve(), fetch: mockTaskFetch, }, }, diff --git a/x-pack/legacy/plugins/task_manager/plugin.test.ts b/x-pack/legacy/plugins/task_manager/plugin.test.ts index 49945ca500315..4f2effb5da3a8 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.test.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.test.ts @@ -42,7 +42,7 @@ describe('Task Manager Plugin', () => { expect(setupResult).toMatchInlineSnapshot(` Object { "addMiddleware": [Function], - "ensureScheduling": [Function], + "ensureScheduled": [Function], "fetch": [Function], "registerTaskDefinitions": [Function], "remove": [Function], diff --git a/x-pack/legacy/plugins/task_manager/plugin.ts b/x-pack/legacy/plugins/task_manager/plugin.ts index 1db6c37a7d995..3e1514bd5234f 100644 --- a/x-pack/legacy/plugins/task_manager/plugin.ts +++ b/x-pack/legacy/plugins/task_manager/plugin.ts @@ -11,7 +11,7 @@ export interface PluginSetupContract { fetch: TaskManager['fetch']; remove: TaskManager['remove']; schedule: TaskManager['schedule']; - ensureScheduling: TaskManager['ensureScheduling']; + ensureScheduled: TaskManager['ensureScheduled']; addMiddleware: TaskManager['addMiddleware']; registerTaskDefinitions: TaskManager['registerTaskDefinitions']; } @@ -60,7 +60,7 @@ export class Plugin { fetch: (...args) => taskManager.fetch(...args), remove: (...args) => taskManager.remove(...args), schedule: (...args) => taskManager.schedule(...args), - ensureScheduling: (...args) => taskManager.ensureScheduling(...args), + ensureScheduled: (...args) => taskManager.ensureScheduled(...args), addMiddleware: (...args) => taskManager.addMiddleware(...args), registerTaskDefinitions: (...args) => taskManager.registerTaskDefinitions(...args), }; diff --git a/x-pack/legacy/plugins/task_manager/task_manager.mock.ts b/x-pack/legacy/plugins/task_manager/task_manager.mock.ts index 2a880475e1469..515099a8bd479 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.mock.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.mock.ts @@ -10,7 +10,7 @@ const createTaskManagerMock = () => { const mocked: jest.Mocked = { registerTaskDefinitions: jest.fn(), addMiddleware: jest.fn(), - ensureScheduling: jest.fn(), + ensureScheduled: jest.fn(), schedule: jest.fn(), fetch: jest.fn(), remove: jest.fn(), diff --git a/x-pack/legacy/plugins/task_manager/task_manager.test.ts b/x-pack/legacy/plugins/task_manager/task_manager.test.ts index 5682bdd4aa6c2..0b4a22910e611 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.test.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.test.ts @@ -136,7 +136,7 @@ describe('TaskManager', () => { client.start(); - const result = await client.ensureScheduling({ + const result = await client.ensureScheduled({ id: 'my-foo-id', taskType: 'foo', params: {}, @@ -162,7 +162,7 @@ describe('TaskManager', () => { client.start(); return expect( - client.ensureScheduling({ + client.ensureScheduled({ id: 'my-foo-id', taskType: 'foo', params: {}, diff --git a/x-pack/legacy/plugins/task_manager/task_manager.ts b/x-pack/legacy/plugins/task_manager/task_manager.ts index 9abcc3cfe7f7d..269d7ff67384b 100644 --- a/x-pack/legacy/plugins/task_manager/task_manager.ts +++ b/x-pack/legacy/plugins/task_manager/task_manager.ts @@ -226,9 +226,9 @@ export class TaskManager { * Schedules a task with an Id * * @param task - The task being scheduled. - * @returns {Promise} + * @returns {Promise} */ - public async ensureScheduling( + public async ensureScheduled( taskInstance: TaskInstanceWithId, options?: any ): Promise { diff --git a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js index 74d6c17debba8..a9dfabae6d609 100644 --- a/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js +++ b/x-pack/test/plugin_api_integration/plugins/task_manager/init_routes.js @@ -39,7 +39,7 @@ export function initRoutes(server) { state: Joi.object().optional(), id: Joi.string().optional() }), - ensureScheduling: Joi.boolean() + ensureScheduled: Joi.boolean() .default(false) .optional(), }), @@ -47,15 +47,15 @@ export function initRoutes(server) { }, async handler(request) { try { - const { ensureScheduling = false, task: taskFields } = request.payload; + const { ensureScheduled = false, task: taskFields } = request.payload; const task = { ...taskFields, scope: [scope], }; const taskResult = await ( - ensureScheduling - ? taskManager.ensureScheduling(task, { request }) + ensureScheduled + ? taskManager.ensureScheduled(task, { request }) : taskManager.schedule(task, { request }) ); diff --git a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js index 172ac1265beff..07877f3c09d84 100644 --- a/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js +++ b/x-pack/test/plugin_api_integration/test_suites/task_manager/task_manager_integration.js @@ -68,7 +68,7 @@ export default function ({ getService }) { function scheduleTaskIfNotExists(task) { return supertest.post('/api/sample_tasks') .set('kbn-xsrf', 'xxx') - .send({ task, ensureScheduling: true }) + .send({ task, ensureScheduled: true }) .expect(200) .then((response) => response.body); } From 55499bc2f791b2eed16696b935a18ccc9c3eaaa2 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 14 Nov 2019 10:37:13 -0500 Subject: [PATCH 8/9] refactor(task-manager): removed unnnecesery typing --- x-pack/legacy/plugins/task_manager/task.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/task_manager/task.ts b/x-pack/legacy/plugins/task_manager/task.ts index 0ab2beda0c86f..250fb87d3823a 100644 --- a/x-pack/legacy/plugins/task_manager/task.ts +++ b/x-pack/legacy/plugins/task_manager/task.ts @@ -10,7 +10,7 @@ import Joi from 'joi'; * Type definitions and validations for tasks. */ -type Require = Omit & Required>; +type Require = Omit & Required>; /** * A loosely typed definition of the elasticjs wrapper. It's beyond the scope From 97ff0aac42a1913aafcaab695bea7aff66524815 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 14 Nov 2019 10:46:30 -0500 Subject: [PATCH 9/9] doc(task-manager): Document Require utility type --- x-pack/legacy/plugins/task_manager/task.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x-pack/legacy/plugins/task_manager/task.ts b/x-pack/legacy/plugins/task_manager/task.ts index 250fb87d3823a..3eeb23685f377 100644 --- a/x-pack/legacy/plugins/task_manager/task.ts +++ b/x-pack/legacy/plugins/task_manager/task.ts @@ -10,6 +10,18 @@ import Joi from 'joi'; * Type definitions and validations for tasks. */ +/** + * Require + * @desc Create a Subtype of type T `T` such that the property under key `P` becomes required + * @example + * type TaskInstance = { + * id?: string; + * name: string; + * }; + * + * // This type is now defined as { id: string; name: string; } + * type TaskInstanceWithId = Require; + */ type Require = Omit & Required>; /**