Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Feb 16, 2024
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this change comes from rebasing the main, this change is from this commit: yujin-emma@e08bf30


### 🐛 Bug Fixes

Expand Down Expand Up @@ -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] Update checkConflictsForDataSource and import_saved_objects to fix bug([#5861](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5871))


### 🚞 Infrastructure

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ describe('#checkConflictsForDataSource', () => {
filteredObjects: [],
errors: [],
importIdMap: new Map(),
pendingOverwrites: new Set(),
});
});

Expand All @@ -83,7 +82,6 @@ describe('#checkConflictsForDataSource', () => {
filteredObjects: [dataSourceObj1, dataSourceObj2],
errors: [],
importIdMap: new Map(),
pendingOverwrites: new Set(),
});
});

Expand Down Expand Up @@ -118,10 +116,6 @@ describe('#checkConflictsForDataSource', () => {
{ id: 'currentDsId_id-2', omitOriginId: true },
],
]),
pendingOverwrites: new Set([
`${dataSourceObj1.type}:${dataSourceObj1.id}`,
`${dataSourceObj2.type}:${dataSourceObj2.id}`,
]),
})
);
});
Expand All @@ -144,7 +138,6 @@ describe('#checkConflictsForDataSource', () => {
},
],
importIdMap: new Map(),
pendingOverwrites: new Set(),
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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 };
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ describe('#importSavedObjectsFromStream', () => {
errors: [],
filteredObjects: [],
importIdMap: new Map(),
pendingOverwrites: new Set(),
});
getMockFn(createSavedObjects).mockResolvedValue({ errors: [], createdObjects: [] });
});
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -438,7 +443,7 @@ describe('#importSavedObjectsFromStream', () => {
};

test('with createNewCopies disabled', async () => {
const options = setupOptions(false, undefined);
const options = setupOptions();
getMockFn(checkConflicts).mockResolvedValue({
errors: [],
filteredObjects: [],
Expand Down Expand Up @@ -500,7 +505,6 @@ describe('#importSavedObjectsFromStream', () => {
errors: [],
filteredObjects: [],
importIdMap: new Map(),
pendingOverwrites: new Set([`${dsSuccess2.type}:${dsSuccess2.id}`]),
});
getMockFn(checkConflicts).mockResolvedValue({
errors: [],
Expand Down Expand Up @@ -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]],
Expand Down
29 changes: 13 additions & 16 deletions src/core/server/saved_objects/import/import_saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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]);
Expand Down
Loading