-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
DocumentMigration: implement downward transformation of documents #153921
Changes from all commits
8c79e4f
74cf5f8
3f3699a
67cdaa2
2b17e30
29d1d36
e11e21d
651c34f
6e22e60
2e629de
f3738f8
79231b5
a615064
a5a43bf
abd80ef
8cfd326
32764a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,10 +12,15 @@ import { | |
} from './document_migrator.test.mock'; | ||
import { set } from '@kbn/safer-lodash-set'; | ||
import _ from 'lodash'; | ||
import type { SavedObjectUnsanitizedDoc, SavedObjectsType } from '@kbn/core-saved-objects-server'; | ||
import type { | ||
SavedObjectUnsanitizedDoc, | ||
SavedObjectsType, | ||
SavedObjectModelTransformationFn, | ||
} from '@kbn/core-saved-objects-server'; | ||
import { | ||
SavedObjectTypeRegistry, | ||
LEGACY_URL_ALIAS_TYPE, | ||
modelVersionToVirtualVersion, | ||
} from '@kbn/core-saved-objects-base-server-internal'; | ||
import { DocumentMigrator } from './document_migrator'; | ||
import { TransformSavedObjectDocumentError } from '../core/transform_saved_object_document_error'; | ||
|
@@ -49,6 +54,17 @@ const createRegistry = (...types: Array<Partial<SavedObjectsType>>) => { | |
return registry; | ||
}; | ||
|
||
const createDoc = ( | ||
parts: Partial<SavedObjectUnsanitizedDoc<any>> = {} | ||
): SavedObjectUnsanitizedDoc<any> => ({ | ||
id: 'test-doc', | ||
type: 'test-type', | ||
attributes: {}, | ||
references: [], | ||
coreMigrationVersion: kibanaVersion, | ||
...parts, | ||
}); | ||
|
||
beforeEach(() => { | ||
mockGetConvertedObjectId.mockClear(); | ||
validateTypeMigrationsMock.mockReset(); | ||
|
@@ -466,7 +482,7 @@ describe('DocumentMigrator', () => { | |
}); | ||
}); | ||
|
||
it('allows changing type', () => { | ||
it('does not allow changing type', () => { | ||
const migrator = new DocumentMigrator({ | ||
...testOpts(), | ||
typeRegistry: createRegistry( | ||
|
@@ -485,20 +501,17 @@ describe('DocumentMigrator', () => { | |
), | ||
}); | ||
migrator.prepareMigrations(); | ||
const actual = migrator.migrate({ | ||
id: 'smelly', | ||
type: 'dog', | ||
attributes: { name: 'Callie' }, | ||
typeMigrationVersion: '', | ||
coreMigrationVersion: '8.8.0', | ||
}); | ||
expect(actual).toEqual({ | ||
id: 'smelly', | ||
type: 'cat', | ||
attributes: { name: 'Kitty Callie' }, | ||
coreMigrationVersion: '8.8.0', | ||
typeMigrationVersion: '1.0.0', | ||
}); | ||
expect(() => | ||
migrator.migrate({ | ||
id: 'smelly', | ||
type: 'dog', | ||
attributes: { name: 'Callie' }, | ||
Comment on lines
+506
to
+508
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Smelly Callie 😄 |
||
typeMigrationVersion: '', | ||
coreMigrationVersion: '8.8.0', | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot( | ||
`"Changing a document's type during a migration is not supported."` | ||
); | ||
}); | ||
|
||
it('disallows updating a typeMigrationVersion prop to a lower version', () => { | ||
|
@@ -1329,6 +1342,175 @@ describe('DocumentMigrator', () => { | |
}); | ||
}); | ||
}); | ||
|
||
describe('down transformation', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the unit tests are directly for
So I added some 'integration' smoke tests here. |
||
let migrator: DocumentMigrator; | ||
let transforms: Record<number, jest.MockedFunction<SavedObjectModelTransformationFn<any>>>; | ||
|
||
const createTransformFn = ( | ||
impl?: SavedObjectModelTransformationFn<any> | ||
): jest.MockedFunction<SavedObjectModelTransformationFn<any>> => { | ||
const defaultImpl: SavedObjectModelTransformationFn = (doc) => ({ | ||
document: doc, | ||
}); | ||
return jest.fn().mockImplementation(impl ?? defaultImpl); | ||
}; | ||
|
||
const transformCall = (num: number) => transforms[num].mock.invocationCallOrder[0]; | ||
|
||
beforeEach(() => { | ||
const migrate1 = createTransformFn((doc) => { | ||
doc.attributes.wentThoughDown1 = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
return { document: doc }; | ||
}); | ||
const migrate2 = createTransformFn((doc) => { | ||
doc.attributes.wentThoughDown2 = true; | ||
return { document: doc }; | ||
}); | ||
const migrate3 = createTransformFn((doc) => { | ||
doc.attributes.wentThoughDown3 = true; | ||
return { document: doc }; | ||
}); | ||
|
||
transforms = { | ||
1: migrate1, | ||
2: migrate2, | ||
3: migrate3, | ||
}; | ||
|
||
migrator = new DocumentMigrator({ | ||
...testOpts(), | ||
typeRegistry: createRegistry({ | ||
name: 'down-test', | ||
migrations: { | ||
'7.8.0': jest.fn(), | ||
'7.9.0': jest.fn(), | ||
}, | ||
switchToModelVersionAt: '8.0.0', | ||
modelVersions: { | ||
1: { | ||
modelChange: { | ||
type: 'expansion', | ||
transformation: { up: jest.fn(), down: migrate1 }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Are we planning to add an FTR that checks up & down migrations are symmetric? I guess this is something we'll do in follow-up PRs, right? P.S.: This comment doesn't belong here... but this line reminded me that... |
||
}, | ||
}, | ||
2: { | ||
modelChange: { | ||
type: 'expansion', | ||
transformation: { up: jest.fn(), down: migrate2 }, | ||
}, | ||
}, | ||
3: { | ||
modelChange: { | ||
type: 'expansion', | ||
transformation: { up: jest.fn(), down: migrate3 }, | ||
}, | ||
}, | ||
}, | ||
}), | ||
}); | ||
migrator.prepareMigrations(); | ||
}); | ||
|
||
it('applies the expected conversion', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(2), | ||
}); | ||
|
||
const result = migrator.transformDown(document, { | ||
targetTypeVersion: modelVersionToVirtualVersion(1), | ||
}); | ||
|
||
expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(1)); | ||
expect(result.attributes).toEqual({ | ||
wentThoughDown2: true, | ||
}); | ||
}); | ||
|
||
it('applies all the expected conversions', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(3), | ||
}); | ||
|
||
const result = migrator.transformDown(document, { | ||
targetTypeVersion: modelVersionToVirtualVersion(0), | ||
}); | ||
|
||
expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(0)); | ||
expect(result.attributes).toEqual({ | ||
wentThoughDown1: true, | ||
wentThoughDown2: true, | ||
wentThoughDown3: true, | ||
}); | ||
}); | ||
|
||
it('applies the conversions in order', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(3), | ||
}); | ||
|
||
const result = migrator.transformDown(document, { | ||
targetTypeVersion: modelVersionToVirtualVersion(0), | ||
}); | ||
|
||
expect(result.typeMigrationVersion).toEqual(modelVersionToVirtualVersion(0)); | ||
|
||
expect(transforms[1]).toHaveBeenCalledTimes(1); | ||
expect(transforms[2]).toHaveBeenCalledTimes(1); | ||
expect(transforms[3]).toHaveBeenCalledTimes(1); | ||
|
||
expect(transformCall(3)).toBeLessThan(transformCall(2)); | ||
expect(transformCall(2)).toBeLessThan(transformCall(1)); | ||
}); | ||
|
||
it('throw when trying to transform to a higher version', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(2), | ||
}); | ||
|
||
expect(() => | ||
migrator.transformDown(document, { | ||
targetTypeVersion: modelVersionToVirtualVersion(3), | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot( | ||
`"Trying to transform down to a higher version: 10.2.0 to 10.3.0"` | ||
); | ||
}); | ||
|
||
it('throw when trying to transform a document from a higher version than the max one', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(4), | ||
}); | ||
|
||
expect(() => | ||
migrator.transformDown(document, { | ||
targetTypeVersion: modelVersionToVirtualVersion(2), | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot( | ||
`"Document \\"test-doc\\" belongs to a more recent version of Kibana [10.4.0] when the last known version is [10.3.0]."` | ||
); | ||
}); | ||
|
||
it('throw when trying to transform to a version without down conversion', () => { | ||
const document = createDoc({ | ||
type: 'down-test', | ||
typeMigrationVersion: modelVersionToVirtualVersion(2), | ||
}); | ||
|
||
expect(() => | ||
migrator.transformDown(document, { | ||
targetTypeVersion: '7.8.0', | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot( | ||
`"Could not apply transformation migrate:7.9.0: no down conversion registered"` | ||
); | ||
}); | ||
}); | ||
}); | ||
|
||
function renameAttr(path: string, newPath: string) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the only place where this function type was used, so I inlined it in
VersionedTransformer
instead