From 6c09a674cfc12e01572bfa54d6b5777418e016d9 Mon Sep 17 00:00:00 2001 From: Court Ewing Date: Fri, 14 Jul 2017 14:30:11 -0400 Subject: [PATCH] Uses uuid for auto-generated ids and prepends type (#12834) (#12877) * Uses uuid for auto-generated ids and prepends type (#12834) Signed-off-by: Tyler Smalley * saved_objects: remove isSingleTypeError reference --- .../saved_objects_client_mappings.js | 150 +++++++++++------- .../saved_objects/client/lib/compatibility.js | 3 +- .../client/lib/handle_es_error.js | 11 -- src/server/saved_objects/client/lib/index.js | 2 +- .../client/saved_objects_client.js | 15 +- 5 files changed, 106 insertions(+), 75 deletions(-) diff --git a/src/server/saved_objects/client/__tests__/saved_objects_client_mappings.js b/src/server/saved_objects/client/__tests__/saved_objects_client_mappings.js index 19ef0d9f84aa9..05b555ec1b346 100644 --- a/src/server/saved_objects/client/__tests__/saved_objects_client_mappings.js +++ b/src/server/saved_objects/client/__tests__/saved_objects_client_mappings.js @@ -8,10 +8,7 @@ const { BadRequest } = elasticsearch.errors; describe('SavedObjectsClient', () => { let callAdminCluster; let savedObjectsClient; - const illegalArgumentException = { - type: 'illegal_argument_exception', - reason: 'Rejecting mapping update to [.kibana-v6] as the final mapping would have more than 1 type: [doc, foo]' - }; + const illegalArgumentException = { type: 'type_missing_exception' }; describe('mapping', () => { beforeEach(() => { @@ -25,7 +22,7 @@ describe('SavedObjectsClient', () => { describe('#create', () => { - it('falls back to v6 mapping', async () => { + it('falls back to single-type mapping', async () => { const error = new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', { body: { error: illegalArgumentException @@ -49,48 +46,66 @@ describe('SavedObjectsClient', () => { } }); }); + + it('prepends id for single-type', async () => { + const id = 'foo'; + const error = new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', { + body: { + error: illegalArgumentException + } + }); + + callAdminCluster + .onFirstCall().throws(error) + .onSecondCall().returns(Promise.resolve()); + + await savedObjectsClient.create('index-pattern', {}, { id }); + + const [, args] = callAdminCluster.getCall(1).args; + expect(args.id).to.eql('index-pattern:foo'); + }); }); describe('#bulkCreate', () => { - it('falls back to v6 mappings', async () => { - const firstResponse = { - errors: true, - items: [{ - create: { - _type: 'config', - _id: 'one', - _version: 2, - status: 400, - error: illegalArgumentException - } - }, { - create: { - _type: 'index-pattern', - _id: 'two', - _version: 2, - status: 400, - error: illegalArgumentException - } - }] - }; - - const secondResponse = { - errors: false, - items: [{ - create: { - _type: 'config', - _id: 'one', - _version: 2 - } - }, { - create: { - _type: 'index-pattern', - _id: 'two', - _version: 2 - } - }] - }; + const firstResponse = { + errors: true, + items: [{ + create: { + _type: 'config', + _id: 'one', + _version: 2, + status: 400, + error: illegalArgumentException + } + }, { + create: { + _type: 'index-pattern', + _id: 'two', + _version: 2, + status: 400, + error: illegalArgumentException + } + }] + }; + + const secondResponse = { + errors: false, + items: [{ + create: { + _type: 'config', + _id: 'one', + _version: 2 + } + }, { + create: { + _type: 'index-pattern', + _id: 'two', + _version: 2 + } + }] + }; + it('falls back to single-type mappings', async () => { callAdminCluster .onFirstCall().returns(Promise.resolve(firstResponse)) .onSecondCall().returns(Promise.resolve(secondResponse)); @@ -116,23 +131,38 @@ describe('SavedObjectsClient', () => { } ]); }); + + it('prepends id for single-type', async () => { + callAdminCluster + .onFirstCall().returns(Promise.resolve(firstResponse)) + .onSecondCall().returns(Promise.resolve(secondResponse)); + + await savedObjectsClient.bulkCreate([ + { type: 'config', id: 'one', attributes: { title: 'Test One' } }, + { type: 'index-pattern', id: 'two', attributes: { title: 'Test Two' } } + ]); + + const [, { body }] = callAdminCluster.getCall(1).args; + expect(body[0].create._id).to.eql('config:one'); + expect(body[2].create._id).to.eql('index-pattern:two'); + // expect(args.id).to.eql('index-pattern:foo'); + }); }); describe('update', () => { - it('falls back to v6 mappings', async () => { - const id = 'logstash-*'; - const type = 'index-pattern'; - const version = 2; - const attributes = { title: 'Testing' }; - - const error = new BadRequest('[document_missing_exception] [config][logstash-*]: document missing', { - body: { - error: { - type: 'document_missing_exception' - } + const id = 'logstash-*'; + const type = 'index-pattern'; + const version = 2; + const attributes = { title: 'Testing' }; + const error = new BadRequest('[document_missing_exception] [config][logstash-*]: document missing', { + body: { + error: { + type: 'document_missing_exception' } - }); + } + }); + beforeEach(() => { callAdminCluster .onFirstCall().throws(error) .onSecondCall().returns(Promise.resolve({ @@ -141,7 +171,10 @@ describe('SavedObjectsClient', () => { _version: version, result: 'updated' })); + }); + + it('falls back to single-type mappings', async () => { const response = await savedObjectsClient.update('index-pattern', 'logstash-*', attributes); expect(response).to.eql({ id, @@ -150,6 +183,13 @@ describe('SavedObjectsClient', () => { attributes }); }); + + it('prepends id for single-type', async () => { + await savedObjectsClient.update('index-pattern', 'logstash-*', attributes); + + const [, args] = callAdminCluster.getCall(1).args; + expect(args.id).to.eql('index-pattern:logstash-*'); + }); }); }); }); diff --git a/src/server/saved_objects/client/lib/compatibility.js b/src/server/saved_objects/client/lib/compatibility.js index 3d78bf19c2105..27ef4da99bb3c 100644 --- a/src/server/saved_objects/client/lib/compatibility.js +++ b/src/server/saved_objects/client/lib/compatibility.js @@ -1,3 +1,4 @@ +import uuid from 'uuid'; import { V6_TYPE } from '../saved_objects_client'; /** @@ -29,7 +30,7 @@ export function v6BulkCreate(objects, options = {}) { acc.push({ [method]: { _type: V6_TYPE, - _id: object.id ? `${object.type}:${object.id}` : undefined + _id: `${object.type}:${object.id || uuid.v1()}`, } }); acc.push(Object.assign({}, diff --git a/src/server/saved_objects/client/lib/handle_es_error.js b/src/server/saved_objects/client/lib/handle_es_error.js index 725d954a1ac61..87bdcf44841b0 100644 --- a/src/server/saved_objects/client/lib/handle_es_error.js +++ b/src/server/saved_objects/client/lib/handle_es_error.js @@ -13,13 +13,6 @@ const { BadRequest } = elasticsearch.errors; -export function isSingleTypeError(error) { - if (!error) return; - - return error.type === 'illegal_argument_exception' && - error.reason.match(/the final mapping would have more than 1 type/); -} - export function handleEsError(error) { if (!(error instanceof Error)) { throw new Error('Expected an instance of Error'); @@ -50,10 +43,6 @@ export function handleEsError(error) { } if (error instanceof BadRequest) { - if (isSingleTypeError(get(error, 'body.error'))) { - details.type = 'is_single_type'; - } - throw Boom.badRequest(reason, details); } diff --git a/src/server/saved_objects/client/lib/index.js b/src/server/saved_objects/client/lib/index.js index f5973cdc01428..2db84b11d49ec 100644 --- a/src/server/saved_objects/client/lib/index.js +++ b/src/server/saved_objects/client/lib/index.js @@ -1,6 +1,6 @@ export { createFindQuery } from './create_find_query'; export { createIdQuery } from './create_id_query'; -export { handleEsError, isSingleTypeError } from './handle_es_error'; +export { handleEsError } from './handle_es_error'; export { v5BulkCreate, v6BulkCreate } from './compatibility'; export { normalizeEsDoc } from './normalize_es_doc'; export { includedFields } from './included_fields'; diff --git a/src/server/saved_objects/client/saved_objects_client.js b/src/server/saved_objects/client/saved_objects_client.js index 578c3281dbdc0..cd69c34fd6d1b 100644 --- a/src/server/saved_objects/client/saved_objects_client.js +++ b/src/server/saved_objects/client/saved_objects_client.js @@ -1,11 +1,11 @@ import Boom from 'boom'; +import uuid from 'uuid'; import { get } from 'lodash'; import { createFindQuery, createIdQuery, handleEsError, - isSingleTypeError, v5BulkCreate, v6BulkCreate, normalizeEsDoc, @@ -40,7 +40,7 @@ export class SavedObjectsClient { refresh: 'wait_for' }, { type: V6_TYPE, - id: options.id ? `${type}:${options.id}` : undefined, + id: `${type}:${options.id || uuid.v1()}`, body: { type, [type]: attributes @@ -71,7 +71,7 @@ export class SavedObjectsClient { const items = get(response, 'items', []); const missingTypesCount = items.filter(item => { const method = Object.keys(item)[0]; - return isSingleTypeError(get(item, `${method}.error`)); + return get(item, `${method}.error.type`) === 'type_missing_exception'; }).length; const formatFallback = format === 'v5' && items.length > 0 && items.length === missingTypesCount; @@ -82,10 +82,10 @@ export class SavedObjectsClient { return get(response, 'items', []).map((resp, i) => { const method = Object.keys(resp)[0]; - const { id, type, attributes } = objects[i]; + const { type, attributes } = objects[i]; return normalizeEsDoc(resp[method], { - id, + id: resp[method]._id, type, attributes, error: resp[method].error ? { message: get(resp[method], 'error.reason') } : undefined @@ -233,6 +233,7 @@ export class SavedObjectsClient { } }, { type: V6_TYPE, + id: `${type}:${id}`, body: { doc: { [type]: attributes @@ -245,8 +246,8 @@ export class SavedObjectsClient { _withKibanaIndexAndMappingFallback(method, params, fallbackParams) { const fallbacks = { - 'create': ['is_single_type'], - 'index': ['is_single_type'], + 'create': ['type_missing_exception'], + 'index': ['type_missing_exception'], 'update': ['document_missing_exception'] };