修复豆包图像编辑(图生图)功能 - #2090
Conversation
WalkthroughAdds form and multipart form parsing to request unmarshalling, introduces an Image field to the image DTO, tightens middleware Content-Type checks using explicit MIME membership, and removes server-side multipart construction for Volcengine image edits, routing edits through the generations JSON endpoint. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware
participant Handler
participant Volcengine
Note over Client,Volcengine: Image Edits Request Flow (updated)
Client->>Middleware: POST /v1/images/edits (form/multipart)
activate Middleware
Middleware->>Middleware: Check Content-Type via slices.Contains
Middleware->>Handler: Forward validated request
deactivate Middleware
activate Handler
Handler->>Handler: UnmarshalBodyReusable
alt form-urlencoded
Handler->>Handler: parseFormData -> map -> JSON -> unmarshal
else multipart/form-data
Handler->>Handler: parseMultipartFormData -> map -> JSON -> unmarshal
end
Handler->>Volcengine: Forward request as JSON (images/generations)
deactivate Handler
activate Volcengine
Volcengine->>Volcengine: Process edits via generations endpoint
Volcengine-->>Client: Response
deactivate Volcengine
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/volcengine/adaptor.go (1)
105-210: Remove commented code instead of leaving it in place.The commented-out multipart handling (lines 106-205) is extensive and will not be used. Leaving large blocks of commented code reduces readability and can confuse future maintainers. Remove it entirely to keep the codebase clean.
Apply this diff to remove the dead code:
switch info.RelayMode { case constant.RelayModeImagesGenerations: return request, nil - // 根据官方文档,并没有发现豆包生图支持表单请求:https://www.volcengine.com/docs/82379/1824121 - //case constant.RelayModeImagesEdits: - // - // var requestBody bytes.Buffer - // writer := multipart.NewWriter(&requestBody) - // - // writer.WriteField("model", request.Model) - // - // formData := c.Request.PostForm - // for key, values := range formData { - // if key == "model" { - // continue - // } - // for _, value := range values { - // writer.WriteField(key, value) - // } - // } - // - // if err := c.Request.ParseMultipartForm(32 << 20); err != nil { - // return nil, errors.New("failed to parse multipart form") - // } - // - // if c.Request.MultipartForm != nil && c.Request.MultipartForm.File != nil { - // var imageFiles []*multipart.FileHeader - // var exists bool - // - // if imageFiles, exists = c.Request.MultipartForm.File["image"]; !exists || len(imageFiles) == 0 { - // if imageFiles, exists = c.Request.MultipartForm.File["image[]"]; !exists || len(imageFiles) == 0 { - // foundArrayImages := false - // for fieldName, files := range c.Request.MultipartForm.File { - // if strings.HasPrefix(fieldName, "image[") && len(files) > 0 { - // foundArrayImages = true - // for _, file := range files { - // imageFiles = append(imageFiles, file) - // } - // } - // } - // - // if !foundArrayImages && (len(imageFiles) == 0) { - // return nil, errors.New("image is required") - // } - // } - // } - // - // for i, fileHeader := range imageFiles { - // file, err := fileHeader.Open() - // if err != nil { - // return nil, fmt.Errorf("failed to open image file %d: %w", i, err) - // } - // defer file.Close() - // - // fieldName := "image" - // if len(imageFiles) > 1 { - // fieldName = "image[]" - // } - // - // mimeType := detectImageMimeType(fileHeader.Filename) - // - // h := make(textproto.MIMEHeader) - // h.Set("Content-Disposition", fmt.Sprintf(`form-data; name="%s"; filename="%s"`, fieldName, fileHeader.Filename)) - // h.Set("Content-Type", mimeType) - // - // part, err := writer.CreatePart(h) - // if err != nil { - // return nil, fmt.Errorf("create form part failed for image %d: %w", i, err) - // } - // - // if _, err := io.Copy(part, file); err != nil { - // return nil, fmt.Errorf("copy file failed for image %d: %w", i, err) - // } - // } - // - // if maskFiles, exists := c.Request.MultipartForm.File["mask"]; exists && len(maskFiles) > 0 { - // maskFile, err := maskFiles[0].Open() - // if err != nil { - // return nil, errors.New("failed to open mask file") - // } - // defer maskFile.Close() - // - // mimeType := detectImageMimeType(maskFiles[0].Filename) - // - // h := make(textproto.MIMEHeader) - // h.Set("Content-Disposition", fmt.Sprintf(`form-data; name="mask"; filename="%s"`, maskFiles[0].Filename)) - // h.Set("Content-Type", mimeType) - // - // maskPart, err := writer.CreatePart(h) - // if err != nil { - // return nil, errors.New("create form file failed for mask") - // } - // - // if _, err := io.Copy(maskPart, maskFile); err != nil { - // return nil, errors.New("copy mask file failed") - // } - // } - // } else { - // return nil, errors.New("no multipart form data found") - // } - // - // writer.Close() - // c.Request.Header.Set("Content-Type", writer.FormDataContentType()) - // return bytes.NewReader(requestBody.Bytes()), nil default:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
common/gin.go(3 hunks)dto/openai_image.go(1 hunks)middleware/distributor.go(2 hunks)relay/channel/volcengine/adaptor.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
middleware/distributor.go (1)
dto/request_common.go (1)
Request(8-12)
relay/channel/volcengine/adaptor.go (1)
relay/constant/relay_mode.go (2)
RelayModeImagesGenerations(14-14)RelayModeImagesEdits(15-15)
common/gin.go (1)
common/json.go (2)
Marshal(21-23)Unmarshal(9-11)
🔇 Additional comments (4)
dto/openai_image.go (1)
30-31: LGTM! Image field addition supports URL-based image edits.The new
Imagefield enables passing image data (likely as URLs) in JSON requests, which aligns with the PR's goal of supporting Doubao's URL-based image editing API.relay/channel/volcengine/adaptor.go (2)
253-257: LGTM! Correct endpoint consolidation for Doubao.Consolidating both
ImagesGenerationsandImagesEditsunder the/images/generationsendpoint with JSON content type correctly reflects the Doubao API behavior documented in the official docs.
281-283: LGTM! Explicit JSON Content-Type for image edits.Setting
Content-Typetoapplication/jsonforRelayModeImagesEditsaligns with the shift away from multipart form data and ensures the request is properly formatted for the Doubao API.common/gin.go (1)
45-48: The review comment is based on an incorrect assumption about the request flow.The generic form parsers (
parseFormDataandparseMultipartFormData) in common/gin.go correctly only extract text fields—but this is not a problem for image edits endpoints because they don't use these generic parsers.Image edits requests (RelayModeImagesEdits) bypass UnmarshalBodyReusable entirely:
valid_request.go (lines 141-151): Image edit validation calls
c.MultipartForm()directly and extracts text fields fromc.Request.PostForm, bypassing the generic parsers.Volcengine/Doubao adapter (line 281): For image edits, the Content-Type is explicitly set to
"application/json", not multipart/form-data. The request is converted to JSON before sending upstream.File upload pattern: When other channels require file uploads (OpenAI audio, Dify files, Cloudflare audio), they use explicit
c.Request.FormFile()extraction, not the generic parsers.The generic form parsers are intentionally text-only utilities for simple form data serialization, not for file upload handling. Image edits leverage URL-based image inputs (as noted in the review) and don't require file upload support—which is correctly reflected in the implementation.
| if boundary == "" { | ||
| return json.Unmarshal(data, v) // Fallback to JSON | ||
| } |
There was a problem hiding this comment.
Questionable fallback: Missing boundary should be an error.
If Content-Type is multipart/form-data but the boundary parameter is missing, that's a malformed request. Falling back to JSON parsing (line 177) masks this error and could lead to confusing behavior.
Consider returning an error instead:
if boundary == "" {
- return json.Unmarshal(data, v) // Fallback to JSON
+ return errors.New("multipart/form-data Content-Type is missing required boundary parameter")
}📝 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 boundary == "" { | |
| return json.Unmarshal(data, v) // Fallback to JSON | |
| } | |
| if boundary == "" { | |
| return errors.New("multipart/form-data Content-Type is missing required boundary parameter") | |
| } |
🤖 Prompt for AI Agents
In common/gin.go around lines 176-178, do not silently fall back to JSON when
Content-Type is multipart/form-data but the boundary is missing; instead return
a clear error indicating a malformed multipart request (e.g., "missing multipart
boundary" or a wrapped error) so callers can handle it as a bad request; remove
the json.Unmarshal fallback and return that error (ensure the function's
signature supports returning the error type expected by callers).
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "slices" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused import after fixing the Content-Type check.
After applying the fix above, the slices import will no longer be needed and should be removed.
🤖 Prompt for AI Agents
In middleware/distributor.go at line 7 the import "slices" is now unused after
the Content-Type check fix; remove the "slices" entry from the import block (and
tidy imports if needed) so the file compiles without unused-import errors.
| contentType := c.Request.Header.Get("Content-Type") | ||
| if slices.Contains([]string{gin.MIMEPOSTForm, gin.MIMEMultipartPOSTForm}, contentType) { | ||
| modelRequest.Model = c.PostForm("model") | ||
| } |
There was a problem hiding this comment.
Critical: Content-Type matching will fail for real requests.
The exact match check using slices.Contains will fail because Content-Type headers typically include parameters. For example, multipart/form-data; boundary=----WebKitFormBoundary... will not match gin.MIMEMultipartPOSTForm ("multipart/form-data").
Note that common/gin.go (lines 45-48) correctly uses strings.Contains() for the same purpose, which handles parameterized headers properly.
Apply this diff to fix the Content-Type matching:
- contentType := c.Request.Header.Get("Content-Type")
- if slices.Contains([]string{gin.MIMEPOSTForm, gin.MIMEMultipartPOSTForm}, contentType) {
+ contentType := c.Request.Header.Get("Content-Type")
+ if strings.Contains(contentType, gin.MIMEPOSTForm) || strings.Contains(contentType, gin.MIMEMultipartPOSTForm) {
modelRequest.Model = c.PostForm("model")
}📝 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.
| contentType := c.Request.Header.Get("Content-Type") | |
| if slices.Contains([]string{gin.MIMEPOSTForm, gin.MIMEMultipartPOSTForm}, contentType) { | |
| modelRequest.Model = c.PostForm("model") | |
| } | |
| contentType := c.Request.Header.Get("Content-Type") | |
| if strings.Contains(contentType, gin.MIMEPOSTForm) || strings.Contains(contentType, gin.MIMEMultipartPOSTForm) { | |
| modelRequest.Model = c.PostForm("model") | |
| } |
🤖 Prompt for AI Agents
In middleware/distributor.go around lines 249 to 252, the Content-Type check
uses slices.Contains which does exact matches and fails for parameterized
headers (e.g. "multipart/form-data; boundary=..."); change the logic to use
strings.Contains(c.Request.Header.Get("Content-Type"), gin.MIMEPOSTForm) ||
strings.Contains(..., gin.MIMEMultipartPOSTForm) so parameterized types match,
and add an import for the strings package if not already present; keep the
existing c.PostForm("model") assignment.
multipart/form-data; boundary
…edit 修复豆包图像编辑(图生图)功能
…n-zero feat(ops): allow retention days = 0 to wipe table on each scheduled cleanup
2 participants
请求示例:
返回示例:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor