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.

Sign up for GitHub

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

Jump to bottom

Case insensitive matching of field names #337

Merged
merged 21 commits into from
Aug 5, 2022
Merged

Case insensitive matching of field names #337

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
834c572
accept upper case in camel style string
ychen-bloxer Jul 21, 2022
588c1c6
handle non-camel model structs
ychen-bloxer Jul 21, 2022
9f192d2
add test case
ychen-bloxer Jul 21, 2022
4c1994d
change print message
ychen-bloxer Jul 21, 2022
4bcae9a
update test cases
ychen-bloxer Jul 21, 2022
bde3d0a
add struct and test case
ychen-bloxer Jul 22, 2022
e95f9dd
update comment
ychen-bloxer Jul 22, 2022
a91b614
update comment
ychen-bloxer Jul 22, 2022
4314b8c
update test case
ychen-bloxer Jul 22, 2022
1e9f300
make find field name more generic
ychen-bloxer Jul 22, 2022
eb88b7c
revert changes
ychen-bloxer Jul 22, 2022
7d3b650
amend revert changes
ychen-bloxer Jul 22, 2022
3de668b
log query result
ychen-bloxer Jul 27, 2022
011fbab
add flag for insensitive field match
ychen-bloxer Aug 1, 2022
6ac4809
correct lint error
ychen-bloxer Aug 1, 2022
7faf8f3
uncomment test case
ychen-bloxer Aug 2, 2022
81d1c6a
revert import order
ychen-bloxer Aug 2, 2022
5dc652f
add tests with ignoreCase set to false
ychen-bloxer Aug 2, 2022
627b66e
revert function signature; add function to set case-sensitive matching
ychen-bloxer Aug 4, 2022
41470e0
update test; remove debug messgess
ychen-bloxer Aug 4, 2022
f23ddc7
add second search if camel search field failed
ychen-bloxer Aug 4, 2022
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
2 changes: 1 addition & 1 deletion gorm/collection_operators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type SortingCriteriaConverter interface {
}

