From c1f08b2bb1f4057573c1d03ee998cb87fbc9e159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 11:23:49 +0200 Subject: [PATCH 1/6] tests/NullInt.Scan: fix integer conversion from int{8,16,32,64} --- stubs_test.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/stubs_test.go b/stubs_test.go index d3c6003..1515adb 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -2,6 +2,7 @@ package sqlmock import ( "database/sql/driver" + "errors" "fmt" "strconv" "time" @@ -25,8 +26,28 @@ func (ni *NullInt) Scan(value interface{}) (err error) { } switch v := value.(type) { - case int, int8, int16, int32, int64: - ni.Integer, ni.Valid = v.(int), true + case int: + ni.Integer, ni.Valid = v, true + return + case int8: + ni.Integer, ni.Valid = int(v), true + return + case int16: + ni.Integer, ni.Valid = int(v), true + return + case int32: + ni.Integer, ni.Valid = int(v), true + return + case int64: + const maxUint = ^uint(0) + const minUint = 0 + const maxInt = int(maxUint >> 1) + const minInt = -maxInt - 1 + + if v > int64(maxInt) || v < int64(minInt) { + return errors.New("value out of int range") + } + ni.Integer, ni.Valid = int(v), true return case []byte: ni.Integer, err = strconv.Atoi(string(v)) From 4f574cddfd1f742f3fc9543c3ffa0a5a868dfadc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 11:47:20 +0200 Subject: [PATCH 2/6] tests/NullInt: fix Scan from string/[]byte Do not change the value if Atoi fails --- stubs_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/stubs_test.go b/stubs_test.go index 1515adb..724a2c7 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -50,13 +50,19 @@ func (ni *NullInt) Scan(value interface{}) (err error) { ni.Integer, ni.Valid = int(v), true return case []byte: - ni.Integer, err = strconv.Atoi(string(v)) - ni.Valid = (err == nil) - return + n, err := strconv.Atoi(string(v)) + if err != nil { + return err + } + ni.Integer, ni.Valid = n, true + return nil case string: - ni.Integer, err = strconv.Atoi(v) - ni.Valid = (err == nil) - return + n, err := strconv.Atoi(v) + if err != nil { + return err + } + ni.Integer, ni.Valid = n, true + return nil } ni.Valid = false From f974ac3c0c7a862ef0599fcf1d4985ed5bea4bc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 11:49:33 +0200 Subject: [PATCH 3/6] tests/NullInt: fix Value() to return a valid driver.Value Value must return int64, not int. --- stubs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stubs_test.go b/stubs_test.go index 724a2c7..71a8ce3 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -74,7 +74,7 @@ func (ni NullInt) Value() (driver.Value, error) { if !ni.Valid { return nil, nil } - return ni.Integer, nil + return int64(ni.Integer), nil } // Satisfy sql.Scanner interface From 9119b1dbff6636b98fe77eb0a7cec39434aa35ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 11:55:17 +0200 Subject: [PATCH 4/6] tests/Null{Int,Time}: refactor Scan for less lines of code Single switch for all cases. Single return for the success case. --- stubs_test.go | 40 ++++++++++++++-------------------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/stubs_test.go b/stubs_test.go index 71a8ce3..ebbb749 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -19,25 +19,18 @@ type NullInt struct { } // Satisfy sql.Scanner interface -func (ni *NullInt) Scan(value interface{}) (err error) { - if value == nil { - ni.Integer, ni.Valid = 0, false - return - } - +func (ni *NullInt) Scan(value interface{}) error { switch v := value.(type) { + case nil: + ni.Integer, ni.Valid = 0, false case int: ni.Integer, ni.Valid = v, true - return case int8: ni.Integer, ni.Valid = int(v), true - return case int16: ni.Integer, ni.Valid = int(v), true - return case int32: ni.Integer, ni.Valid = int(v), true - return case int64: const maxUint = ^uint(0) const minUint = 0 @@ -48,25 +41,23 @@ func (ni *NullInt) Scan(value interface{}) (err error) { return errors.New("value out of int range") } ni.Integer, ni.Valid = int(v), true - return case []byte: n, err := strconv.Atoi(string(v)) if err != nil { return err } ni.Integer, ni.Valid = n, true - return nil case string: n, err := strconv.Atoi(v) if err != nil { return err } ni.Integer, ni.Valid = n, true - return nil + default: + ni.Valid = false + return fmt.Errorf("Can't convert %T to integer", value) } - - ni.Valid = false - return fmt.Errorf("Can't convert %T to integer", value) + return nil } // Satisfy sql.Valuer interface. @@ -78,20 +69,17 @@ func (ni NullInt) Value() (driver.Value, error) { } // Satisfy sql.Scanner interface -func (nt *NullTime) Scan(value interface{}) (err error) { - if value == nil { - nt.Time, nt.Valid = time.Time{}, false - return - } - +func (nt *NullTime) Scan(value interface{}) error { switch v := value.(type) { + case nil: + nt.Time, nt.Valid = time.Time{}, false case time.Time: nt.Time, nt.Valid = v, true - return + default: + nt.Valid = false + return fmt.Errorf("Can't convert %T to time.Time", value) } - - nt.Valid = false - return fmt.Errorf("Can't convert %T to time.Time", value) + return nil } // Satisfy sql.Valuer interface. From 87bbc720504ef970a5d2aa24c44c1a4336550e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 11:58:55 +0200 Subject: [PATCH 5/6] tests/Null{Int,Time}: do not change the value in case of error Do not touch the existing value in case of error. No capital on error message. --- stubs_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/stubs_test.go b/stubs_test.go index ebbb749..410d76c 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -54,8 +54,7 @@ func (ni *NullInt) Scan(value interface{}) error { } ni.Integer, ni.Valid = n, true default: - ni.Valid = false - return fmt.Errorf("Can't convert %T to integer", value) + return fmt.Errorf("can't convert %T to integer", value) } return nil } @@ -76,8 +75,7 @@ func (nt *NullTime) Scan(value interface{}) error { case time.Time: nt.Time, nt.Valid = v, true default: - nt.Valid = false - return fmt.Errorf("Can't convert %T to time.Time", value) + return fmt.Errorf("can't convert %T to time.Time", value) } return nil } From 5c0fec018b2a4ce97e510dfc735cf28ec085326b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Thu, 12 Jul 2018 13:57:47 +0200 Subject: [PATCH 6/6] tests/NullInt.Scan: add FIXME about removing int{,8,16,32} input support The input of Scan is a driver.Value. Which means the only integer type on input is a int64. But NullInt.Scan also handles int/int8/int16/int32 while that should not be necessary. Unfortunately, sqlmock doesn't enforce strict driver.Value in its implementation and the testsuite (that uses NullInt) relies on this bug. So, this patch just adds a FIXME until sqlmock internals and testsuite are fixed (will maybe require breaking the API). --- stubs_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/stubs_test.go b/stubs_test.go index 410d76c..fcd8b29 100644 --- a/stubs_test.go +++ b/stubs_test.go @@ -23,6 +23,11 @@ func (ni *NullInt) Scan(value interface{}) error { switch v := value.(type) { case nil: ni.Integer, ni.Valid = 0, false + + // FIXME int, int8, int16, int32 types are handled here but that should not + // be necessary: only int64 is a driver.Value + // Unfortunately, the sqlmock testsuite currently relies on that because + // sqlmock doesn't properly limits itself internally to pure driver.Value. case int: ni.Integer, ni.Valid = v, true case int8: @@ -31,6 +36,7 @@ func (ni *NullInt) Scan(value interface{}) error { ni.Integer, ni.Valid = int(v), true case int32: ni.Integer, ni.Valid = int(v), true + case int64: const maxUint = ^uint(0) const minUint = 0