Skip to content

Commit

Permalink
fix: Use model.Type() as cache key instead of model itself (#62)
Browse files Browse the repository at this point in the history
* fix: Use model.Type() as cache key instead of model itself

* fix: update TestValuesReadsFromCacheFirst to match TestColumnsReadsFromCacheFirst
  • Loading branch information
max-stytch authored Jul 27, 2023
1 parent 0eac140 commit 3e43b34
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 9 deletions.
11 changes: 9 additions & 2 deletions columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ var (

var columnsCache cache = &sync.Map{}

type cacheKey struct {
Type reflect.Type
Strict bool
}

// Columns scans a struct and returns a list of strings
// that represent the assumed column names based on the
// db struct tag, or the field name. Any field or struct
Expand All @@ -53,7 +58,9 @@ func columns(v interface{}, strict bool, excluded ...string) ([]string, error) {
return nil, fmt.Errorf("columns: %w", err)
}

if cache, ok := columnsCache.Load(model); ok {
key := cacheKey{model.Type(), strict}

if cache, ok := columnsCache.Load(key); ok {
cached := cache.([]string)
res := make([]string, 0, len(cached))

Expand All @@ -76,7 +83,7 @@ func columns(v interface{}, strict bool, excluded ...string) ([]string, error) {

names := columnNames(model, strict, excluded...)
toCache := append(names, excluded...)
columnsCache.Store(model, toCache)
columnsCache.Store(key, toCache)
return names, nil
}

Expand Down
33 changes: 31 additions & 2 deletions columns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,44 @@ func TestColumnsReadsFromCacheFirst(t *testing.T) {
Name string
}

v := reflect.Indirect(reflect.ValueOf(&person))
key := cacheKey{
Type: reflect.Indirect(reflect.ValueOf(&person)).Type(),
Strict: false,
}
expected := []string{"fake"}
columnsCache.Store(v, expected)
columnsCache.Store(key, expected)

cols, err := Columns(&person)
assert.NoError(t, err)
assert.EqualValues(t, expected, cols)
}

func TestColumnsStoresOneCacheEntryPerInstance(t *testing.T) {
type person struct {
ID int64
Name string
}

before := 0
columnsCache.Range(func(key interface{}, value interface{}) bool {
before += 1
return true
})

for i := 0; i < 10; i++ {
_, err := Columns(&person{})
assert.NoError(t, err)
}

after := 0
columnsCache.Range(func(key interface{}, value interface{}) bool {
after += 1
return true
})

assert.Equal(t, 1, after-before, "Cache size grew unexpectedly")
}

func BenchmarkColumnsLargeStruct(b *testing.B) {
ls := &largeStruct{ID: "test", Index: 88, UUID: "test", IsActive: false, Balance: "test", Picture: "test", Age: 88, EyeColor: "test", Name: "test", Gender: "test", Company: "test", Email: "test", Phone: "test", Address: "test", About: "test", Registered: "test", Latitude: 0.566439688205719, Longitude: 0.48440760374069214, Greeting: "test", FavoriteFruit: "test", AID: "test", AIndex: 19, AUUID: "test", AIsActive: true, ABalance: "test", APicture: "test", AAge: 12, AEyeColor: "test", AName: "test", AGender: "test", ACompany: "test", AEmail: "test", APhone: "test", AAddress: "test", AAbout: "test", ARegistered: "test", ALatitude: 0.16338545083999634, ALongitude: 0.24648870527744293, AGreeting: "test", AFavoriteFruit: "test"}

Expand Down
4 changes: 2 additions & 2 deletions values.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func Values(cols []string, v interface{}) ([]interface{}, error) {
}

func loadFields(val reflect.Value) map[string][]int {
if cache, cached := valuesCache.Load(val); cached {
if cache, cached := valuesCache.Load(val.Type()); cached {
return cache.(map[string][]int)
}
return writeFieldsCache(val)
Expand All @@ -41,7 +41,7 @@ func loadFields(val reflect.Value) map[string][]int {
func writeFieldsCache(val reflect.Value) map[string][]int {
m := map[string][]int{}
writeFields(val, m, []int{})
valuesCache.Store(val, m)
valuesCache.Store(val.Type(), m)
return m
}

Expand Down
6 changes: 3 additions & 3 deletions values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ func TestValuesReadsFromCacheFirst(t *testing.T) {
Name: "Brett",
}

v := reflect.Indirect(reflect.ValueOf(&person))
valuesCache.Store(v, map[string][]int{"Name": {0}})
v := reflect.Indirect(reflect.ValueOf(&person)).Type()
valuesCache.Store(v, map[string][]int{"fake": {0}})

vals, err := Values([]string{"Name"}, &person)
vals, err := Values([]string{"fake"}, &person)
require.NoError(t, err)
assert.EqualValues(t, []interface{}{"Brett"}, vals)
}
Expand Down

0 comments on commit 3e43b34

Please sign in to comment.