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

[Saved Objects] Update the migrationVersion property to hold a plain string value #150075

Merged
merged 12 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dev_docs/tutorials/saved_objects.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ Having said that, if a document is encountered that is not in the expected shape
fail an upgrade than to silently ignore a corrupt document which can cause unexpected behaviour at some future point in time. When such a scenario is encountered,
the error should be verbose and informative so that the corrupt document can be corrected, if possible.

**WARNING:** Do not attempt to change the `migrationVersion`, `id`, or `type` fields within a migration function, this is not supported.
**WARNING:** Do not attempt to change the `typeMigrationVersion`, `id`, or `type` fields within a migration function, this is not supported.

### Testing Migrations

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,15 @@ export interface SavedObjectsCreateOptions {
id?: string;
/** If a document with the given `id` already exists, overwrite it's contents (default=false). */
overwrite?: boolean;
/** {@inheritDoc SavedObjectsMigrationVersion} */
/**
* {@inheritDoc SavedObjectsMigrationVersion}
* @deprecated
*/
migrationVersion?: SavedObjectsMigrationVersion;
/** A semver value that is used when upgrading objects between Kibana versions. */
coreMigrationVersion?: string;
/** A semver value that is used when migrating documents between Kibana versions. */
typeMigrationVersion?: string;
/** Array of referenced saved objects. */
references?: SavedObjectReference[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ export interface SavedObjectsClientContract {
* @param {object} attributes - the attributes to update
* @param {object} options {@link SavedObjectsUpdateOptions}
* @prop {integer} options.version - ensures version matches that of persisted object
* @prop {object} options.migrationVersion - The optional migrationVersion of this document
* @returns the udpated simple saved object
* @deprecated See https://github.com/elastic/kibana/issues/149098
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ export interface SimpleSavedObject<T = unknown> {
id: SavedObjectType<T>['id'];
/** Type of the saved object */
type: SavedObjectType<T>['type'];
/** Migration version of the saved object */
/**
* Migration version of the saved object
* @deprecated
*/
migrationVersion: SavedObjectType<T>['migrationVersion'];
/** Core migration version of the saved object */
coreMigrationVersion: SavedObjectType<T>['coreMigrationVersion'];
/** Core migration version of the saved object */
typeMigrationVersion: SavedObjectType<T>['typeMigrationVersion'];
/** Error associated with this object, undefined if no error */
error: SavedObjectType<T>['error'];
/** References to other saved objects */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('getRootFields', () => {
"references",
"migrationVersion",
"coreMigrationVersion",
"typeMigrationVersion",
"updated_at",
"created_at",
"originId",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const ROOT_FIELDS = [
'references',
'migrationVersion',
'coreMigrationVersion',
'typeMigrationVersion',
'updated_at',
'created_at',
'originId',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('#getSavedObjectFromSource', () => {
const references = [{ type: 'ref-type', id: 'ref-id', name: 'ref-name' }];
const migrationVersion = { foo: 'migrationVersion' };
const coreMigrationVersion = 'coreMigrationVersion';
const typeMigrationVersion = 'typeMigrationVersion';
const originId = 'originId';
// eslint-disable-next-line @typescript-eslint/naming-convention
const updated_at = 'updatedAt';
Expand All @@ -112,6 +113,7 @@ describe('#getSavedObjectFromSource', () => {
references,
migrationVersion,
coreMigrationVersion,
typeMigrationVersion,
originId,
updated_at,
...namespaceAttrs,
Expand All @@ -127,6 +129,7 @@ describe('#getSavedObjectFromSource', () => {
expect(result).toEqual({
attributes,
coreMigrationVersion,
typeMigrationVersion,
id,
migrationVersion,
namespaces: expect.anything(), // see specific test cases below
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export function getSavedObjectFromSource<T>(
references: doc._source.references || [],
migrationVersion: doc._source.migrationVersion,
coreMigrationVersion: doc._source.coreMigrationVersion,
typeMigrationVersion: doc._source.typeMigrationVersion,
dokmic marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,8 @@ describe('SavedObjectsRepository', () => {
_source: {
...response.items[0].create._source,
namespaces: response.items[0].create._source.namespaces,
coreMigrationVersion: expect.any(String),
typeMigrationVersion: '1.1.1',
},
_id: expect.stringMatching(/^myspace:config:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$/),
});
Expand All @@ -942,6 +944,8 @@ describe('SavedObjectsRepository', () => {
_source: {
...response.items[1].create._source,
namespaces: response.items[1].create._source.namespaces,
coreMigrationVersion: expect.any(String),
typeMigrationVersion: '1.1.1',
},
});

Expand Down Expand Up @@ -2946,7 +2950,8 @@ describe('SavedObjectsRepository', () => {
attributes,
references,
namespaces: [namespace ?? 'default'],
migrationVersion: { [MULTI_NAMESPACE_TYPE]: '1.1.1' },
coreMigrationVersion: expect.any(String),
typeMigrationVersion: '1.1.1',
});
});
});
Expand Down Expand Up @@ -3533,6 +3538,7 @@ describe('SavedObjectsRepository', () => {
'references',
'migrationVersion',
'coreMigrationVersion',
'typeMigrationVersion',
'updated_at',
'created_at',
'originId',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
const {
migrationVersion,
coreMigrationVersion,
typeMigrationVersion,
overwrite = false,
references = [],
refresh = DEFAULT_REFRESH_SETTING,
Expand Down Expand Up @@ -381,6 +382,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
),
migrationVersion,
coreMigrationVersion,
typeMigrationVersion,
created_at: time,
updated_at: time,
...(Array.isArray(references) && { references }),
Expand Down Expand Up @@ -591,6 +593,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
),
migrationVersion: object.migrationVersion,
coreMigrationVersion: object.coreMigrationVersion,
typeMigrationVersion: object.typeMigrationVersion,
...(savedObjectNamespace && { namespace: savedObjectNamespace }),
...(savedObjectNamespaces && { namespaces: savedObjectNamespaces }),
updated_at: time,
Expand Down Expand Up @@ -2311,6 +2314,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
): Promise<SavedObject<T>> {
const {
migrationVersion,
typeMigrationVersion,
refresh = DEFAULT_REFRESH_SETTING,
initialize = false,
upsertAttributes,
Expand Down Expand Up @@ -2384,6 +2388,7 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
}, {} as Record<string, number>),
},
migrationVersion,
typeMigrationVersion,
updated_at: time,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,23 +589,25 @@ export const getMockBulkCreateResponse = (
return {
errors: false,
took: 1,
items: objects.map(({ type, id, originId, attributes, references, migrationVersion }) => ({
create: {
// status: 1,
// _index: '.kibana',
_id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
_source: {
[type]: attributes,
type,
namespace,
...(originId && { originId }),
references,
...mockTimestampFieldsWithCreated,
migrationVersion: migrationVersion || { [type]: '1.1.1' },
items: objects.map(
({ type, id, originId, attributes, references, migrationVersion, typeMigrationVersion }) => ({
create: {
// status: 1,
// _index: '.kibana',
_id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
_source: {
[type]: attributes,
type,
namespace,
...(originId && { originId }),
references,
...mockTimestampFieldsWithCreated,
typeMigrationVersion: typeMigrationVersion || migrationVersion?.[type] || '1.1.1',
},
...mockVersionProps,
},
...mockVersionProps,
},
})),
})
),
} as unknown as estypes.BulkResponse;
};

Expand All @@ -627,7 +629,8 @@ export const expectCreateResult = (obj: {
namespaces?: string[];
}) => ({
...obj,
migrationVersion: { [obj.type]: '1.1.1' },
coreMigrationVersion: expect.any(String),
typeMigrationVersion: '1.1.1',
version: mockVersion,
namespaces: obj.namespaces ?? [obj.namespace ?? 'default'],
...mockTimestampFieldsWithCreated,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
version?: string;
/** Array of references to other saved objects */
references?: SavedObjectReference[];
/** {@inheritDoc SavedObjectsMigrationVersion} */
/**
* {@inheritDoc SavedObjectsMigrationVersion}
* @deprecated
*/
migrationVersion?: SavedObjectsMigrationVersion;
/**
* A semver value that is used when upgrading objects between Kibana versions. If undefined, this will be automatically set to the current
Expand All @@ -37,6 +40,8 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
* field set and you want to create it again.
*/
coreMigrationVersion?: string;
/** A semver value that is used when migrating documents between Kibana versions. */
typeMigrationVersion?: string;
/** Optional ID of the original saved object, if this object's `id` was regenerated */
originId?: string;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
* Can be used in conjunction with `overwrite` for implementing optimistic concurrency control.
**/
version?: string;
/** {@inheritDoc SavedObjectsMigrationVersion} */
/**
* {@inheritDoc SavedObjectsMigrationVersion}
* @deprecated Use {@link SavedObjectsCreateOptions.typeMigrationVersion} instead.
*/
migrationVersion?: SavedObjectsMigrationVersion;
/**
* A semver value that is used when upgrading objects between Kibana versions. If undefined, this will be automatically set to the current
Expand All @@ -37,6 +40,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
* field set and you want to create it again.
*/
coreMigrationVersion?: string;
/**
* A semver value that is used when migrating documents between Kibana versions.
*/
typeMigrationVersion?: string;
/** Array of references to other saved objects */
references?: SavedObjectReference[];
/** The Elasticsearch Refresh setting for this operation */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,15 @@ export interface SavedObjectsIncrementCounterOptions<Attributes = unknown>
* already exist. Existing fields will be left as-is and won't be incremented.
*/
initialize?: boolean;
/** {@link SavedObjectsMigrationVersion} */
/**
* {@link SavedObjectsMigrationVersion}
* @deprecated
*/
migrationVersion?: SavedObjectsMigrationVersion;
/**
* A semver value that is used when migrating documents between Kibana versions.
*/
typeMigrationVersion?: string;
/**
* (default='wait_for') The Elasticsearch refresh setting for this
* operation. See {@link MutatingOperationRefreshSetting}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export interface ISavedObjectsRepository {
* @param {object} [options={}] {@link SavedObjectsCreateOptions} - options for the create operation
* @property {string} [options.id] - force id on creation, not recommended
* @property {boolean} [options.overwrite=false]
* @property {object} [options.migrationVersion=undefined]
* @property {string} [options.namespace]
* @property {array} [options.references=[]] - [{ name, type, id }]
* @returns {promise} the created saved object { id, type, version, attributes }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,15 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {

const { namespaceTreatment = 'strict' } = options;
const { _id, _source, _seq_no, _primary_term } = doc;
const { type, namespaces, originId, migrationVersion, references, coreMigrationVersion } =
_source;
const {
type,
namespaces,
originId,
migrationVersion,
references,
coreMigrationVersion,
typeMigrationVersion,
} = _source;

const version =
_seq_no != null || _primary_term != null
Expand All @@ -109,6 +116,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
references: references || [],
...(migrationVersion && { migrationVersion }),
...(coreMigrationVersion && { coreMigrationVersion }),
...(typeMigrationVersion != null ? { typeMigrationVersion } : {}),
Comment on lines 117 to +119
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: There's a subtle difference of behavior between typeMigrationVersion and the other attributes here (X && { X } vs X != null ? { X } : {}) . Not saying that one is better than the other, but isn't that a risk somehow to have this specific prop handled differently here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's good notice. That's done so to keep backward compatibility -- previously, we used migrationVersion: {} to trigger migrations, but now an empty string does that trick. In order to keep the value, we are using this ternary operator.

...(_source.updated_at && { updated_at: _source.updated_at }),
...(_source.created_at && { created_at: _source.created_at }),
...(version && { version }),
Expand All @@ -135,6 +143,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
version,
references,
coreMigrationVersion,
typeMigrationVersion,
} = savedObj;
const source = {
[type]: attributes,
Expand All @@ -145,6 +154,7 @@ export class SavedObjectsSerializer implements ISavedObjectsSerializer {
...(originId && { originId }),
...(migrationVersion && { migrationVersion }),
...(coreMigrationVersion && { coreMigrationVersion }),
...(typeMigrationVersion != null ? { typeMigrationVersion } : {}),
...(updated_at && { updated_at }),
...(createdAt && { created_at: createdAt }),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const createSavedObjectSanitizedDocSchema = (attributesSchema: SavedObjec
namespaces: schema.maybe(schema.arrayOf(schema.string())),
migrationVersion: schema.maybe(schema.recordOf(schema.string(), schema.string())),
coreMigrationVersion: schema.maybe(schema.string()),
typeMigrationVersion: schema.maybe(schema.string()),
updated_at: schema.maybe(schema.string()),
created_at: schema.maybe(schema.string()),
version: schema.maybe(schema.string()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ export class SavedObjectsClient implements SavedObjectsClientContract {
body: JSON.stringify({
attributes,
migrationVersion: options.migrationVersion,
typeMigrationVersion: options.typeMigrationVersion,
references: options.references,
}),
});
Expand All @@ -216,7 +217,7 @@ export class SavedObjectsClient implements SavedObjectsClientContract {
/**
* Creates multiple documents at once
*
* @param {array} objects - [{ type, id, attributes, references, migrationVersion }]
* @param {array} objects - [{ type, id, attributes, references, migrationVersion, typeMigrationVersion }]
* @param {object} [options={}]
* @property {boolean} [options.overwrite=false]
* @returns The result of the create operation containing created saved objects.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
public _version?: SavedObjectType<T>['version'];
public id: SavedObjectType<T>['id'];
public type: SavedObjectType<T>['type'];
/** @deprecated */
public migrationVersion: SavedObjectType<T>['migrationVersion'];
public coreMigrationVersion: SavedObjectType<T>['coreMigrationVersion'];
public typeMigrationVersion: SavedObjectType<T>['typeMigrationVersion'];
public error: SavedObjectType<T>['error'];
public references: SavedObjectType<T>['references'];
public updatedAt: SavedObjectType<T>['updated_at'];
Expand All @@ -44,6 +46,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
references,
migrationVersion,
coreMigrationVersion,
typeMigrationVersion,
namespaces,
updated_at: updatedAt,
created_at: createdAt,
Expand All @@ -56,6 +59,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
this._version = version;
this.migrationVersion = migrationVersion;
this.coreMigrationVersion = coreMigrationVersion;
this.typeMigrationVersion = typeMigrationVersion;
this.namespaces = namespaces;
this.updatedAt = updatedAt;
this.createdAt = createdAt;
Expand Down Expand Up @@ -91,6 +95,7 @@ export class SimpleSavedObjectImpl<T = unknown> implements SimpleSavedObject<T>
.create(this.type, this.attributes, {
migrationVersion: this.migrationVersion,
coreMigrationVersion: this.coreMigrationVersion,
typeMigrationVersion: this.typeMigrationVersion,
references: this.references,
})
.then((sso) => {
Expand Down
Loading