Fixed Previously attached MongoDB RU accounts get lost #2649

[tnaum-ms]

This pull request adds migration support for Cosmos DB accounts with the MongoDB API. It improves the handling of legacy account data.

The new migration code is enclosed in the function postPickSupportedAccountsCleanUp that corrects the storage location of MongoDB and MongoClusters storage items.

Additional work has been done around refactoring. The migration code is now located in one file. This file should be deleted "in a couple of releases" once we assume the migration to be completed.

Fixes #2649

---

Copilot:

Migration Functions and Logic:

• Added pickSupportedAccounts, postPickSupportedAccountsCleanUp, and migrateV1AccountsToV2 functions in src/tree/workspace-view/accountMigration.ts to handle account migration from the old storage format to the new one and address specific migration issues. These functions include telemetry tracking and error handling.

Integration with Existing Code:

• Updated CosmosDBWorkspaceItem and AccountsItem classes to invoke the new postPickSupportedAccountsCleanUp function during their getChildren operations to ensure migration cleanup is performed. [1] [2]

Code Simplification:

• Removed redundant pickSupportedAccounts and migrateV1AccountsToV2 methods from CosmosDBWorkspaceItem after moving their logic into the new accountMigration.ts file for better modularity.

Refactoring:

• Adjusted imports in CosmosDBWorkspaceItem.ts and AccountsItem.ts to use the new migration functions from accountMigration.ts. [1] [2]

[Copilot]

Pull Request Overview

This PR improves migration support for Cosmos DB accounts with the MongoDB API by consolidating legacy migration logic into a dedicated module and updating relevant workspace items. Key changes include:

• Introducing migration functions (pickSupportedAccounts, postPickSupportedAccountsCleanUp, migrateV1AccountsToV2) in a new file for modularity.
• Updating AccountsItem.ts and CosmosDBWorkspaceItem.ts to invoke the new migration functions and remove redundant legacy code.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/tree/workspace-view/documentdb/AccountsItem.ts	Updated to call postPickSupportedAccountsCleanUp in getChildren for migration cleanup.
src/tree/workspace-view/cosmosdb/CosmosDBWorkspaceItem.ts	Updated to invoke pickSupportedAccounts and postPickSupportedAccountsCleanUp and removed obsolete migration functions.
src/tree/workspace-view/accountMigration.ts	New migration module consolidating and refining account migration logic.

[Copilot]

Pull Request Overview

This pull request introduces migration support for Cosmos DB accounts with the MongoDB API by moving legacy account data to a new storage location and refactoring migration logic into a dedicated module.

• Introduces new migration functions (pickSupportedAccounts, postPickSupportedAccountsCleanUp, migrateV1AccountsToV2) in accountMigration.ts.
• Refactors CosmosDBWorkspaceItem and AccountsItem to leverage the new migration functions and removes redundant legacy methods.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/tree/workspace-view/documentdb/AccountsItem.ts	Added a migration call (postPickSupportedAccountsCleanUp) in getChildren to clean up legacy storage.
src/tree/workspace-view/cosmosdb/CosmosDBWorkspaceItem.ts	Updated to call pickSupportedAccounts and postPickSupportedAccountsCleanUp and removed redundant migration methods.
src/tree/workspace-view/accountMigration.ts	New module encapsulating migration functions with telemetry tracking and error handling.

Comments suppressed due to low confidence (1)

src/tree/workspace-view/accountMigration.ts:129

• [nitpick] Consider using a centralized logging or telemetry mechanism instead of direct console logging for consistency in error reporting across the migration process.

console.log(`Failed to delete item ${item.id} from AttachedAccounts, will retry on next run: ${deleteError}`);

[Copilot]

Pull Request Overview

Adds migration support to correctly handle legacy Cosmos DB accounts using the MongoDB API by extracting and centralizing migration functions, and integrating them into account tree items.

• Extracted migration logic into src/tree/workspace-view/accountMigration.ts, including pickSupportedAccounts, postPickSupportedAccountsCleanUp, and migrateV1AccountsToV2.
• Updated AccountsItem and CosmosDBWorkspaceItem to invoke the new migration functions and removed outdated methods.
• Refactored imports and removed duplicate legacy migration code from CosmosDBWorkspaceItem.ts.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/tree/workspace-view/documentdb/AccountsItem.ts	Import and call postPickSupportedAccountsCleanUp()
src/tree/workspace-view/cosmosdb/CosmosDBWorkspaceItem.ts	Replace inline migration methods with shared functions
src/tree/workspace-view/accountMigration.ts	New file containing extracted migration functions

Comments suppressed due to low confidence (2)

src/tree/workspace-view/accountMigration.ts:82

• [nitpick] The name postPickSupportedAccountsCleanUp is verbose and less descriptive of its purpose. Consider renaming it to something like cleanupMongoClusterAccounts to clarify intent.

export async function postPickSupportedAccountsCleanUp(): Promise<void> {

src/tree/workspace-view/documentdb/AccountsItem.ts:31

• It looks like pickSupportedAccounts() isn't invoked here before the cleanup step, which may leave some legacy accounts unprocessed. Consider calling pickSupportedAccounts() first to ensure all old-format accounts are migrated.

await postPickSupportedAccountsCleanUp();

[Copilot]

Calling getItems() inside the loop causes an N+1 read pattern. For better performance, fetch the target items once outside the loop or track which IDs have been pushed without repeated fetches.

Suggested change

	const migratedItems = await StorageService.get(StorageNames.Workspace).getItems(
	WorkspaceResourceType.MongoClusters,
	);
	const isMigrated = migratedItems.some((migratedItem) => migratedItem.id === newStorageId);
	const isMigrated = migratedItemIds.has(newStorageId);

[tnaum-ms]

This approach is intended as we really want to verify that the newly added item has been saved.

[tnaum-ms]

@sevoku Now it's better - it works with our storageId generator