diff --git a/core/collection/cache.go b/core/collection/cache.go index 56b31811b..75bb8d48a 100644 --- a/core/collection/cache.go +++ b/core/collection/cache.go @@ -81,6 +81,10 @@ func (c *Cache) Del(key string) { delete(c.data, key) c.lruCache.remove(key) c.lock.Unlock() + + // RemoveTimer is called outside the lock to avoid performance impact from this + // potentially time-consuming operation. Data integrity is maintained by lruCache, + // which will eventually evict any remaining entries when capacity is exceeded. c.timingWheel.RemoveTimer(key) } diff --git a/core/stores/sqlx/orm.go b/core/stores/sqlx/orm.go index 5b308581d..b395f3c52 100644 --- a/core/stores/sqlx/orm.go +++ b/core/stores/sqlx/orm.go @@ -70,25 +70,16 @@ func getTaggedFieldValueMap(v reflect.Value) (map[string]any, error) { } func getValueInterface(value reflect.Value) (any, error) { - switch value.Kind() { - case reflect.Ptr: - if !value.CanAddr() || !value.Addr().CanInterface() { - return nil, ErrNotReadableValue - } - - if value.IsNil() { - baseValueType := mapping.Deref(value.Type()) - value.Set(reflect.New(baseValueType)) - } - - return value.Addr().Interface(), nil - default: - if !value.CanAddr() || !value.Addr().CanInterface() { - return nil, ErrNotReadableValue - } - - return value.Addr().Interface(), nil + if !value.CanAddr() || !value.Addr().CanInterface() { + return nil, ErrNotReadableValue } + + if value.Kind() == reflect.Pointer && value.IsNil() { + baseValueType := mapping.Deref(value.Type()) + value.Set(reflect.New(baseValueType)) + } + + return value.Addr().Interface(), nil } func isScanFailed(err error) bool { diff --git a/core/stores/sqlx/orm_test.go b/core/stores/sqlx/orm_test.go index c3e9e46ea..d8561c869 100644 --- a/core/stores/sqlx/orm_test.go +++ b/core/stores/sqlx/orm_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "reflect" "testing" "time" @@ -2205,6 +2206,145 @@ func TestUnmarshalRowsSqlNullStringEmptyVsNull(t *testing.T) { }) } +func TestGetValueInterface(t *testing.T) { + t.Run("non_pointer_field", func(t *testing.T) { + type testStruct struct { + Name string + Age int + } + s := testStruct{} + v := reflect.ValueOf(&s).Elem() + + nameField := v.Field(0) + result, err := getValueInterface(nameField) + assert.NoError(t, err) + assert.NotNil(t, result) + + // Should return pointer to the field + ptr, ok := result.(*string) + assert.True(t, ok) + *ptr = "test" + assert.Equal(t, "test", s.Name) + }) + + t.Run("pointer_field_nil", func(t *testing.T) { + type testStruct struct { + NamePtr *string + AgePtr *int64 + } + s := testStruct{} + v := reflect.ValueOf(&s).Elem() + + // Test with nil pointer field + namePtrField := v.Field(0) + assert.True(t, namePtrField.IsNil(), "initial pointer should be nil") + + result, err := getValueInterface(namePtrField) + assert.NoError(t, err) + assert.NotNil(t, result) + + // Should have allocated the pointer + assert.False(t, namePtrField.IsNil(), "pointer should be allocated after getValueInterface") + + // Should return pointer to pointer field + ptrPtr, ok := result.(**string) + assert.True(t, ok) + testValue := "initialized" + *ptrPtr = &testValue + assert.NotNil(t, s.NamePtr) + assert.Equal(t, "initialized", *s.NamePtr) + }) + + t.Run("pointer_field_already_allocated", func(t *testing.T) { + type testStruct struct { + NamePtr *string + } + initial := "existing" + s := testStruct{NamePtr: &initial} + v := reflect.ValueOf(&s).Elem() + + namePtrField := v.Field(0) + assert.False(t, namePtrField.IsNil(), "pointer should not be nil initially") + + result, err := getValueInterface(namePtrField) + assert.NoError(t, err) + assert.NotNil(t, result) + + // Should return pointer to pointer field + ptrPtr, ok := result.(**string) + assert.True(t, ok) + + // Verify it points to the existing value + assert.Equal(t, "existing", **ptrPtr) + + // Modify through the returned pointer + newValue := "modified" + *ptrPtr = &newValue + assert.Equal(t, "modified", *s.NamePtr) + }) + + t.Run("pointer_field_zero_value", func(t *testing.T) { + type testStruct struct { + IntPtr *int + } + s := testStruct{} + v := reflect.ValueOf(&s).Elem() + + intPtrField := v.Field(0) + result, err := getValueInterface(intPtrField) + assert.NoError(t, err) + + // After calling getValueInterface, nil pointer should be allocated + assert.NotNil(t, s.IntPtr) + + // Set zero value through returned interface + ptrPtr, ok := result.(**int) + assert.True(t, ok) + zero := 0 + *ptrPtr = &zero + assert.Equal(t, 0, *s.IntPtr) + }) + + t.Run("not_addressable_value", func(t *testing.T) { + type testStruct struct { + Name string + } + s := testStruct{Name: "test"} + v := reflect.ValueOf(s) // Non-pointer, not addressable + + nameField := v.Field(0) + result, err := getValueInterface(nameField) + assert.Error(t, err) + assert.Equal(t, ErrNotReadableValue, err) + assert.Nil(t, result) + }) + + t.Run("multiple_pointer_types", func(t *testing.T) { + type testStruct struct { + StringPtr *string + IntPtr *int + Int64Ptr *int64 + FloatPtr *float64 + BoolPtr *bool + } + s := testStruct{} + v := reflect.ValueOf(&s).Elem() + + // Test each pointer type gets properly initialized + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + assert.True(t, field.IsNil(), "field %d should start as nil", i) + + result, err := getValueInterface(field) + assert.NoError(t, err, "field %d should not error", i) + assert.NotNil(t, result, "field %d result should not be nil", i) + + // After getValueInterface, pointer should be allocated + assert.False(t, field.IsNil(), "field %d should be allocated", i) + } + }) +} + func stringPtr(s string) *string { return &s }