From ac3481ed43d50ce49ff78a0de3d08782c9d52f05 Mon Sep 17 00:00:00 2001 From: John Guo Date: Thu, 17 Aug 2023 20:29:06 +0800 Subject: [PATCH] it returns error when `Scan` to a none empty slice with empty `Result` for package `gdb` (#2858) --- contrib/drivers/mssql/mssql_z_basic_test.go | 4 +- contrib/drivers/mysql/mysql_core_test.go | 4 +- .../mysql/mysql_feature_model_struct_test.go | 4 +- contrib/drivers/mysql/mysql_model_test.go | 41 ++++++++++ contrib/drivers/sqlite/sqlite_core_test.go | 4 +- contrib/drivers/sqlitecgo/sqlite_core_test.go | 4 +- database/gdb/gdb_type_result.go | 9 +++ frame/g/g_func.go | 10 ++- frame/g/g_z_unit_test.go | 6 ++ internal/empty/empty.go | 77 +++++++++++-------- os/gfpool/gfpool_pool.go | 2 +- 11 files changed, 118 insertions(+), 47 deletions(-) diff --git a/contrib/drivers/mssql/mssql_z_basic_test.go b/contrib/drivers/mssql/mssql_z_basic_test.go index b4d26aa92..2dfb4239f 100644 --- a/contrib/drivers/mssql/mssql_z_basic_test.go +++ b/contrib/drivers/mssql/mssql_z_basic_test.go @@ -8,6 +8,7 @@ package mssql_test import ( "context" + "database/sql" "fmt" "testing" "time" @@ -804,8 +805,7 @@ func Test_DB_ToJson(t *testing.T) { t.Assert(users[0].CreateTime, resultJson.Get("0.CREATE_TIME").String()) result = nil - err = result.Structs(&users) - t.AssertNil(err) + t.Assert(result.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { diff --git a/contrib/drivers/mysql/mysql_core_test.go b/contrib/drivers/mysql/mysql_core_test.go index 00cc599bc..9e6533c03 100644 --- a/contrib/drivers/mysql/mysql_core_test.go +++ b/contrib/drivers/mysql/mysql_core_test.go @@ -8,6 +8,7 @@ package mysql_test import ( "context" + "database/sql" "fmt" "testing" "time" @@ -892,8 +893,7 @@ func Test_DB_ToJson(t *testing.T) { t.Assert(users[0].CreateTime, resultJson.Get("0.create_time").String()) result = nil - err = result.Structs(&users) - t.AssertNil(err) + t.Assert(result.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { diff --git a/contrib/drivers/mysql/mysql_feature_model_struct_test.go b/contrib/drivers/mysql/mysql_feature_model_struct_test.go index 5da8d6f68..7d9902c79 100644 --- a/contrib/drivers/mysql/mysql_feature_model_struct_test.go +++ b/contrib/drivers/mysql/mysql_feature_model_struct_test.go @@ -301,7 +301,7 @@ func Test_Structs_Empty(t *testing.T) { all, err := db.Model(table).Where("id>100").All() t.AssertNil(err) users := make([]User, 10) - t.Assert(all.Structs(&users), nil) + t.Assert(all.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { all, err := db.Model(table).Where("id>100").All() @@ -320,7 +320,7 @@ func Test_Structs_Empty(t *testing.T) { all, err := db.Model(table).Where("id>100").All() t.AssertNil(err) users := make([]*User, 10) - t.Assert(all.Structs(&users), nil) + t.Assert(all.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { all, err := db.Model(table).Where("id>100").All() diff --git a/contrib/drivers/mysql/mysql_model_test.go b/contrib/drivers/mysql/mysql_model_test.go index 8e3501eb4..0aa8d494a 100644 --- a/contrib/drivers/mysql/mysql_model_test.go +++ b/contrib/drivers/mysql/mysql_model_test.go @@ -4665,3 +4665,44 @@ func Test_Builder_OmitEmptyWhere(t *testing.T) { t.Assert(count, int64(TableSize)) }) } + +func Test_Scan_Nil_Result_Error(t *testing.T) { + table := createInitTable() + defer dropTable(table) + + type S struct { + Id int + Name string + Age int + Score int + } + gtest.C(t, func(t *gtest.T) { + var s *S + err := db.Model(table).Where("id", 1).Scan(&s) + t.AssertNil(err) + t.Assert(s.Id, 1) + }) + gtest.C(t, func(t *gtest.T) { + var s *S + err := db.Model(table).Where("id", 100).Scan(&s) + t.AssertNil(err) + t.Assert(s, nil) + }) + gtest.C(t, func(t *gtest.T) { + var s S + err := db.Model(table).Where("id", 100).Scan(&s) + t.Assert(err, sql.ErrNoRows) + }) + gtest.C(t, func(t *gtest.T) { + var ss []*S + err := db.Model(table).Scan(&ss) + t.AssertNil(err) + t.Assert(len(ss), TableSize) + }) + // If the result is empty, it returns error. + gtest.C(t, func(t *gtest.T) { + var ss = make([]*S, 10) + err := db.Model(table).WhereGT("id", 100).Scan(&ss) + t.Assert(err, sql.ErrNoRows) + }) +} diff --git a/contrib/drivers/sqlite/sqlite_core_test.go b/contrib/drivers/sqlite/sqlite_core_test.go index 1ad5721c8..f20b051d5 100644 --- a/contrib/drivers/sqlite/sqlite_core_test.go +++ b/contrib/drivers/sqlite/sqlite_core_test.go @@ -8,6 +8,7 @@ package sqlite_test import ( "context" + "database/sql" "fmt" "testing" "time" @@ -839,8 +840,7 @@ func Test_DB_ToJson(t *testing.T) { t.Assert(users[0].CreateTime, resultJson.Get("0.create_time").String()) result = nil - err = result.Structs(&users) - t.AssertNil(err) + t.Assert(result.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { diff --git a/contrib/drivers/sqlitecgo/sqlite_core_test.go b/contrib/drivers/sqlitecgo/sqlite_core_test.go index f6f3a2af8..4539bbe34 100644 --- a/contrib/drivers/sqlitecgo/sqlite_core_test.go +++ b/contrib/drivers/sqlitecgo/sqlite_core_test.go @@ -8,6 +8,7 @@ package sqlitecgo_test import ( "context" + "database/sql" "fmt" "testing" "time" @@ -839,8 +840,7 @@ func Test_DB_ToJson(t *testing.T) { t.Assert(users[0].CreateTime, resultJson.Get("0.create_time").String()) result = nil - err = result.Structs(&users) - t.AssertNil(err) + t.Assert(result.Structs(&users), sql.ErrNoRows) }) gtest.C(t, func(t *gtest.T) { diff --git a/database/gdb/gdb_type_result.go b/database/gdb/gdb_type_result.go index 343a75f5d..03e7051d3 100644 --- a/database/gdb/gdb_type_result.go +++ b/database/gdb/gdb_type_result.go @@ -7,10 +7,12 @@ package gdb import ( + "database/sql" "math" "github.com/gogf/gf/v2/container/gvar" "github.com/gogf/gf/v2/encoding/gjson" + "github.com/gogf/gf/v2/internal/empty" "github.com/gogf/gf/v2/util/gconv" ) @@ -191,5 +193,12 @@ func (r Result) RecordKeyUint(key string) map[uint]Record { // Structs converts `r` to struct slice. // Note that the parameter `pointer` should be type of *[]struct/*[]*struct. func (r Result) Structs(pointer interface{}) (err error) { + // If the result is empty and the target pointer is not empty, it returns error. + if r.IsEmpty() { + if !empty.IsEmpty(pointer, true) { + return sql.ErrNoRows + } + return nil + } return gconv.StructsTag(r, pointer, OrmTagForStruct) } diff --git a/frame/g/g_func.go b/frame/g/g_func.go index 1725daceb..c7092b17f 100644 --- a/frame/g/g_func.go +++ b/frame/g/g_func.go @@ -80,7 +80,7 @@ func TryCatch(ctx context.Context, try func(ctx context.Context), catch ...func( // IsNil checks whether given `value` is nil. // Parameter `traceSource` is used for tracing to the source variable if given `value` is type -// of pinter that also points to a pointer. It returns nil if the source is nil when `traceSource` +// of pointer that also points to a pointer. It returns nil if the source is nil when `traceSource` // is true. // Note that it might use reflect feature which affects performance a little. func IsNil(value interface{}, traceSource ...bool) bool { @@ -90,8 +90,12 @@ func IsNil(value interface{}, traceSource ...bool) bool { // IsEmpty checks whether given `value` empty. // It returns true if `value` is in: 0, nil, false, "", len(slice/map/chan) == 0. // Or else it returns true. -func IsEmpty(value interface{}) bool { - return empty.IsEmpty(value) +// +// The parameter `traceSource` is used for tracing to the source variable if given `value` is type of pointer +// that also points to a pointer. It returns true if the source is empty when `traceSource` is true. +// Note that it might use reflect feature which affects performance a little. +func IsEmpty(value interface{}, traceSource ...bool) bool { + return empty.IsEmpty(value, traceSource...) } // RequestFromCtx retrieves and returns the Request object from context. diff --git a/frame/g/g_z_unit_test.go b/frame/g/g_z_unit_test.go index d0f71778b..80dc13b5d 100644 --- a/frame/g/g_z_unit_test.go +++ b/frame/g/g_z_unit_test.go @@ -1,3 +1,9 @@ +// Copyright GoFrame Author(https://goframe.org). All Rights Reserved. +// +// This Source Code Form is subject to the terms of the MIT License. +// If a copy of the MIT was not distributed with this file, +// You can obtain one at https://github.com/gogf/gf. + package g_test import ( diff --git a/internal/empty/empty.go b/internal/empty/empty.go index 4e42d1c94..07fee1e24 100644 --- a/internal/empty/empty.go +++ b/internal/empty/empty.go @@ -37,7 +37,11 @@ type iTime interface { // IsEmpty checks whether given `value` empty. // It returns true if `value` is in: 0, nil, false, "", len(slice/map/chan) == 0, // or else it returns false. -func IsEmpty(value interface{}) bool { +// +// The parameter `traceSource` is used for tracing to the source variable if given `value` is type of pointer +// that also points to a pointer. It returns true if the source is empty when `traceSource` is true. +// Note that it might use reflect feature which affects performance a little. +func IsEmpty(value interface{}, traceSource ...bool) bool { if value == nil { return true } @@ -88,38 +92,39 @@ func IsEmpty(value interface{}) bool { return len(result) == 0 default: - // ========================= - // Common interfaces checks. - // ========================= - if f, ok := value.(iTime); ok { - if f == (*time.Time)(nil) { - return true - } - return f.IsZero() - } - if f, ok := value.(iString); ok { - if f == nil { - return true - } - return f.String() == "" - } - if f, ok := value.(iInterfaces); ok { - if f == nil { - return true - } - return len(f.Interfaces()) == 0 - } - if f, ok := value.(iMapStrAny); ok { - if f == nil { - return true - } - return len(f.MapStrAny()) == 0 - } // Finally, using reflect. var rv reflect.Value if v, ok := value.(reflect.Value); ok { rv = v } else { + // ========================= + // Common interfaces checks. + // ========================= + if f, ok := value.(iTime); ok { + if f == (*time.Time)(nil) { + return true + } + return f.IsZero() + } + if f, ok := value.(iString); ok { + if f == nil { + return true + } + return f.String() == "" + } + if f, ok := value.(iInterfaces); ok { + if f == nil { + return true + } + return len(f.Interfaces()) == 0 + } + if f, ok := value.(iMapStrAny); ok { + if f == nil { + return true + } + return len(f.MapStrAny()) == 0 + } + rv = reflect.ValueOf(value) } @@ -169,21 +174,27 @@ func IsEmpty(value interface{}) bool { reflect.Array: return rv.Len() == 0 + case reflect.Ptr: + if len(traceSource) > 0 && traceSource[0] { + return IsEmpty(rv.Elem()) + } + return rv.IsNil() + case reflect.Func, - reflect.Ptr, reflect.Interface, reflect.UnsafePointer: - if rv.IsNil() { - return true - } + return rv.IsNil() + + case reflect.Invalid: + return true } } return false } // IsNil checks whether given `value` is nil, especially for interface{} type value. -// Parameter `traceSource` is used for tracing to the source variable if given `value` is type of pinter +// Parameter `traceSource` is used for tracing to the source variable if given `value` is type of pointer // that also points to a pointer. It returns nil if the source is nil when `traceSource` is true. // Note that it might use reflect feature which affects performance a little. func IsNil(value interface{}, traceSource ...bool) bool { diff --git a/os/gfpool/gfpool_pool.go b/os/gfpool/gfpool_pool.go index 6cee707e6..c8d9942a6 100644 --- a/os/gfpool/gfpool_pool.go +++ b/os/gfpool/gfpool_pool.go @@ -62,7 +62,7 @@ func newFilePool(p *Pool, path string, flag int, perm os.FileMode, ttl time.Dura // File retrieves file item from the file pointer pool and returns it. It creates one if // the file pointer pool is empty. // Note that it should be closed when it will never be used. When it's closed, it is not -// really closed the underlying file pointer but put back to the file pinter pool. +// really closed the underlying file pointer but put back to the file pointer pool. func (p *Pool) File() (*File, error) { if v, err := p.pool.Get(); err != nil { return nil, err