Bug 5861 Fix: fix import api always display overwritten after impor…

…ting 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 <yujin.emma.work@gmail.com> * clean unused parameters in test Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> * bug fix 5861 Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> * clean unused parameters in test Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> * fix failed UT Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> * update CHANGELOG.md Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> * Update changelog message Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> --------- Signed-off-by: yujin-emma <yujin.emma.work@gmail.com> Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com>
opensearch-project · Feb 16, 2024 · d8aefae · d8aefae
1 parent 30aea11
commit d8aefae
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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 };
 }
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
@@ -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
@@ -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]);