From d8aefaec70c448383387f09a44bad3e07b21edeb Mon Sep 17 00:00:00 2001 From: Yu Jin <112784385+yujin-emma@users.noreply.github.com> Date: Fri, 16 Feb 2024 14:06:38 -0800 Subject: [PATCH] Bug 5861 Fix: fix import api always display `overwritten` after importing saved objects, after this fix, the first time import will display `new` and after that will display `overwritten` if import the same objects (#5871) * bug fix 5861 Signed-off-by: yujin-emma * clean unused parameters in test Signed-off-by: yujin-emma * bug fix 5861 Signed-off-by: yujin-emma * clean unused parameters in test Signed-off-by: yujin-emma * fix failed UT Signed-off-by: yujin-emma * update CHANGELOG.md Signed-off-by: yujin-emma * Update changelog message Signed-off-by: yujin-emma --------- Signed-off-by: yujin-emma Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com> --- CHANGELOG.md | 4 ++- .../check_conflict_for_data_source.test.ts | 7 ----- .../import/check_conflict_for_data_source.ts | 14 ++++----- .../import/import_saved_objects.test.ts | 12 +++++--- .../import/import_saved_objects.ts | 29 +++++++++---------- 5 files changed, 31 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 308d8fb52a54..0ce198952176 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Discover] Enhanced the data source selector with added sorting functionality ([#5609](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5609)) - [Multiple Datasource] Add datasource picker component and use it in devtools and tutorial page when multiple datasource is enabled ([#5756](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5756)) - [Multiple Datasource] Add datasource picker to import saved object flyout when multiple data source is enabled ([#5781](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5781)) -- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) +- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) ### 🐛 Bug Fixes @@ -664,6 +664,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Search Telemetry] Fix search telemetry's observable object that won't be GC-ed([#3390](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3390)) - [Region Maps] Add ui setting to configure custom vector map's size parameter([#3399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3399)) +- [Import API] Fix import saved objects always display overwritten issue([#5861](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5871)) + ### 🚞 Infrastructure diff --git a/src/core/server/saved_objects/import/check_conflict_for_data_source.test.ts b/src/core/server/saved_objects/import/check_conflict_for_data_source.test.ts index 4bf3a8110454..2b50ba8e9b35 100644 --- a/src/core/server/saved_objects/import/check_conflict_for_data_source.test.ts +++ b/src/core/server/saved_objects/import/check_conflict_for_data_source.test.ts @@ -72,7 +72,6 @@ describe('#checkConflictsForDataSource', () => { filteredObjects: [], errors: [], importIdMap: new Map(), - pendingOverwrites: new Set(), }); }); @@ -83,7 +82,6 @@ describe('#checkConflictsForDataSource', () => { filteredObjects: [dataSourceObj1, dataSourceObj2], errors: [], importIdMap: new Map(), - pendingOverwrites: new Set(), }); }); @@ -118,10 +116,6 @@ describe('#checkConflictsForDataSource', () => { { id: 'currentDsId_id-2', omitOriginId: true }, ], ]), - pendingOverwrites: new Set([ - `${dataSourceObj1.type}:${dataSourceObj1.id}`, - `${dataSourceObj2.type}:${dataSourceObj2.id}`, - ]), }) ); }); @@ -144,7 +138,6 @@ describe('#checkConflictsForDataSource', () => { }, ], importIdMap: new Map(), - pendingOverwrites: new Set(), }); }); }); diff --git a/src/core/server/saved_objects/import/check_conflict_for_data_source.ts b/src/core/server/saved_objects/import/check_conflict_for_data_source.ts index 50005777aba0..6611b01dfb2a 100644 --- a/src/core/server/saved_objects/import/check_conflict_for_data_source.ts +++ b/src/core/server/saved_objects/import/check_conflict_for_data_source.ts @@ -18,13 +18,13 @@ interface ImportIdMapEntry { } /** - * function to check the conflict when multiple data sources are enabled. + * function to only check the data souerce conflict when multiple data sources are enabled. * the purpose of this function is to check the conflict of the imported saved objects and data source * @param objects, this the array of saved objects to be verified whether contains the data source conflict * @param ignoreRegularConflicts whether to override * @param retries import operations list * @param dataSourceId the id to identify the data source - * @returns {filteredObjects, errors, importIdMap, pendingOverwrites } + * @returns {filteredObjects, errors, importIdMap } */ export async function checkConflictsForDataSource({ objects, @@ -35,11 +35,9 @@ export async function checkConflictsForDataSource({ const filteredObjects: Array> = []; const errors: SavedObjectsImportError[] = []; const importIdMap = new Map(); - const pendingOverwrites = new Set(); - // exit early if there are no objects to check if (objects.length === 0) { - return { filteredObjects, errors, importIdMap, pendingOverwrites }; + return { filteredObjects, errors, importIdMap }; } const retryMap = retries.reduce( (acc, cur) => acc.set(`${cur.type}:${cur.id}`, cur), @@ -73,13 +71,15 @@ export async function checkConflictsForDataSource({ } else if (previoudDataSourceId && previoudDataSourceId === dataSourceId) { filteredObjects.push(object); } else { + /** + * Only update importIdMap and filtered objects + */ const omitOriginId = ignoreRegularConflicts; importIdMap.set(`${type}:${id}`, { id: `${dataSourceId}_${rawId}`, omitOriginId }); - pendingOverwrites.add(`${type}:${id}`); filteredObjects.push({ ...object, id: `${dataSourceId}_${rawId}` }); } } }); - return { filteredObjects, errors, importIdMap, pendingOverwrites }; + return { filteredObjects, errors, importIdMap }; } diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 2a9c4b0ff5ed..3dda6931bd1e 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -86,7 +86,6 @@ describe('#importSavedObjectsFromStream', () => { errors: [], filteredObjects: [], importIdMap: new Map(), - pendingOverwrites: new Set(), }); getMockFn(createSavedObjects).mockResolvedValue({ errors: [], createdObjects: [] }); }); @@ -248,6 +247,12 @@ describe('#importSavedObjectsFromStream', () => { collectedObjects, importIdMap: new Map(), }); + getMockFn(checkConflicts).mockResolvedValue({ + errors: [], + filteredObjects: collectedObjects, + importIdMap: new Map([['bar', { id: 'newId1' }]]), + pendingOverwrites: new Set(), + }); await importSavedObjectsFromStream(options); const checkConflictsForDataSourceParams = { @@ -438,7 +443,7 @@ describe('#importSavedObjectsFromStream', () => { }; test('with createNewCopies disabled', async () => { - const options = setupOptions(false, undefined); + const options = setupOptions(); getMockFn(checkConflicts).mockResolvedValue({ errors: [], filteredObjects: [], @@ -500,7 +505,6 @@ describe('#importSavedObjectsFromStream', () => { errors: [], filteredObjects: [], importIdMap: new Map(), - pendingOverwrites: new Set([`${dsSuccess2.type}:${dsSuccess2.id}`]), }); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -559,7 +563,7 @@ describe('#importSavedObjectsFromStream', () => { }); test('accumulates multiple errors', async () => { - const options = setupOptions(false, undefined); + const options = setupOptions(); const errors = [createError(), createError(), createError(), createError(), createError()]; getMockFn(collectSavedObjects).mockResolvedValue({ errors: [errors[0]], diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 6cbb1059fdc3..a5744478fd7d 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -94,22 +94,6 @@ export async function importSavedObjectsFromStream({ ignoreRegularConflicts: overwrite, }; - // resolve when data source exist, pass the filtered objects to next check conflict - if (dataSourceId) { - const checkConflictsForDataSourceResult = await checkConflictsForDataSource({ - objects: collectSavedObjectsResult.collectedObjects, - ignoreRegularConflicts: overwrite, - dataSourceId, - }); - - checkConflictsParams.objects = checkConflictsForDataSourceResult.filteredObjects; - - pendingOverwrites = new Set([ - ...pendingOverwrites, - ...checkConflictsForDataSourceResult.pendingOverwrites, - ]); - } - const checkConflictsResult = await checkConflicts(checkConflictsParams); errorAccumulator = [...errorAccumulator, ...checkConflictsResult.errors]; importIdMap = new Map([...importIdMap, ...checkConflictsResult.importIdMap]); @@ -124,6 +108,19 @@ export async function importSavedObjectsFromStream({ ignoreRegularConflicts: overwrite, importIdMap, }; + + /** + * If dataSourceId exist, + */ + if (dataSourceId) { + const checkConflictsForDataSourceResult = await checkConflictsForDataSource({ + objects: checkConflictsResult.filteredObjects, + ignoreRegularConflicts: overwrite, + dataSourceId, + }); + checkOriginConflictsParams.objects = checkConflictsForDataSourceResult.filteredObjects; + } + const checkOriginConflictsResult = await checkOriginConflicts(checkOriginConflictsParams); errorAccumulator = [...errorAccumulator, ...checkOriginConflictsResult.errors]; importIdMap = new Map([...importIdMap, ...checkOriginConflictsResult.importIdMap]);