Skip to content

feat(ops): implement MoveFile and CopyItem across all providers#6

Merged
brunolnetto merged 7 commits into
mainfrom
feat/implement-stub-operations
Jul 8, 2026
Merged

feat(ops): implement MoveFile and CopyItem across all providers#6
brunolnetto merged 7 commits into
mainfrom
feat/implement-stub-operations

Conversation

@brunolnetto

Copy link
Copy Markdown
Contributor

Closes #5

What

Implements MoveFile() (was throwing "not yet implemented") and fills the CopyItem() gap in OneDrive.

Changes

Interface (ICloudBackend)

  • New MoveItem() virtual method with default "not supported" fallback
  • All 6 backends override it with their native move/rename API

CloudFileSystem::MoveFile()

  • Validates same-provider and same-root constraint (cross-root moves unsupported)
  • Resolves root ID (with cache), stat source item, stat destination parent
  • Delegates to backend.MoveItem() then invalidates cache for both paths

Per-provider implementations

Provider API Notes
SharePoint PATCH /drives/{root}/items/{id} with name + parentReference Microsoft Graph
OneDrive Same Graph PATCH pattern + adds missing CopyItem()
Google Drive GET /files/{id}?fields=parents then PATCH with addParents/removeParents Extra GET prevents multi-parent artifacts
Dropbox POST /files/move_v2 Symmetric to existing CopyItem
SFTP libssh2_sftp_rename_ex() with OVERWRITE|ATOMIC|NATIVE flags Atomic on POSIX
VFS Agent POST /v1/move with {"from": ..., "to": ...} Requires agent-side endpoint

OneDrive CopyItem() fix

OneDriveBackend::Capabilities() declared supports_server_side_copy = true but CopyItem() was never implemented (fell through to the base class error). Added POST /items/{id}/copy (202 Accepted pattern, same as SharePoint).

Test

Build: 0 errors, 0 warnings
Binary: cloudfs.duckdb_extension (11 MB)

Coverage after this PR

                 GDrive  Dropbox  OneDrive  SharePoint  SFTP  VFS
MoveFile()         ✅      ✅        ✅         ✅        ✅    ✅
CopyItem()         ✅      ✅        ✅         ✅        ❌    ❌

SFTP and VFS copy remain unsupported (no native server-side copy in those protocols).

Closes #5

Adds MoveItem() virtual method to ICloudBackend interface.
Implements server-side move/rename for all 6 providers:

SharePoint:
  - PATCH /drives/{root}/items/{id} with name + parentReference
  - Cache invalidation on source and destination parent

OneDrive:
  - PATCH /drives/{root}/items/{id} (same Graph API as SharePoint)
  - Also adds CopyItem() (was missing despite supports_server_side_copy=true)

Google Drive:
  - GET /files/{id}?fields=parents to retrieve old parent IDs
  - PATCH with addParents/removeParents to avoid multi-parent artifacts

Dropbox:
  - POST /files/move_v2 (symmetric to existing CopyItem implementation)

SFTP:
  - libssh2_sftp_rename_ex() with OVERWRITE|ATOMIC|NATIVE flags

VFS Agent:
  - POST /v1/move with {from, to} JSON body

CloudFileSystem::MoveFile():
  - Validates same-provider and same-root constraint
  - Resolves root (with cache), src item, dst parent
  - Delegates to backend MoveItem()
  - Invalidates cache for both src and dst paths

Build: 0 errors, 0 warnings
Copilot AI review requested due to automatic review settings July 7, 2026 23:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Implements a server-side move/rename pathway across all CloudFS providers by introducing ICloudBackend::MoveItem() and wiring CloudFileSystem::MoveFile() to validate same-provider/same-root moves and delegate to provider-native APIs. Also fills the OneDrive CopyItem() implementation gap to match its declared capabilities.

Changes:

  • Adds ICloudBackend::MoveItem() with a default “not supported” fallback and overrides it in all providers.
  • Implements CloudFileSystem::MoveFile() to parse/validate URLs, resolve roots, stat source and destination parent, and execute MoveItem().
  • Implements provider-specific move APIs (Graph PATCH, Google Drive parents patch, Dropbox move_v2, SFTP rename, VFS agent move) and adds OneDrive CopyItem().

Reviewed changes

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

Show a summary per file
File Description
src/core/cloud_filesystem.cpp Implements MoveFile() with same-provider/same-root validation, stats, delegation, and cache invalidation.
src/include/core/cloud_backend.hpp Introduces virtual MoveItem() with a default unsupported fallback.
src/providers/vfs/vfs_backend.cpp Adds VFS agent-backed MoveItem() calling /v1/move.
src/include/providers/vfs_backend.hpp Declares VFSBackend::MoveItem() override.
src/providers/sharepoint/sharepoint_backend.cpp Adds SharePoint Graph MoveItem() via PATCH on the item.
src/include/providers/sharepoint_backend.hpp Declares SharePointBackend::MoveItem() override.
src/providers/sftp/sftp_backend.cpp Adds SFTP MoveItem() via libssh2_sftp_rename_ex().
src/include/providers/sftp_backend.hpp Declares SFTPBackend::MoveItem() override.
src/providers/onedrive/onedrive_backend.cpp Adds OneDrive Graph CopyItem() and MoveItem() implementations.
src/include/providers/onedrive_backend.hpp Declares OneDriveBackend::CopyItem() and MoveItem() overrides.
src/providers/gdrive/gdrive_backend.cpp Adds Google Drive MoveItem() by adjusting parents and renaming.
src/include/providers/gdrive_backend.hpp Declares GDriveBackend::MoveItem() override.
src/providers/dropbox/dropbox_backend.cpp Adds Dropbox MoveItem() via /files/move_v2.
src/include/providers/dropbox_backend.hpp Declares DropboxBackend::MoveItem() override.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/cloud_filesystem.cpp
Comment thread src/providers/gdrive/gdrive_backend.cpp Outdated
Comment on lines +234 to +250
// Extract first parent ID from JSON array ["id1","id2",...]
std::string old_parent;
auto parents_pos = meta_resp.body.find("\"parents\"");
if (parents_pos != std::string::npos) {
auto arr_start = meta_resp.body.find('[', parents_pos);
auto first_q = meta_resp.body.find('"', arr_start + 1);
auto second_q = meta_resp.body.find('"', first_q + 1);
if (first_q != std::string::npos && second_q != std::string::npos)
old_parent = meta_resp.body.substr(first_q + 1, second_q - first_q - 1);
}

