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

refactor: binding #541

Merged
merged 91 commits into from
Sep 22, 2023
Merged

refactor: binding #541

merged 91 commits into from
Sep 22, 2023

Conversation

FGYFFFF
Copy link
Contributor

@FGYFFFF FGYFFFF commented Jan 13, 2023

What type of PR is this?

refactor

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.

(Optional) Translate the PR title into Chinese.

重构 hertz 参数绑定

(Optional) More detail description for this PR(en: English/zh: Chinese).

zh:
绑定:

  • 支持的类型:基本类型、数组/切片、map、struct、typedef、多重指针
  • 多 tag 绑定:优先级:path > form > query > cookie > header > json > raw_body
  • 最大努力绑定:先尝试解析 body 做 unmarshal,再绑定 http 信息
  • required 校验
  • 自定义类型绑定
  • 设置默认值
  • 绑定文件
  • 没加 tag 自动遍历所有 tag
  • 复杂类型的默认值(map、slice、struct 等类型的默认值)
  • 替换自定义绑定函数的注册方式,便于对一些 Known-Struct 进行拓展
  • json unmarshal 后的默认值覆盖以及required校验
  • 对于 struct Field 要不要也提供一个 decoder,用于直接 unmarshal(实现类似 map field)

en:
binding:

  • supported type:base type、array/slice、map、struct、typedef、multiple pointer
  • multiple tag binding:Priority:path > form > query > cookie > header > json > raw_body
  • best effort binding:First try to parse the body to do unmarshal, then bind the http information
  • required validate
  • custom type binding
  • set default value
  • file binding
  • without tag automatically traverses all tags

Which issue(s) this PR fixes:

@FGYFFFF FGYFFFF requested review from a team as code owners January 13, 2023 09:01
@FGYFFFF FGYFFFF force-pushed the refactor/binding branch 4 times, most recently from ae0cf9d to 28f6f5f Compare January 17, 2023 07:23
@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

Patch coverage: 80.35% and project coverage change: -0.19% ⚠️

Comparison is base (02112cc) 82.01% compared to head (1dc1af0) 81.82%.

❗ Current head 1dc1af0 differs from pull request most recent head 256f9bb. Consider uploading reports for the commit 256f9bb to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #541      +/-   ##
===========================================
- Coverage    82.01%   81.82%   -0.19%     
===========================================
  Files           97       98       +1     
  Lines         9663     9900     +237     
===========================================
+ Hits          7925     8101     +176     
- Misses        1261     1311      +50     
- Partials       477      488      +11     
Files Changed Coverage Δ
pkg/common/config/option.go 100.00% <ø> (ø)
pkg/app/server/binding/config.go 71.11% <71.11%> (ø)
pkg/app/context.go 93.41% <71.73%> (-2.02%) ⬇️
pkg/route/engine.go 61.58% <76.92%> (+0.83%) ⬆️
pkg/common/utils/utils.go 85.71% <80.00%> (-0.78%) ⬇️
pkg/app/server/binding/default.go 82.53% <82.53%> (ø)
pkg/app/server/binding/reflect.go 100.00% <100.00%> (ø)
pkg/app/server/option.go 98.73% <100.00%> (+0.10%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@FGYFFFF FGYFFFF changed the title [WIP]refactor: binding refactor: binding Jan 31, 2023
@FGYFFFF FGYFFFF force-pushed the refactor/binding branch 6 times, most recently from 2ffa30e to c305aad Compare February 1, 2023 12:06
go.mod Outdated Show resolved Hide resolved
pkg/app/server/binding/binder.go Outdated Show resolved Hide resolved
pkg/app/server/binding/default_binder.go Outdated Show resolved Hide resolved
pkg/app/server/binding/default_binder.go Outdated Show resolved Hide resolved
pkg/app/server/binding/text_decoder/bool.go Outdated Show resolved Hide resolved
pkg/app/server/binding/default_validator.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
pkg/app/context.go Outdated Show resolved Hide resolved
Copy link
Member

@welkeyever welkeyever left a comment

Choose a reason for hiding this comment

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

LGTM & welldone, GL.

@FGYFFFF FGYFFFF merged commit 8904972 into cloudwego:develop Sep 22, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants