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 (opensearch-project#5871) (opensearch-project#5891) * bug fix 5861 Signed-off-by: yujin-emma <[email protected]> * clean unused parameters in test Signed-off-by: yujin-emma <[email protected]> * bug fix 5861 Signed-off-by: yujin-emma <[email protected]> * clean unused parameters in test Signed-off-by: yujin-emma <[email protected]> * fix failed UT Signed-off-by: yujin-emma <[email protected]> * update CHANGELOG.md Signed-off-by: yujin-emma <[email protected]> * Update changelog message Signed-off-by: yujin-emma <[email protected]> --------- Signed-off-by: yujin-emma <[email protected]> Signed-off-by: Yu Jin <[email protected]> (cherry picked from commit d8aefae) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
bandinib-amzn · Feb 20, 2024 · 57200af · 57200af
1 parent 2b7fdf2
commit 57200af
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 34 deletions.
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]);