type FieldSelectionConverter interface {
FieldSelectionToGorm(ctx context.Context, fs *query.FieldSelection, obj interface{}) ([]string, error)
FieldSelectionToGorm(ctx context.Context, fs *query.FieldSelection, obj interface{}, ignoreCase ...bool) ([]string, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be a option struct{} not a boolean. consider the meaning is for multiple options.

}

type PaginationConverter interface {
Expand Down
59 changes: 40 additions & 19 deletions gorm/fields.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gorm
import (
"context"
"fmt"
"log"
"reflect"
"sort"
"strings"
Expand All @@ -18,22 +19,23 @@ type DefaultFieldSelectionConverter struct{}

// FieldSelectionStringToGorm is a shortcut to parse a string into FieldSelection struct and
// receive a list of associations to preload.
func FieldSelectionStringToGorm(ctx context.Context, fs string, obj interface{}) ([]string, error) {
func FieldSelectionStringToGorm(ctx context.Context, fs string, obj interface{}, ignoreCase ...bool) ([]string, error) {
c := &DefaultFieldSelectionConverter{}
return c.FieldSelectionToGorm(ctx, query.ParseFieldSelection(fs), obj)
return c.FieldSelectionToGorm(ctx, query.ParseFieldSelection(fs), obj, ignoreCase...)
}

// FieldSelectionToGorm receives FieldSelection struct and returns a list of associations to preload.
func (converter *DefaultFieldSelectionConverter) FieldSelectionToGorm(ctx context.Context, fs *query.FieldSelection, obj interface{}) ([]string, error) {
func (converter *DefaultFieldSelectionConverter) FieldSelectionToGorm(ctx context.Context, fs *query.FieldSelection, obj interface{}, ignoreCase ...bool) ([]string, error) {
objType := indirectType(reflect.TypeOf(obj))
if fs.GetFields() == nil {
selectedFields := fs.GetFields()
if selectedFields == nil {
return preloadEverything(objType, nil)
}
var toPreload []string
fieldNames := getSortedFieldNames(fs.GetFields())
fieldNames := getSortedFieldNames(selectedFields)
for _, fieldName := range fieldNames {
f := fs.GetFields()[fieldName]
subPreload, err := handlePreloads(f, objType)
f := selectedFields[fieldName]
subPreload, err := handlePreloads(f, objType, ignoreCase...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -75,36 +77,55 @@ fields:
return toPreload, nil
}

func handlePreloads(f *query.Field, objType reflect.Type) ([]string, error) {
sf, ok := objType.FieldByName(util.Camel(f.GetName()))
func handlePreloads(f *query.Field, objType reflect.Type, ignoreCase ...bool) ([]string, error) {
queryFieldName := f.GetName()
log.Printf("Query name = %v\n", queryFieldName)

var sf reflect.StructField
var ok bool
if len(ignoreCase) > 0 && ignoreCase[0] {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe I'm missing something but why is ignoreCase a ...bool instead of just a plain old bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an "optional" parameter for backward compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok that makes sense. if that's the case then make sure that there is a unti test for ignoreCase being false, and one for there being no ignoreCase value at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there are. In test file, the function has been called as followed
FieldSelectionStringToGorm(context.Background(), test.fs, &Model{}) // no "ignoreCase" = false
FieldSelectionStringToGorm(context.Background(), test.fs, &Model{}, true)

Copy link
Contributor

Choose a reason for hiding this comment

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

it might be nice to have at least one FieldSelectionStringToGorm(context.Background(), test.fs, &Model{}, false) just for completion's sake

sf, ok = objType.FieldByNameFunc(func(name string) bool {
return strings.EqualFold(name, strings.ToLower(strings.ReplaceAll(queryFieldName, "_", "")))
})
} else {
sf, ok = objType.FieldByName(util.Camel(queryFieldName))
}

if !ok {
log.Printf("no field found for query '%v\n'", queryFieldName)
return nil, nil
}

fType := indirectType(sf.Type)
if f.GetSubs() == nil {
fName := sf.Name
log.Printf("found field by name '%v'\n", fName)

fieldSubs := f.GetSubs()

if fieldSubs == nil {
if isModel(fType) {
return []string{util.Camel(f.GetName())}, nil
return []string{fName}, nil
} else {
return nil, nil
}
}
if !isModel(fType) {
return nil, fmt.Errorf("%s is expected to be a model, but got %s ", f.GetName(), fType)
return nil, fmt.Errorf("%s is expected to be a model, but got %s ", queryFieldName, fType)
}
var toPreload []string
fieldNames := getSortedFieldNames(f.GetSubs())
fieldNames := getSortedFieldNames(fieldSubs)
for _, fieldName := range fieldNames {
subField := f.GetSubs()[fieldName]
subPreload, err := handlePreloads(subField, fType)
subField := fieldSubs[fieldName]
subPreload, err := handlePreloads(subField, fType, ignoreCase...)
if err != nil {
return nil, err
}
for i, e := range subPreload {
subPreload[i] = util.Camel(f.GetName()) + "." + e
subPreload[i] = fName + "." + e
}
toPreload = append(toPreload, subPreload...)
}
return append(toPreload, util.Camel(f.GetName())), nil
return append(toPreload, fName), nil
}

func getSortedFieldNames(fields map[string]*query.Field) []string {
Expand All @@ -126,7 +147,7 @@ func preload(db *gorm.DB, obj interface{}, assoc string) (*gorm.DB, error) {
for i, part := range assocPath {
sf, ok := objType.FieldByName(part)
if !ok {
return nil, fmt.Errorf("Cannot find %s in %s", part, objType)
return nil, fmt.Errorf("cannot find %s in %s", part, objType)
}
objType = indirectType(sf.Type)
if !isModel(objType) {
Expand All @@ -143,5 +164,5 @@ func preload(db *gorm.DB, obj interface{}, assoc string) (*gorm.DB, error) {
}
}
}
return nil, fmt.Errorf("Cannot preload empty association")
return nil, fmt.Errorf("cannot preload empty association")
}
177 changes: 166 additions & 11 deletions gorm/fields_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,54 @@ package gorm

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

type Model struct {
Property string
SubModel SubModel
SubModels []SubModel
CycleModel *CycleModel
NotPreloadObj SubModel `gorm:"preload:false"`
PreloadObj SubModel `gorm:"preload:true"`
Property string
SubModel SubModel
SubModels []SubModel
CycleModel *CycleModel
NotPreloadObj SubModel `gorm:"preload:false"`
PreloadObj SubModel `gorm:"preload:true"`
NonCamelMODEL NonCamelMODEL
NonCAMEL2Model NonCAMEL2Model
NonCamelSUBMODEL NonCamelSUBMODEL
SubModelMix SubModelMix
SubModelMix2 SubModelMix2
NonCamelModelMIX NonCamelModelMIX
}

type NonCamelModelMIX struct {
NonCamelMixProperty string
SubModel SubModel
}

type SubModelMix struct {
SubModelMixProperty string
NonCamelMODEL NonCamelMODEL
}

type SubModelMix2 struct {
SubModelMixProperty string
NonCamelSUBMODEL NonCamelSUBMODEL
}

type NonCamelMODEL struct {
NonCamelProperty string
NonCamelSUBMODEL NonCamelSUBMODEL
}

type NonCamelSUBMODEL struct {
NonCamelSubProperty string
}

type NonCAMEL2Model struct {
NonCamelProperty string
Model *Model
}

type CycleModel struct {
Expand All @@ -32,53 +68,145 @@ type SubSubModel struct {

func TestGormFieldSelection(t *testing.T) {
tests := []struct {
fs string
toPreload []string
err bool
fs string
toPreload []string
toPreloadIgnoreCase []string
err bool
}{
{
"sub_model_mix2.non_camel_submodel",
[]string{"SubModelMix2"},
[]string{"SubModelMix2.NonCamelSUBMODEL", "SubModelMix2"},
false,
},
{
"non_camel_model_mix.sub_model.sub_property",
nil,
[]string{"NonCamelModelMIX.SubModel", "NonCamelModelMIX"},
false,
},
{
"non_camel_model_mix,sub_model,non_camel_2_model,cycle_model",
[]string{"CycleModel", "SubModel"},
[]string{"CycleModel", "NonCAMEL2Model", "NonCamelModelMIX", "SubModel"},
false,
},
{
"sub_model_mix.non_camel_model",
[]string{"SubModelMix"},
[]string{"SubModelMix.NonCamelMODEL", "SubModelMix"},
false,
},
{
"non_camel_model.non_camel_submodel.noncamelsubproperty",
nil,
[]string{"NonCamelMODEL.NonCamelSUBMODEL", "NonCamelMODEL"},
false,
},
{
"non_camel_model.non_camel_submodel.noncamelsubproperty",
nil,
[]string{"NonCamelMODEL.NonCamelSUBMODEL", "NonCamelMODEL"},
false,
},
{
"non_camel_model.non_camel_submodel",
nil,
[]string{"NonCamelMODEL.NonCamelSUBMODEL", "NonCamelMODEL"},
false,
},
{
"non_camel_model",
nil,
[]string{"NonCamelMODEL"},
false,
},
{
"non_camel_model.noncamelproperty",
nil,
[]string{"NonCamelMODEL"},
false,
},
{
"non_CAMEL_2_Model,Non_camel_2_model,non_camel2_model,non_camel_2model",
nil,
[]string{"NonCAMEL2Model", "NonCAMEL2Model", "NonCAMEL2Model", "NonCAMEL2Model"},
false,
},
{
"property",
nil,
nil,
false,
},
{
"property,sub_model",
[]string{"SubModel"},
[]string{"SubModel"},
false,
},
{
"property,sub_model.sub_property",
[]string{"SubModel"},
[]string{"SubModel"},
false,
},
{
"sub_model,sub_models",
[]string{"SubModel", "SubModels"},
[]string{"SubModel", "SubModels"},
false,
},
{
"sub_model,sub_models.sub_property",
[]string{"SubModel", "SubModels"},
[]string{"SubModel", "SubModels"},
false,
},
{
"sub_model,sub_model.sub_sub_model.sub_sub_property",
"sub_model",
[]string{"SubModel"},
[]string{"SubModel"},
false,
},
{
"sub_model.sub_sub_model",
[]string{"SubModel.SubSubModel", "SubModel"},
[]string{"SubModel.SubSubModel", "SubModel"},
false,
},
{
"sub_model.sub_sub_model.sub_sub_property",
[]string{"SubModel.SubSubModel", "SubModel"},
[]string{"SubModel.SubSubModel", "SubModel"},
false,
},
{
"unknown_property",
nil,
nil,
false,
},
{
"not_preload_obj,preload_obj",
[]string{"NotPreloadObj", "PreloadObj"},
[]string{"NotPreloadObj", "PreloadObj"},
false,
},
{
"",
[]string{"SubModel.SubSubModel", "SubModel", "SubModels.SubSubModel", "SubModels", "CycleModel", "PreloadObj.SubSubModel", "PreloadObj"},
[]string{"SubModel.SubSubModel", "SubModel", "SubModels.SubSubModel", "SubModels",
"CycleModel", "PreloadObj.SubSubModel", "PreloadObj",
"NonCamelMODEL.NonCamelSUBMODEL", "NonCamelMODEL", "NonCAMEL2Model", "NonCamelSUBMODEL",
"SubModelMix.NonCamelMODEL.NonCamelSUBMODEL", "SubModelMix.NonCamelMODEL", "SubModelMix",
"SubModelMix2.NonCamelSUBMODEL", "SubModelMix2",
"NonCamelModelMIX.SubModel.SubSubModel", "NonCamelModelMIX.SubModel", "NonCamelModelMIX"},
[]string{"SubModel.SubSubModel", "SubModel", "SubModels.SubSubModel", "SubModels",
"CycleModel", "PreloadObj.SubSubModel", "PreloadObj",
"NonCamelMODEL.NonCamelSUBMODEL", "NonCamelMODEL", "NonCAMEL2Model", "NonCamelSUBMODEL",
"SubModelMix.NonCamelMODEL.NonCamelSUBMODEL", "SubModelMix.NonCamelMODEL", "SubModelMix",
"SubModelMix2.NonCamelSUBMODEL", "SubModelMix2",
"NonCamelModelMIX.SubModel.SubSubModel", "NonCamelModelMIX.SubModel", "NonCamelModelMIX"},
false,
},
}
Expand All @@ -88,8 +216,35 @@ func TestGormFieldSelection(t *testing.T) {
assert.Nil(t, toPreload)
assert.NotNil(t, err)
} else {
fmt.Printf("expected=%v\nactual=%v\n\n", test.toPreload, toPreload)
assert.Equal(t, test.toPreload, toPreload)
assert.Nil(t, err)
}
}

for _, test := range tests {
toPreload, err := FieldSelectionStringToGorm(context.Background(), test.fs, &Model{}, false)
if test.err {
assert.Nil(t, toPreload)
assert.NotNil(t, err)
} else {
fmt.Printf("Ignore case = false\nexpected=%v\nactual=%v\n\n", test.toPreload, toPreload)
assert.Equal(t, test.toPreload, toPreload)
assert.Nil(t, err)
}
}

// re-test with "ignoreCase" flag set to true
for _, test := range tests {
toPreloadIgnoreCase, err := FieldSelectionStringToGorm(context.Background(), test.fs, &Model{}, true)
if test.err {
assert.Nil(t, toPreloadIgnoreCase)
assert.NotNil(t, err)
} else {
fmt.Printf("Ignore case = true \nexpected=%v\nactual=%v\n\n", test.toPreloadIgnoreCase, toPreloadIgnoreCase)
assert.Equal(t, test.toPreloadIgnoreCase, toPreloadIgnoreCase)
assert.Nil(t, err)
}
}

}