std::string url = kApiBase + "/files/" + src_id + "?supportsAllDrives=true";
if (!old_parent.empty())
url += "&removeParents=" + old_parent;
url += "&addParents=" + dst_parent_id;

std::string body = "{\"name\":\"" + dst_name + "\"}";
Comment on lines +284 to +291
HttpRequest req;
req.method = "POST";
req.SetBearerAuth(tok);
req.headers["Content-Type"] = "application/json";
req.url = std::string(kApiBase) + "/files/move_v2";
req.body =
"{\"from_path\":\"" + src_id + "\",\"to_path\":\"" + dst_parent_id + "/" + dst_name + "\"}";
auto resp = http_.Execute(req);
Comment on lines +237 to +238
std::string body =
"{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}";
Comment thread src/providers/onedrive/onedrive_backend.cpp Outdated
Comment on lines +186 to +187
std::string body =
"{\"name\":\"" + dst_name + "\",\"parentReference\":{\"id\":\"" + dst_parent_id + "\"}}";
brunolnetto and others added 3 commits July 7, 2026 21:44
…, Dropbox path, GDrive parents

Resolves all 6 review comments on PR #6:

1. dst_name empty validation (MoveFile):
   - Throw clear error when destination path ends with '/' (no filename)

2. MoveItem interface: CloudItem objects instead of bare string IDs
   - Dropbox move_v2 needs .path (not opaque id) for to_path
   - Graph/GDrive backends continue using .id (unchanged semantics)
   - Signature: MoveItem(root, const CloudItem& src, const CloudItem& dst_parent, ...)

3. JSON injection — add JsonUtil::EscapeJsonString() for dst_name in:
   - SharePoint, OneDrive, Google Drive, Dropbox MoveItem bodies

4. Dropbox MoveItem: from_path uses src_item.path, to_path uses
   dst_parent.path + '/' + dst_name (not opaque Dropbox IDs)

5. GDrive parent parsing: safe extraction of ALL parents from JSON array
   - Eliminate arr_start+1 overflow (was size_t wrap when '[' not found)
   - Remove ALL parents (not just first) to prevent multi-parent artifacts

6. Format: clang-format -style=LLVM applied to all changed files
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@brunolnetto

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts and pushed the result in 756fa4d, with a small follow-up cleanup in b994b28.

@brunolnetto

Copy link
Copy Markdown
Contributor Author

@copilot



Found differences in file src/include/providers/gdrive_backend.hpp



+++

@@ -99,7 +99,7 @@

               const std::string& dst_name, const std::string& token, std::string& err) override;

 bool AbortUpload(const CloudUploadSession& session, const std::string& token,
  •                std::string& err) override;
    
  •                 std::string& err) override;
    

    private:
    // Path resolution: Google Drive has no native path API.
    src/providers/gdrive/gdrive_auth.cpp
    src/providers/gdrive/gdrive_backend.cpp
    src/providers/onedrive/onedrive_auth.cpp
    src/providers/onedrive/onedrive_backend.cpp
    src/providers/sftp/sftp_backend.cpp
    src/providers/sharepoint/sharepoint_auth.cpp
    src/providers/sharepoint/sharepoint_backend.cpp
    src/providers/vfs/vfs_backend.cpp
    test/sql/all_providers.test
    test/test_suite.py



Found differences in file src/providers/onedrive/onedrive_backend.cpp



+++

@@ -168,9 +168,9 @@

                            const std::string& tok, std::string& err) {
 std::string url =
     "https://graph.microsoft.com/v1.0/drives/" + root + "/items/" + src_id + "/copy";
  • std::string body =
  •    "{\"parentReference\":{\"id\":\"" + JsonUtil::EscapeJsonString(dst_parent_id) +
    
  •    "\"},\"name\":\"" + JsonUtil::EscapeJsonString(dst_name) + "\"}";
    
  • std::string body = "{"parentReference":{"id":"" +
  •                   JsonUtil::EscapeJsonString(dst_parent_id) + "\"},\"name\":\"" +
    
  •                   JsonUtil::EscapeJsonString(dst_name) + "\"}";
    
    auto resp = http_.Post(url, tok, body);
    if (resp.status != 202) {
    err = "copy failed (" + std::to_string(resp.status) + ")";

Failed format-check: differences were found in the following files:

  • src/core/cloud_filesystem.cpp
  • src/include/providers/gdrive_backend.hpp
  • src/providers/onedrive/onedrive_backend.cpp
    Run "make format-fix" to fix these differences automatically @copilot

@brunolnetto brunolnetto merged commit 27f90e9 into main Jul 8, 2026
25 of 26 checks passed
@brunolnetto brunolnetto deleted the feat/implement-stub-operations branch July 8, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement MoveFile and CopyItem across all providers (stub audit follow-up)

3 participants