-
Notifications
You must be signed in to change notification settings - Fork 177
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.
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
tweaked scan method to work with most sql drivers // Added Must #44
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,56 @@ | ||||||
package ksuid | ||||||
|
||||||
import ( | ||||||
"database/sql/driver" | ||||||
"fmt" | ||||||
) | ||||||
|
||||||
// Scan implements the sql.Scanner interface. It supports converting from | ||||||
// string, []byte, or nil into a KSUID value. Attempting to convert from | ||||||
// another type will return an error. | ||||||
func (i *KSUID) Scan(src interface{}) error { | ||||||
switch src := src.(type) { | ||||||
case nil: | ||||||
return nil | ||||||
case string: | ||||||
// if an empty KSUID comes from a table, we return a null KSUID | ||||||
if src == "" { | ||||||
return nil | ||||||
} | ||||||
k, err := Parse(src) | ||||||
if err != nil { | ||||||
return fmt.Errorf("Scan: %v", err) | ||||||
} | ||||||
*i = k | ||||||
case []byte: | ||||||
// if an empty KSUID comes from a table, we return a null KSUID | ||||||
if len(src) == 0 { | ||||||
return nil | ||||||
} | ||||||
// assumes a simple slice of bytes if [byteLength] bytes | ||||||
if len(src) == byteLength { | ||||||
copy((*i)[:], src) | ||||||
return nil | ||||||
} | ||||||
|
||||||
if len(src) == stringEncodedLength { | ||||||
return i.Scan(string(src)) | ||||||
} | ||||||
|
||||||
return i.Scan(string(src)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These recursive calls to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @achille-roussel thanks for your time to review my request! I see your Point.
Suggested change
Using the UnmarshalText is better. Both functions (scan old, scan new) are now allocating the same. Beside that the new Scan is consuming 2,79% less memory + is slightly faster. |
||||||
|
||||||
default: | ||||||
return fmt.Errorf("Scan: unable to scan type %T into KSUID", src) | ||||||
} | ||||||
return nil | ||||||
} | ||||||
|
||||||
// Value implements sql.Valuer so that KSUIDs can be written to databases | ||||||
// transparently. Currently, KSUIDs map to strings. Please consult database-specific | ||||||
// driver documentation for matching types. | ||||||
func (i *KSUID) Value() (driver.Value, error) { | ||||||
if i == nil || i.IsNil() { | ||||||
return nil, nil | ||||||
} | ||||||
return i.String(), nil | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
package ksuid | ||
|
||
import ( | ||
"strings" | ||
"testing" | ||
) | ||
|
||
func TestScan(t *testing.T) { | ||
stringTest := "1al9byIH8Ze6OLkD5tZqmByJkSX" | ||
badTypeTest := 6 | ||
invalidTest := "1al9byIH8Ze6OLkD5tZqmByJk" | ||
|
||
byteTest := make([]byte, byteLength) | ||
byteTestKSUID := Must(Parse(stringTest)) | ||
copy(byteTest, byteTestKSUID[:]) | ||
textTest := []byte(stringTest) | ||
|
||
// valid tests | ||
|
||
var ksuid KSUID | ||
err := (&ksuid).Scan(stringTest) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
err = (&ksuid).Scan([]byte(stringTest)) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
err = (&ksuid).Scan(byteTest) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
err = (&ksuid).Scan(textTest) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// bad type tests | ||
|
||
err = (&ksuid).Scan(badTypeTest) | ||
if err == nil { | ||
t.Error("int correctly parsed and shouldn't have") | ||
} | ||
if !strings.Contains(err.Error(), "unable to scan type") { | ||
t.Error("attempting to parse an int returned an incorrect error message") | ||
} | ||
|
||
// invalid/incomplete ksuids | ||
err = (&ksuid).Scan(invalidTest) | ||
if err == nil { | ||
t.Error("invalid uuid was parsed without error") | ||
} | ||
if !strings.Contains(err.Error(), "Valid encoded KSUIDs") { | ||
t.Error("attempting to parse an invalid KSUID returned an incorrect error message") | ||
} | ||
|
||
err = (&ksuid).Scan(byteTest[:len(byteTest)-2]) | ||
if err == nil { | ||
t.Error("invalid byte ksuid was parsed without error") | ||
} | ||
if !strings.Contains(err.Error(), "Valid encoded KSUIDs") { | ||
t.Error("attempting to parse an invalid byte KSUID returned an incorrect error message") | ||
} | ||
|
||
// empty tests | ||
|
||
ksuid = KSUID{} | ||
var emptySlice []byte | ||
err = (&ksuid).Scan(emptySlice) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
for _, v := range ksuid { | ||
if v != 0 { | ||
t.Error("KSUID was not nil after scanning empty byte slice") | ||
} | ||
} | ||
|
||
ksuid = KSUID{} | ||
var emptyString string | ||
err = (&ksuid).Scan(emptyString) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
for _, v := range ksuid { | ||
if v != 0 { | ||
t.Error("KSUID was not nil after scanning empty string") | ||
} | ||
} | ||
|
||
ksuid = KSUID{} | ||
err = (&ksuid).Scan(nil) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
for _, v := range ksuid { | ||
if v != 0 { | ||
t.Error("KSUID was not nil after scanning nil") | ||
} | ||
} | ||
} | ||
|
||
func TestValue(t *testing.T) { | ||
stringTest := "1al9byIH8Ze6OLkD5tZqmByJkSX" | ||
ksuid := Must(Parse(stringTest)) | ||
val, _ := ksuid.Value() | ||
if val != stringTest { | ||
t.Error("Value() did ot return expected string") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this function seems unrelated to the modification of
Scan
itself, maybe we can split it in a separate PR?