Remove id attribute from canvas attributes (#30736) (#31138)

* Remove id attribute from canvas attributes

* Fix create and update API

* Add migration tests

* Use constants

* Add API tests

* Cleanup tests

* Apply PR feedback

x-pack/plugins/canvas/index.js

@@ -8,6 +8,7 @@ import { resolve } from 'path';
 import init from './init';
 import { mappings } from './server/mappings';
 import { CANVAS_APP } from './common/lib';
+import { migrations } from './migrations';
 
 export function canvas(kibana) {
   return new kibana.Plugin({
@@ -30,6 +31,7 @@ export function canvas(kibana) {
       ],
       home: ['plugins/canvas/register_feature'],
       mappings,
+      migrations,
     },
 
     config: Joi => {

x-pack/plugins/canvas/migrations.js

@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { CANVAS_TYPE } from './common/lib';
+
+export const migrations = {
+  [CANVAS_TYPE]: {
+    '7.0.0': doc => {
+      if (doc.attributes) {
+        delete doc.attributes.id;
+      }
+      return doc;
+    },
+  },
+};

x-pack/plugins/canvas/migrations.test.js

@@ -0,0 +1,37 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { migrations } from './migrations';
+import { CANVAS_TYPE } from './common/lib';
+
+describe(CANVAS_TYPE, () => {
+  describe('7.0.0', () => {
+    const migrate = doc => migrations[CANVAS_TYPE]['7.0.0'](doc);
+
+    it('does not throw error on empty object', () => {
+      const migratedDoc = migrate({});
+      expect(migratedDoc).toMatchInlineSnapshot(`Object {}`);
+    });
+
+    it('removes id from "attributes"', () => {
+      const migratedDoc = migrate({
+        foo: true,
+        attributes: {
+          id: '123',
+          bar: true,
+        },
+      });
+      expect(migratedDoc).toMatchInlineSnapshot(`
+Object {
+  "attributes": Object {
+    "bar": true,
+  },
+  "foo": true,
+}
+`);
+    });
+  });
+});

x-pack/plugins/canvas/server/mappings.js

@@ -18,7 +18,6 @@ export const mappings = {
           },
         },
       },
-      id: { type: 'text', index: false },
       '@timestamp': { type: 'date' },
       '@created': { type: 'date' },
     },

x-pack/plugins/canvas/server/routes/workpad.js

@@ -5,6 +5,7 @@
  */
 
 import boom from 'boom';
+import { omit } from 'lodash';
 import {
   CANVAS_TYPE,
   API_ROUTE_WORKPAD,
@@ -44,7 +45,7 @@ export function workpad(server) {
     return resp;
   }
 
-  function createWorkpad(req, id) {
+  function createWorkpad(req) {
     const savedObjectsClient = req.getSavedObjectsClient();
 
     if (!req.payload) {
@@ -52,14 +53,15 @@ export function workpad(server) {
     }
 
     const now = new Date().toISOString();
+    const { id, ...payload } = req.payload;
     return savedObjectsClient.create(
       CANVAS_TYPE,
       {
-        ...req.payload,
+        ...payload,
         '@timestamp': now,
         '@created': now,
       },
-      { id: id || req.payload.id || getId('workpad') }
+      { id: id || getId('workpad') }
     );
   }
 
@@ -74,7 +76,7 @@ export function workpad(server) {
       return savedObjectsClient.create(
         CANVAS_TYPE,
         {
-          ...req.payload,
+          ...omit(req.payload, 'id'),
           '@timestamp': now,
           '@created': workpad.attributes['@created'],
         },
@@ -158,7 +160,7 @@ export function workpad(server) {
 
       return savedObjectsClient
         .get(CANVAS_TYPE, id)
-        .then(obj => obj.attributes)
+        .then(obj => ({ id: obj.id, ...obj.attributes }))
         .then(formatResponse)
         .catch(formatResponse);
     },
@@ -233,7 +235,7 @@ export function workpad(server) {
         .then(resp => {
           return {
             total: resp.total,
-            workpads: resp.saved_objects.map(hit => hit.attributes),
+            workpads: resp.saved_objects.map(hit => ({ id: hit.id, ...hit.attributes })),
           };
         })
         .catch(() => {

x-pack/plugins/canvas/server/routes/workpad.test.js

@@ -0,0 +1,357 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import Hapi from 'hapi';
+import { CANVAS_TYPE, API_ROUTE_WORKPAD } from '../../common/lib/constants';
+import { workpad } from './workpad';
+
+jest.mock('uuid/v4', () => jest.fn().mockReturnValue('123abc'));
+
+describe(`${CANVAS_TYPE} API`, () => {
+  const savedObjectsClient = {
+    get: jest.fn(),
+    create: jest.fn(),
+    delete: jest.fn(),
+    find: jest.fn(),
+  };
+
+  afterEach(() => {
+    savedObjectsClient.get.mockReset();
+    savedObjectsClient.create.mockReset();
+    savedObjectsClient.delete.mockReset();
+    savedObjectsClient.find.mockReset();
+  });
+
+  // Mock toISOString function of all Date types
+  global.Date = class Date extends global.Date {
+    toISOString() {
+      return '2019-02-12T21:01:22.479Z';
+    }
+  };
+
+  // Setup mock server
+  const mockServer = new Hapi.Server({ debug: false, port: 0 });
+  mockServer.plugins = {
+    elasticsearch: {
+      getCluster: () => ({
+        errors: {
+          // formatResponse will fail without objects here
+          '400': Error,
+          '401': Error,
+          '403': Error,
+          '404': Error,
+        },
+      }),
+    },
+  };
+  mockServer.ext('onRequest', (req, h) => {
+    req.getSavedObjectsClient = () => savedObjectsClient;
+    return h.continue;
+  });
+  workpad(mockServer);
+
+  describe(`GET ${API_ROUTE_WORKPAD}/{id}`, () => {
+    test('returns successful response', async () => {
+      const request = {
+        method: 'GET',
+        url: `${API_ROUTE_WORKPAD}/123`,
+      };
+
+      savedObjectsClient.get.mockResolvedValueOnce({ id: '123', attributes: { foo: true } });
+
+      const { payload, statusCode } = await mockServer.inject(request);
+      const response = JSON.parse(payload);
+
+      expect(statusCode).toBe(200);
+      expect(response).toMatchInlineSnapshot(`
+Object {
+  "foo": true,
+  "id": "123",
+}
+`);
+      expect(savedObjectsClient.get).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      "123",
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+    });
+  });
+
+  describe(`POST ${API_ROUTE_WORKPAD}`, () => {
+    test('returns successful response without id in payload', async () => {
+      const request = {
+        method: 'POST',
+        url: API_ROUTE_WORKPAD,
+        payload: {
+          foo: true,
+        },
+      };
+
+      savedObjectsClient.create.mockResolvedValueOnce({});
+
+      const { payload, statusCode } = await mockServer.inject(request);
+      const response = JSON.parse(payload);
+
+      expect(statusCode).toBe(200);
+      expect(response).toMatchInlineSnapshot(`
+Object {
+  "ok": true,
+}
+`);
+      expect(savedObjectsClient.create).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      Object {
+        "@created": "2019-02-12T21:01:22.479Z",
+        "@timestamp": "2019-02-12T21:01:22.479Z",
+        "foo": true,
+      },
+      Object {
+        "id": "workpad-123abc",
+      },
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+    });
+
+    test('returns succesful response with id in payload', async () => {
+      const request = {
+        method: 'POST',
+        url: API_ROUTE_WORKPAD,
+        payload: {
+          id: '123',
+          foo: true,
+        },
+      };
+
+      savedObjectsClient.create.mockResolvedValueOnce({});
+
+      const { payload, statusCode } = await mockServer.inject(request);
+      const response = JSON.parse(payload);
+
+      expect(statusCode).toBe(200);
+      expect(response).toMatchInlineSnapshot(`
+Object {
+  "ok": true,
+}
+`);
+      expect(savedObjectsClient.create).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      Object {
+        "@created": "2019-02-12T21:01:22.479Z",
+        "@timestamp": "2019-02-12T21:01:22.479Z",
+        "foo": true,
+      },
+      Object {
+        "id": "123",
+      },
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+    });
+  });
+
+  describe(`PUT ${API_ROUTE_WORKPAD}/{id}`, () => {
+    test('formats successful response', async () => {
+      const request = {
+        method: 'PUT',
+        url: `${API_ROUTE_WORKPAD}/123`,
+        payload: {
+          id: '234',
+          foo: true,
+        },
+      };
+
+      savedObjectsClient.get.mockResolvedValueOnce({
+        attributes: {
+          '@created': new Date().toISOString(),
+        },
+      });
+      savedObjectsClient.create.mockResolvedValueOnce({});
+
+      const { payload, statusCode } = await mockServer.inject(request);
+      const response = JSON.parse(payload);
+
+      expect(statusCode).toBe(200);
+      expect(response).toMatchInlineSnapshot(`
+Object {
+  "ok": true,
+}
+`);
+      expect(savedObjectsClient.get).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      "123",
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+      expect(savedObjectsClient.create).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      Object {
+        "@created": "2019-02-12T21:01:22.479Z",
+        "@timestamp": "2019-02-12T21:01:22.479Z",
+        "foo": true,
+      },
+      Object {
+        "id": "123",
+        "overwrite": true,
+      },
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+    });
+  });
+
+  describe(`DELETE ${API_ROUTE_WORKPAD}/{id}`, () => {
+    test('formats successful response', async () => {
+      const request = {
+        method: 'DELETE',
+        url: `${API_ROUTE_WORKPAD}/123`,
+      };
+
+      savedObjectsClient.delete.mockResolvedValueOnce({});
+
+      const { payload, statusCode } = await mockServer.inject(request);
+      const response = JSON.parse(payload);
+
+      expect(statusCode).toBe(200);
+      expect(response).toMatchInlineSnapshot(`
+Object {
+  "ok": true,
+}
+`);
+      expect(savedObjectsClient.delete).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      "canvas-workpad",
+      "123",
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+    });
+  });
+
+  describe(`GET ${API_ROUTE_WORKPAD}/find`, async () => {
+    const request = {
+      method: 'GET',
+      url: `${API_ROUTE_WORKPAD}/find?name=abc&page=2&perPage=10`,
+    };
+
+    savedObjectsClient.find.mockResolvedValueOnce({
+      saved_objects: [
+        {
+          id: '1',
+          attributes: {
+            foo: true,
+          },
+        },
+      ],
+    });
+
+    const { payload, statusCode } = await mockServer.inject(request);
+    const response = JSON.parse(payload);
+
+    expect(statusCode).toBe(200);
+    expect(response).toMatchInlineSnapshot(`
+Object {
+  "workpads": Array [
+    Object {
+      "foo": true,
+      "id": "1",
+    },
+  ],
+}
+`);
+    expect(savedObjectsClient.find).toMatchInlineSnapshot(`
+[MockFunction] {
+  "calls": Array [
+    Array [
+      Object {
+        "fields": Array [
+          "id",
+          "name",
+          "@created",
+          "@timestamp",
+        ],
+        "page": "2",
+        "perPage": "10",
+        "search": "abc* | abc",
+        "searchFields": Array [
+          "name",
+        ],
+        "sortField": "@timestamp",
+        "sortOrder": "desc",
+        "type": "canvas-workpad",
+      },
+    ],
+  ],
+  "results": Array [
+    Object {
+      "isThrow": false,
+      "value": Promise {},
+    },
+  ],
+}
+`);
+  });
+});

x-pack/plugins/canvas/server/sample_data/ecommerce_saved_objects.json

@@ -4,10 +4,11 @@
     "type": "canvas-workpad",
     "updated_at": "2018-10-22T15:19:02.081Z",
     "version": 1,
-    "migrationVersion": {},
+    "migrationVersion": {
+      "canvas-workpad": "7.0.0"
+    },
     "attributes": {
       "name": "[eCommerce] Revenue Tracking",
-      "id": "workpad-e08b9bdb-ec14-4339-94c4-063bddfd610e",
       "width": 1080,
       "height": 720,
       "page": 0,

x-pack/plugins/canvas/server/sample_data/flights_saved_objects.json

@@ -4,10 +4,11 @@
     "type": "canvas-workpad",
     "updated_at": "2018-10-22T14:17:04.040Z",
     "version": 1,
-    "migrationVersion": {},
+    "migrationVersion": {
+      "canvas-workpad": "7.0.0"
+    },
     "attributes": {
       "name": "[Flights] Overview",
-      "id": "workpad-a474e74b-aedc-47c3-894a-db77e62c41e0",
       "width": 1280,
       "height": 720,
       "page": 0,

x-pack/plugins/canvas/server/sample_data/web_logs_saved_objects.json

@@ -4,10 +4,11 @@
     "type": "canvas-workpad",
     "updated_at": "2018-10-22T12:41:57.071Z",
     "version": 1,
-    "migrationVersion": {},
+    "migrationVersion": {
+      "canvas-workpad": "7.0.0"
+    },
     "attributes": {
       "name": "[Logs] Web Traffic",
-      "id": "workpad-5563cc40-5760-4afe-bf33-9da72fac53b7",
       "width": 1280,
       "height": 720,
       "page": 0,