mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
SavedObjects bulkCreate API should return migrationVersion and strip the type & namespace from the id (#65150)
* Deserialize bulkCreate response to remove namespace type from id
* Index operations don't return _source in response
* Fix integration tests
* repository: make id generation and seq_no/primary_term spreading more explicit
* API Integration test for bulk create without ids
* Fix copy_to_space snapshot
* Revert "Fix copy_to_space snapshot"
This reverts commit 9c2b7433e3.
* Move test into returns block
* repository.test.js stricter regexp matching
This commit is contained in:
Rudolf Meijering
GitHub
parent
86bafec3a4
commit
1baf0b0e47
3 changed files with 91 additions and 30 deletions
|
|
@ -23,6 +23,7 @@ import { SavedObjectsErrorHelpers } from './errors';
|
|||
import { SavedObjectsSerializer } from '../../serialization';
|
||||
import { encodeHitVersion } from '../../version';
|
||||
import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry';
|
||||
import { DocumentMigrator } from '../../migrations/core/document_migrator';
|
||||
|
||||
jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: jest.fn() }));
|
||||
|
||||
|
|
@ -115,6 +116,7 @@ describe('SavedObjectsRepository', () => {
|
|||
const createType = type => ({
|
||||
name: type,
|
||||
mappings: { properties: mappings.properties[type].properties },
|
||||
migrations: { '1.1.1': doc => doc },
|
||||
});
|
||||
|
||||
const registry = new SavedObjectTypeRegistry();
|
||||
|
|
@ -144,6 +146,13 @@ describe('SavedObjectsRepository', () => {
|
|||
namespaceType: 'agnostic',
|
||||
});
|
||||
|
||||
const documentMigrator = new DocumentMigrator({
|
||||
typeRegistry: registry,
|
||||
kibanaVersion: '2.0.0',
|
||||
log: {},
|
||||
validateDoc: jest.fn(),
|
||||
});
|
||||
|
||||
const getMockGetResponse = ({ type, id, references, namespace }) => ({
|
||||
// NOTE: Elasticsearch returns more fields (_index, _type) but the SavedObjectsRepository method ignores these
|
||||
found: true,
|
||||
|
|
@ -207,7 +216,7 @@ describe('SavedObjectsRepository', () => {
|
|||
beforeEach(() => {
|
||||
callAdminCluster = jest.fn();
|
||||
migrator = {
|
||||
migrateDocument: jest.fn(doc => doc),
|
||||
migrateDocument: jest.fn().mockImplementation(documentMigrator.migrate),
|
||||
runMigrations: async () => ({ status: 'skipped' }),
|
||||
};
|
||||
|
||||
|
|
@ -424,9 +433,17 @@ describe('SavedObjectsRepository', () => {
|
|||
|
||||
const getMockBulkCreateResponse = (objects, namespace) => {
|
||||
return {
|
||||
items: objects.map(({ type, id }) => ({
|
||||
items: objects.map(({ type, id, attributes, references, migrationVersion }) => ({
|
||||
create: {
|
||||
_id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
|
||||
_source: {
|
||||
[type]: attributes,
|
||||
type,
|
||||
namespace,
|
||||
references,
|
||||
...mockTimestampFields,
|
||||
migrationVersion: migrationVersion || { [type]: '1.1.1' },
|
||||
},
|
||||
...mockVersionProps,
|
||||
},
|
||||
})),
|
||||
|
|
@ -474,7 +491,7 @@ describe('SavedObjectsRepository', () => {
|
|||
|
||||
const expectSuccessResult = obj => ({
|
||||
...obj,
|
||||
migrationVersion: undefined,
|
||||
migrationVersion: { [obj.type]: '1.1.1' },
|
||||
version: mockVersion,
|
||||
...mockTimestampFields,
|
||||
});
|
||||
|
|
@ -619,13 +636,16 @@ describe('SavedObjectsRepository', () => {
|
|||
};
|
||||
|
||||
const bulkCreateError = async (obj, esError, expectedError) => {
|
||||
const objects = [obj1, obj, obj2];
|
||||
const response = getMockBulkCreateResponse(objects);
|
||||
let response;
|
||||
if (esError) {
|
||||
response = getMockBulkCreateResponse([obj1, obj, obj2]);
|
||||
response.items[1].create = { error: esError };
|
||||
} else {
|
||||
response = getMockBulkCreateResponse([obj1, obj2]);
|
||||
}
|
||||
callAdminCluster.mockResolvedValue(response); // this._writeToCluster('bulk', ...)
|
||||
|
||||
const objects = [obj1, obj, obj2];
|
||||
const result = await savedObjectsRepository.bulkCreate(objects);
|
||||
expectClusterCalls('bulk');
|
||||
const objCall = esError ? expectObjArgs(obj) : [];
|
||||
|
|
@ -781,7 +801,7 @@ describe('SavedObjectsRepository', () => {
|
|||
id: 'three',
|
||||
};
|
||||
const objects = [obj1, obj, obj2];
|
||||
const response = getMockBulkCreateResponse(objects);
|
||||
const response = getMockBulkCreateResponse([obj1, obj2]);
|
||||
callAdminCluster.mockResolvedValue(response); // this._writeToCluster('bulk', ...)
|
||||
const result = await savedObjectsRepository.bulkCreate(objects);
|
||||
expect(callAdminCluster).toHaveBeenCalledTimes(1);
|
||||
|
|
@ -789,6 +809,32 @@ describe('SavedObjectsRepository', () => {
|
|||
saved_objects: [expectSuccessResult(obj1), expectError(obj), expectSuccessResult(obj2)],
|
||||
});
|
||||
});
|
||||
|
||||
it(`a deserialized saved object`, async () => {
|
||||
// Test for fix to https://github.com/elastic/kibana/issues/65088 where
|
||||
// we returned raw ID's when an object without an id was created.
|
||||
const namespace = 'myspace';
|
||||
const response = getMockBulkCreateResponse([obj1, obj2], namespace);
|
||||
callAdminCluster.mockResolvedValueOnce(response); // this._writeToCluster('bulk', ...)
|
||||
|
||||
// Bulk create one object with id unspecified, and one with id specified
|
||||
const result = await savedObjectsRepository.bulkCreate([{ ...obj1, id: undefined }, obj2], {
|
||||
namespace,
|
||||
});
|
||||
|
||||
// Assert that both raw docs from the ES response are deserialized
|
||||
expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(1, {
|
||||
...response.items[0].create,
|
||||
_id: expect.stringMatching(/^myspace:config:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/),
|
||||
});
|
||||
expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(2, response.items[1].create);
|
||||
|
||||
// Assert that ID's are deserialized to remove the type and namespace
|
||||
expect(result.saved_objects[0].id).toEqual(
|
||||
expect.stringMatching(/^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/)
|
||||
);
|
||||
expect(result.saved_objects[1].id).toEqual(obj2.id);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -1604,6 +1650,7 @@ describe('SavedObjectsRepository', () => {
|
|||
version: mockVersion,
|
||||
attributes,
|
||||
references,
|
||||
migrationVersion: { [type]: '1.1.1' },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@
|
|||
*/
|
||||
|
||||
import { omit } from 'lodash';
|
||||
import uuid from 'uuid';
|
||||
import { retryCallCluster } from '../../../elasticsearch/retry_call_cluster';
|
||||
import { APICaller } from '../../../elasticsearch/';
|
||||
|
||||
|
|
@ -299,6 +300,8 @@ export class SavedObjectsRepository {
|
|||
const requiresNamespacesCheck =
|
||||
method === 'index' && this._registry.isMultiNamespace(object.type);
|
||||
|
||||
if (object.id == null) object.id = uuid.v1();
|
||||
|
||||
return {
|
||||
tag: 'Right' as 'Right',
|
||||
value: {
|
||||
|
|
@ -404,35 +407,25 @@ export class SavedObjectsRepository {
|
|||
}
|
||||
|
||||
const { requestedId, rawMigratedDoc, esRequestIndex } = expectedResult.value;
|
||||
const response = bulkResponse.items[esRequestIndex];
|
||||
const {
|
||||
error,
|
||||
_id: responseId,
|
||||
_seq_no: seqNo,
|
||||
_primary_term: primaryTerm,
|
||||
} = Object.values(response)[0] as any;
|
||||
const { error, ...rawResponse } = Object.values(
|
||||
bulkResponse.items[esRequestIndex]
|
||||
)[0] as any;
|
||||
|
||||
const {
|
||||
_source: { type, [type]: attributes, references = [], namespaces },
|
||||
} = rawMigratedDoc;
|
||||
|
||||
const id = requestedId || responseId;
|
||||
if (error) {
|
||||
return {
|
||||
id,
|
||||
type,
|
||||
error: getBulkOperationError(error, type, id),
|
||||
id: requestedId,
|
||||
type: rawMigratedDoc._source.type,
|
||||
error: getBulkOperationError(error, rawMigratedDoc._source.type, requestedId),
|
||||
};
|
||||
}
|
||||
return {
|
||||
id,
|
||||
type,
|
||||
...(namespaces && { namespaces }),
|
||||
updated_at: time,
|
||||
version: encodeVersion(seqNo, primaryTerm),
|
||||
attributes,
|
||||
references,
|
||||
};
|
||||
|
||||
// When method == 'index' the bulkResponse doesn't include the indexed
|
||||
// _source so we return rawMigratedDoc but have to spread the latest
|
||||
// _seq_no and _primary_term values from the rawResponse.
|
||||
return this._serializer.rawToSavedObject({
|
||||
...rawMigratedDoc,
|
||||
...{ _seq_no: rawResponse._seq_no, _primary_term: rawResponse._primary_term },
|
||||
});
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -72,11 +72,26 @@ export default function({ getService }) {
|
|||
attributes: {
|
||||
title: 'A great new dashboard',
|
||||
},
|
||||
migrationVersion: {
|
||||
dashboard: resp.body.saved_objects[1].migrationVersion.dashboard,
|
||||
},
|
||||
references: [],
|
||||
},
|
||||
],
|
||||
});
|
||||
}));
|
||||
|
||||
it('should not return raw id when object id is unspecified', async () =>
|
||||
await supertest
|
||||
.post(`/api/saved_objects/_bulk_create`)
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
.send(BULK_REQUESTS.map(({ id, ...rest }) => rest))
|
||||
.expect(200)
|
||||
.then(resp => {
|
||||
resp.body.saved_objects.map(({ id }) =>
|
||||
expect(id).not.match(/visualization|dashboard/)
|
||||
);
|
||||
}));
|
||||
});
|
||||
|
||||
describe('without kibana index', () => {
|
||||
|
|
@ -106,6 +121,9 @@ export default function({ getService }) {
|
|||
title: 'An existing visualization',
|
||||
},
|
||||
references: [],
|
||||
migrationVersion: {
|
||||
visualization: resp.body.saved_objects[0].migrationVersion.visualization,
|
||||
},
|
||||
},
|
||||
{
|
||||
type: 'dashboard',
|
||||
|
|
@ -116,6 +134,9 @@ export default function({ getService }) {
|
|||
title: 'A great new dashboard',
|
||||
},
|
||||
references: [],
|
||||
migrationVersion: {
|
||||
dashboard: resp.body.saved_objects[1].migrationVersion.dashboard,
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue