From a8ba66482a4663f67a7caa0ddc94e2b12d01e51e Mon Sep 17 00:00:00 2001 From: Ian Chiles Date: Wed, 10 May 2017 14:37:47 +0200 Subject: [PATCH 1/2] add hash:"string" support using fmt.Stringer --- README.md | 3 +++ hashstructure.go | 29 +++++++++++++++++++++++------ hashstructure_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 619a7fa..28ce45a 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ sending data across the network, caching values locally (de-dup), and so on. * Optionally specify a custom hash function to optimize for speed, collision avoidance for your data set, etc. + + * Optionally hash the output of `.String()` on structs that implement fmt.Stringer, + allowing effective hashing of time.Time ## Installation diff --git a/hashstructure.go b/hashstructure.go index 1800f43..9f36d11 100644 --- a/hashstructure.go +++ b/hashstructure.go @@ -2,12 +2,17 @@ package hashstructure import ( "encoding/binary" + "errors" "fmt" "hash" "hash/fnv" "reflect" ) +var ( + ErrNotStringer = errors.New("field tag set to string, but struct does not implement fmt.Stringer") +) + // HashOptions are options that are available for hashing. type HashOptions struct { // Hasher is the hash function to use. If this isn't set, it will @@ -27,8 +32,8 @@ type HashOptions struct { // // If opts is nil, then default options will be used. See HashOptions // for the default values. The same *HashOptions value cannot be used -// concurrently. None of the values within a *HashOptions struct are -// safe to read/write while hashing is being done. +// concurrently. None of the values within a *HashOptions struct are +// safe to read/write while hashing is being done. // // Notes on the value: // @@ -52,6 +57,9 @@ type HashOptions struct { // * "set" - The field will be treated as a set, where ordering doesn't // affect the hash code. This only works for slices. // +// * "string" - The field will be hashed as a string, only works when the +// field implements fmt.Stringer +// func Hash(v interface{}, opts *HashOptions) (uint64, error) { // Create default options if opts == nil { @@ -201,8 +209,8 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) { return h, nil case reflect.Struct: - var include Includable parent := v.Interface() + var include Includable if impl, ok := parent.(Includable); ok { include = impl } @@ -215,7 +223,7 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) { l := v.NumField() for i := 0; i < l; i++ { - if v := v.Field(i); v.CanSet() || t.Field(i).Name != "_" { + if innerV := v.Field(i); v.CanSet() || t.Field(i).Name != "_" { var f visitFlag fieldType := t.Field(i) if fieldType.PkgPath != "" { @@ -229,9 +237,18 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) { continue } + // if string is set, use the string value + if tag == "string" { + if impl, ok := innerV.Interface().(fmt.Stringer); ok { + innerV = reflect.ValueOf(impl.String()) + } else { + return 0, ErrNotStringer + } + } + // Check if we implement includable and check it if include != nil { - incl, err := include.HashInclude(fieldType.Name, v) + incl, err := include.HashInclude(fieldType.Name, innerV) if err != nil { return 0, err } @@ -250,7 +267,7 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) { return 0, err } - vh, err := w.visit(v, &visitOpts{ + vh, err := w.visit(innerV, &visitOpts{ Flags: f, Struct: parent, StructField: fieldType.Name, diff --git a/hashstructure_test.go b/hashstructure_test.go index 29edcac..64506d9 100644 --- a/hashstructure_test.go +++ b/hashstructure_test.go @@ -3,6 +3,7 @@ package hashstructure import ( "fmt" "testing" + "time" ) func TestHash_identity(t *testing.T) { @@ -195,6 +196,17 @@ func TestHash_equalIgnore(t *testing.T) { UUID string `hash:"-"` } + type TestTime struct { + Name string + Time time.Time `hash:"string"` + } + + type TestTime2 struct { + Name string + Time time.Time + } + + now := time.Now() cases := []struct { One, Two interface{} Match bool @@ -222,6 +234,21 @@ func TestHash_equalIgnore(t *testing.T) { Test2{Name: "foo", UUID: "foo"}, true, }, + { + TestTime{Name: "foo", Time: now}, + TestTime{Name: "foo", Time: time.Time{}}, + false, + }, + { + TestTime{Name: "foo", Time: now}, + TestTime{Name: "foo", Time: now}, + true, + }, + { + TestTime2{Name: "foo", Time: now}, + TestTime2{Name: "foo", Time: time.Time{}}, + true, + }, } for _, tc := range cases { From fe40ba3ce75821cf5926988980b659de3cc73b09 Mon Sep 17 00:00:00 2001 From: Ian Chiles Date: Thu, 11 May 2017 10:19:05 +0200 Subject: [PATCH 2/2] say what field is broken for hash:"string" --- hashstructure.go | 17 ++++++++++----- hashstructure_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/hashstructure.go b/hashstructure.go index 9f36d11..76dbea0 100644 --- a/hashstructure.go +++ b/hashstructure.go @@ -2,16 +2,21 @@ package hashstructure import ( "encoding/binary" - "errors" "fmt" "hash" "hash/fnv" "reflect" ) -var ( - ErrNotStringer = errors.New("field tag set to string, but struct does not implement fmt.Stringer") -) +// ErrNotStringer is returned when there's an error with hash:"string" +type ErrNotStringer struct { + Field string +} + +// Error implements error for ErrNotStringer +func (ens *ErrNotStringer) Error() string { + return fmt.Sprintf("hashstructure: %s has hash:\"string\" set, but does not implement fmt.Stringer", ens.Field) +} // HashOptions are options that are available for hashing. type HashOptions struct { @@ -242,7 +247,9 @@ func (w *walker) visit(v reflect.Value, opts *visitOpts) (uint64, error) { if impl, ok := innerV.Interface().(fmt.Stringer); ok { innerV = reflect.ValueOf(impl.String()) } else { - return 0, ErrNotStringer + return 0, &ErrNotStringer{ + Field: v.Type().Field(i).Name, + } } } diff --git a/hashstructure_test.go b/hashstructure_test.go index 64506d9..13ab834 100644 --- a/hashstructure_test.go +++ b/hashstructure_test.go @@ -273,6 +273,54 @@ func TestHash_equalIgnore(t *testing.T) { } } +func TestHash_stringTagError(t *testing.T) { + type Test1 struct { + Name string + BrokenField string `hash:"string"` + } + + type Test2 struct { + Name string + BustedField int `hash:"string"` + } + + type Test3 struct { + Name string + Time time.Time `hash:"string"` + } + + cases := []struct { + Test interface{} + Field string + }{ + { + Test1{Name: "foo", BrokenField: "bar"}, + "BrokenField", + }, + { + Test2{Name: "foo", BustedField: 23}, + "BustedField", + }, + { + Test3{Name: "foo", Time: time.Now()}, + "", + }, + } + + for _, tc := range cases { + _, err := Hash(tc.Test, nil) + if err != nil { + if ens, ok := err.(*ErrNotStringer); ok { + if ens.Field != tc.Field { + t.Fatalf("did not get expected field %#v: got %s wanted %s", tc.Test, ens.Field, tc.Field) + } + } else { + t.Fatalf("unknown error %#v: got %s", tc, err) + } + } + } +} + func TestHash_equalNil(t *testing.T) { type Test struct { Str *string