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

fix(util/gconv): cached field indexes append issue caused incorrect field converting #3790

Merged
merged 5 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions net/ghttp/ghttp_z_unit_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,3 +568,53 @@ func Test_Issue3245(t *testing.T) {
t.Assert(c.GetContent(ctx, "/hello?nickname=oldme"), expect)
})
}

type ItemSecondThird struct {
SecondID uint64 `json:"secondId,string"`
ThirdID uint64 `json:"thirdId,string"`
}
type ItemFirst struct {
ID uint64 `json:"id,string"`
ItemSecondThird
}
type ItemInput struct {
ItemFirst
}
type Issue3789Req struct {
g.Meta `path:"/hello" method:"GET"`
ItemInput
}
type Issue3789Res struct {
ItemInput
}

type Issue3789 struct{}

func (Issue3789) Say(ctx context.Context, req *Issue3789Req) (res *Issue3789Res, err error) {
res = &Issue3789Res{
ItemInput: req.ItemInput,
}
return
}

// https://github.com/gogf/gf/issues/3789
func TestIssue3789(t *testing.T) {
gtest.C(t, func(t *gtest.T) {
s := g.Server()
s.Use(ghttp.MiddlewareHandlerResponse)
s.Group("/", func(group *ghttp.RouterGroup) {
group.Bind(
new(Issue3789),
)
})
s.SetDumpRouterMap(false)
s.Start()
defer s.Shutdown()
time.Sleep(100 * time.Millisecond)

c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))
expect := `{"code":0,"message":"","data":{"id":"0","secondId":"2","thirdId":"3"}}`
t.Assert(c.GetContent(ctx, "/hello?id=&secondId=2&thirdId=3"), expect)
})
}
152 changes: 78 additions & 74 deletions util/gconv/gconv_struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func doStruct(
paramsInterface interface{} // DO NOT use `params` directly as it might be type `reflect.Value`
pointerReflectValue reflect.Value
pointerReflectKind reflect.Kind
pointerElemReflectValue reflect.Value // The pointed element.
pointerElemReflectValue reflect.Value // The reflection value to struct element.
)
if v, ok := params.(reflect.Value); ok {
paramsReflectValue = v
Expand Down Expand Up @@ -183,33 +183,37 @@ func doStruct(
var (
// Indicates that those values have been used and cannot be reused.
usedParamsKeyOrTagNameMap = structcache.GetUsedParamsKeyOrTagNameMapFromPool()
cachedFieldInfo *structcache.CachedFieldInfo
paramsValue interface{}
)
defer structcache.PutUsedParamsKeyOrTagNameMapToPool(usedParamsKeyOrTagNameMap)

// Firstly, search according to custom mapping rules.
// If a possible direct assignment is found, reduce the number of subsequent map searches.
for paramKey, fieldName := range paramKeyToAttrMap {
fieldInfo := cachedStructInfo.GetFieldInfo(fieldName)
if fieldInfo != nil {
if paramsValue, ok := paramsMap[paramKey]; ok {
fieldValue := fieldInfo.GetFieldReflectValue(pointerElemReflectValue)
if err = bindVarToStructField(
fieldValue,
paramsValue,
fieldInfo,
paramKeyToAttrMap,
paramsValue, ok = paramsMap[paramKey]
if !ok {
continue
}
cachedFieldInfo = cachedStructInfo.GetFieldInfo(fieldName)
if cachedFieldInfo != nil {
fieldValue := cachedFieldInfo.GetFieldReflectValueFrom(pointerElemReflectValue)
if err = bindVarToStructField(
fieldValue,
paramsValue,
cachedFieldInfo,
paramKeyToAttrMap,
); err != nil {
return err
}
if len(cachedFieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(
cachedFieldInfo, paramsValue, pointerReflectValue, paramKeyToAttrMap,
); err != nil {
return err
}
if len(fieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(
fieldInfo, paramsValue, pointerReflectValue, paramKeyToAttrMap,
); err != nil {
return err
}
}
usedParamsKeyOrTagNameMap[paramKey] = struct{}{}
}
usedParamsKeyOrTagNameMap[paramKey] = struct{}{}
}
}
// Already done converting for given `paramsMap`.
Expand All @@ -228,15 +232,15 @@ func doStruct(
}

func setOtherSameNameField(
fieldInfo *structcache.CachedFieldInfo,
cachedFieldInfo *structcache.CachedFieldInfo,
srcValue any,
structValue reflect.Value,
paramKeyToAttrMap map[string]string,
) (err error) {
// loop the same field name of all sub attributes.
for i := range fieldInfo.OtherSameNameFieldIndex {
fieldValue := fieldInfo.GetOtherFieldReflectValue(structValue, i)
if err = bindVarToStructField(fieldValue, srcValue, fieldInfo, paramKeyToAttrMap); err != nil {
for i := range cachedFieldInfo.OtherSameNameFieldIndex {
fieldValue := cachedFieldInfo.GetOtherFieldReflectValueFrom(structValue, i)
if err = bindVarToStructField(fieldValue, srcValue, cachedFieldInfo, paramKeyToAttrMap); err != nil {
return err
}
}
Expand All @@ -251,36 +255,36 @@ func bindStructWithLoopParamsMap(
cachedStructInfo *structcache.CachedStructInfo,
) (err error) {
var (
fieldName string
fieldInfo *structcache.CachedFieldInfo
fuzzLastKey string
fieldValue reflect.Value
paramKey string
paramValue any
ok bool
fieldName string
cachedFieldInfo *structcache.CachedFieldInfo
fuzzLastKey string
fieldValue reflect.Value
paramKey string
paramValue any
ok bool
)
for paramKey, paramValue = range paramsMap {
if _, ok = usedParamsKeyOrTagNameMap[paramKey]; ok {
continue
}
fieldInfo = cachedStructInfo.GetFieldInfo(paramKey)
if fieldInfo != nil {
fieldName = fieldInfo.FieldName()
cachedFieldInfo = cachedStructInfo.GetFieldInfo(paramKey)
if cachedFieldInfo != nil {
fieldName = cachedFieldInfo.FieldName()
// already converted using its field name?
// the field name has the more priority than tag name.
_, ok = usedParamsKeyOrTagNameMap[fieldName]
if ok && fieldInfo.IsField {
if ok && cachedFieldInfo.IsField {
continue
}
fieldValue = fieldInfo.GetFieldReflectValue(structValue)
fieldValue = cachedFieldInfo.GetFieldReflectValueFrom(structValue)
if err = bindVarToStructField(
fieldValue, paramValue, fieldInfo, paramKeyToAttrMap,
fieldValue, paramValue, cachedFieldInfo, paramKeyToAttrMap,
); err != nil {
return err
}
// handle same field name in nested struct.
if len(fieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(fieldInfo, paramValue, structValue, paramKeyToAttrMap); err != nil {
if len(cachedFieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(cachedFieldInfo, paramValue, structValue, paramKeyToAttrMap); err != nil {
return err
}
}
Expand All @@ -289,40 +293,40 @@ func bindStructWithLoopParamsMap(
}

// fuzzy matching.
for _, fieldInfo = range cachedStructInfo.FieldConvertInfos {
fieldName = fieldInfo.FieldName()
for _, cachedFieldInfo = range cachedStructInfo.FieldConvertInfos {
fieldName = cachedFieldInfo.FieldName()
if _, ok = usedParamsKeyOrTagNameMap[fieldName]; ok {
continue
}
fuzzLastKey = fieldInfo.LastFuzzyKey.Load().(string)
fuzzLastKey = cachedFieldInfo.LastFuzzyKey.Load().(string)
paramValue, ok = paramsMap[fuzzLastKey]
if !ok {
if strings.EqualFold(
fieldInfo.RemoveSymbolsFieldName, utils.RemoveSymbols(paramKey),
cachedFieldInfo.RemoveSymbolsFieldName, utils.RemoveSymbols(paramKey),
) {
paramValue, ok = paramsMap[paramKey]
// If it is found this time, update it based on what was not found last time.
fieldInfo.LastFuzzyKey.Store(paramKey)
cachedFieldInfo.LastFuzzyKey.Store(paramKey)
}
}
if ok {
fieldValue = fieldInfo.GetFieldReflectValue(structValue)
fieldValue = cachedFieldInfo.GetFieldReflectValueFrom(structValue)
if paramValue != nil {
if err = bindVarToStructField(
fieldValue, paramValue, fieldInfo, paramKeyToAttrMap,
fieldValue, paramValue, cachedFieldInfo, paramKeyToAttrMap,
); err != nil {
return err
}
// handle same field name in nested struct.
if len(fieldInfo.OtherSameNameFieldIndex) > 0 {
if len(cachedFieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(
fieldInfo, paramValue, structValue, paramKeyToAttrMap,
cachedFieldInfo, paramValue, structValue, paramKeyToAttrMap,
); err != nil {
return err
}
}
}
usedParamsKeyOrTagNameMap[fieldInfo.FieldName()] = struct{}{}
usedParamsKeyOrTagNameMap[cachedFieldInfo.FieldName()] = struct{}{}
break
}
}
Expand All @@ -338,33 +342,33 @@ func bindStructWithLoopFieldInfos(
cachedStructInfo *structcache.CachedStructInfo,
) (err error) {
var (
fieldInfo *structcache.CachedFieldInfo
fuzzLastKey string
fieldValue reflect.Value
paramKey string
paramValue any
matched bool
ok bool
cachedFieldInfo *structcache.CachedFieldInfo
fuzzLastKey string
fieldValue reflect.Value
paramKey string
paramValue any
matched bool
ok bool
)
for _, fieldInfo = range cachedStructInfo.FieldConvertInfos {
for _, fieldTag := range fieldInfo.PriorityTagAndFieldName {
for _, cachedFieldInfo = range cachedStructInfo.FieldConvertInfos {
for _, fieldTag := range cachedFieldInfo.PriorityTagAndFieldName {
if paramValue, ok = paramsMap[fieldTag]; !ok {
continue
}
if _, ok = usedParamsKeyOrTagNameMap[fieldTag]; ok {
matched = true
break
}
fieldValue = fieldInfo.GetFieldReflectValue(structValue)
fieldValue = cachedFieldInfo.GetFieldReflectValueFrom(structValue)
if err = bindVarToStructField(
fieldValue, paramValue, fieldInfo, paramKeyToAttrMap,
fieldValue, paramValue, cachedFieldInfo, paramKeyToAttrMap,
); err != nil {
return err
}
// handle same field name in nested struct.
if len(fieldInfo.OtherSameNameFieldIndex) > 0 {
if len(cachedFieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(
fieldInfo, paramValue, structValue, paramKeyToAttrMap,
cachedFieldInfo, paramValue, structValue, paramKeyToAttrMap,
); err != nil {
return err
}
Expand All @@ -378,26 +382,26 @@ func bindStructWithLoopFieldInfos(
continue
}

fuzzLastKey = fieldInfo.LastFuzzyKey.Load().(string)
fuzzLastKey = cachedFieldInfo.LastFuzzyKey.Load().(string)
if paramValue, ok = paramsMap[fuzzLastKey]; !ok {
paramKey, paramValue = fuzzyMatchingFieldName(
fieldInfo.RemoveSymbolsFieldName, paramsMap, usedParamsKeyOrTagNameMap,
cachedFieldInfo.RemoveSymbolsFieldName, paramsMap, usedParamsKeyOrTagNameMap,
)
ok = paramKey != ""
fieldInfo.LastFuzzyKey.Store(paramKey)
cachedFieldInfo.LastFuzzyKey.Store(paramKey)
}
if ok {
fieldValue = fieldInfo.GetFieldReflectValue(structValue)
fieldValue = cachedFieldInfo.GetFieldReflectValueFrom(structValue)
if paramValue != nil {
if err = bindVarToStructField(
fieldValue, paramValue, fieldInfo, paramKeyToAttrMap,
fieldValue, paramValue, cachedFieldInfo, paramKeyToAttrMap,
); err != nil {
return err
}
// handle same field name in nested struct.
if len(fieldInfo.OtherSameNameFieldIndex) > 0 {
if len(cachedFieldInfo.OtherSameNameFieldIndex) > 0 {
if err = setOtherSameNameField(
fieldInfo, paramValue, structValue, paramKeyToAttrMap,
cachedFieldInfo, paramValue, structValue, paramKeyToAttrMap,
); err != nil {
return err
}
Expand Down Expand Up @@ -432,7 +436,7 @@ func fuzzyMatchingFieldName(
func bindVarToStructField(
fieldValue reflect.Value,
srcValue interface{},
fieldInfo *structcache.CachedFieldInfo,
cachedFieldInfo *structcache.CachedFieldInfo,
paramKeyToAttrMap map[string]string,
) (err error) {
if !fieldValue.IsValid() {
Expand All @@ -445,7 +449,7 @@ func bindVarToStructField(
defer func() {
if exception := recover(); exception != nil {
if err = bindVarToReflectValue(fieldValue, srcValue, paramKeyToAttrMap); err != nil {
err = gerror.Wrapf(err, `error binding srcValue to attribute "%s"`, fieldInfo.FieldName())
err = gerror.Wrapf(err, `error binding srcValue to attribute "%s"`, cachedFieldInfo.FieldName())
}
}
}()
Expand All @@ -460,27 +464,27 @@ func bindVarToStructField(
customConverterInput reflect.Value
ok bool
)
if fieldInfo.IsCustomConvert {
if cachedFieldInfo.IsCustomConvert {
if customConverterInput, ok = srcValue.(reflect.Value); !ok {
customConverterInput = reflect.ValueOf(srcValue)
}
if ok, err = callCustomConverter(customConverterInput, fieldValue); ok || err != nil {
return
}
}
if fieldInfo.IsCommonInterface {
if cachedFieldInfo.IsCommonInterface {
if ok, err = bindVarToReflectValueWithInterfaceCheck(fieldValue, srcValue); ok || err != nil {
return
}
}
// Common types use fast assignment logic
if fieldInfo.ConvertFunc != nil {
fieldInfo.ConvertFunc(srcValue, fieldValue)
if cachedFieldInfo.ConvertFunc != nil {
cachedFieldInfo.ConvertFunc(srcValue, fieldValue)
return nil
}
doConvertWithReflectValueSet(fieldValue, doConvertInput{
FromValue: srcValue,
ToTypeName: fieldInfo.StructField.Type.String(),
ToTypeName: cachedFieldInfo.StructField.Type.String(),
ReferValue: fieldValue,
})
return nil
Expand Down
Loading
Loading