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

feat: support custom filename when #508

Merged
merged 1 commit into from
May 7, 2024

Conversation

elrrrrrrr
Copy link
Contributor

@elrrrrrrr elrrrrrrr commented May 7, 2024

Uploading files with Buffer defaults the filename to the key or path, which can conflict with middleware checks.

  • 🤖 files as an object defaults filename to the key.
    * 🚨 File paths or streams also override the filename.
  • ♻️ only work for buffer scence.

当直接通过 Buffer 来上传文件时,无法自定义文件名,这在某些中间件会校验 filename,导致无法上传。

  • 🤖 files 参数为 object 时,默认将 key 作为文件名传输
    * 🚨 传入文件路径或 Readable 流时也同样覆盖
  • ♻️ 仅处理 buffer 场景

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features
    • Enhanced file upload functionality to support optional custom file names, allowing users to specify filenames when uploading files.

Copy link

coderabbitai bot commented May 7, 2024

Warning

Rate Limit Exceeded

@elrrrrrrr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 5 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between bc21ec3 and 684e410.

Walkthrough

The recent improvements focus on enhancing file upload functionality within the HttpClient class by enabling custom file names. This update extends to managing the uploadFiles array and adjusting file appending methods for form data. Furthermore, a new test scenario has been added to validate the usage of custom file names in multipart requests.

Changes

File Change Summary
src/HttpClient.ts Added support for custom file names in file uploads. Updated uploadFiles array and form data appending logic.
test/options.files.test.ts Introduced a test case for validating custom file names in multipart requests.

🐇✨
In the code where files glide,
A rabbit tweaked the upload tide.
With a hop, a skip, custom names abide,
In tests, they're checked, with pride they're tried.
Oh, how smoothly the data now slides! 🌟
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@fengmk2 fengmk2 added the enhancement New feature or request label May 7, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between bc21ec3 and aabe7fe.
Files selected for processing (2)
  • src/HttpClient.ts (3 hunks)
  • test/options.files.test.ts (1 hunks)
Additional Context Used
GitHub Check Runs (11)
Node.js / Test (windows-latest, 21) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (windows-latest, 18) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (windows-latest, 16) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (ubuntu-latest, 21) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (ubuntu-latest, 20) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (ubuntu-latest, 18) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (ubuntu-latest, 16) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (macos-latest, 21) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (macos-latest, 20) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (macos-latest, 18) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Node.js / Test (macos-latest, 16) failure (4)

test/options.files.test.ts: [failure] 100-100: test/options.files.test.ts > options.files.test.ts > should upload multi files: Record<field, string> success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'foo.js'

  • 'foo.txt'
    ^

  • Expected

  • Received
  • foo.txt
  • foo.js

❯ test/options.files.test.ts:100:12


test/options.files.test.ts: [failure] 123-123: test/options.files.test.ts > options.files.test.ts > should upload files as object success with default POST method
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'hello'

  • 'bufferfile0'

  • Expected

  • Received
  • bufferfile0
  • hello

❯ test/options.files.test.ts:123:12


test/options.files.test.ts: [failure] 262-262: test/options.files.test.ts > options.files.test.ts > should upload same field name between files and data
AssertionError: Expected values to be strictly equal:

  • actual - expected

  • 'uploadfile'

  • 'options.files.test.ts'

  • Expected

  • Received
  • options.files.test.ts
  • uploadfile

❯ test/options.files.test.ts:262:12

Additional comments not posted (2)
test/options.files.test.ts (1)

291-293: Verify the MIME type for 'file.js'.

Verification successful

The MIME type for 'file.js' is confirmed as 'video/mp2t' in the test case, as shown in the script output. This matches the MIME type specified in the test snippet provided in the review comment. Therefore, the MIME type is correctly implemented as intended in the test case.

  • File: test/options.files.test.ts
  • Line: Assert statement confirming MIME type as 'video/mp2t'
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the MIME type for 'file.js' in the test case.

# Test: Search for the MIME type setting in the test case. Expect: Correct MIME type for a JavaScript file.
rg --type typescript $'file.js' | grep 'mimeType'

Length of output: 81



Script:

#!/bin/bash
# Description: Verify the MIME type for 'file.js' in the test case.

# Correcting the file type to 'ts' for TypeScript files and searching for the MIME type setting.
rg --type ts 'mimeType' | grep 'file.js'

Length of output: 138

src/HttpClient.ts (1)

Line range hint 443-477: The modifications to handle custom filenames during file uploads are correctly implemented.

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

+1

@@ -470,7 +471,7 @@ export class HttpClient extends EventEmitter {
const fileReadable = createReadStream(file);
formData.append(field, new BlobFromStream(fileReadable, mime.lookup(fileName) || ''), fileName);
} else if (Buffer.isBuffer(file)) {
formData.append(field, new Blob([ file ]), `bufferfile${index}`);
formData.append(field, new Blob([ file ]), customFileName || `bufferfile${index}`);
} else if (file instanceof Readable || isReadable(file as any)) {
const fileName = getFileName(file) || `streamfile${index}`;
Copy link
Member

Choose a reason for hiding this comment

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

stream 找不到 filename 的时候也使用 customFileName?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between bc21ec3 and be4de7c.
Files selected for processing (2)
  • src/HttpClient.ts (4 hunks)
  • test/options.files.test.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/HttpClient.ts
  • test/options.files.test.ts

@fengmk2
Copy link
Member

fengmk2 commented May 7, 2024

mac node v14 我处理一下,不再使用了。

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

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

+1

@fengmk2 fengmk2 merged commit 032f439 into node-modules:master May 7, 2024
22 of 24 checks passed
fengmk2 pushed a commit that referenced this pull request May 7, 2024
[skip ci]

## [3.25.0](v3.24.0...v3.25.0) (2024-05-07)

### Features

* support custom filename when file is Buffer or Readable  ([#508](#508)) ([032f439](032f439))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants