feat: support file upload via application/octet-stream #248
Conversation
WalkthroughAdds single-file upload support end-to-end: new Express POST /uploads endpoint and tests (uploads via path, stream, buffer; missing-file 400; GET /failed-test 404). Introduces a DSLRequestFile type and RequestBuilder.file() overloads (descriptor or DSLRequestFile) that are mutually exclusive with body() and forbid setting Content-Type via header(). Propagates file support through ResponseBuilder (sending path/buffer/stream), TestCaseConfig.requestFile, and TestResult.request.file. RequestBodyBuilder and OpenAPIGenerator updated to emit binary or inferred requestBody and handle missing Content-Type. Adds docs and localized guides for file APIs, updates expected OAS, package scripts, and removes an ESLint directive in logger. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
lib/dsl/interface/field.ts (2)
38-42: Consider renaming to clarify single-file vs. multipart context.The interface name
DSLRequestFileis generic. Since this PR explicitly supports single-fileapplication/octet-streamuploads (not multipart), consider a name likeDSLBinaryRequestFileor add a JSDoc comment clarifying the scope to prevent confusion when multipart support is added later.
86-91: Strengthen stream validation.The current check only verifies that
stream.pipeis a function, which is insufficient to confirm it's a readable stream. Consider using Node's built-in stream detection.+import { Readable } from "stream" + - if ( - typeof stream !== "object" || - typeof (stream as NodeJS.ReadableStream).pipe !== "function" - ) { - throw new Error("fileField(): stream must be a Readable stream.") + if (!(stream instanceof Readable)) { + throw new Error("fileField(): stream must be a Readable stream instance.") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
examples/express/__tests__/expressApp.test.js(3 hunks)examples/express/expressApp.js(1 hunks)lib/dsl/generator/OpenAPIGenerator.ts(1 hunks)lib/dsl/generator/builders/operation/RequestBodyBuilder.ts(2 hunks)lib/dsl/generator/types/TestResult.ts(1 hunks)lib/dsl/index.ts(1 hunks)lib/dsl/interface/field.ts(3 hunks)lib/dsl/interface/index.ts(1 hunks)lib/dsl/test-builders/RequestBuilder.ts(3 hunks)lib/dsl/test-builders/ResponseBuilder.ts(4 hunks)lib/dsl/test-builders/TestCaseConfig.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
lib/dsl/test-builders/RequestBuilder.ts (3)
lib/dsl/interface/field.ts (1)
DSLRequestFile(38-42)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)lib/dsl/test-builders/RootBuilder.ts (2)
RootBuilder(23-35)req(32-34)
lib/dsl/test-builders/TestCaseConfig.ts (2)
lib/dsl/interface/field.ts (1)
DSLRequestFile(38-42)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)
lib/dsl/interface/index.ts (3)
lib/dsl/generator/builders/schema/generators/DSLFieldSchemaGenerator.ts (2)
DSLFieldSchemaGenerator(24-85)enrichSchemaWithMetadata(58-84)lib/__tests__/unit/dsl/interface/field.test.ts (5)
it(20-52)expect(21-27)expect(37-43)expect(29-35)expect(45-51)lib/dsl/interface/ItdocBuilderEntry.ts (1)
ApiDocOptions(52-57)
lib/dsl/generator/OpenAPIGenerator.ts (2)
lib/dsl/generator/builders/operation/interfaces.ts (1)
RequestBodyBuilderInterface(65-72)lib/dsl/generator/types/OpenAPITypes.ts (1)
RequestBodyObject(56-60)
examples/express/expressApp.js (1)
examples/express/__tests__/expressApp.test.js (1)
app(1-1)
lib/dsl/generator/types/TestResult.ts (1)
lib/__tests__/unit/dsl/OpenAPIGenerator.test.ts (5)
testResult(44-63)testResult(89-110)testResult(65-87)testResult1(203-232)testResult(112-156)
lib/dsl/generator/builders/operation/RequestBodyBuilder.ts (2)
lib/dsl/generator/types/OpenAPITypes.ts (2)
Content(34-36)RequestBodyObject(56-60)lib/dsl/generator/builders/schema/index.ts (1)
SchemaBuilder(39-60)
lib/dsl/interface/field.ts (2)
lib/dsl/interface/index.ts (2)
DSLRequestFile(23-23)fileField(22-22)lib/dsl/index.ts (1)
fileField(19-19)
lib/dsl/test-builders/ResponseBuilder.ts (3)
lib/dsl/test-builders/RequestBuilder.ts (1)
body(120-136)lib/dsl/interface/field.ts (1)
DSLRequestFile(38-42)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)
examples/express/__tests__/expressApp.test.js (2)
lib/dsl/interface/describeAPI.ts (1)
describeAPI(30-58)lib/dsl/interface/field.ts (2)
fileField(68-101)field(52-58)
🪛 GitHub Actions: CI Pipeline
examples/express/__tests__/expressApp.test.js
[error] 530-530: ReferenceError: before is not defined. Likely using Mocha-style 'before' in a Jest test environment.
🔇 Additional comments (5)
lib/dsl/index.ts (1)
19-19: LGTM!The addition of
fileFieldto the public API exports is consistent with the existing export pattern.lib/dsl/test-builders/TestCaseConfig.ts (1)
19-19: LGTM!The addition of
requestFiletoTestCaseConfigis well-typed and properly integrated with the newDSLRequestFileinterface.Also applies to: 37-37
lib/dsl/generator/OpenAPIGenerator.ts (1)
526-529: LGTM!The refactor correctly decouples
requestBodyassignment fromresult.request.body, enabling support for file-based requests that don't have a traditional body. This aligns with the PR's goal of supporting binary file uploads.lib/dsl/interface/index.ts (1)
19-19: LGTM!The exports properly extend the public API surface to include the new file upload functionality, maintaining consistency with the existing export pattern.
Also applies to: 22-23
examples/express/__tests__/expressApp.test.js (1)
554-565: Confirm.file()without arguments simulates missing file
Check the DSL’sfile()implementation or documentation to ensure that invoking.file()with no parameters sends an empty upload (triggering HTTP 400 “No file uploaded”) rather than throwing an error at the builder level.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/dsl/test-builders/RequestBuilder.ts (3)
57-62: Content-Type guard still checks wrong object and loses normalization.This issue was flagged in a previous review but remains unfixed. You normalize headers to
normalizedHeaders(line 57), then check the rawheadersobject for"content-type"(line 58), and finally overwrite with rawheaders(line 61). This allows"Content-Type"(capital C, capital T) to slip through, duplicates survive, and normalization is lost.Apply the previously suggested fix:
this.config.requestHeaders = normalizedHeaders -if (headers["content-type"]) { +if (normalizedHeaders["content-type"]) { throw new Error('You cannot set "Content-Type" header using header().') } -this.config.requestHeaders = headers return this
140-142: file() still blocks every header including Authorization.This issue was flagged in a previous review but remains unfixed. The guard
if (this.config.requestHeaders)throws when ANY header exists, making binary uploads unusable for authenticated APIs that needAuthorizationor similar headers.Apply the previously suggested fix to check only for content-type conflicts:
-if (this.config.requestHeaders) { - throw new Error("already defined headers. can't use file()") -} - -if (!requestFile || typeof requestFile !== "object") { - this.config.requestHeaders = { - "content-type": "application/octet-stream", - } - logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") - return this -} - -this.config.requestHeaders = { - "content-type": requestFile.opts?.contentType ?? "application/octet-stream", -} +const existingHeaders = this.config.requestHeaders ?? {} +const hasContentType = Object.keys(existingHeaders).some( + (key) => key.toLowerCase() === "content-type", +) +if (hasContentType) { + throw new Error('You cannot set "Content-Type" header when using file().') +} + +if (!requestFile || typeof requestFile !== "object") { + this.config.requestHeaders = { + ...existingHeaders, + "content-type": "application/octet-stream", + } + logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") + return this +} + +this.config.requestHeaders = { + ...existingHeaders, + "content-type": requestFile.opts?.contentType ?? "application/octet-stream", +}
191-202: body() still allows mixing with file().This issue was flagged in a previous review but remains unfixed. The guard only checks
this.config.requestBody, so calling.file()then.body()succeeds and leaves bothrequestFileandrequestBodypopulated.Apply the previously suggested fix:
-if (this.config.requestBody) { +if (this.config.requestBody || this.config.requestFile) { throw new Error( [ "❌ Conflict: request body has already been set using .body().",
🧹 Nitpick comments (4)
lib/dsl/test-builders/RequestBuilder.ts (1)
158-166: Redundant validation.This check duplicates the validation already performed in
normalizeFileArguments()at lines 113-118. WhennormalizeFileArgumentsreturns a validDSLRequestFile, it has already ensured exactly one source exists.Consider removing this redundant check:
this.config.requestHeaders = { "content-type": requestFile.opts?.contentType ?? "application/octet-stream", } const { file } = requestFile - -const sources = [file.path ? 1 : 0, file.buffer ? 1 : 0, file.stream ? 1 : 0].reduce( - (a, b) => a + b, - 0, -) -if (sources > 1) { - throw new Error( - "req().file(): only one of file.path | file.buffer | file.stream must be provided.", - ) -} if (this.config.requestBody) {lib/dsl/test-builders/ResponseBuilder.ts (3)
101-147: File upload handling is correct with minor duplication.The three branches correctly handle path, buffer, and stream sources, and all properly send the data (the previously flagged buffer issue has been fixed). However, the content-type checking logic is duplicated in each branch.
Consider extracting the content-type check:
if (this.config.requestFile) { const requestFile: DSLRequestFile = this.config.requestFile const contentType = requestFile.opts?.contentType ?? "application/octet-stream" if ( !requestFile.file || (!requestFile.file.path && !requestFile.file.buffer && !requestFile.file.stream) ) { logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") } else { const hasContentType = !!this.config.requestHeaders && Object.keys(this.config.requestHeaders).some( (k) => k.toLowerCase() === "content-type", ) if (!hasContentType) { req.set("Content-Type", contentType) } if (requestFile.file.path) { const buf = fs.readFileSync(requestFile.file.path) req = req.send(buf) } else if (requestFile.file.buffer) { req = req.send(requestFile.file.buffer) } else if (requestFile.file.stream) { const chunks: Buffer[] = [] for await (const chunk of requestFile.file.stream as any) { chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) } const streamBuffer = Buffer.concat(chunks) req = req.send(streamBuffer) } } }
209-216: Remove commented-out code.This commented block appears to be unused and should be removed to keep the codebase clean.
-// let requestType: string = "" -// if (this.config.requestFile) { -// requestType = "binary" -// } else if (this.config.requestBody) { -// requestType = "body" -// } else { -// throw new Error("Could not define requestType") -// } - try {
199-201: Consider the naming mismatch.The property is named
requestBodybut can contain eitherthis.config.requestFileorthis.config.requestBody. While functionally correct, this naming could be confusing for future maintainers.Consider renaming to something more generic like
requestPayloador splitting into separaterequestBodyandrequestFileproperties:request: { path: finalUrl, method: this.method, headers: this.config.requestHeaders, queryParams: this.config.queryParams, pathParams: this.config.pathParams, requestBody: this.config.requestBody, requestFile: this.config.requestFile, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/express/__tests__/expressApp.test.js(2 hunks)lib/dsl/interface/field.ts(1 hunks)lib/dsl/interface/index.ts(1 hunks)lib/dsl/test-builders/RequestBuilder.ts(3 hunks)lib/dsl/test-builders/ResponseBuilder.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/dsl/interface/field.ts
- lib/dsl/interface/index.ts
- examples/express/tests/expressApp.test.js
🧰 Additional context used
🧬 Code graph analysis (2)
lib/dsl/test-builders/ResponseBuilder.ts (2)
lib/dsl/test-builders/RequestBuilder.ts (1)
body(190-206)lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)
lib/dsl/test-builders/RequestBuilder.ts (2)
lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)
🔇 Additional comments (1)
lib/dsl/test-builders/ResponseBuilder.ts (1)
97-99: LGTM!The conditional body send prevents sending an empty JSON object when using file uploads, correctly ensuring only one payload type is sent.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
examples/express/__tests__/expressApp.test.js (1)
573-584: Consider the API design for missing file scenario.Calling
.file()with no arguments to test the missing file case seems counterintuitive. Consider whether:
- The test should simply not call
.file()at all, or- The DSL API should provide a clearer way to express "no file provided" (e.g.,
.file(null)or a separate method)However, if this is the intended API design for testing missing files, the test is functionally correct.
lib/dsl/test-builders/ResponseBuilder.ts (1)
105-109: Consider throwing an error for invalid file specifications.Currently, the code only logs a warning when
requestFileis provided without a valid source. This allows the request to proceed with no file data, which could lead to confusing test failures downstream.Consider applying this diff to fail fast:
if ( !requestFile.file || (!requestFile.file.path && !requestFile.file.buffer && !requestFile.file.stream) ) { - logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") + throw new Error("req().file(): must provide one of file.path, file.buffer, or file.stream")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/express/__tests__/expressApp.test.js(2 hunks)lib/dsl/test-builders/ResponseBuilder.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/express/__tests__/expressApp.test.js (3)
lib/dsl/interface/describeAPI.ts (1)
describeAPI(30-58)lib/dsl/interface/itDoc.ts (1)
itDoc(21-48)lib/dsl/interface/field.ts (1)
field(50-56)
lib/dsl/test-builders/ResponseBuilder.ts (2)
lib/dsl/test-builders/RequestBuilder.ts (1)
body(190-206)lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)
🪛 GitHub Actions: CI Pipeline
examples/express/__tests__/expressApp.test.js
[error] 1-1: Command failed: pnpm test:jest
🔇 Additional comments (7)
examples/express/__tests__/expressApp.test.js (3)
497-518: LGTM!The test structure is correct and follows the established pattern. The intentional 404 response test is useful for verifying error handling.
532-571: LGTM! Good coverage of upload methods.The tests properly cover three upload methods (path, stream, buffer) and use appropriate Node.js APIs. Each test correctly manages file resources.
Note: These tests depend on the file path from line 530, so ensure that path resolves correctly (see previous comment).
520-586: Re-run upload tests locally with dependencies installed
Install dev dependencies inexamples/express(pnpm install), then execute the upload-related tests with verbose output:pnpm test:jest -- --testNamePattern="파일 업로드" --verboseand share the full console output for further diagnosis.
lib/dsl/test-builders/ResponseBuilder.ts (4)
18-18: LGTM!The imports are appropriate for the new file upload functionality.
Also applies to: 26-26
97-99: LGTM!Guarding against sending an empty body object is correct, especially when file uploads are used instead of JSON bodies.
199-201: LGTM!Preferring
requestFileoverrequestBodyin the logged request is the correct choice for file uploads.
228-228: LGTM!Adding the
filefield toTestResult.requestproperly captures file upload metadata in test results.
| const hasContentType = | ||
| !!this.config.requestHeaders && | ||
| Object.keys(this.config.requestHeaders).some( | ||
| (k) => k.toLowerCase() === "content-type", | ||
| ) | ||
| if (!hasContentType) { | ||
| req.set("Content-Type", contentType) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract repeated Content-Type checking logic.
The Content-Type header detection is duplicated three times across the path, buffer, and stream branches. Extract this into a helper method to improve maintainability.
Consider adding a private helper method:
private setContentTypeIfNotPresent(req: supertest.Test, contentType: string): void {
const hasContentType =
!!this.config.requestHeaders &&
Object.keys(this.config.requestHeaders).some(
(k) => k.toLowerCase() === "content-type",
)
if (!hasContentType) {
req.set("Content-Type", contentType)
}
}Then replace each duplicated block with:
this.setContentTypeIfNotPresent(req, contentType)Also applies to: 122-129, 132-139
🤖 Prompt for AI Agents
In lib/dsl/test-builders/ResponseBuilder.ts around lines 111-118 (and similarly
at 122-129 and 132-139) the logic that checks for an existing Content-Type
header and sets it if absent is duplicated; extract that into a private helper
method (e.g., setContentTypeIfNotPresent(req, contentType)) that performs the
null-check on this.config.requestHeaders, lowercases header names to find
"content-type", and conditionally calls req.set, then replace each duplicated
block with a call to that helper to centralize the behavior and reduce
repetition.
| const chunks: Buffer[] = [] | ||
| for await (const chunk of requestFile.file.stream as any) { | ||
| chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) | ||
| } | ||
| const streamBuffer = Buffer.concat(chunks) | ||
| req = req.send(streamBuffer) |
There was a problem hiding this comment.
Buffering the entire stream defeats its purpose.
The stream branch reads the entire stream into memory before sending it, which negates the memory efficiency benefits of using a stream. For large files, this could lead to high memory consumption.
If supertest's API doesn't support streaming directly, consider documenting this limitation or evaluating whether the stream option should be supported at all.
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does supertest support streaming request bodies or piping streams directly?
💡 Result:
Short answer: No—supertest itself doesn’t reliably support piping a readable stream directly into the request body. Superagent (the HTTP client supertest is built on) does support piping (stream.pipe(req) and req.pipe(stream)), but many users find piping into a supertest request doesn't behave as expected (and TypeScript types/objects may not expose pipe). [1][2][3]
Recommended workarounds:
- Use superagent directly (it supports piping streams). [1][4]
- In tests, read the file/buffer first and use .send(buffer) or .attach()/multipart helpers instead of piping. [3][5]
Sources:
- Superagent docs (piping examples). [1]
- supertest GitHub issues / discussion about pipe behavior and tests. [2]
- Stack Overflow (users reporting piping into supertest request not working). [3]
- Superagent tests/docs showing request pipe behavior. [4]
- supertest issue threads proposing .send/.attach alternatives. [5]
@PENEKhun, based on my research, supertest doesn't reliably support streaming request bodies directly. The underlying library (superagent) does support piping streams, but it doesn't work well with supertest's API.
Here are your options:
Option 1: Keep the current implementation (Recommended)
The current approach of buffering the stream is actually the standard way to handle streams with supertest. While it does buffer the entire stream in memory, this is acceptable for testing scenarios where:
- Test files are typically small
- Tests run in isolation with controlled data
If you go this route, I suggest documenting this behavior in the code:
} else if (requestFile.file.stream) {
// Note: Supertest doesn't support streaming directly, so we buffer the entire stream.
// This is acceptable for testing scenarios with reasonably-sized files.
const hasContentType = /* ... */
if (!hasContentType) {
req.set("Content-Type", contentType)
}
const chunks: Buffer[] = []
for await (const chunk of requestFile.file.stream as any) {
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
}
const streamBuffer = Buffer.concat(chunks)
req = req.send(streamBuffer)
}Option 2: Drop stream support
Since the buffering limitation exists, consider removing the stream option and only supporting path and buffer. This would:
- Simplify the API
- Avoid confusion about stream behavior
- Still cover most testing use cases
Option 3: Switch to superagent directly
Use superagent instead of supertest for file uploads, which natively supports streaming. However, this would require significant refactoring and you'd lose supertest's convenient assertion API.
My recommendation: Go with Option 1 and add a comment documenting the buffering behavior. For a testing library, buffering small-to-medium test files is a reasonable trade-off for API simplicity.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
itdoc-doc/docs/guides/file-related-api-guide.mdx (1)
15-20: Clarify.file()zero-argument usage.You list only the signatures that take a description argument, yet the tip below calls
.file()with no parameters. This contradiction makes it unclear whether zero-argument usage is actually supported. Please either documentreq().file()as a supported form or adjust the tip to match the declared signatures.-**Supported signatures** - -- `req().file("description", { path: string, filename?, contentType? })` -- `req().file("description", { buffer: Buffer, filename?, contentType? })` -- `req().file("description", { stream: Readable, filename?, contentType? })` +**Supported signatures** + +- `req().file("description", { path: string, filename?, contentType? })` +- `req().file("description", { buffer: Buffer, filename?, contentType? })` +- `req().file("description", { stream: Readable, filename?, contentType? })` +- `req().file() // sends an empty body with Content-Type only`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
itdoc-doc/docs/guides/configuration.mdx(2 hunks)itdoc-doc/docs/guides/file-related-api-guide.mdx(1 hunks)itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/configuration.mdx(1 hunks)itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/file-related-api-guide.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- itdoc-doc/docs/guides/configuration.mdx
- itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/file-related-api-guide.mdx
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/dsl/test-builders/RequestBuilder.ts (1)
41-63: Stop dropping previously set headersCalling
.file()populates"content-type"inthis.config.requestHeaders, but chaining.header()afterwards replaces the entire map with the new argument, so the stored metadata (and later OpenAPI output) loses the content type. Merge normalized headers with the existing set instead of overwriting it.- const normalizedHeaders: Record<string, DSLField<string>> = {} - const seen = new Set<string>() - - Object.entries(headers).forEach(([headerName, headerValue]) => { - const normalized = headerName.toLowerCase() - - if (seen.has(normalized)) { - logger.warn(`Duplicate header detected: "${headerName}" (already set)`) - return - } - - seen.add(normalized) - normalizedHeaders[normalized] = headerValue - }) - - this.config.requestHeaders = normalizedHeaders - if (normalizedHeaders["content-type"]) { + const normalizedHeaders: Record<string, DSLField<string> | string> = {} + const existing = Object.entries(this.config.requestHeaders ?? {}).reduce< + Record<string, DSLField<string> | string> + >((acc, [name, value]) => { + acc[name.toLowerCase()] = value + return acc + }, {}) + const seen = new Set<string>(Object.keys(existing)) + + Object.entries(headers).forEach(([headerName, headerValue]) => { + const normalized = headerName.toLowerCase() + + if (seen.has(normalized)) { + logger.warn( + `Duplicate header detected: "${headerName}" (already set, will be overridden)`, + ) + } + + normalizedHeaders[normalized] = headerValue + seen.add(normalized) + }) + + if (normalizedHeaders["content-type"]) { throw new Error('You cannot set "Content-Type" header using header().') } - this.config.requestHeaders = headers + this.config.requestHeaders = { + ...existing, + ...normalizedHeaders, + } return this
♻️ Duplicate comments (1)
lib/dsl/test-builders/RequestBuilder.ts (1)
139-155: Allow.file()to coexist with other headersThe early
if (this.config.requestHeaders)throws when callers set innocuous headers (e.g.,Authorization) before.file(), making binary uploads unusable for authenticated APIs. Even when.file()runs first, later.header()calls overwrite the stored"content-type"because we never merge. Normalize and merge headers while preventing duplicate content-type instead of rejecting the call.- if (this.config.requestHeaders) { - throw new Error("already defined headers. can't use file()") - } - - if (!requestFile || typeof requestFile !== "object") { - this.config.requestHeaders = { - "content-type": "application/octet-stream", - } + const existing = Object.entries(this.config.requestHeaders ?? {}).reduce< + Record<string, string | DSLField<string>> + >((acc, [name, value]) => { + acc[name.toLowerCase()] = value + return acc + }, {}) + if ("content-type" in existing) { + throw new Error('You cannot set "Content-Type" header when using file().') + } + + if (!requestFile || typeof requestFile !== "object") { + this.config.requestHeaders = { + ...existing, + "content-type": "application/octet-stream", + } logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") return this } - this.config.requestHeaders = { - "content-type": requestFile.opts?.contentType ?? "application/octet-stream", - } + this.config.requestHeaders = { + ...existing, + "content-type": requestFile.opts?.contentType ?? "application/octet-stream", + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/express/expressApp.js(1 hunks)lib/dsl/generator/builders/operation/RequestBodyBuilder.ts(3 hunks)lib/dsl/test-builders/RequestBuilder.ts(3 hunks)lib/dsl/test-builders/ResponseBuilder.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
lib/dsl/generator/builders/operation/RequestBodyBuilder.ts (3)
lib/dsl/generator/types/OpenAPITypes.ts (1)
Content(34-36)lib/dsl/generator/builders/SchemaBuilder.ts (1)
SchemaBuilder(24-43)lib/dsl/generator/builders/schema/index.ts (1)
SchemaBuilder(39-60)
lib/dsl/test-builders/ResponseBuilder.ts (3)
lib/dsl/test-builders/RequestBuilder.ts (1)
body(190-206)lib/dsl/test-builders/RootBuilder.ts (1)
req(32-34)lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)
examples/express/expressApp.js (1)
examples/express/__tests__/expressApp.test.js (1)
app(1-1)
lib/dsl/test-builders/RequestBuilder.ts (1)
lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)
PENEKhun
changed the title
PENEKhun
changed the title
PENEKhun
added
the
enhancement
|
@coderabbitai review it |
|
@PENEKhun: I'll review the changes in PR #248 that add support for application/octet-stream file uploads. Let me analyze the implementation across the core library, tests, and documentation files. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/dsl/test-builders/RequestBuilder.ts (1)
64-186: Consider removing redundant source validation.The applyFile method (lines 161-169) re-validates that exactly one file source is provided, but normalizeFileArguments already performs this check at lines 112-117. This creates duplicate validation logic.
Since normalizeFileArguments guarantees exactly one source or throws, you can safely remove the redundant check in applyFile:
- const sources = [file.path ? 1 : 0, file.buffer ? 1 : 0, file.stream ? 1 : 0].reduce( - (a, b) => a + b, - 0, - ) - if (sources > 1) { - throw new Error( - "req().file(): only one of file.path | file.buffer | file.stream must be provided.", - ) - } - if (this.config.requestBody) {Alternatively, if you want defense-in-depth validation, consider adding a comment explaining why the check is intentionally duplicated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/express/expected/oas.json(1 hunks)itdoc-doc/docs/api-reference/interface.mdx(1 hunks)lib/config/logger.ts(0 hunks)lib/dsl/generator/types/TestResult.ts(2 hunks)lib/dsl/test-builders/RequestBuilder.ts(3 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- lib/config/logger.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/dsl/generator/types/TestResult.ts
🔇 Additional comments (3)
itdoc-doc/docs/api-reference/interface.mdx (1)
216-234: LGTM! Clear and comprehensive documentation.The file upload API documentation is well-structured:
- Both overload signatures are clearly explained
- Mutual exclusivity with body() is noted
- The example demonstrates proper usage with all relevant options
- Content-Type management is explicitly documented
lib/dsl/test-builders/RequestBuilder.ts (2)
41-62: LGTM! Content-Type guard correctly implemented.The header normalization and content-type guard now correctly check the normalized headers object, addressing the previous review concern.
Based on past review comments, this implementation properly:
- Normalizes headers to lowercase
- Checks for duplicates
- Guards against setting Content-Type via header()
193-209: LGTM! Symmetric conflict detection implemented.The body() method now correctly checks both
requestBodyandrequestFile, preventing mixing of JSON and file payloads, which addresses the previous review concern.Based on past review comments, the mutual exclusivity between body() and file() is now properly enforced in both directions.
const multer = require("multer")
const upload = multer({ dest: "uploads/" })
app.post("/uploads2", upload.single("file"), (req, res) => {
...Is uploading using multer(not a stream) still not implemented? Except for this part, it works well. testcode |
Multipart form data is not supported yet. |
now, just direct pass args with `.req().file(...)`
* fix: apply header key normalized to lowercase * update expected oas.json
|
I finished rebase. |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
examples/express/__tests__/expressApp.test.js(2 hunks)examples/express/expected/oas.json(1 hunks)examples/express/expressApp.js(1 hunks)itdoc-doc/docs/api-reference/interface.mdx(1 hunks)itdoc-doc/docs/guides/configuration.mdx(2 hunks)itdoc-doc/docs/guides/file-related-api-guide.mdx(1 hunks)itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/configuration.mdx(1 hunks)itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/file-related-api-guide.mdx(1 hunks)lib/config/logger.ts(0 hunks)lib/dsl/generator/OpenAPIGenerator.ts(1 hunks)lib/dsl/generator/builders/operation/RequestBodyBuilder.ts(3 hunks)lib/dsl/generator/types/TestResult.ts(2 hunks)lib/dsl/interface/field.ts(1 hunks)lib/dsl/interface/index.ts(1 hunks)lib/dsl/test-builders/RequestBuilder.ts(3 hunks)lib/dsl/test-builders/ResponseBuilder.ts(4 hunks)lib/dsl/test-builders/TestCaseConfig.ts(2 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- lib/config/logger.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- lib/dsl/test-builders/TestCaseConfig.ts
- lib/dsl/interface/field.ts
- lib/dsl/interface/index.ts
- itdoc-doc/i18n/ko/docusaurus-plugin-content-docs/current/guides/configuration.mdx
- itdoc-doc/docs/guides/file-related-api-guide.mdx
- examples/express/expected/oas.json
- lib/dsl/generator/builders/operation/RequestBodyBuilder.ts
- examples/express/tests/expressApp.test.js
🧰 Additional context used
🧬 Code graph analysis (3)
lib/dsl/test-builders/ResponseBuilder.ts (3)
lib/dsl/test-builders/RequestBuilder.ts (1)
body(193-209)lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)
lib/dsl/test-builders/RequestBuilder.ts (2)
lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)lib/dsl/interface/index.ts (1)
DSLRequestFile(23-23)
lib/dsl/generator/types/TestResult.ts (1)
lib/dsl/interface/field.ts (1)
DSLRequestFile(36-40)
🔇 Additional comments (5)
lib/dsl/test-builders/RequestBuilder.ts (5)
20-20: LGTM! Clean interface definition.The
FileDescriptorinterface and imports are well-structured and support the three file source types (path, buffer, stream) with optional metadata (filename, contentType).Also applies to: 24-30
58-62: LGTM! Content-Type guard now works correctly.The fix correctly checks
normalizedHeadersfor the lowercase"content-type"key and returnsthisimmediately after validation, preventing the previous issue whereContent-Typewould slip through.
64-78: LGTM! Clean overload design.The public API provides both a convenient shorthand style and advanced DSLRequestFile support, with clear delegation to helper methods.
130-136: LGTM! Standard stream detection.The type guard correctly checks for the
pipefunction, which is the standard way to detect Node.js readable streams.
194-205: LGTM! Symmetric conflict prevention.The
body()method now correctly checks bothrequestBodyandrequestFile, preventing mixing file and body payloads in either direction. The error message clearly explains the mutual exclusivity.
| if (req.headers["content-type"] !== "application/octet-stream") { | ||
| return res.status(400).json({ error: "Invalid content type" }) | ||
| } | ||
|
|
||
| let uploadedBytes = 0 | ||
|
|
||
| req.on("data", (chunk) => { | ||
| uploadedBytes += chunk.length | ||
| }) | ||
|
|
||
| req.on("end", () => { | ||
| if (uploadedBytes === 0) { | ||
| return res.status(400).json({ error: "No file uploaded" }) | ||
| } | ||
| return res.status(201).json() |
There was a problem hiding this comment.
Allow non‑octet-stream binary uploads
The route rejects valid uploads whenever the caller customizes contentType (e.g., "application/pdf" from the new .file() examples in itdoc-doc/docs/api-reference/interface.mdx). With the current strict equality check, those requests always get 400 despite being legitimate binary bodies, which contradicts the PR goal of handling “application/octet-stream or similar”. Please normalize the header and accept other application/* types (and parameters) instead of failing outright.
- if (req.headers["content-type"] !== "application/octet-stream") {
+ const contentType =
+ req.headers["content-type"]?.split(";")[0]?.trim().toLowerCase() ?? ""
+ if (!contentType || !contentType.startsWith("application/")) {
return res.status(400).json({ error: "Invalid content type" })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (req.headers["content-type"] !== "application/octet-stream") { | |
| return res.status(400).json({ error: "Invalid content type" }) | |
| } | |
| let uploadedBytes = 0 | |
| req.on("data", (chunk) => { | |
| uploadedBytes += chunk.length | |
| }) | |
| req.on("end", () => { | |
| if (uploadedBytes === 0) { | |
| return res.status(400).json({ error: "No file uploaded" }) | |
| } | |
| return res.status(201).json() | |
| const contentType = | |
| req.headers["content-type"]?.split(";")[0]?.trim().toLowerCase() ?? "" | |
| if (!contentType || !contentType.startsWith("application/")) { | |
| return res.status(400).json({ error: "Invalid content type" }) | |
| } | |
| let uploadedBytes = 0 | |
| req.on("data", (chunk) => { | |
| uploadedBytes += chunk.length | |
| }) | |
| req.on("end", () => { | |
| if (uploadedBytes === 0) { | |
| return res.status(400).json({ error: "No file uploaded" }) | |
| } | |
| return res.status(201).json() |
🤖 Prompt for AI Agents
In examples/express/expressApp.js around lines 255 to 269, the content-type
check is too strict and rejects valid binary uploads (e.g., "application/pdf" or
values with parameters); update the validation to normalize and accept any
"application/*" media type (including "application/octet-stream") by reading
req.headers['content-type'] safely, trimming and lowercasing it, stripping any
parameters (after ';') and then checking that the type starts with
"application/"; keep the uploadedBytes tracking and existing empty-upload check
unchanged.
| private normalizeFileArguments( | ||
| descriptionOrRequest: string | DSLRequestFile | undefined, | ||
| descriptor?: FileDescriptor, | ||
| ): DSLRequestFile | undefined { | ||
| if (typeof descriptionOrRequest !== "string") { | ||
| return descriptionOrRequest | ||
| } | ||
|
|
||
| if (!descriptor || typeof descriptor !== "object") { | ||
| return undefined | ||
| } | ||
|
|
||
| const file: DSLRequestFile["file"] = {} | ||
|
|
||
| const { path, buffer, stream } = descriptor | ||
|
|
||
| if (path !== undefined) { | ||
| file.path = path | ||
| } | ||
| if (buffer !== undefined) { | ||
| if (!Buffer.isBuffer(buffer)) { | ||
| throw new Error("req().file(): buffer must be a Buffer instance.") | ||
| } | ||
| file.buffer = buffer | ||
| } | ||
| if (stream !== undefined) { | ||
| if (!this.isReadableStream(stream)) { | ||
| throw new Error("req().file(): stream must be a readable stream.") | ||
| } | ||
| file.stream = stream | ||
| } | ||
|
|
||
| const providedSources = [file.path, file.buffer, file.stream].filter((value) => value) | ||
| if (providedSources.length !== 1) { | ||
| throw new Error( | ||
| "req().file(): provide exactly one of path | buffer | stream in the descriptor.", | ||
| ) | ||
| } | ||
|
|
||
| const normalizedContentType = descriptor.contentType ?? "application/octet-stream" | ||
|
|
||
| return { | ||
| description: descriptionOrRequest, | ||
| file, | ||
| opts: descriptor.filename | ||
| ? { contentType: normalizedContentType, filename: descriptor.filename } | ||
| : { contentType: normalizedContentType }, | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Returning undefined for invalid arguments creates ambiguity.
When descriptor is missing or not an object (lines 88-90), the function silently returns undefined, which is later handled by applyFile with partial state mutations. This makes the API confusing—req().file("desc", undefined) would succeed but set headers without an actual file.
Consider throwing an explicit error instead:
if (!descriptor || typeof descriptor !== "object") {
- return undefined
+ throw new Error(
+ "req().file(): descriptor must be an object with path | buffer | stream."
+ )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private normalizeFileArguments( | |
| descriptionOrRequest: string | DSLRequestFile | undefined, | |
| descriptor?: FileDescriptor, | |
| ): DSLRequestFile | undefined { | |
| if (typeof descriptionOrRequest !== "string") { | |
| return descriptionOrRequest | |
| } | |
| if (!descriptor || typeof descriptor !== "object") { | |
| return undefined | |
| } | |
| const file: DSLRequestFile["file"] = {} | |
| const { path, buffer, stream } = descriptor | |
| if (path !== undefined) { | |
| file.path = path | |
| } | |
| if (buffer !== undefined) { | |
| if (!Buffer.isBuffer(buffer)) { | |
| throw new Error("req().file(): buffer must be a Buffer instance.") | |
| } | |
| file.buffer = buffer | |
| } | |
| if (stream !== undefined) { | |
| if (!this.isReadableStream(stream)) { | |
| throw new Error("req().file(): stream must be a readable stream.") | |
| } | |
| file.stream = stream | |
| } | |
| const providedSources = [file.path, file.buffer, file.stream].filter((value) => value) | |
| if (providedSources.length !== 1) { | |
| throw new Error( | |
| "req().file(): provide exactly one of path | buffer | stream in the descriptor.", | |
| ) | |
| } | |
| const normalizedContentType = descriptor.contentType ?? "application/octet-stream" | |
| return { | |
| description: descriptionOrRequest, | |
| file, | |
| opts: descriptor.filename | |
| ? { contentType: normalizedContentType, filename: descriptor.filename } | |
| : { contentType: normalizedContentType }, | |
| } | |
| } | |
| private normalizeFileArguments( | |
| descriptionOrRequest: string | DSLRequestFile | undefined, | |
| descriptor?: FileDescriptor, | |
| ): DSLRequestFile | undefined { | |
| if (typeof descriptionOrRequest !== "string") { | |
| return descriptionOrRequest | |
| } | |
| if (!descriptor || typeof descriptor !== "object") { | |
| throw new Error( | |
| "req().file(): descriptor must be an object with path | buffer | stream." | |
| ) | |
| - return undefined | |
| } | |
| const file: DSLRequestFile["file"] = {} | |
| const { path, buffer, stream } = descriptor | |
| if (path !== undefined) { | |
| file.path = path | |
| } | |
| if (buffer !== undefined) { | |
| if (!Buffer.isBuffer(buffer)) { | |
| throw new Error("req().file(): buffer must be a Buffer instance.") | |
| } | |
| file.buffer = buffer | |
| } | |
| if (stream !== undefined) { | |
| if (!this.isReadableStream(stream)) { | |
| throw new Error("req().file(): stream must be a readable stream.") | |
| } | |
| file.stream = stream | |
| } | |
| const providedSources = [file.path, file.buffer, file.stream].filter((value) => value) | |
| if (providedSources.length !== 1) { | |
| throw new Error( | |
| "req().file(): provide exactly one of path | buffer | stream in the descriptor.", | |
| ) | |
| } | |
| const normalizedContentType = descriptor.contentType ?? "application/octet-stream" | |
| return { | |
| description: descriptionOrRequest, | |
| file, | |
| opts: descriptor.filename | |
| ? { contentType: normalizedContentType, filename: descriptor.filename } | |
| : { contentType: normalizedContentType }, | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/dsl/test-builders/RequestBuilder.ts around lines 80 to 128, the function
normalizeFileArguments currently returns undefined when descriptor is missing or
not an object which allows callers like req().file("desc", undefined) to
silently proceed and later produce partial state; change this to throw a clear,
specific Error when descriptor is absent or not an object (e.g., "req().file():
descriptor must be a FileDescriptor object") so callers receive immediate
feedback; update the function to validate descriptor at the top, throw the error
instead of returning undefined, and ensure any callers/tests expecting undefined
are adjusted to handle/expect the thrown error.
| private applyFile(requestFile: DSLRequestFile | undefined): this { | ||
| const existingHeaders = this.config.requestHeaders ?? {} | ||
| const hasContentType = Object.keys(existingHeaders).some( | ||
| (key) => key.toLowerCase() === "content-type", | ||
| ) | ||
| if (hasContentType) { | ||
| throw new Error('You cannot set "Content-Type" header when using file().') | ||
| } | ||
|
|
||
| if (!requestFile || typeof requestFile !== "object") { | ||
| this.config.requestHeaders = { | ||
| "content-type": "application/octet-stream", | ||
| } | ||
| logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") | ||
| return this | ||
| } | ||
|
|
||
| this.config.requestHeaders = { | ||
| "content-type": requestFile.opts?.contentType ?? "application/octet-stream", | ||
| } | ||
|
|
||
| const { file } = requestFile | ||
|
|
||
| const sources = [file.path ? 1 : 0, file.buffer ? 1 : 0, file.stream ? 1 : 0].reduce( | ||
| (a, b) => a + b, | ||
| 0, | ||
| ) | ||
| if (sources > 1) { | ||
| throw new Error( | ||
| "req().file(): only one of file.path | file.buffer | file.stream must be provided.", | ||
| ) | ||
| } | ||
|
|
||
| if (this.config.requestBody) { | ||
| throw new Error( | ||
| [ | ||
| "❌ Conflict: request body has already been set using .body().", | ||
| "", | ||
| "You cannot mix JSON body (.body()) and raw file (.file()) in the same request.", | ||
| "Please choose exactly one of:", | ||
| " • req().body(...) → for JSON payloads", | ||
| " • req().file(...) → for raw binary uploads (application/octet-stream)", | ||
| ].join("\n"), | ||
| ) | ||
| } | ||
|
|
||
| this.config.requestFile = requestFile | ||
| return this | ||
| } |
There was a problem hiding this comment.
State mutations happen before all validations complete.
The current flow has several issues:
-
Lines 147-153: When
requestFileis invalid, you still setContent-Typeheaders and return successfully with just a warning. This allowsreq().file(undefined)to succeed, leaving partial state (headers set but no file). -
Lines 155-157: Headers are set before validating the number of sources (lines 161-169), creating partial state if validation fails later.
-
Lines 161-169: This validation duplicates the check already performed in
normalizeFileArguments(lines 112-117). -
Lines 171-182: Headers are modified before checking for conflicts with
requestBody.
The correct order should be: validate everything → mutate state → return.
Apply this refactor:
private applyFile(requestFile: DSLRequestFile | undefined): this {
+ // Validate requestFile first
+ if (!requestFile || typeof requestFile !== "object") {
+ throw new Error(
+ "req().file(): Invalid file configuration. Provide one of file.path | file.buffer | file.stream."
+ )
+ }
+
+ // Check for requestBody conflict early
+ if (this.config.requestBody) {
+ throw new Error(
+ [
+ "❌ Conflict: request body has already been set using .body().",
+ "",
+ "You cannot mix JSON body (.body()) and raw file (.file()) in the same request.",
+ "Please choose exactly one of:",
+ " • req().body(...) → for JSON payloads",
+ " • req().file(...) → for raw binary uploads (application/octet-stream)",
+ ].join("\n"),
+ )
+ }
+
+ // Check for existing Content-Type header
const existingHeaders = this.config.requestHeaders ?? {}
const hasContentType = Object.keys(existingHeaders).some(
(key) => key.toLowerCase() === "content-type",
)
if (hasContentType) {
throw new Error('You cannot set "Content-Type" header when using file().')
}
- if (!requestFile || typeof requestFile !== "object") {
- this.config.requestHeaders = {
- "content-type": "application/octet-stream",
- }
- logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.")
- return this
- }
-
+ // All validations passed - now mutate state
this.config.requestHeaders = {
"content-type": requestFile.opts?.contentType ?? "application/octet-stream",
}
-
- const { file } = requestFile
-
- const sources = [file.path ? 1 : 0, file.buffer ? 1 : 0, file.stream ? 1 : 0].reduce(
- (a, b) => a + b,
- 0,
- )
- if (sources > 1) {
- throw new Error(
- "req().file(): only one of file.path | file.buffer | file.stream must be provided.",
- )
- }
-
- if (this.config.requestBody) {
- throw new Error(
- [
- "❌ Conflict: request body has already been set using .body().",
- "",
- "You cannot mix JSON body (.body()) and raw file (.file()) in the same request.",
- "Please choose exactly one of:",
- " • req().body(...) → for JSON payloads",
- " • req().file(...) → for raw binary uploads (application/octet-stream)",
- ].join("\n"),
- )
- }
-
this.config.requestFile = requestFile
return this
}Note: The duplicate sources validation (lines 161-169) is removed since it's already enforced in normalizeFileArguments (lines 112-117).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private applyFile(requestFile: DSLRequestFile | undefined): this { | |
| const existingHeaders = this.config.requestHeaders ?? {} | |
| const hasContentType = Object.keys(existingHeaders).some( | |
| (key) => key.toLowerCase() === "content-type", | |
| ) | |
| if (hasContentType) { | |
| throw new Error('You cannot set "Content-Type" header when using file().') | |
| } | |
| if (!requestFile || typeof requestFile !== "object") { | |
| this.config.requestHeaders = { | |
| "content-type": "application/octet-stream", | |
| } | |
| logger.warn("req().file(): provide one of file.path | file.buffer | file.stream.") | |
| return this | |
| } | |
| this.config.requestHeaders = { | |
| "content-type": requestFile.opts?.contentType ?? "application/octet-stream", | |
| } | |
| const { file } = requestFile | |
| const sources = [file.path ? 1 : 0, file.buffer ? 1 : 0, file.stream ? 1 : 0].reduce( | |
| (a, b) => a + b, | |
| 0, | |
| ) | |
| if (sources > 1) { | |
| throw new Error( | |
| "req().file(): only one of file.path | file.buffer | file.stream must be provided.", | |
| ) | |
| } | |
| if (this.config.requestBody) { | |
| throw new Error( | |
| [ | |
| "❌ Conflict: request body has already been set using .body().", | |
| "", | |
| "You cannot mix JSON body (.body()) and raw file (.file()) in the same request.", | |
| "Please choose exactly one of:", | |
| " • req().body(...) → for JSON payloads", | |
| " • req().file(...) → for raw binary uploads (application/octet-stream)", | |
| ].join("\n"), | |
| ) | |
| } | |
| this.config.requestFile = requestFile | |
| return this | |
| } | |
| private applyFile(requestFile: DSLRequestFile | undefined): this { | |
| // Validate requestFile first | |
| if (!requestFile || typeof requestFile !== "object") { | |
| throw new Error( | |
| "req().file(): Invalid file configuration. Provide one of file.path | file.buffer | file.stream." | |
| ) | |
| } | |
| // Check for requestBody conflict early | |
| if (this.config.requestBody) { | |
| throw new Error( | |
| [ | |
| "❌ Conflict: request body has already been set using .body().", | |
| "", | |
| "You cannot mix JSON body (.body()) and raw file (.file()) in the same request.", | |
| "Please choose exactly one of:", | |
| " • req().body(...) → for JSON payloads", | |
| " • req().file(...) → for raw binary uploads (application/octet-stream)", | |
| ].join("\n"), | |
| ) | |
| } | |
| // Check for existing Content-Type header | |
| const existingHeaders = this.config.requestHeaders ?? {} | |
| const hasContentType = Object.keys(existingHeaders).some( | |
| (key) => key.toLowerCase() === "content-type", | |
| ) | |
| if (hasContentType) { | |
| throw new Error('You cannot set "Content-Type" header when using file().') | |
| } | |
| // All validations passed – now mutate state | |
| this.config.requestHeaders = { | |
| "content-type": requestFile.opts?.contentType ?? "application/octet-stream", | |
| } | |
| this.config.requestFile = requestFile | |
| return this | |
| } |
2 participants
related #247
Changes
Summary by CodeRabbit
New Features
Documentation
Tests
Chores