Copy-and-Paste 2: Basic copy and paste task by tnaum-ms · Pull Request #147 · microsoft/vscode-documentdb

tnaum-ms · 2025-06-10T07:16:45Z

Closes #111

…aste-2-implement-basic-copy-and-paste-task

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…-documentdb into dev/xingfan/111-copy-and-paste-2-implement-basic-copy-and-paste-task

…into dev/xingfan/111-copy-and-paste-2-implement-basic-copy-and-paste-task

tnaum-ms · 2025-06-18T17:23:47Z

@xingfan-git look at the new additions to our base feature-branch, the Task API is now simpler to use, and, progress monitoring is in place ( #114 ) . I didn't have enough time to implement the fancy one - it's in the queue for later ( #182 ).

There is a test command in there in the feature branch, just open the command palette and search for demo. You'll see the DemoTask in action with its progress reporting.

Recording.2025-06-18.192942.mp4

This reverts commit 52eec8d.

Copilot

Pull Request Overview

This PR implements a basic copy-paste functionality for MongoDB collections, allowing users to copy documents from one collection to another. The implementation follows a database-agnostic design pattern using interfaces for document reading and writing operations.

Key changes:

Adds copy-paste utility interfaces and types for generic document operations
Implements a task-based copy-paste collection feature with memory-efficient streaming
Provides MongoDB-specific implementations for document reading and writing operations

Reviewed Changes

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

Show a summary per file

File	Description
src/utils/copyPasteUtils.ts	Defines interfaces and types for copy-paste operations including conflict resolution strategies
src/services/tasks/CopyPasteCollectionTask.ts	Implements the main task logic for copying collections with buffer-based streaming
src/documentdb/DocumentProvider.ts	MongoDB-specific implementations of DocumentReader and DocumentWriter interfaces
src/documentdb/ClustersClient.ts	Adds transaction support and helper methods for MongoDB operations
src/commands/copyCollection/copyCollection.ts	Simple command to mark a collection for copying
src/commands/pasteCollection/pasteCollection.ts	Command to execute the copy-paste operation with user confirmation
src/commands/importDocuments/importDocuments.ts	Updates to use new MongoDB error handling patterns
src/extensionVariables.ts	Adds temporary storage for copied collection reference
package.json	Registers new copy and paste collection commands
l10n/bundle.l10n.json	Adds localization strings for the new functionality

Copilot · 2025-08-12T20:04:52Z


+    // TODO: TN improve this: This is a temporary solution to get going.
+    export let copiedCollectionNode: CollectionItem | undefined;
+


This TODO comment indicates technical debt. Consider implementing a more robust clipboard-like service instead of storing state in global extension variables, which could lead to issues with concurrent operations or extension state management.

Suggested change

/**

* Clipboard-like service for managing copied collection nodes.

*/

export class CollectionClipboardService {

private static _copiedCollectionNode: CollectionItem | undefined;

public static setCopiedCollectionNode(node: CollectionItem | undefined): void {

CollectionClipboardService._copiedCollectionNode = node;

}

public static getCopiedCollectionNode(): CollectionItem | undefined {

return CollectionClipboardService._copiedCollectionNode;

}

public static clearCopiedCollectionNode(): void {

CollectionClipboardService._copiedCollectionNode = undefined;

}

}

Copilot · 2025-08-12T20:04:52Z

+
+    // Check type of sourceNode or targetNodeAdd commentMore actions
+    // Currently we only support CollectionItem types
+    // Later we need to check if they are supported types that with document reader and writer implementations


The comment contains a formatting error: 'targetNodeAdd commentMore actions' should be properly formatted and clarified.

Suggested change

// Later we need to check if they are supported types that with document reader and writer implementations

// Check type of sourceNode and targetNode

// Currently we only support CollectionItem types

// Later we need to check if they are supported types that have document reader and writer implementations

Copilot · 2025-08-12T20:04:52Z

+    );
+
+    // void vscode.window.showInformationMessage(`${sourceInfo}\n${targetInfo}`);
+    // Confirm the copy operation with the userAdd commentMore actions


The comment contains a formatting error: 'userAdd commentMore actions' should be properly formatted and clarified.

Suggested change

// Confirm the copy operation with the userAdd commentMore actions

// Confirm the copy operation with the user

Copilot · 2025-08-12T20:04:53Z

+        //     config.source.databaseName,
+        //     config.source.collectionName,
+        // );
+


This commented-out code block should be removed if it's not needed, as it adds unnecessary clutter to the codebase.

Suggested change

Copilot · 2025-08-12T20:04:53Z

+        if (result.errors !== null) {
+            // For basic implementation with abort strategy, any error should fail the task
+            if (this.config.onConflict === ConflictResolutionStrategy.Abort) {
+                const firstError = result.errors[0] as { error: Error };


Using type assertion with 'as' violates the TypeScript strict mode guidelines. The code should use proper type guards or handle the possibility that errors[0] might not exist or have the expected structure.

Copilot · 2025-08-12T20:04:53Z

+                    const session = client.startTransaction();
+                    const collection = client.getCollection(databaseName, collectionName);
+                    try {
+                        // eslint-disable-next-line @typescript-eslint/no-unsafe-return


The eslint-disable comment indicates unsafe type handling. The getOperation() method should be properly typed instead of disabling type safety warnings.

…aste-2-implement-basic-copy-and-paste-task

tnaum-ms

@xingfan-git I added my review comments and now I'll move forward with changes, as discussed.

This means that my comments might be out of date, but please read these anyway to receive feedback.

We'll discuss next step in a dedicated PR.

tnaum-ms · 2025-08-12T20:31:52Z

        collectionName: string,
        documents: Document[],
-    ): Promise<InsertDocumentsResult> {
+        ordered: boolean = true,


we should improve documentation here

tnaum-ms · 2025-08-12T20:33:33Z

+    ): AsyncIterable<DocumentDetails> {
+        const client = await ClustersClient.getClient(connectionId);
+
+        const documentStream = client.streamDocuments(databaseName, collectionName, new AbortController().signal);


let's make sure we can abort this.

tnaum-ms · 2025-08-13T10:11:09Z

+    try {
+        const insertResult = await client.insertDocuments(
+            databaseName,
+            collectionName,
+            documentsToProcess as Document[],
+            false,
+        );
+        return {
+            count: insertResult.insertedCount,
+            errorOccurred: false,
+        };
+    } catch (error) {
+        if (isMongoBulkWriteError(error)) {
+            // Handle MongoDB bulk write errors
+            // It could be a partial failure, so we need to check the result
+            return {
+                count: error.result.insertedCount,
+                errorOccurred: true,
+            };
+        } else {
+            // Handle other errors
+            return {
+                count: 0,
+                errorOccurred: true,
+            };
+        }
+    }
 }


@xingfan-git thanks for this update. Let's keep features/fixes separate to simplify feature delivery. This could be in a separate issue/branch/PR

tnaum-ms · 2025-08-13T10:15:51Z

+
+        // Wait for the task to complete
+        while (task.getStatus().state === TaskState.Running || task.getStatus().state === TaskState.Initializing) {
+            // Simple polling with a small delay
+            await new Promise((resolve) => setTimeout(resolve, 100));
+        }
+
+        // Check final status and show result
+        const finalStatus = task.getStatus();
+        if (finalStatus.state === TaskState.Completed) {
+            void vscode.window.showInformationMessage(
+                l10n.t('Collection copied successfully: {0}', finalStatus.message || ''),
+            );
+        } else if (finalStatus.state === TaskState.Failed) {
+            const errorToThrow =
+                finalStatus.error instanceof Error ? finalStatus.error : new Error('Copy operation failed');
+            throw errorToThrow;
+        } else if (finalStatus.state === TaskState.Stopped) {
+            void vscode.window.showInformationMessage(l10n.t('Copy operation was cancelled.'));
+        }
+    } catch (error) {
+        const errorMessage = error instanceof Error ? error.message : String(error);
+        void vscode.window.showErrorMessage(l10n.t('Failed to copy collection: {0}', errorMessage));
+        throw error;
+    } finally {
+        // Clean up - remove the task from the service after completion
+        try {
+            const task = TaskService.listTasks().find((t) => t.type === 'copy-paste-collection');
+            if (task) {
+                await TaskService.deleteTask(task.id);
+            }
+        } catch (cleanupError) {
+            // Log cleanup error but don't throw
+            console.warn('Failed to clean up copy-paste task:', cleanupError);
+        }
+    }


@xingfan-git we should not wait for the task to complete here - it's a fire and forget scenario. If the task needs work to be done once it finishes, it needs to be done in the task itself. if we need extra "post task" UI step etc., and you think that the architecture does not support it, then it's something we should consier in the architecture and extend the Task API.

I'll remove/comment it out for discussion.

tnaum-ms · 2025-08-13T10:18:06Z

        return documents;
    }

