fix: address code review feedback on copyObject, e2e helpers, and tests#3
Merged
Cybersoulja merged 2 commits intomainfrom Apr 13, 2026
Merged
Conversation
- helpers.ts: replace browser-only btoa/unescape with Node-safe
Buffer.from(...).toString('base64') in encodeKey and seedEmail
- copyObject.ts: wrap atob decoding in try/catch, return 400 with
descriptive messages for invalid sourceKey/destinationKey; return
c.json({ success: true }) instead of raw R2Object for consistency
- multipart.test.ts: add test for non-base64 httpMetadata in
CreateUpload to complement the existing malformed-JSON test
- object.test.ts: add test asserting Content-Disposition sanitizes
non-ASCII and quote characters in filename while preserving the
original key in the RFC 5987 filename* parameter
- FileOptions.vue: stop folder duplication on first copy failure and
show an error notification instead of silently continuing
https://claude.ai/code/session_018aqegB3K211sW1c1xf8Eux
Expands documentation to reflect the latest codebase including: - Complete API route table with all methods, paths, and handlers - Full TypeScript type definitions (R2ExplorerConfig, ShareMetadata, AppEnv) - Detailed testing infrastructure (worker integration, dashboard unit/component, E2E) - Dashboard router routes and appUtils.js function inventory - Email integration details (receive/send flow) - Module directory layout for worker src/ - Environment bindings (wrangler.toml) example - Debugging tips and security considerations - Key dependencies for both worker and dashboard packages https://claude.ai/code/session_01Fc3LXwYaGM3GKyCQiiQMPa
|
Review these changes at https://app.gitnotebooks.com/Cybersoulja/R2-Explorer/pull/3 |
Reviewer's GuideAddresses review feedback around key encoding/decoding robustness, copyObject error handling/response shape, test coverage for edge cases, and UX for folder copy failures, plus expands CLAUDE.md documentation and project overview details. Sequence diagram for folder duplication error handling in FileOptions componentsequenceDiagram
actor User
participant FileOptions as FileOptions
participant ApiHandler as ApiHandler
participant CopyObjectEndpoint as CopyObjectEndpoint
participant R2Bucket as R2Bucket
participant Notifier as Notifier
User->>FileOptions: Click duplicateFolder
FileOptions->>ApiHandler: createFolder(destPrefix, selectedBucket)
ApiHandler->>CopyObjectEndpoint: POST create folder placeholder
CopyObjectEndpoint->>R2Bucket: put(destPrefix placeholder)
R2Bucket-->>CopyObjectEndpoint: ok
CopyObjectEndpoint-->>ApiHandler: json success
ApiHandler-->>FileOptions: resolved
loop for each innerFile in folderContents
FileOptions->>FileOptions: compute newKey from sourcePrefix and destPrefix
alt innerFile is file
FileOptions->>ApiHandler: copyObject(selectedBucket, innerFile.key, newKey)
ApiHandler->>CopyObjectEndpoint: POST /api/buckets/:bucket/copy
CopyObjectEndpoint->>R2Bucket: get(sourceKey)
alt get succeeds
R2Bucket-->>CopyObjectEndpoint: object
CopyObjectEndpoint->>R2Bucket: put(destinationKey, body, metadata)
R2Bucket-->>CopyObjectEndpoint: ok
CopyObjectEndpoint-->>ApiHandler: json success
ApiHandler-->>FileOptions: resolved
FileOptions->>Notifier: show progress percent
else get or put fails
R2Bucket-->>CopyObjectEndpoint: error
CopyObjectEndpoint-->>ApiHandler: error response
ApiHandler-->>FileOptions: throw error
FileOptions->>FileOptions: set failed true
FileOptions->>Notifier: show negative error notification
FileOptions->>FileOptions: break loop
end
else innerFile is folder
FileOptions->>Notifier: show progress percent
end
end
alt failed is false
FileOptions->>Notifier: show success Folder duplicated
else failed is true
FileOptions->>Notifier: do not show success notification
end
Sequence diagram for CopyObject endpoint with key decoding validationsequenceDiagram
participant ApiHandler as ApiHandler
participant CopyObjectEndpoint as CopyObjectEndpoint
participant R2Bucket as R2Bucket
ApiHandler->>CopyObjectEndpoint: POST /api/buckets/:bucket/copy with base64 keys
alt invalid sourceKey base64
CopyObjectEndpoint->>CopyObjectEndpoint: atob(sourceKey) throws
CopyObjectEndpoint-->>ApiHandler: HTTP 400 Invalid sourceKey
else valid sourceKey base64
CopyObjectEndpoint->>CopyObjectEndpoint: decode sourceKey
alt invalid destinationKey base64
CopyObjectEndpoint->>CopyObjectEndpoint: atob(destinationKey) throws
CopyObjectEndpoint-->>ApiHandler: HTTP 400 Invalid destinationKey
else valid destinationKey base64
CopyObjectEndpoint->>R2Bucket: get(sourceKey)
alt object not found
R2Bucket-->>CopyObjectEndpoint: null
CopyObjectEndpoint-->>ApiHandler: HTTP 404 Object not found
else object found
R2Bucket-->>CopyObjectEndpoint: object with body and metadata
CopyObjectEndpoint->>R2Bucket: put(destinationKey, body, metadata)
R2Bucket-->>CopyObjectEndpoint: ok
CopyObjectEndpoint-->>ApiHandler: json { success: true }
end
end
end
Updated class diagram for CopyObject handler and related typesclassDiagram
class OpenAPIRoute {
<<abstract>>
}
class CopyObject {
+handle(c)
}
class R2Bucket {
+get(key)
+put(key, body, options)
}
class HTTPException {
+HTTPException(status, options)
}
CopyObject --|> OpenAPIRoute
CopyObject --> R2Bucket : uses
CopyObject --> HTTPException : throws
class CopyObjectHandleFlow {
+validateBody()
+decodeSourceKey()
+decodeDestinationKey()
+getSourceObject()
+copyToDestination()
+returnJsonSuccess()
}
CopyObjectHandleFlow <-- CopyObject : handle delegates logic
CopyObjectHandleFlow --> HTTPException : 400 invalid keys
CopyObjectHandleFlow --> HTTPException : 404 object not found
CopyObjectHandleFlow --> R2Bucket : get and put operations
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
FileOptions.vue, consider simplifying the failure branch in the folder-copy loop by returning early from the method inside the catch block instead of using a separatefailedflag plusbreak, which will make the control flow easier to follow. - There are multiple base64 encode/decode implementations across the codebase (worker routes, dashboard
appUtils, e2e helpers); introducing a shared utility for key encoding/decoding would reduce the risk of subtle inconsistencies and keep future changes centralized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `FileOptions.vue`, consider simplifying the failure branch in the folder-copy loop by returning early from the method inside the catch block instead of using a separate `failed` flag plus `break`, which will make the control flow easier to follow.
- There are multiple base64 encode/decode implementations across the codebase (worker routes, dashboard `appUtils`, e2e helpers); introducing a shared utility for key encoding/decoding would reduce the risk of subtle inconsistencies and keep future changes centralized.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Buffer.from(...).toString('base64') in encodeKey and seedEmail
descriptive messages for invalid sourceKey/destinationKey; return
c.json({ success: true }) instead of raw R2Object for consistency
CreateUpload to complement the existing malformed-JSON test
non-ASCII and quote characters in filename while preserving the
original key in the RFC 5987 filename* parameter
show an error notification instead of silently continuing
https://claude.ai/code/session_018aqegB3K211sW1c1xf8Eux
Summary by Sourcery
Document test suites and API surface, harden copy operation and multipart upload validation, improve folder copy UX, and add regression coverage for headers and metadata handling.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: