From 3601aefff03e371aa3fa8827dfdfa5ac444b0fc8 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 28 Sep 2022 13:48:06 +0200 Subject: [PATCH 1/3] Fix SO export sorting algorithm --- .../src/export/sort_objects.test.ts | 37 +++++++++++++++++++ .../src/export/sort_objects.ts | 21 ++++++----- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.test.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.test.ts index 1f663ea5dbc56..27fbb09a37018 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.test.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.test.ts @@ -6,7 +6,9 @@ * Side Public License, v 1. */ +import { range } from 'lodash'; import { sortObjects } from './sort_objects'; +import type { SavedObject } from '@kbn/core-saved-objects-common'; describe('sortObjects()', () => { test('should return on empty array', () => { @@ -309,6 +311,7 @@ describe('sortObjects()', () => { ] `); }); + test('should not fail on complex circular dependencies', () => { const docs = [ { @@ -424,4 +427,38 @@ describe('sortObjects()', () => { ] `); }); + + test('should not fail on large graph of objects', () => { + // create an object that references all objects with a higher `index` up to `depth`. + const createComplexNode = (index: number, depth: number): SavedObject => { + return { + type: 'test', + id: `${index}`, + attributes: {}, + references: range(index + 1, depth).map((refIndex) => ({ + type: 'test', + id: `${refIndex}`, + name: `test-${refIndex}`, + })), + }; + }; + + const createComplexGraph = (depth: number): SavedObject[] => { + const nodes: SavedObject[] = []; + for (let i = 0; i < depth; i++) { + nodes.push(createComplexNode(i, depth)); + } + return nodes; + }; + + const depth = 100; + const graph = createComplexGraph(depth); + const sorted = sortObjects(graph); + + expect(sorted.map(({ type, id }) => `${type}:${id}`)).toEqual( + range(depth) + .reverse() + .map((index) => `test:${index}`) + ); + }); }); diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts index 487622877e25c..f63f0f6cf0790 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts @@ -8,27 +8,29 @@ import type { SavedObject } from '@kbn/core-saved-objects-common'; +const getId = (object: { type: string; id: string }) => `${object.type}:${object.id}`; + export function sortObjects(savedObjects: SavedObject[]): SavedObject[] { - const path = new Set(); + const path = new Set(); const sorted = new Set(); const objectsByTypeId = new Map( - savedObjects.map((object) => [`${object.type}:${object.id}`, object] as [string, SavedObject]) + savedObjects.map((object) => [getId(object), object] as [string, SavedObject]) ); function includeObjects(objects: SavedObject[]) { for (const object of objects) { - if (path.has(object)) { + const objectId = getId(object); + if (path.has(objectId)) { continue; } - const refdObjects = object.references - .map((ref) => objectsByTypeId.get(`${ref.type}:${ref.id}`)) + const objectRefs = object.references + .map((ref) => objectsByTypeId.get(getId(ref))) .filter((ref): ref is SavedObject => !!ref); - if (refdObjects.length) { - path.add(object); - includeObjects(refdObjects); - path.delete(object); + path.add(objectId); + if (objectRefs.length) { + includeObjects(objectRefs); } sorted.add(object); @@ -36,5 +38,6 @@ export function sortObjects(savedObjects: SavedObject[]): SavedObject[] { } includeObjects(savedObjects); + return [...sorted]; } From 00262b6b8b214f23df9e457996f57eed5625fbee Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 28 Sep 2022 13:54:03 +0200 Subject: [PATCH 2/3] improve var name --- .../src/export/sort_objects.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts index f63f0f6cf0790..551ba3989e527 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/sort_objects.ts @@ -11,7 +11,7 @@ import type { SavedObject } from '@kbn/core-saved-objects-common'; const getId = (object: { type: string; id: string }) => `${object.type}:${object.id}`; export function sortObjects(savedObjects: SavedObject[]): SavedObject[] { - const path = new Set(); + const traversed = new Set(); const sorted = new Set(); const objectsByTypeId = new Map( savedObjects.map((object) => [getId(object), object] as [string, SavedObject]) @@ -20,7 +20,7 @@ export function sortObjects(savedObjects: SavedObject[]): SavedObject[] { function includeObjects(objects: SavedObject[]) { for (const object of objects) { const objectId = getId(object); - if (path.has(objectId)) { + if (traversed.has(objectId)) { continue; } @@ -28,7 +28,7 @@ export function sortObjects(savedObjects: SavedObject[]): SavedObject[] { .map((ref) => objectsByTypeId.get(getId(ref))) .filter((ref): ref is SavedObject => !!ref); - path.add(objectId); + traversed.add(objectId); if (objectRefs.length) { includeObjects(objectRefs); } From 6abc46d09d6afdd4b47c4a89a85ca83af973cc0e Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 28 Sep 2022 15:09:36 +0200 Subject: [PATCH 3/3] adapt another unit test --- .../src/export/saved_objects_exporter.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts index 614c9e3680acc..fed06cbf2f740 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts @@ -146,16 +146,18 @@ describe('getSortedObjectsForExport()', () => { attributes = {}, sort = [], type = 'index-pattern', + idPrefix = '', }: { attributes?: Record; sort?: string[]; type?: string; + idPrefix?: string; } = {} ) { const hits = []; for (let i = 1; i <= hitCount; i++) { hits.push({ - id: `${i}`, + id: `${idPrefix}${i}`, type, attributes, sort, @@ -247,7 +249,7 @@ describe('getSortedObjectsForExport()', () => { describe('>1k hits', () => { const firstMockHits = generateHits(1000, { sort: ['a', 'b'] }); - const secondMockHits = generateHits(500); + const secondMockHits = generateHits(500, { idPrefix: 'second-hit-' }); test('requests multiple pages', async () => { savedObjectsClient.find.mockResolvedValueOnce({