+    async countDocuments(databaseName: string, collectionName: string, findQuery: string = '{}'): Promise<number> {


TODO: create an issue to add support for "abort" support to runQuery and for countDocuments

tnaum-ms · 2025-08-13T11:03:08Z

+                if (signal.aborted) {
+                    // Cleanup any remaining buffer
+                    if (buffer.length > 0) {
+                        await this.flushBuffer(buffer);
+                    }
+                    return;


hm, let's just abort without writing more into the target collection.

tnaum-ms · 2025-08-13T11:07:29Z

+export interface DocumentDetails {
+    /**
+     * The document's unique identifier (e.g., _id in MongoDB)
+     */
+    id: unknown;
+
+    /**
+     * The document content treated as opaque data by the core task logic.
+     * Specific readers/writers will know how to interpret/serialize this.
+     * For MongoDB, this would typically be a BSON document.
+     */
+    documentContent: unknown;
+}


@xingfan-git let's discuss adding a size estimate here so maybe we can get this from the document reader and keep it so that it's always available.

tnaum-ms

submitting comments

tnaum-ms · 2025-08-13T11:18:34Z

+        const count = await collection.countDocuments(findQueryObj, {
+            // Use a read preference of 'primary' to ensure we get the most up-to-date
+            // count, especially important for sharded clusters.
+            readPreference: 'primary',


@xingfan-git good catch!

… calls

tnaum-ms added 4 commits June 10, 2025 09:00

added copy/paste commands to package.json

52a7720

Merge branch 'feature/copy-and-paste' into dev/xingfan/111-copy-and-p…

59a970b

…aste-2-implement-basic-copy-and-paste-task

feat: basic copy+paste UX for experimenting

33f7d40

fix: updated eslint config to exclude the api/dist folder

aca6916

tnaum-ms linked an issue Jun 10, 2025 that may be closed by this pull request

Copy-and-Paste: 2. Implement Basic Copy-and-Paste Task #111

Closed

tnaum-ms requested a review from Copilot June 10, 2025 07:17

This comment was marked as outdated.

Sign in to view

tnaum-ms and others added 4 commits June 10, 2025 09:59

Apply suggestions from code review

eab7593

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'copilot/fix-130' of https://github.com/microsoft/vscode…

30d5a72

…-documentdb into dev/xingfan/111-copy-and-paste-2-implement-basic-copy-and-paste-task

Merge branch 'next' of https://github.com/microsoft/vscode-documentdb …

a4df3f6

…into dev/xingfan/111-copy-and-paste-2-implement-basic-copy-and-paste-task

draft copy paste task

52eec8d

xingfan-git added 7 commits June 19, 2025 08:59

Revert "draft copy paste task"

3a737fb

This reverts commit 52eec8d.

merge feature/copy-and-paste branch to pull changes for task progress

1e76085

draft version for copy paste collection task

95ee9da

refine insert result parsing

fbf169a

two strategies

c1e8552

l10n

b721aaf

overwrite

2458adc

tnaum-ms mentioned this pull request Aug 6, 2025

Implement basic copy and paste task engine #188

Closed

tnaum-ms requested a review from Copilot August 12, 2025 20:03

Copilot AI reviewed Aug 12, 2025

View reviewed changes

Merge branch 'feature/copy-and-paste' into dev/xingfan/111-copy-and-p…

1f4c942

…aste-2-implement-basic-copy-and-paste-task

tnaum-ms commented Aug 13, 2025

View reviewed changes

Updated copy-paste-task, minor changes + update to final state reporting

d8eff8d

tnaum-ms commented Aug 13, 2025

View reviewed changes

tnaum-ms added 4 commits August 13, 2025 13:55

l10n updates

21b409b

feat: centralized task status udpates in the outputChannel

96a841c

refreshed Copy-Paste task implememtation for readability

a955b24

ux tweak / emojii...

7582e3b

tnaum-ms added 5 commits August 13, 2025 12:24

refactoring file locations

3037998

finalized refactoring

cd7983d

removed obsolete code / commented out code.

e567242

tweak to the copy-and-paste task display name

884116d

fix: updated taskService tests to ignore ext.outputChannel.appendLine…

85c31a4

… calls

tnaum-ms merged commit 35221cb into feature/copy-and-paste Aug 13, 2025
5 checks passed

tnaum-ms deleted the dev/xingfan/111-copy-and-paste-2-implement-basic-copy-and-paste-task branch August 13, 2025 13:04


		// TODO: TN improve this: This is a temporary solution to get going.
		export let copiedCollectionNode: CollectionItem \| undefined;

+    /**
+     * Clipboard-like service for managing copied collection nodes.
+     */
+    export class CollectionClipboardService {
+        private static _copiedCollectionNode: CollectionItem | undefined;
+        public static setCopiedCollectionNode(node: CollectionItem | undefined): void {
+            CollectionClipboardService._copiedCollectionNode = node;
+        }
+        public static getCopiedCollectionNode(): CollectionItem | undefined {
+            return CollectionClipboardService._copiedCollectionNode;
+        }
+        public static clearCopiedCollectionNode(): void {
+            CollectionClipboardService._copiedCollectionNode = undefined;
+        }
+    }

	// Confirm the copy operation with the userAdd commentMore actions
	// Confirm the copy operation with the user

Conversation

tnaum-ms commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

tnaum-ms commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tnaum-ms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnaum-ms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tnaum-ms commented Jun 10, 2025 •

edited

Loading

tnaum-ms commented Jun 18, 2025 •

edited

Loading