Skip to content

Commit

Permalink
Update response format & tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeelmers committed Sep 1, 2021
1 parent 5266277 commit 3bced11
Show file tree
Hide file tree
Showing 31 changed files with 586 additions and 263 deletions.
2 changes: 2 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,9 @@ export type {
SavedObjectsImportHookResult,
SavedObjectsImportSimpleWarning,
SavedObjectsImportActionRequiredWarning,
SavedObjectsImportActionRequiredWarningConfig,
SavedObjectsImportWarning,
SavedObjectsImportWarningConfig,
} from './saved_objects';

export type {
Expand Down
14 changes: 12 additions & 2 deletions src/core/server/saved_objects/import/import_saved_objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ describe('#importSavedObjectsFromStream', () => {
const options = setupOptions();

const result = await importSavedObjectsFromStream(options);
expect(result).toEqual({ success: true, successCount: 0, warnings: [] });
expect(result).toEqual({ success: true, successCount: 0, success_count: 0, warnings: [] });
});

test('returns success=false if an error occurred', async () => {
Expand All @@ -363,6 +363,7 @@ describe('#importSavedObjectsFromStream', () => {
expect(result).toEqual({
success: false,
successCount: 0,
success_count: 0,
errors: [expect.any(Object)],
warnings: [],
});
Expand Down Expand Up @@ -408,12 +409,14 @@ describe('#importSavedObjectsFromStream', () => {
id: obj2.id,
meta: { title: obj2.attributes.title, icon: `${obj2.type}-icon` },
destinationId: obj2.destinationId,
destination_id: obj2.destinationId,
};
const success3 = {
type: obj3.type,
id: obj3.id,
meta: { title: obj3.attributes.title, icon: `${obj3.type}-icon` },
destinationId: obj3.destinationId,
destination_id: obj3.destinationId,
};
const errors = [error1, error2];

Expand All @@ -439,7 +442,7 @@ describe('#importSavedObjectsFromStream', () => {
// originId; hence, this specific object is a new copy -- we would need this information for rendering the appropriate originId
// in the client UI, and we would need it to construct a retry for this object if other objects had errors that needed to be
// resolved
{ ...success3, createNewCopy: true },
{ ...success3, createNewCopy: true, create_new_copy: true },
];
const errorResults = [
{ ...error1, meta: { ...error1.meta, icon: `${error1.type}-icon` } },
Expand All @@ -448,7 +451,9 @@ describe('#importSavedObjectsFromStream', () => {
expect(result).toEqual({
success: false,
successCount: 3,
success_count: 3,
successResults,
success_results: successResults,
errors: errorResults,
warnings: [],
});
Expand All @@ -470,7 +475,9 @@ describe('#importSavedObjectsFromStream', () => {
expect(result).toEqual({
success: false,
successCount: 3,
success_count: 3,
successResults,
success_results: successResults,
errors: errorResults,
warnings: [],
});
Expand Down Expand Up @@ -521,7 +528,9 @@ describe('#importSavedObjectsFromStream', () => {
expect(result).toEqual({
success: true,
successCount: 2,
success_count: 2,
successResults,
success_results: successResults,
warnings: [],
});
});
Expand Down Expand Up @@ -553,6 +562,7 @@ describe('#importSavedObjectsFromStream', () => {
expect(result).toEqual({
success: false,
successCount: 0,
success_count: 0,
errors: expectedErrors,
warnings: [],
});
Expand Down
34 changes: 29 additions & 5 deletions src/core/server/saved_objects/import/import_saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Readable } from 'stream';
import { ISavedObjectTypeRegistry } from '../saved_objects_type_registry';
import { SavedObjectsClientContract } from '../types';
import {
isSavedObjectsImportSimpleWarning,
SavedObjectsImportFailure,
SavedObjectsImportResponse,
SavedObjectsImportHook,
Expand Down Expand Up @@ -137,13 +138,18 @@ export async function importSavedObjectsFromStream({
icon: typeRegistry.getType(type)?.management?.icon,
};
const attemptedOverwrite = pendingOverwrites.has(`${type}:${id}`);
const deprecatedFields = {
...(destinationId && { destinationId }),
...(destinationId && !originId && !createNewCopies && { createNewCopy: true }),
};
return {
type,
id,
meta,
...(attemptedOverwrite && { overwrite: true }),
...(destinationId && { destinationId }),
...(destinationId && !originId && !createNewCopies && { createNewCopy: true }),
...(destinationId && { destination_id: destinationId }),
...(destinationId && !originId && !createNewCopies && { create_new_copy: true }),
...deprecatedFields,
};
});
const errorResults = errorAccumulator.map((error) => {
Expand All @@ -160,11 +166,29 @@ export async function importSavedObjectsFromStream({
importHooks,
});

return {
const deprecatedResponseFields = {
successCount: createSavedObjectsResult.createdObjects.length,
success: errorAccumulator.length === 0,
warnings,
...(successResults.length && { successResults }),
};

return {
success_count: createSavedObjectsResult.createdObjects.length,
success: errorAccumulator.length === 0,
warnings: warnings.map((w) => {
if (isSavedObjectsImportSimpleWarning(w)) {
return w;
}
return {
type: w.type,
message: w.message,
action_path: w.actionPath,
actionPath: w.actionPath, // deprecated
...(w.buttonLabel && { button_label: w.buttonLabel }),
...(w.buttonLabel && { buttonLabel: w.buttonLabel }), // deprecated
};
}),
...(successResults.length && { success_results: successResults }),
...(errorResults.length && { errors: errorResults }),
...deprecatedResponseFields,
};
}
2 changes: 2 additions & 0 deletions src/core/server/saved_objects/import/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export type {
SavedObjectsImportHookResult,
SavedObjectsImportSimpleWarning,
SavedObjectsImportActionRequiredWarning,
SavedObjectsImportActionRequiredWarningConfig,
SavedObjectsImportWarning,
SavedObjectsImportWarningConfig,
} from './types';
export { SavedObjectsImportError } from './errors';
26 changes: 21 additions & 5 deletions src/core/server/saved_objects/import/lib/check_conflicts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const createObject = (type: string, id: string): SavedObjectType => ({
const getResultMock = {
conflict: (type: string, id: string) => {
const error = SavedObjectsErrorHelpers.createConflictError(type, id).output.payload;
return { type, id, error };
return { type, id, error: { ...error, status_code: error.statusCode } };
},
unresolvableConflict: (type: string, id: string) => {
const conflictMock = getResultMock.conflict(type, id);
Expand All @@ -38,7 +38,7 @@ const getResultMock = {
},
invalidType: (type: string, id: string) => {
const error = SavedObjectsErrorHelpers.createUnsupportedTypeError(type).output.payload;
return { type, id, error };
return { type, id, error: { ...error, status_code: error.statusCode } };
},
};

Expand Down Expand Up @@ -154,8 +154,20 @@ describe('#checkConflicts', () => {
const _objects = [...objects, obj5];
const retries = [
{ id: obj1.id, type: obj1.type }, // find no conflict for obj1
{ id: obj2.id, type: obj2.type, destinationId: 'some-object-id' }, // find a conflict for obj2, and return it with the specified destinationId
{ id: obj3.id, type: obj3.type, destinationId: 'another-object-id', createNewCopy: true }, // find an unresolvable conflict for obj3, regenerate the destinationId, and then omit originId because of the createNewCopy flag
{
id: obj2.id,
type: obj2.type,
destinationId: 'some-object-id',
destination_id: 'some-object-id',
}, // find a conflict for obj2, and return it with the specified destinationId
{
id: obj3.id,
type: obj3.type,
destinationId: 'another-object-id',
destination_id: 'another-object-id',
createNewCopy: true,
create_new_copy: true,
}, // find an unresolvable conflict for obj3, regenerate the destinationId, and then omit originId because of the createNewCopy flag
{ id: obj4.id, type: obj4.type }, // get an unknown error for obj4
{ id: obj5.id, type: obj5.type, overwrite: true }, // find a conflict for obj5, but ignore it because of the overwrite flag
] as SavedObjectsImportRetry[];
Expand All @@ -178,7 +190,11 @@ describe('#checkConflicts', () => {
...obj2Error,
title: obj2.attributes.title,
meta: { title: obj2.attributes.title },
error: { type: 'conflict', destinationId: 'some-object-id' },
error: {
type: 'conflict',
destinationId: 'some-object-id',
destination_id: 'some-object-id',
},
},
{
...obj4Error,
Expand Down
21 changes: 19 additions & 2 deletions src/core/server/saved_objects/import/lib/check_conflicts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,26 @@ export async function checkConflicts({
importIdMap.set(`${type}:${id}`, { id: uuidv4(), omitOriginId });
filteredObjects.push(object);
} else if (errorObj && errorObj.statusCode !== 409) {
errors.push({ type, id, title, meta: { title }, error: { ...errorObj, type: 'unknown' } });
errors.push({
type,
id,
title,
meta: { title },
error: {
type: 'unknown',
message: errorObj.message,
status_code: errorObj.statusCode,
statusCode: errorObj.statusCode, // deprecated
error: errorObj.error, // deprecated
...(errorObj.metadata && { metadata: errorObj.metadata }), // deprecated
},
});
} else if (errorObj?.statusCode === 409 && !ignoreRegularConflicts && !overwrite) {
const error = { type: 'conflict' as 'conflict', ...(destinationId && { destinationId }) };
const error = {
type: 'conflict' as 'conflict',
...(destinationId && { destination_id: destinationId }),
...(destinationId && { destinationId }), // deprecated
};
errors.push({ type, id, title, meta: { title }, error });
} else {
filteredObjects.push(object);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ describe('#checkOriginConflicts', () => {
id,
title: attributes?.title,
updatedAt,
updated_at: updatedAt,
}));
const createAmbiguousConflictError = (
object: SavedObjectType,
Expand All @@ -180,6 +181,7 @@ describe('#checkOriginConflicts', () => {
error: {
type: 'conflict',
...(destinationId && { destinationId }),
...(destinationId && { destination_id: destinationId }),
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,17 @@ const transformObjectsToAmbiguousConflictFields = (
objects: Array<SavedObject<{ title?: string }>>
) =>
objects
.map(({ id, attributes, updated_at: updatedAt }) => ({
// eslint-disable-next-line @typescript-eslint/naming-convention
.map(({ id, attributes, updated_at }) => ({
id,
title: attributes?.title,
updatedAt,
updated_at,
updatedAt: updated_at, // deprecated
}))
// Sort to ensure that integration tests are not flaky
.sort((a, b) => {
const aUpdatedAt = a.updatedAt ?? '';
const bUpdatedAt = b.updatedAt ?? '';
const aUpdatedAt = a.updated_at ?? '';
const bUpdatedAt = b.updated_at ?? '';
if (aUpdatedAt !== bUpdatedAt) {
return aUpdatedAt < bUpdatedAt ? 1 : -1; // descending
}
Expand Down Expand Up @@ -185,7 +187,8 @@ export async function checkOriginConflicts({ objects, ...params }: CheckOriginCo
meta: { title },
error: {
type: 'conflict',
destinationId: destinations[0].id,
destination_id: destinations[0].id,
destinationId: destinations[0].id, // deprecated
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { SavedObject } from '../../types';
import { SavedObjectsImportHook, SavedObjectsImportWarning } from '../types';
import { SavedObjectsImportHook, SavedObjectsImportWarningConfig } from '../types';

interface ExecuteImportHooksOptions {
objects: SavedObject[];
Expand All @@ -17,9 +17,9 @@ interface ExecuteImportHooksOptions {
export const executeImportHooks = async ({
objects,
importHooks,
}: ExecuteImportHooksOptions): Promise<SavedObjectsImportWarning[]> => {
}: ExecuteImportHooksOptions): Promise<SavedObjectsImportWarningConfig[]> => {
const objsByType = splitByType(objects);
let warnings: SavedObjectsImportWarning[] = [];
let warnings: SavedObjectsImportWarningConfig[] = [];

for (const [type, typeObjs] of Object.entries(objsByType)) {
const hooks = importHooks[type] ?? [];
Expand Down
82 changes: 42 additions & 40 deletions src/core/server/saved_objects/import/lib/extract_errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,45 +51,47 @@ describe('extractErrors()', () => {
];
const result = extractErrors(savedObjects, savedObjects);
expect(result).toMatchInlineSnapshot(`
Array [
Object {
"error": Object {
"type": "conflict",
},
"id": "2",
"meta": Object {
"title": "My Dashboard 2",
},
"title": "My Dashboard 2",
"type": "dashboard",
},
Object {
"error": Object {
"error": "Bad Request",
"message": "Bad Request",
"statusCode": 400,
"type": "unknown",
},
"id": "3",
"meta": Object {
"title": "My Dashboard 3",
},
"title": "My Dashboard 3",
"type": "dashboard",
},
Object {
"error": Object {
"destinationId": "foo",
"type": "conflict",
},
"id": "4",
"meta": Object {
"title": "My Dashboard 4",
},
"title": "My Dashboard 4",
"type": "dashboard",
},
]
`);
Array [
Object {
"error": Object {
"type": "conflict",
},
"id": "2",
"meta": Object {
"title": "My Dashboard 2",
},
"title": "My Dashboard 2",
"type": "dashboard",
},
Object {
"error": Object {
"error": "Bad Request",
"message": "Bad Request",
"statusCode": 400,
"status_code": 400,
"type": "unknown",
},
"id": "3",
"meta": Object {
"title": "My Dashboard 3",
},
"title": "My Dashboard 3",
"type": "dashboard",
},
Object {
"error": Object {
"destinationId": "foo",
"destination_id": "foo",
"type": "conflict",
},
"id": "4",
"meta": Object {
"title": "My Dashboard 4",
},
"title": "My Dashboard 4",
"type": "dashboard",
},
]
`);
});
});
Loading

0 comments on commit 3bced11

Please sign in to comment.