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

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
@@ -663,6 +663,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
 

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(),
     });
   });
 });

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<SavedObject<{ title?: string }>> = [];
   const errors: SavedObjectsImportError[] = [];
   const importIdMap = new Map<string, ImportIdMapEntry>();
-  const pendingOverwrites = new Set<string>();
-
   // 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 };
 }

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]],

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]);