-
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
[Saved Objects] Adds managed to import options #155677
Changes from 9 commits
2359b68
b81f813
03fdfe8
1267270
e19f896
c52bb41
e7c0319
cbd4561
0669c42
4eeb530
f753e32
a396a9f
bb9b6cc
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 |
---|---|---|
|
@@ -91,6 +91,7 @@ export interface SavedObjectsImportFailure { | |
* If `overwrite` is specified, an attempt was made to overwrite an existing object. | ||
*/ | ||
overwrite?: boolean; | ||
managed?: boolean; | ||
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. Same question here, I don't see it as being a problem, but just so I understand, why do we need to surface the 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. As a reference, that's all. |
||
error: | ||
| SavedObjectsImportConflictError | ||
| SavedObjectsImportAmbiguousConflictError | ||
|
@@ -125,6 +126,14 @@ export interface SavedObjectsImportSuccess { | |
* If `overwrite` is specified, this object overwrote an existing one (or will do so, in the case of a pending resolution). | ||
*/ | ||
overwrite?: boolean; | ||
/** | ||
* Flag indicating if a saved object is managed by Kibana (default=false) | ||
* | ||
* This can be leveraged by applications to e.g. prevent edits to a managed | ||
* saved object. Instead, users can be guided to create a copy first and | ||
* make their edits to the copy. | ||
*/ | ||
managed?: boolean; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,10 +80,12 @@ describe('#importSavedObjectsFromStream', () => { | |
management: { icon: `${type}-icon` }, | ||
} as any), | ||
importHooks = {}, | ||
managed, | ||
}: { | ||
createNewCopies?: boolean; | ||
getTypeImpl?: (name: string) => any; | ||
importHooks?: Record<string, SavedObjectsImportHook[]>; | ||
managed?: boolean; | ||
} = {}): ImportSavedObjectsOptions => { | ||
readStream = new Readable(); | ||
savedObjectsClient = savedObjectsClientMock.create(); | ||
|
@@ -98,19 +100,23 @@ describe('#importSavedObjectsFromStream', () => { | |
namespace, | ||
createNewCopies, | ||
importHooks, | ||
managed, | ||
}; | ||
}; | ||
const createObject = ({ | ||
type = 'foo-type', | ||
title = 'some-title', | ||
}: { type?: string; title?: string } = {}): SavedObject<{ | ||
managed = undefined, // yea, explicitly declare this as not set to test against existing objects | ||
}: { type?: string; title?: string; managed?: boolean } = {}): SavedObject<{ | ||
title: string; | ||
managed?: boolean; | ||
}> => { | ||
return { | ||
type, | ||
id: uuidv4(), | ||
references: [], | ||
attributes: { title }, | ||
managed, | ||
}; | ||
}; | ||
const createError = (): SavedObjectsImportFailure => { | ||
|
@@ -320,6 +326,55 @@ describe('#importSavedObjectsFromStream', () => { | |
importStateMap, | ||
overwrite, | ||
namespace, | ||
managed: options.managed, | ||
}; | ||
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); | ||
}); | ||
|
||
test('creates managed saved objects', async () => { | ||
const options = setupOptions({ managed: true }); | ||
const collectedObjects = [createObject({ managed: true })]; | ||
const filteredObjects = [createObject({ managed: false })]; | ||
const errors = [createError(), createError(), createError(), createError()]; | ||
mockCollectSavedObjects.mockResolvedValue({ | ||
errors: [errors[0]], | ||
collectedObjects, | ||
importStateMap: new Map([ | ||
['foo', {}], | ||
['bar', {}], | ||
['baz', { isOnlyReference: true }], | ||
]), | ||
}); | ||
mockCheckReferenceOrigins.mockResolvedValue({ | ||
importStateMap: new Map([['baz', { isOnlyReference: true, destinationId: 'newId1' }]]), | ||
}); | ||
mockValidateReferences.mockResolvedValue([errors[1]]); | ||
mockCheckConflicts.mockResolvedValue({ | ||
errors: [errors[2]], | ||
filteredObjects, | ||
importStateMap: new Map([['foo', { destinationId: 'newId2' }]]), | ||
pendingOverwrites: new Set(), | ||
}); | ||
mockCheckOriginConflicts.mockResolvedValue({ | ||
errors: [errors[3]], | ||
importStateMap: new Map([['bar', { destinationId: 'newId3' }]]), | ||
pendingOverwrites: new Set(), | ||
}); | ||
|
||
await importSavedObjectsFromStream(options); | ||
const importStateMap = new Map([ | ||
['foo', { destinationId: 'newId2' }], | ||
['bar', { destinationId: 'newId3' }], | ||
['baz', { isOnlyReference: true, destinationId: 'newId1' }], | ||
]); | ||
const createSavedObjectsParams = { | ||
objects: collectedObjects, | ||
accumulatedErrors: errors, | ||
savedObjectsClient, | ||
importStateMap, | ||
overwrite, | ||
namespace, | ||
managed: options.managed, | ||
}; | ||
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); | ||
}); | ||
|
@@ -382,6 +437,214 @@ describe('#importSavedObjectsFromStream', () => { | |
}; | ||
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); | ||
}); | ||
|
||
test('creates managed saved objects', async () => { | ||
const options = setupOptions({ createNewCopies: true, managed: true }); | ||
const collectedObjects = [createObject({ managed: true })]; | ||
const errors = [createError(), createError()]; | ||
mockCollectSavedObjects.mockResolvedValue({ | ||
errors: [errors[0]], | ||
collectedObjects, | ||
importStateMap: new Map([ | ||
['foo', {}], | ||
['bar', { isOnlyReference: true }], | ||
]), | ||
}); | ||
mockCheckReferenceOrigins.mockResolvedValue({ | ||
importStateMap: new Map([['bar', { isOnlyReference: true, destinationId: 'newId' }]]), | ||
}); | ||
mockValidateReferences.mockResolvedValue([errors[1]]); | ||
mockRegenerateIds.mockReturnValue(new Map([['foo', { destinationId: `randomId1` }]])); | ||
|
||
await importSavedObjectsFromStream(options); | ||
// assert that the importStateMap is correctly composed of the results from the three modules | ||
const importStateMap: ImportStateMap = new Map([ | ||
['foo', { destinationId: `randomId1` }], | ||
['bar', { isOnlyReference: true, destinationId: 'newId' }], | ||
]); | ||
const createSavedObjectsParams = { | ||
objects: collectedObjects, | ||
accumulatedErrors: errors, | ||
savedObjectsClient, | ||
importStateMap, | ||
overwrite, | ||
namespace, | ||
managed: options.managed, | ||
}; | ||
expect(mockCreateSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); | ||
}); | ||
}); | ||
|
||
describe('managed option', () => { | ||
test('if not provided, does not override existing property when already present, defaults to false for others', async () => { | ||
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. Just looked briefly, but I think this test should focus on asserting the paramaters that we send to the create API
We already test 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. We already indirectly test that the params don't include the new option in I'll be more explicit in this one then. 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. I refactored all of them in this group of tests to assert on the params rather than the objects getting created. |
||
const obj1 = createObject({ type: 'foo', managed: true }); | ||
const obj2 = createObject({ type: 'bar', title: 'bar-title', managed: false }); | ||
|
||
const options = setupOptions({ | ||
createNewCopies: false, | ||
getTypeImpl: (type) => { | ||
if (type === 'foo') { | ||
return { | ||
management: { getTitle: () => 'getTitle-foo', icon: `${type}-icon` }, | ||
}; | ||
} | ||
return { | ||
management: { icon: `${type}-icon` }, | ||
}; | ||
}, | ||
}); | ||
|
||
mockCheckConflicts.mockResolvedValue({ | ||
errors: [], | ||
filteredObjects: [], | ||
importStateMap: new Map(), | ||
pendingOverwrites: new Set(), | ||
}); | ||
mockCreateSavedObjects.mockResolvedValue({ | ||
errors: [], | ||
createdObjects: [obj1, { ...obj2, managed: false }], // default applied in createSavedObjects | ||
}); | ||
|
||
const result = await importSavedObjectsFromStream(options); | ||
// successResults only includes the imported object's type, id, and destinationId (if a new one was generated) | ||
const successResults = [ | ||
{ | ||
type: obj1.type, | ||
id: obj1.id, | ||
meta: { title: 'getTitle-foo', icon: `${obj1.type}-icon` }, | ||
managed: true, | ||
}, | ||
{ | ||
type: obj2.type, | ||
id: obj2.id, | ||
meta: { title: 'bar-title', icon: `${obj2.type}-icon` }, | ||
managed: false, | ||
}, | ||
]; | ||
|
||
expect(result).toEqual({ | ||
success: true, | ||
successCount: 2, | ||
successResults, | ||
warnings: [], | ||
}); | ||
}); // assert that the documents being imported retain their prop or have the default applied | ||
|
||
test('creates and converts objects from unmanaged to managed', async () => { | ||
const obj1 = createObject({ type: 'foo', managed: false }); | ||
const obj2 = createObject({ type: 'bar', title: 'bar-title' }); | ||
|
||
const options = setupOptions({ | ||
createNewCopies: false, | ||
managed: true, | ||
getTypeImpl: (type) => { | ||
if (type === 'foo') { | ||
return { | ||
management: { getTitle: () => 'getTitle-foo', icon: `${type}-icon` }, | ||
}; | ||
} | ||
return { | ||
management: { icon: `${type}-icon` }, | ||
}; | ||
}, | ||
}); | ||
|
||
mockCheckConflicts.mockResolvedValue({ | ||
errors: [], | ||
filteredObjects: [], | ||
importStateMap: new Map(), | ||
pendingOverwrites: new Set(), | ||
}); | ||
mockCreateSavedObjects.mockResolvedValue({ | ||
errors: [], | ||
createdObjects: [ | ||
{ ...obj1, managed: true }, | ||
{ ...obj2, managed: true }, | ||
], // default applied in createSavedObjects | ||
}); | ||
|
||
const result = await importSavedObjectsFromStream(options); | ||
// successResults only includes the imported object's type, id, and destinationId (if a new one was generated) | ||
const successResults = [ | ||
{ | ||
type: obj1.type, | ||
id: obj1.id, | ||
meta: { title: 'getTitle-foo', icon: `${obj1.type}-icon` }, | ||
managed: true, | ||
}, | ||
{ | ||
type: obj2.type, | ||
id: obj2.id, | ||
meta: { title: 'bar-title', icon: `${obj2.type}-icon` }, | ||
managed: true, | ||
}, | ||
]; | ||
|
||
expect(result).toEqual({ | ||
success: true, | ||
successCount: 2, | ||
successResults, | ||
warnings: [], | ||
}); | ||
}); // assert that the documents being imported retain their prop or have the default applied | ||
|
||
test('creates and converts objects from managed to unmanaged', async () => { | ||
const obj1 = createObject({ type: 'foo', managed: true }); | ||
const obj2 = createObject({ type: 'bar', title: 'bar-title' }); | ||
|
||
const options = setupOptions({ | ||
createNewCopies: false, | ||
managed: false, | ||
getTypeImpl: (type) => { | ||
if (type === 'foo') { | ||
return { | ||
management: { getTitle: () => 'getTitle-foo', icon: `${type}-icon` }, | ||
}; | ||
} | ||
return { | ||
management: { icon: `${type}-icon` }, | ||
}; | ||
}, | ||
}); | ||
|
||
mockCheckConflicts.mockResolvedValue({ | ||
errors: [], | ||
filteredObjects: [], | ||
importStateMap: new Map(), | ||
pendingOverwrites: new Set(), | ||
}); | ||
mockCreateSavedObjects.mockResolvedValue({ | ||
errors: [], | ||
createdObjects: [ | ||
{ ...obj1, managed: false }, | ||
{ ...obj2, managed: false }, | ||
], | ||
}); | ||
|
||
const result = await importSavedObjectsFromStream(options); | ||
// successResults only includes the imported object's type, id, destinationId (if a new one was generated) and managed | ||
const successResults = [ | ||
{ | ||
type: obj1.type, | ||
id: obj1.id, | ||
meta: { title: 'getTitle-foo', icon: `${obj1.type}-icon` }, | ||
managed: false, | ||
}, | ||
{ | ||
type: obj2.type, | ||
id: obj2.id, | ||
meta: { title: 'bar-title', icon: `${obj2.type}-icon` }, | ||
managed: false, | ||
}, | ||
]; | ||
|
||
expect(result).toEqual({ | ||
success: true, | ||
successCount: 2, | ||
successResults, | ||
warnings: [], | ||
}); | ||
}); // assert that the documents being imported retain their prop or have the default applied | ||
}); | ||
}); | ||
|
||
|
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.
What's the purpose of surfacing this
managed
flag on our legacy url alias definition?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.
It's to set all objects created during
import
asmanaged
if the option is provided. Legacy alias get created if they need to and the repository will set the default for managed as false if it's not specified.In short: to bypass adding a default.