Repository: cloudwego/hertz
Pull request #1135 (2024-06-11 04:54:24)

optimize: non-existent file error for binding
#### What type of PR is this?
<!--
Add one of the following kinds:

build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
docs: Documentation only changes
feat: A new feature
optimize: A new optimization
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that neither fixes a bug nor adds a feature
style: Changes that do not affect the meaning of the code (white space, formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests
chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
-->
optimize
#### Check the PR title.
<!--
The description of the title will be attached in Release Notes, 
so please describe it from user-oriented, what this PR does / why we need it.
Please check your PR title with the below requirements:
-->
- [x] This PR title match the format: \<type\>(optional scope): \<description\>
- [x] The description of this PR title is user-oriented and clear enough for others to understand.
- [x] Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)


#### (Optional) Translate the PR title into Chinese.
优化绑定文件时，如果文件不存在则不报错，而是 warning

#### (Optional) More detailed description for this PR(en: English/zh: Chinese).
<!--
Provide more detailed info for review(e.g., it's recommended to provide perf data if this is a perf type PR).
-->
en: optimize file unexist error for binding
zh(optional): 优化绑定文件场景；如果文件不存在，则打warning 提醒；而不是返回 error，导致用户其他 field 无法绑定

#### (Optional) Which issue(s) this PR fixes:
<!--
Automatically closes linked issue when PR is merged.
Eg: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

#### (Optional) The PR that updates user documentation:
<!--
If the current PR requires user awareness at the usage level, please submit a PR to update user docs. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)
-->


Patch:
diff --git a/pkg/app/server/binding/internal/decoder/multipart_file_decoder.go b/pkg/app/server/binding/internal/decoder/multipart_file_decoder.go
index ae32dfe..b114177 100644
--- a/pkg/app/server/binding/internal/decoder/multipart_file_decoder.go
+++ b/pkg/app/server/binding/internal/decoder/multipart_file_decoder.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/cloudwego/hertz/pkg/common/hlog"
 	"github.com/cloudwego/hertz/pkg/protocol"
 	"github.com/cloudwego/hertz/pkg/route/param"
 )
@@ -52,7 +53,8 @@ func (d *fileTypeDecoder) Decode(req *protocol.Request, params param.Params, req
 	}
 	file, err := req.FormFile(fileName)
 	if err != nil {
-		return fmt.Errorf("can not get file '%s', err: %v", fileName, err)
+		hlog.SystemLogger().Warnf("can not get file '%s' form request, reason: %v, so skip '%s' field binding", fileName, err, d.fieldName)
+		return nil
 	}
 	if field.Kind() == reflect.Ptr {
 		t := field.Type()
@@ -105,11 +107,13 @@ func (d *fileTypeDecoder) fileSliceDecode(req *protocol.Request, params param.Pa
 	}
 	multipartForm, err := req.MultipartForm()
 	if err != nil {
-		return fmt.Errorf("can not get multipartForm info, err: %v", err)
+		hlog.SystemLogger().Warnf("can not get MultipartForm from request, reason: %v, so skip '%s' field binding", fileName, err, d.fieldName)
+		return nil
 	}
 	files, exist := multipartForm.File[fileName]
 	if !exist {
-		return fmt.Errorf("the file '%s' is not existed", fileName)
+		hlog.SystemLogger().Warnf("the file '%s' is not existed in request, so skip '%s' field binding", fileName, d.fieldName)
+		return nil
 	}
 
 	if field.Kind() == reflect.Array {
diff --git a/pkg/app/server/binding/internal/decoder/struct_type_decoder.go b/pkg/app/server/binding/internal/decoder/struct_type_decoder.go
index c2af2c0..4a3ded1 100644
--- a/pkg/app/server/binding/internal/decoder/struct_type_decoder.go
+++ b/pkg/app/server/binding/internal/decoder/struct_type_decoder.go
@@ -83,7 +83,7 @@ func (d *structTypeFieldTextDecoder) Decode(req *protocol.Request, params param.
 		var vv reflect.Value
 		vv, err := stringToValue(t, text, req, params, d.config)
 		if err != nil {
-			hlog.Infof("unable to decode '%s' as %s: %v, but it may not affect correctness, so skip it", text, d.fieldType.Name(), err)
+			hlog.SystemLogger().Infof("unable to decode '%s' as %s: %v, but it may not affect correctness, so skip it", text, d.fieldType.Name(), err)
 			return nil
 		}
 		field.Set(ReferenceValue(vv, ptrDepth))
@@ -92,7 +92,7 @@ func (d *structTypeFieldTextDecoder) Decode(req *protocol.Request, params param.
 
 	err = hjson.Unmarshal(bytesconv.S2b(text), field.Addr().Interface())
 	if err != nil {
-		hlog.Infof("unable to decode '%s' as %s: %v, but it may not affect correctness, so skip it", text, d.fieldType.Name(), err)
+		hlog.SystemLogger().Infof("unable to decode '%s' as %s: %v, but it may not affect correctness, so skip it", text, d.fieldType.Name(), err)
 	}
 
 	return nil


Test patch:
diff --git a/pkg/app/server/binding/binder_test.go b/pkg/app/server/binding/binder_test.go
index ec962da..1ea7b1e 100644
--- a/pkg/app/server/binding/binder_test.go
+++ b/pkg/app/server/binding/binder_test.go
@@ -692,6 +692,31 @@ func TestBind_FileBind(t *testing.T) {
 	assert.DeepEqual(t, fileName, (**s.D).N.Filename)
 }
 
+func TestBind_FileBindWithNoFile(t *testing.T) {
+	var s struct {
+		A *multipart.FileHeader `file_name:"a"`
+		B *multipart.FileHeader `form:"b"`
+		C *multipart.FileHeader
+	}
+	fileName := "binder_test.go"
+	req := newMockRequest().
+		SetRequestURI("http://foobar.com").
+		SetFile("a", fileName).
+		SetFile("b", fileName)
+	// to parse multipart files
+	req2 := req2.GetHTTP1Request(req.Req)
+	_ = req2.String()
+	err := DefaultBinder().Bind(req.Req, &s, nil)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	assert.DeepEqual(t, fileName, s.A.Filename)
+	assert.DeepEqual(t, fileName, s.B.Filename)
+	if s.C != nil {
+		t.Fatalf("expected a nil for s.C")
+	}
+}
+
 func TestBind_FileSliceBind(t *testing.T) {
 	type Nest struct {
 		N *[]*multipart.FileHeader `form:"b"`