Skip to content

Commit

Permalink
fix(util/gconv): cached field indexes append issue caused incorrect f…
Browse files Browse the repository at this point in the history 
…ield converting (#3790)
  • Loading branch information
@wln32
wln32 authored Sep 23, 2024
1 parent d8e3e9d commit 8a1c97f
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 81 deletions.
50 changes: 50 additions & 0 deletions net/ghttp/ghttp_z_unit_issue_test.go
View file
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
View file
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

0 comments on commit 8a1c97f

Please sign in to comment.