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.

Sign up for GitHub

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

Fixes #9680 (wrong title when overwriting saved objects) #44441

Closed
wants to merge 5 commits into from
Closed

Fixes #9680 (wrong title when overwriting saved objects) #44441

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96d3640
Fixes #9680 (wrong title when overwriting saved objects)
friol Aug 29, 2019
cfd882d
As asked by @kobelb, fetching all the titles with one bulk call
friol Aug 30, 2019
5a2e961
Reverted the srcSpaceId changes and modified srcSpaceId in destSpaceI…
friol Sep 5, 2019
ff9c4f0
Merge remote-tracking branch 'upstream/master' into wrongtitlefix
friol Sep 14, 2019
b42bed1
Changes requested in review
friol Sep 15, 2019
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
8 changes: 4 additions & 4 deletions src/core/server/saved_objects/import/extract_errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ import { SavedObject } from '../types';
import { extractErrors } from './extract_errors';

describe('extractErrors()', () => {
test('returns empty array when no errors exist', () => {
test('returns empty array when no errors exist', async () => {
const savedObjects: SavedObject[] = [];
const result = extractErrors(savedObjects, savedObjects);
const result = await extractErrors(savedObjects, savedObjects);
expect(result).toMatchInlineSnapshot(`Array []`);
});

test('extracts errors from saved objects', () => {
test('extracts errors from saved objects', async () => {
const savedObjects: SavedObject[] = [
{
id: '1',
Expand Down Expand Up @@ -62,7 +62,7 @@ describe('extractErrors()', () => {
},
},
];
const result = extractErrors(savedObjects, savedObjects);
const result = await extractErrors(savedObjects, savedObjects);
expect(result).toMatchInlineSnapshot(`
Array [
Object {
Expand Down
30 changes: 27 additions & 3 deletions src/core/server/saved_objects/import/extract_errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,36 @@
* specific language governing permissions and limitations
* under the License.
*/
import { SavedObjectsClientContract } from 'src/core/server';
import { SavedObject } from '../types';
import { SavedObjectsImportError } from './types';

export function extractErrors(
export async function extractErrors(
savedObjectResults: SavedObject[],
savedObjectsToImport: SavedObject[]
savedObjectsToImport: SavedObject[],
savedObjectsClient?: SavedObjectsClientContract,
destSpaceId?: string
) {
const errors: SavedObjectsImportError[] = [];
const originalSavedObjectsMap = new Map<string, SavedObject>();
const savedObjectsTitlesMap = new Map<string, string>();

for (const savedObject of savedObjectsToImport) {
originalSavedObjectsMap.set(`${savedObject.type}:${savedObject.id}`, savedObject);
}
if (savedObjectsClient) {
const bulkGetResult = await savedObjectsClient.bulkGet(savedObjectsToImport, {
namespace: destSpaceId,
});

for (const savedObject of bulkGetResult.saved_objects) {
savedObjectsTitlesMap.set(
`${savedObject.type}:${savedObject.id}`,
`${savedObject.attributes.title}`
);
}
}

for (const savedObject of savedObjectResults) {
if (savedObject.error) {
const originalSavedObject = originalSavedObjectsMap.get(
Expand All @@ -37,11 +55,17 @@ export function extractErrors(
originalSavedObject &&
originalSavedObject.attributes &&
originalSavedObject.attributes.title;

if (savedObject.error.statusCode === 409) {
// pick the correct title of the saved object that conflicts
let realTitle = title;
if (savedObjectsClient) {
realTitle = savedObjectsTitlesMap.get(`${savedObject.type}:${savedObject.id}`);
}
errors.push({
id: savedObject.id,
type: savedObject.type,
title,
title: realTitle,
Copy link
Contributor

Choose a reason for hiding this comment

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

When the import API returns an error, I think the correct response is to include the title of the object that failed to import. The API is saying "I failed while trying to import the title you gave me". Perhaps we can add a new field to the response like existingTitle.

error: {
type: 'conflict',
},
Expand Down
11 changes: 7 additions & 4 deletions src/core/server/saved_objects/import/import_saved_objects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@ export async function importSavedObjects({
overwrite,
namespace,
});
errorAccumulator = [
...errorAccumulator,
...extractErrors(bulkCreateResult.saved_objects, filteredObjects),
];

const soErrors = await extractErrors(
bulkCreateResult.saved_objects,
filteredObjects,
savedObjectsClient,
namespace
);
errorAccumulator = [...errorAccumulator, ...soErrors];
return {
success: errorAccumulator.length === 0,
successCount: bulkCreateResult.saved_objects.filter(obj => !obj.error).length,
Expand Down
14 changes: 6 additions & 8 deletions src/core/server/saved_objects/import/resolve_import_errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,18 @@ export async function resolveImportErrors({
overwrite: true,
namespace,
});
errorAccumulator = [
...errorAccumulator,
...extractErrors(bulkCreateResult.saved_objects, objectsToOverwrite),
];

const extractErrs = await extractErrors(bulkCreateResult.saved_objects, objectsToOverwrite);
errorAccumulator = [...errorAccumulator, ...extractErrs];
successCount += bulkCreateResult.saved_objects.filter(obj => !obj.error).length;
}
if (objectsToNotOverwrite.length) {
const bulkCreateResult = await savedObjectsClient.bulkCreate(objectsToNotOverwrite, {
namespace,
});
errorAccumulator = [
...errorAccumulator,
...extractErrors(bulkCreateResult.saved_objects, objectsToNotOverwrite),
];

const extractErrs = await extractErrors(bulkCreateResult.saved_objects, objectsToNotOverwrite);
errorAccumulator = [...errorAccumulator, ...extractErrs];
successCount += bulkCreateResult.saved_objects.filter(obj => !obj.error).length;
}

Expand Down