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

Remove dependency on doc versions #29906

Merged
merged 24 commits into from Feb 5, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a53f0a2
tag discovered uses of version
Feb 2, 2019
f36cfd2
discovery round two
Feb 2, 2019
24cbf26
[savedObjects] version = _seq_no + _primary_term
Feb 2, 2019
62d3a73
[spaces] update spaces integration test fixtures
Feb 2, 2019
1af3057
[apiIntegration] update saved object api fixtures
Feb 2, 2019
e55ab0a
[saveObjects/types] fix version type
Feb 2, 2019
c528f78
update several uses of savedObjects versions to expect strings
Feb 2, 2019
35343fc
disable --bail
Feb 2, 2019
9212637
revert changes to fixture
Feb 2, 2019
20b7579
[infra/tests[ update assertions
Feb 2, 2019
b386f40
[reporting] audit version usage in esqueue
Feb 2, 2019
498301b
[courier] audit version usage
Feb 2, 2019
5645f7b
[ui/indexPatterns] audit version usage
Feb 2, 2019
a5550bb
audit some misc version usage
Feb 2, 2019
91363de
[taskManager] update to use seqNo/primaryTerm
Feb 3, 2019
077245f
[ml] job_version is a semver version for the ML api https://www.elast…
Feb 3, 2019
d563488
Merge branch 'master' of github.com:elastic/kibana into fix/remove-de…
Feb 3, 2019
52d890f
[beatscm] update to use seqNo/primaryTerm, remove unused method
Feb 3, 2019
5719035
[beatscm] these versions refer to release versions
Feb 4, 2019
3b95811
Revert "disable --bail"
Feb 4, 2019
8512cee
Merge branch 'master' of github.com:elastic/kibana into fix/remove-de…
Feb 4, 2019
9a9ee93
Merge branch 'master' of github.com:elastic/kibana into fix/remove-de…
Feb 4, 2019
935f7cc
[savedObjects/version] break up version mode and thoroughly test it
Feb 5, 2019
687e1ec
fix broken import
Feb 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -61,7 +61,6 @@ describe('plugins/elasticsearch', () => {

cluster = { callWithInternalUser: sinon.stub() };
cluster.callWithInternalUser.withArgs('index', sinon.match.any).returns(Promise.resolve());
cluster.callWithInternalUser.withArgs('create', sinon.match.any).returns(Promise.resolve({ _id: '1', _version: 1 }));
cluster.callWithInternalUser.withArgs('mget', sinon.match.any).returns(Promise.resolve({ ok: true }));
cluster.callWithInternalUser.withArgs('get', sinon.match.any).returns(Promise.resolve({ found: false }));
cluster.callWithInternalUser.withArgs('search', sinon.match.any).returns(Promise.resolve({ hits: { hits: [] } }));
Expand Down
2 changes: 1 addition & 1 deletion src/server/saved_objects/routes/bulk_create.js
Expand Up @@ -35,7 +35,7 @@ export const createBulkCreateRoute = prereqs => ({
type: Joi.string().required(),
id: Joi.string(),
attributes: Joi.object().required(),
version: Joi.number(),
version: Joi.string(),
migrationVersion: Joi.object().optional(),
references: Joi.array().items(
Joi.object()
Expand Down
2 changes: 1 addition & 1 deletion src/server/saved_objects/routes/bulk_get.test.js
Expand Up @@ -59,7 +59,7 @@ describe('POST /api/saved_objects/_bulk_get', () => {
id: 'abc123',
type: 'index-pattern',
title: 'logstash-*',
version: 2,
version: 'foo',
references: [],
}]
};
Expand Down
2 changes: 1 addition & 1 deletion src/server/saved_objects/routes/update.js
Expand Up @@ -32,7 +32,7 @@ export const createUpdateRoute = (prereqs) => {
}).required(),
payload: Joi.object({
attributes: Joi.object().required(),
version: Joi.number().min(1),
version: Joi.string(),
references: Joi.array().items(
Joi.object()
.keys({
Expand Down
2 changes: 1 addition & 1 deletion src/server/saved_objects/routes/update.test.js
Expand Up @@ -67,7 +67,7 @@ describe('PUT /api/saved_objects/{type}/{id?}', () => {

it('calls upon savedObjectClient.update', async () => {
const attributes = { title: 'Testing' };
const options = { version: 2, references: [] };
const options = { version: 'foo', references: [] };
const request = {
method: 'PUT',
url: '/api/saved_objects/index-pattern/logstash-*',
Expand Down
19 changes: 14 additions & 5 deletions src/server/saved_objects/serialization/index.ts
Expand Up @@ -24,6 +24,7 @@

import uuid from 'uuid';
import { SavedObjectsSchema } from '../schema';
import { decodeVersion, encodeVersion } from '../service/lib/version';

/**
* A raw document as represented directly in the saved object index.
Expand All @@ -32,7 +33,8 @@ export interface RawDoc {
_id: string;
_source: any;
_type?: string;
_version?: number;
_seq_no?: number;
_primary_term?: number;
}

/**
Expand Down Expand Up @@ -64,7 +66,7 @@ interface SavedObjectDoc {
type: string;
namespace?: string;
migrationVersion?: MigrationVersion;
version?: number;
version?: string;
updated_at?: Date;

[rootProp: string]: any;
Expand Down Expand Up @@ -116,8 +118,15 @@ export class SavedObjectsSerializer {
*
* @param {RawDoc} rawDoc - The raw ES document to be converted to saved object format.
*/
public rawToSavedObject({ _id, _source, _version }: RawDoc): SanitizedSavedObjectDoc {
public rawToSavedObject(doc: RawDoc): SanitizedSavedObjectDoc {
const { _id, _source, _seq_no, _primary_term } = doc;
const { type, namespace } = _source;

const version =
_seq_no != null || _primary_term != null
? encodeVersion(_seq_no!, _primary_term!)
: undefined;

return {
type,
id: this.trimIdPrefix(namespace, type, _id),
Expand All @@ -126,7 +135,7 @@ export class SavedObjectsSerializer {
references: _source.references || [],
...(_source.migrationVersion && { migrationVersion: _source.migrationVersion }),
...(_source.updated_at && { updated_at: _source.updated_at }),
...(_version != null && { version: _version }),
...(version && { version }),
};
}

Expand Down Expand Up @@ -158,7 +167,7 @@ export class SavedObjectsSerializer {
return {
_id: this.generateRawId(namespace, type, id),
_source: source,
...(version != null && { _version: version }),
...(version != null && decodeVersion(version)),
};
}

Expand Down
67 changes: 56 additions & 11 deletions src/server/saved_objects/serialization/serialization.test.ts
Expand Up @@ -20,6 +20,7 @@
import _ from 'lodash';
import { SavedObjectsSerializer } from '.';
import { SavedObjectsSchema } from '../schema';
import { encodeVersion } from '../service/lib/version';

describe('saved object conversion', () => {
describe('#rawToSavedObject', () => {
Expand Down Expand Up @@ -86,7 +87,8 @@ describe('saved object conversion', () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
const actual = serializer.rawToSavedObject({
_id: 'hello:world',
_version: 3,
_seq_no: 3,
_primary_term: 1,
_source: {
type: 'hello',
hello: {
Expand All @@ -103,7 +105,7 @@ describe('saved object conversion', () => {
const expected = {
id: 'world',
type: 'hello',
version: 3,
version: encodeVersion(3, 1),
attributes: {
a: 'b',
c: 'd',
Expand All @@ -130,17 +132,46 @@ describe('saved object conversion', () => {
expect(actual).not.toHaveProperty('version');
});

test(`if specified it copies _version to version`, () => {
test(`if specified it encodes _seq_no and _primary_term to version`, () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
const actual = serializer.rawToSavedObject({
_id: 'foo:bar',
_version: 4,
_seq_no: 4,
_primary_term: 1,
_source: {
type: 'foo',
hello: {},
},
});
expect(actual).toHaveProperty('version', 4);
expect(actual).toHaveProperty('version', encodeVersion(4, 1));
spalger marked this conversation as resolved.
Show resolved Hide resolved
});

test(`if only _seq_no is specified it throws`, () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
expect(() =>
serializer.rawToSavedObject({
_id: 'foo:bar',
_seq_no: 4,
_source: {
type: 'foo',
hello: {},
},
})
).toThrowErrorMatchingInlineSnapshot(`"_primary_term from elasticsearch must be an integer"`);
});

test(`if only _primary_term is throws`, () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
expect(() =>
serializer.rawToSavedObject({
_id: 'foo:bar',
_primary_term: 1,
_source: {
type: 'foo',
hello: {},
},
})
).toThrowErrorMatchingInlineSnapshot(`"_seq_no from elasticsearch must be an integer"`);
});

test('if specified it copies the _source.updated_at property to updated_at', () => {
Expand Down Expand Up @@ -222,7 +253,8 @@ describe('saved object conversion', () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
const raw = {
_id: 'foo-namespace:foo:bar',
_version: 24,
_primary_term: 24,
_seq_no: 42,
_source: {
type: 'foo',
foo: {
Expand Down Expand Up @@ -473,25 +505,38 @@ describe('saved object conversion', () => {
expect(actual._source).not.toHaveProperty('migrationVersion');
});

test('it copies the version property to _version', () => {
test('it decodes the version property to _seq_no and _primary_term', () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
const actual = serializer.savedObjectToRaw({
type: '',
attributes: {},
version: 4,
version: encodeVersion(1, 2),
} as any);

expect(actual).toHaveProperty('_version', 4);
expect(actual).toHaveProperty('_seq_no', 1);
expect(actual).toHaveProperty('_primary_term', 2);
});

test(`if unspecified it doesn't add _version property`, () => {
test(`if unspecified it doesn't add _seq_no or _primary_term properties`, () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
const actual = serializer.savedObjectToRaw({
type: '',
attributes: {},
} as any);

expect(actual).not.toHaveProperty('_version');
expect(actual).not.toHaveProperty('_seq_no');
expect(actual).not.toHaveProperty('_primary_term');
});

test(`if version invalid it throws`, () => {
const serializer = new SavedObjectsSerializer(new SavedObjectsSchema());
expect(() =>
serializer.savedObjectToRaw({
type: '',
attributes: {},
version: 'foo',
} as any)
).toThrowErrorMatchingInlineSnapshot(`"Invalid version [foo]"`);
});

test('it copies attributes to _source[type]', () => {
Expand Down
3 changes: 3 additions & 0 deletions src/server/saved_objects/service/lib/errors.d.ts
Expand Up @@ -25,3 +25,6 @@ export function isNotFoundError(maybeError: any): boolean;
export function isConflictError(maybeError: any): boolean;
export function isEsUnavailableError(maybeError: any): boolean;
export function isEsAutoCreateIndexError(maybeError: any): boolean;

export function createInvalidVersionError(version: any): Error;
export function isInvalidVersionError(maybeError: Error): boolean;
9 changes: 9 additions & 0 deletions src/server/saved_objects/service/lib/errors.js
Expand Up @@ -50,6 +50,15 @@ export function isBadRequestError(error) {
return error && error[code] === CODE_BAD_REQUEST;
}

// 400 - invalid version
const CODE_INVALID_VERSION = 'SavedObjectsClient/invalidVersion';
export function createInvalidVersionError(versionInput) {
return decorate(Boom.badRequest(`Invalid version [${versionInput}]`), CODE_INVALID_VERSION, 400);
}
export function isInvalidVersionError(error) {
return error && error[code] === CODE_INVALID_VERSION;
}


// 401 - Not Authorized
const CODE_NOT_AUTHORIZED = 'SavedObjectsClient/notAuthorized';
Expand Down
21 changes: 11 additions & 10 deletions src/server/saved_objects/service/lib/repository.js
Expand Up @@ -23,6 +23,7 @@ import { getSearchDsl } from './search_dsl';
import { includedFields } from './included_fields';
import { decorateEsError } from './decorate_es_error';
import * as errors from './errors';
import { decodeRequestVersion, encodeVersion, encodeHitVersion } from './version';

// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
Expand Down Expand Up @@ -173,7 +174,8 @@ export class SavedObjectsRepository {
const {
error,
_id: responseId,
_version: version,
_seq_no: seqNo,
_primary_term: primaryTerm,
} = Object.values(response)[0];

const {
Expand Down Expand Up @@ -208,7 +210,7 @@ export class SavedObjectsRepository {
id,
type,
updated_at: time,
version,
version: encodeVersion(seqNo, primaryTerm),
attributes,
references,
};
Expand Down Expand Up @@ -262,7 +264,6 @@ export class SavedObjectsRepository {
* @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures }
*/
async deleteByNamespace(namespace) {

if (!namespace || typeof namespace !== 'string') {
throw new TypeError(`namespace is required, and must be a string`);
}
Expand Down Expand Up @@ -338,7 +339,7 @@ export class SavedObjectsRepository {
ignore: [404],
rest_total_hits_as_int: true,
body: {
version: true,
seq_no_primary_term: true,
...getSearchDsl(this._mappings, this._schema, {
search,
defaultSearchOperator,
Expand Down Expand Up @@ -423,7 +424,7 @@ export class SavedObjectsRepository {
id,
type,
...time && { updated_at: time },
version: doc._version,
version: encodeHitVersion(doc),
attributes: doc._source[type],
references: doc._source.references || [],
migrationVersion: doc._source.migrationVersion,
Expand Down Expand Up @@ -466,7 +467,7 @@ export class SavedObjectsRepository {
id,
type,
...updatedAt && { updated_at: updatedAt },
version: response._version,
version: encodeHitVersion(response),
attributes: response._source[type],
references: response._source.references || [],
migrationVersion: response._source.migrationVersion,
Expand All @@ -479,7 +480,7 @@ export class SavedObjectsRepository {
* @param {string} type
* @param {string} id
* @param {object} [options={}]
* @property {integer} options.version - ensures version matches that of persisted object
* @property {string} options.version - ensures version matches that of persisted object
* @property {string} [options.namespace]
* @property {array} [options.references] - [{ name, type, id }]
* @returns {promise}
Expand All @@ -496,7 +497,7 @@ export class SavedObjectsRepository {
id: this._serializer.generateRawId(namespace, type, id),
type: this._type,
index: this._index,
version,
...(version && decodeRequestVersion(version)),
refresh: 'wait_for',
ignore: [404],
body: {
Expand All @@ -517,7 +518,7 @@ export class SavedObjectsRepository {
id,
type,
updated_at: time,
version: response._version,
version: encodeHitVersion(response),
references,
attributes
};
Expand Down Expand Up @@ -593,7 +594,7 @@ export class SavedObjectsRepository {
type,
updated_at: time,
references: response.get._source.references,
version: response._version,
version: encodeHitVersion(response),
attributes: response.get._source[type],
};

Expand Down