From 053ac9610a0ec8f8ad666d1d54ad922628518e46 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Mon, 24 Jan 2022 16:18:06 +0100 Subject: [PATCH 01/13] Add StringMap[Un]Marshaler interfaces --- marshal.go | 8 +++++++- unmarshal.go | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/marshal.go b/marshal.go index 54c3bd2..4f7aa77 100644 --- a/marshal.go +++ b/marshal.go @@ -9,6 +9,11 @@ import ( const inlineSep = "." +// StringMapMarshaler is the interface implemented by types that can marshal themselves into a map of strings. +type StringMapMarshaler interface { + MarshalStringMap() (map[string]string, error) +} + // Marshal returns the map[string]string representation of v, which must be a struct, // by reading every exported field and translating it into a (key, value) pair to be // added to the resulting map. Interfaces or pointers to struct are also accepted. @@ -149,6 +154,7 @@ func fieldToString(val reflect.Value) (string, error) { } var ( - textMarshalerType = reflect.TypeOf(new(encoding.TextMarshaler)).Elem() + mapMarshalerType = reflect.TypeOf(new(StringMapMarshaler)).Elem() stringerType = reflect.TypeOf(new(fmt.Stringer)).Elem() + textMarshalerType = reflect.TypeOf(new(encoding.TextMarshaler)).Elem() ) diff --git a/unmarshal.go b/unmarshal.go index 80cb83d..9b90eec 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -7,6 +7,12 @@ import ( "strconv" ) +// StringMapUnmarshaler is the interface implemented by types that can unmarshal themselves +// from a map of strings. Implementations must copy the given map if they wish to modify it. +type StringMapUnmarshaler interface { + UnmarshalStringMap(map[string]string) error +} + // Unmarshal sets v's fields according to its map representation contained by data. // v must be a pointer to struct or an interface. Neither data nor v can be nil. // @@ -176,5 +182,6 @@ func stringToField(str string, field reflect.Value, omitempty bool) error { } var ( + mapUnmarshalerType = reflect.TypeOf(new(StringMapUnmarshaler)).Elem() textUnmarshalerType = reflect.TypeOf(new(encoding.TextUnmarshaler)).Elem() ) From 036b440f87bf60d3f0bd0885ee9a36803e8872be Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Mon, 24 Jan 2022 21:28:41 +0100 Subject: [PATCH 02/13] Unexport types from tests --- marshal_test.go | 30 +++++++++++++++--------------- unmarshal_test.go | 18 +++++++++--------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/marshal_test.go b/marshal_test.go index 342817a..600900e 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -15,24 +15,24 @@ const ( textMarshalerOut = "stubtext" ) -type StubStringer struct{} +type stubStringer struct{} -func (s StubStringer) String() string { return stringerOut } +func (s stubStringer) String() string { return stringerOut } -// StubIntStringer implements fmt.Stringer but doesn't rely on an +// stubIntStringer implements fmt.Stringer but doesn't rely on an // underlying struct. Useful to test whether we can detect // interfaces independently from their underlying type. -type StubIntStringer int +type stubIntStringer int -func (s StubIntStringer) String() string { return stringerOut } +func (s stubIntStringer) String() string { return stringerOut } -type StubTextMarshaler struct{} +type stubTextMarshaler struct{} -func (s StubTextMarshaler) MarshalText() ([]byte, error) { return []byte(textMarshalerOut), nil } +func (s stubTextMarshaler) MarshalText() ([]byte, error) { return []byte(textMarshalerOut), nil } func TestMarshalValidType(t *testing.T) { var ( - stub StubStringer = StubStringer{} + stub stubStringer = stubStringer{} ifac fmt.Stringer = stub ) types := []interface{}{ @@ -50,7 +50,7 @@ func TestMarshalValidType(t *testing.T) { func TestMarshalNil(t *testing.T) { var ( - stub *StubStringer = nil + stub *stubStringer = nil ifac fmt.Stringer = stub ) nils := []interface{}{nil, stub, ifac} @@ -87,14 +87,14 @@ func TestMarshalScalars(t *testing.T) { {In: struct{ V string }{"str"}, Out: map[string]string{"V": "str"}}, // // Marshal interfaces by passing the real value. - {In: struct{ V StubStringer }{StubStringer{}}, Out: map[string]string{"V": stringerOut}}, - {In: struct{ V StubIntStringer }{StubIntStringer(100)}, Out: map[string]string{"V": stringerOut}}, - {In: struct{ V StubTextMarshaler }{StubTextMarshaler{}}, Out: map[string]string{"V": textMarshalerOut}}, + {In: struct{ V stubStringer }{stubStringer{}}, Out: map[string]string{"V": stringerOut}}, + {In: struct{ V stubIntStringer }{stubIntStringer(100)}, Out: map[string]string{"V": stringerOut}}, + {In: struct{ V stubTextMarshaler }{stubTextMarshaler{}}, Out: map[string]string{"V": textMarshalerOut}}, // Marshal interfaces by interfaces. - {In: struct{ V fmt.Stringer }{StubStringer{}}, Out: map[string]string{"V": stringerOut}}, - {In: struct{ V fmt.Stringer }{StubIntStringer(100)}, Out: map[string]string{"V": stringerOut}}, - {In: struct{ V encoding.TextMarshaler }{StubTextMarshaler{}}, Out: map[string]string{"V": textMarshalerOut}}, + {In: struct{ V fmt.Stringer }{stubStringer{}}, Out: map[string]string{"V": stringerOut}}, + {In: struct{ V fmt.Stringer }{stubIntStringer(100)}, Out: map[string]string{"V": stringerOut}}, + {In: struct{ V encoding.TextMarshaler }{stubTextMarshaler{}}, Out: map[string]string{"V": textMarshalerOut}}, } for _, test := range tests { out, err := redmap.Marshal(test.In) diff --git a/unmarshal_test.go b/unmarshal_test.go index e594172..33c0e4c 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -10,9 +10,9 @@ import ( "github.com/livingsilver94/redmap" ) -type StubTextUnmarshaler struct{ S string } +type stubTextUnmarshaler struct{ S string } -func (s *StubTextUnmarshaler) UnmarshalText(text []byte) error { +func (s *stubTextUnmarshaler) UnmarshalText(text []byte) error { s.S = string(text) return nil } @@ -35,8 +35,8 @@ var emptyMap = make(map[string]string) func TestUnmarshalValidType(t *testing.T) { tests := []interface{}{ - StubStringer{}, - &StubStringer{}, + stubStringer{}, + &stubStringer{}, } for _, test := range tests { val := reflect.New(reflect.TypeOf(test)) @@ -48,7 +48,7 @@ func TestUnmarshalValidType(t *testing.T) { } func TestUnmarshalNil(t *testing.T) { - err := redmap.Unmarshal(nil, &StubStringer{}) + err := redmap.Unmarshal(nil, &stubStringer{}) if !errors.Is(err, redmap.ErrNilValue) { t.Fatal("Unmarshal() with a nil map did not return the specific error") } @@ -56,7 +56,7 @@ func TestUnmarshalNil(t *testing.T) { if !errors.Is(err, redmap.ErrNilValue) { t.Fatal("Unmarshal() with invalid target did not return the specific error") } - var nilPtr *StubStringer + var nilPtr *stubStringer err = redmap.Unmarshal(emptyMap, nilPtr) if !errors.Is(err, redmap.ErrNilValue) { t.Fatal("Unmarshal() with nil pointer did not return the specific error") @@ -82,7 +82,7 @@ func TestUnmarshalInvalidType(t *testing.T) { { // Interface is not a struct. val: func() reflect.Value { - v := fmt.Stringer(StubStringer{}) + v := fmt.Stringer(stubStringer{}) return reflect.ValueOf(&v) }, expErr: redmap.ErrNotStruct}, @@ -116,7 +116,7 @@ func TestUnmarshalScalars(t *testing.T) { {In: map[string]string{"V": "(100.1+80.1i)"}, Out: struct{ V complex64 }{100.1 + 80.1i}}, {In: map[string]string{"V": "(100.1+80.1i)"}, Out: struct{ V complex128 }{100.1 + 80.1i}}, {In: map[string]string{"V": "str"}, Out: struct{ V string }{"str"}}, - {In: map[string]string{"V": "a test"}, Out: struct{ V StubTextUnmarshaler }{StubTextUnmarshaler{S: "a test"}}}, + {In: map[string]string{"V": "a test"}, Out: struct{ V stubTextUnmarshaler }{stubTextUnmarshaler{S: "a test"}}}, {In: map[string]string{"V": "100"}, Out: struct{ V StubIntUnmarshaler }{StubIntUnmarshaler(100)}}, } for _, test := range tests { @@ -185,7 +185,7 @@ func TestUnarshalUnexported(t *testing.T) { }{Exp: "atest"}}, {Out: struct { Exp string - unexp StubTextUnmarshaler + unexp stubTextUnmarshaler }{Exp: "atest"}}, } for _, test := range tests { From d84e0bf3550411937f2adc01eef0ad6a8d305aa9 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Mon, 24 Jan 2022 21:38:01 +0100 Subject: [PATCH 03/13] Document types and variables in tests --- marshal_test.go | 11 ++++++----- unmarshal_test.go | 7 ++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/marshal_test.go b/marshal_test.go index 600900e..efb37e8 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -11,21 +11,22 @@ import ( ) const ( - stringerOut = "stub" - textMarshalerOut = "stubtext" + stringerOut = "stub" // stringerOut is the output of fmt.Stringer implementations. + textMarshalerOut = "stubtext" // textMarshalerOut is the output of encoding.TextMarshaler implementations. ) +// stubStringer implements the fmt.Stringer interface. type stubStringer struct{} func (s stubStringer) String() string { return stringerOut } -// stubIntStringer implements fmt.Stringer but doesn't rely on an -// underlying struct. Useful to test whether we can detect -// interfaces independently from their underlying type. +// stubIntStringer is an int that implements fmt.Stringer, +// so that we can test if a non-struct type is correctly handled as an interface. type stubIntStringer int func (s stubIntStringer) String() string { return stringerOut } +// stubTextMarshaler implements the encoding.TextMarshaler interface. type stubTextMarshaler struct{} func (s stubTextMarshaler) MarshalText() ([]byte, error) { return []byte(textMarshalerOut), nil } diff --git a/unmarshal_test.go b/unmarshal_test.go index 33c0e4c..1f79d33 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -10,6 +10,7 @@ import ( "github.com/livingsilver94/redmap" ) +// stubTextUnmarshaler implements the encoding.TextUnmarshaler interface. type stubTextUnmarshaler struct{ S string } func (s *stubTextUnmarshaler) UnmarshalText(text []byte) error { @@ -17,9 +18,8 @@ func (s *stubTextUnmarshaler) UnmarshalText(text []byte) error { return nil } -// StubIntUnmarshaler implements encoding.TextUnmarshaler but doesn't rely on an -// underlying struct. Useful to test whether we can detect -// interfaces independently from their underlying type. +// StubIntUnmarshaler is an int that implements encoding.TextUnmarshaler, +// so that we can test if a non-struct type is correctly handled as an interface. type StubIntUnmarshaler int func (s *StubIntUnmarshaler) UnmarshalText(text []byte) error { @@ -31,6 +31,7 @@ func (s *StubIntUnmarshaler) UnmarshalText(text []byte) error { return nil } +// emptyMap is a non-nil, zero-length map. var emptyMap = make(map[string]string) func TestUnmarshalValidType(t *testing.T) { From 1b59aa0a3b4ce005490571dd775ecad81dfc1fc7 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Mon, 24 Jan 2022 21:58:53 +0100 Subject: [PATCH 04/13] Marshal structs implementing StringMapMarshaler --- marshal.go | 14 ++++++++++++ marshal_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/marshal.go b/marshal.go index 4f7aa77..6242e2d 100644 --- a/marshal.go +++ b/marshal.go @@ -83,6 +83,9 @@ func structValue(v interface{}) (reflect.Value, error) { // name in case of an inlined inner struct. func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) error { typ := stru.Type() + if typ.Implements(mapMarshalerType) { + return structToMap(mp, prefix, stru) + } for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) if field.PkgPath != "" { @@ -122,6 +125,17 @@ func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) e return nil } +func structToMap(mp map[string]string, prefix string, stru reflect.Value) error { + conv, err := stru.Interface().(StringMapMarshaler).MarshalStringMap() + if err != nil { + return err + } + for k, v := range conv { + mp[prefix+k] = v + } + return nil +} + func fieldToString(val reflect.Value) (string, error) { typ := val.Type() if typ.Implements(textMarshalerType) { diff --git a/marshal_test.go b/marshal_test.go index efb37e8..d02044c 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -15,6 +15,11 @@ const ( textMarshalerOut = "stubtext" // textMarshalerOut is the output of encoding.TextMarshaler implementations. ) +var ( + // mapMarshalerOut is the output of redmap.StringMapMarshaler implementations. + mapMarshalerOut = map[string]string{"field1": "value1", "field2": "value2"} +) + // stubStringer implements the fmt.Stringer interface. type stubStringer struct{} @@ -31,6 +36,21 @@ type stubTextMarshaler struct{} func (s stubTextMarshaler) MarshalText() ([]byte, error) { return []byte(textMarshalerOut), nil } +// stubMapMarshaler implements the redmap.StringMapMarshaler interface. +type stubMapMarshaler struct{} + +func (s stubMapMarshaler) MarshalStringMap() (map[string]string, error) { + return mapMarshalerOut, nil +} + +// stubIntMapMarshaler is an int that implements redmap.StringMapMarshaler, +// so that we can test if a non-struct type is correctly handled as an interface. +type stubIntMapMarshaler struct{} + +func (s stubIntMapMarshaler) MarshalStringMap() (map[string]string, error) { + return mapMarshalerOut, nil +} + func TestMarshalValidType(t *testing.T) { var ( stub stubStringer = stubStringer{} @@ -190,3 +210,44 @@ func TestMarshalWithTags(t *testing.T) { t.Fatalf("Marshal's output doesn't respect struct tags\n\tExpected: %v\n\tOut: %v", expected, out) } } + +func TestMapMarshaler(t *testing.T) { + tests := []struct { + In redmap.StringMapMarshaler + Out map[string]string + }{ + {In: stubMapMarshaler{}, Out: mapMarshalerOut}, + {In: stubIntMapMarshaler{}, Out: mapMarshalerOut}, + } + for _, test := range tests { + out, err := redmap.Marshal(test.In) + if err != nil { + t.Fatalf("Marshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(out, test.Out) { + t.Fatalf("Marshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", test.In, test.Out, out) + } + } +} + +func TestInnerMapMarshaler(t *testing.T) { + stru := struct { + RegularField string + Struct stubMapMarshaler `redmap:",inline"` + }{ + RegularField: "regular", + Struct: stubMapMarshaler{}, + } + expected := map[string]string{"RegularField": "regular"} + for k, v := range mapMarshalerOut { + expected["Struct."+k] = v + } + + out, err := redmap.Marshal(stru) + if err != nil { + t.Fatalf("Marshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(out, expected) { + t.Fatalf("Marshal's output doesn't respect struct tags\n\tExpected: %v\n\tOut: %v", expected, out) + } +} From b7d981dfa5ee2226aef9ebbb238e6712bc34ba75 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 10:33:03 +0100 Subject: [PATCH 05/13] Fix StringMapMarshaler for non-struct types --- marshal.go | 5 ++++- marshal_test.go | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/marshal.go b/marshal.go index 6242e2d..087c94d 100644 --- a/marshal.go +++ b/marshal.go @@ -73,7 +73,10 @@ func structValue(v interface{}) (reflect.Value, error) { case reflect.Invalid: return reflect.Value{}, ErrNilValue default: - return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + if !val.Type().Implements(mapMarshalerType) { + return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + } + return val, nil } } diff --git a/marshal_test.go b/marshal_test.go index d02044c..90ee175 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -45,7 +45,7 @@ func (s stubMapMarshaler) MarshalStringMap() (map[string]string, error) { // stubIntMapMarshaler is an int that implements redmap.StringMapMarshaler, // so that we can test if a non-struct type is correctly handled as an interface. -type stubIntMapMarshaler struct{} +type stubIntMapMarshaler int func (s stubIntMapMarshaler) MarshalStringMap() (map[string]string, error) { return mapMarshalerOut, nil @@ -217,7 +217,7 @@ func TestMapMarshaler(t *testing.T) { Out map[string]string }{ {In: stubMapMarshaler{}, Out: mapMarshalerOut}, - {In: stubIntMapMarshaler{}, Out: mapMarshalerOut}, + {In: stubIntMapMarshaler(666), Out: mapMarshalerOut}, } for _, test := range tests { out, err := redmap.Marshal(test.In) From 9c791298997938ca65603d80616abe3972ec2229 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 11:31:58 +0100 Subject: [PATCH 06/13] Marshal types implementing StringMapUnmarshaler --- unmarshal.go | 30 +++++++++++++++--- unmarshal_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 9 deletions(-) diff --git a/unmarshal.go b/unmarshal.go index 9b90eec..5168685 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strconv" + "strings" ) // StringMapUnmarshaler is the interface implemented by types that can unmarshal themselves @@ -55,11 +56,17 @@ func ptrStructValue(v interface{}) (reflect.Value, error) { case reflect.Invalid: return reflect.Value{}, errIs(reflect.TypeOf(v), ErrNilValue) default: - return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + if !val.Addr().Type().Implements(mapUnmarshalerType) { + return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + } + return val, nil } } -func unmarshalRecursive(data map[string]string, prefix string, stru reflect.Value) error { +func unmarshalRecursive(mp map[string]string, prefix string, stru reflect.Value) error { + if ptr := stru.Addr(); ptr.Type().Implements(mapUnmarshalerType) { + return mapToStruct(mp, prefix, ptr) + } typ := stru.Type() for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) @@ -92,12 +99,12 @@ func unmarshalRecursive(data map[string]string, prefix string, stru reflect.Valu if kind := value.Kind(); kind != reflect.Struct { return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNotStruct)) } - err := unmarshalRecursive(data, tags.name+inlineSep, value) + err := unmarshalRecursive(mp, tags.name+inlineSep, value) if err != nil { return err } } else { - str, ok := data[tags.name] + str, ok := mp[tags.name] if !ok { continue } @@ -110,6 +117,21 @@ func unmarshalRecursive(data map[string]string, prefix string, stru reflect.Valu return nil } +func mapToStruct(mp map[string]string, prefix string, stru reflect.Value) error { + if prefix != "" { + // FIXME: Creating a submap is O(n). Can we think of a better algorithm? + subMP := make(map[string]string, len(mp)) + for k, v := range mp { + if !strings.HasPrefix(k, prefix) { + continue + } + subMP[k[len(prefix):]] = v + } + mp = subMP + } + return stru.Interface().(StringMapUnmarshaler).UnmarshalStringMap(mp) +} + func stringToField(str string, field reflect.Value, omitempty bool) error { addr := field.Addr() // Unmarshaling always requires a pointer receiver. if addr.Type().Implements(textUnmarshalerType) { diff --git a/unmarshal_test.go b/unmarshal_test.go index 1f79d33..aff510d 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -18,16 +18,41 @@ func (s *stubTextUnmarshaler) UnmarshalText(text []byte) error { return nil } -// StubIntUnmarshaler is an int that implements encoding.TextUnmarshaler, +// stubIntTextUnmarshaler is an int that implements encoding.TextUnmarshaler, // so that we can test if a non-struct type is correctly handled as an interface. -type StubIntUnmarshaler int +type stubIntTextUnmarshaler int -func (s *StubIntUnmarshaler) UnmarshalText(text []byte) error { +func (s *stubIntTextUnmarshaler) UnmarshalText(text []byte) error { v, err := strconv.Atoi(string(text)) if err != nil { return err } - *s = StubIntUnmarshaler(v) + *s = stubIntTextUnmarshaler(v) + return nil +} + +// stubMapMarshaler implements the redmap.StringMapUnmarshaler interface. +type stubMapUnmarshaler struct { + Field1 string + Field2 string +} + +func (s *stubMapUnmarshaler) UnmarshalStringMap(mp map[string]string) error { + s.Field1 = mp["Field1"] + s.Field2 = mp["Field2"] + return nil +} + +// StubIntUnmarshaler is an int that implements redmap.StringMapUnmarshaler, +// so that we can test if a non-struct type is correctly handled as an interface. +type stubIntMapUnmarshaler int + +func (s *stubIntMapUnmarshaler) UnmarshalStringMap(mp map[string]string) error { + v, err := strconv.Atoi(mp["Field1"]) + if err != nil { + return err + } + *s = stubIntMapUnmarshaler(v) return nil } @@ -118,7 +143,7 @@ func TestUnmarshalScalars(t *testing.T) { {In: map[string]string{"V": "(100.1+80.1i)"}, Out: struct{ V complex128 }{100.1 + 80.1i}}, {In: map[string]string{"V": "str"}, Out: struct{ V string }{"str"}}, {In: map[string]string{"V": "a test"}, Out: struct{ V stubTextUnmarshaler }{stubTextUnmarshaler{S: "a test"}}}, - {In: map[string]string{"V": "100"}, Out: struct{ V StubIntUnmarshaler }{StubIntUnmarshaler(100)}}, + {In: map[string]string{"V": "100"}, Out: struct{ V stubIntTextUnmarshaler }{stubIntTextUnmarshaler(100)}}, } for _, test := range tests { zero := reflect.New(reflect.TypeOf(test.Out)) @@ -227,3 +252,48 @@ func TestUnmarshalWithTags(t *testing.T) { t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", mp, expected, copy) } } + +func TestMapUnmarshaler(t *testing.T) { + intUn := stubIntMapUnmarshaler(666) + tests := []struct { + In map[string]string + Out redmap.StringMapUnmarshaler + }{ + {In: map[string]string{"Field1": "value1", "Field2": "value2"}, Out: &stubMapUnmarshaler{Field1: "value1", Field2: "value2"}}, + {In: map[string]string{"Field1": "666"}, Out: &intUn}, + } + for _, test := range tests { + actual := reflect.New(reflect.TypeOf(test.Out).Elem()) + err := redmap.Unmarshal(test.In, actual.Interface()) + if err != nil { + t.Fatalf("Unmarshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(actual.Interface(), test.Out) { + t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", test.In, test.Out, actual) + } + } +} + +func TestInnerMapUnmarshaler(t *testing.T) { + expected := struct { + RegularField string + Struct stubMapUnmarshaler `redmap:",inline"` + }{ + RegularField: "regular", + Struct: stubMapUnmarshaler{Field1: "value1", Field2: "value2"}, + } + mp := map[string]string{ + "RegularField": "regular", + "Struct.Field1": "value1", + "Struct.Field2": "value2", + } + + actual := reflect.New(reflect.TypeOf(expected)) + err := redmap.Unmarshal(mp, actual.Interface()) + if err != nil { + t.Fatalf("Unmarshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(actual.Elem().Interface(), expected) { + t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", mp, expected, actual) + } +} From 368b6bed7538f08b50664872d72044406a3792be Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:03:06 +0100 Subject: [PATCH 07/13] unmarshal_test.go: Make TestInnerMapUnmarshaler table-driven --- unmarshal_test.go | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/unmarshal_test.go b/unmarshal_test.go index aff510d..fefb116 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -104,14 +104,14 @@ func TestUnmarshalInvalidType(t *testing.T) { v := 100 return reflect.ValueOf(&v) }, - expErr: redmap.ErrNotStruct}, + expErr: redmap.ErrNoCodec}, { // Interface is not a struct. val: func() reflect.Value { v := fmt.Stringer(stubStringer{}) return reflect.ValueOf(&v) }, - expErr: redmap.ErrNotStruct}, + expErr: redmap.ErrNoCodec}, } for _, test := range tests { err := redmap.Unmarshal(emptyMap, test.val().Interface()) @@ -275,25 +275,32 @@ func TestMapUnmarshaler(t *testing.T) { } func TestInnerMapUnmarshaler(t *testing.T) { - expected := struct { - RegularField string - Struct stubMapUnmarshaler `redmap:",inline"` + tests := []struct { + In map[string]string + Out interface{} }{ - RegularField: "regular", - Struct: stubMapUnmarshaler{Field1: "value1", Field2: "value2"}, - } - mp := map[string]string{ - "RegularField": "regular", - "Struct.Field1": "value1", - "Struct.Field2": "value2", + {In: map[string]string{"RegularField": "regular", "Struct.Field1": "value1", "Struct.Field2": "value2"}, + Out: struct { + RegularField string + Struct stubMapUnmarshaler `redmap:",inline"` + }{"regular", stubMapUnmarshaler{Field1: "value1", Field2: "value2"}}, + }, + // {In: map[string]string{"RegularField": "regular", "Inner.Field1": "666"}, + // Out: struct { + // RegularField string + // Inner stubIntMapUnmarshaler `redmap:",inline"` + // }{"regular", stubIntMapUnmarshaler(666)}, + // }, } - actual := reflect.New(reflect.TypeOf(expected)) - err := redmap.Unmarshal(mp, actual.Interface()) - if err != nil { - t.Fatalf("Unmarshal returned unexpected error %q", err) - } - if !reflect.DeepEqual(actual.Elem().Interface(), expected) { - t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", mp, expected, actual) + for _, test := range tests { + actual := reflect.New(reflect.TypeOf(test.Out)) + err := redmap.Unmarshal(test.In, actual.Interface()) + if err != nil { + t.Fatalf("Unmarshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(actual.Elem().Interface(), test.Out) { + t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", test.In, test.Out, actual) + } } } From 742841f3947c5b2771761898ba92dd12cf70daa9 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:06:38 +0100 Subject: [PATCH 08/13] Replace ErrNotStruct with ErrNoCodec --- error.go | 5 +++-- marshal.go | 4 ++-- marshal_test.go | 4 ++-- unmarshal.go | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/error.go b/error.go index 13b236c..83b8dc5 100644 --- a/error.go +++ b/error.go @@ -10,8 +10,9 @@ var ( ErrNilValue = errors.New("nil") // ErrNotPointer is retuned when an argument passed is not a pointer but it should be. ErrNotPointer = errors.New("not a pointer") - // ErrNotStruct is returned when an argument passed is not a struct but it should be. - ErrNotStruct = errors.New("not a struct type") + // ErrNoCodec is returned when a type cannot be marshaled or unmarshaled, + // e.g. it is neither a struct nor implements StringMap(Un)marshaler. + ErrNoCodec = errors.New("not an encodable or decodable type") ) func errIs(something interface{}, err error) error { diff --git a/marshal.go b/marshal.go index 087c94d..317570a 100644 --- a/marshal.go +++ b/marshal.go @@ -74,7 +74,7 @@ func structValue(v interface{}) (reflect.Value, error) { return reflect.Value{}, ErrNilValue default: if !val.Type().Implements(mapMarshalerType) { - return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + return reflect.Value{}, errIs(val.Type(), ErrNoCodec) } return val, nil } @@ -111,7 +111,7 @@ func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) e if tags.inline { if kind := value.Kind(); kind != reflect.Struct { - return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNotStruct)) + return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNoCodec)) } err := marshalRecursive(mp, prefix+tags.name+inlineSep, value) if err != nil { diff --git a/marshal_test.go b/marshal_test.go index 90ee175..a73b99c 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -87,8 +87,8 @@ func TestMarshalInvalidType(t *testing.T) { tests := []interface{}{noStruct, &noStruct} for _, test := range tests { _, err := redmap.Marshal(test) - if !errors.Is(err, redmap.ErrNotStruct) { - t.Fatalf("Unmarshal returned error %q but %q was expected", err, redmap.ErrNotStruct) + if !errors.Is(err, redmap.ErrNoCodec) { + t.Fatalf("Unmarshal returned error %q but %q was expected", err, redmap.ErrNoCodec) } } } diff --git a/unmarshal.go b/unmarshal.go index 5168685..8c4275f 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -57,7 +57,7 @@ func ptrStructValue(v interface{}) (reflect.Value, error) { return reflect.Value{}, errIs(reflect.TypeOf(v), ErrNilValue) default: if !val.Addr().Type().Implements(mapUnmarshalerType) { - return reflect.Value{}, errIs(val.Type(), ErrNotStruct) + return reflect.Value{}, errIs(val.Type(), ErrNoCodec) } return val, nil } @@ -97,7 +97,7 @@ func unmarshalRecursive(mp map[string]string, prefix string, stru reflect.Value) if tags.inline { if kind := value.Kind(); kind != reflect.Struct { - return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNotStruct)) + return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNoCodec)) } err := unmarshalRecursive(mp, tags.name+inlineSep, value) if err != nil { From c0b565c40337adbace34e13baae525ddf56eb9b7 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:11:57 +0100 Subject: [PATCH 09/13] marshal.go: marshalRecursive: Check if encodable first --- marshal.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/marshal.go b/marshal.go index 317570a..67a1d58 100644 --- a/marshal.go +++ b/marshal.go @@ -52,7 +52,7 @@ type StringMapMarshaler interface { // // are constructed in the "customName.subFieldName" format. // Field int `redmap:"customName,inline"` func Marshal(v interface{}) (map[string]string, error) { - val, err := structValue(v) + val, err := validValue(v) if err != nil { return nil, err } @@ -60,24 +60,17 @@ func Marshal(v interface{}) (map[string]string, error) { return ret, marshalRecursive(ret, "", val) } -func structValue(v interface{}) (reflect.Value, error) { +func validValue(v interface{}) (reflect.Value, error) { val := reflect.ValueOf(v) kin := val.Kind() for kin == reflect.Interface || kin == reflect.Ptr { val = val.Elem() kin = val.Kind() } - switch kin { - case reflect.Struct: - return val, nil - case reflect.Invalid: + if kin == reflect.Invalid { return reflect.Value{}, ErrNilValue - default: - if !val.Type().Implements(mapMarshalerType) { - return reflect.Value{}, errIs(val.Type(), ErrNoCodec) - } - return val, nil } + return val, nil } // marshalRecursive marshal a struct represented by val into a map[string]string. @@ -89,6 +82,9 @@ func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) e if typ.Implements(mapMarshalerType) { return structToMap(mp, prefix, stru) } + if stru.Kind() != reflect.Struct { + return errIs(stru.Type(), ErrNoCodec) + } for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) if field.PkgPath != "" { @@ -110,9 +106,6 @@ func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) e } if tags.inline { - if kind := value.Kind(); kind != reflect.Struct { - return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNoCodec)) - } err := marshalRecursive(mp, prefix+tags.name+inlineSep, value) if err != nil { return err From 16f4a129aa90d3825597f48dc28fa84569651ddb Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:18:55 +0100 Subject: [PATCH 10/13] unmarshal.go: unmarshalRecursive: Check if decodable first --- unmarshal.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/unmarshal.go b/unmarshal.go index 8c4275f..63e1023 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -26,17 +26,16 @@ func Unmarshal(data map[string]string, v interface{}) error { if data == nil { return errIs("map passed", ErrNilValue) } - val, err := ptrStructValue(v) + val, err := ptrValidValue(v) if err != nil { return err } return unmarshalRecursive(data, "", val) } -func ptrStructValue(v interface{}) (reflect.Value, error) { +func ptrValidValue(v interface{}) (reflect.Value, error) { val := reflect.ValueOf(v) kin := val.Kind() - switch kin { case reflect.Ptr: case reflect.Invalid: @@ -44,29 +43,23 @@ func ptrStructValue(v interface{}) (reflect.Value, error) { default: return reflect.Value{}, errIs(val.Type(), ErrNotPointer) } - for kin == reflect.Ptr { val = val.Elem() kin = val.Kind() } - - switch kin { - case reflect.Struct: - return val, nil - case reflect.Invalid: + if kin == reflect.Invalid { return reflect.Value{}, errIs(reflect.TypeOf(v), ErrNilValue) - default: - if !val.Addr().Type().Implements(mapUnmarshalerType) { - return reflect.Value{}, errIs(val.Type(), ErrNoCodec) - } - return val, nil } + return val, nil } func unmarshalRecursive(mp map[string]string, prefix string, stru reflect.Value) error { if ptr := stru.Addr(); ptr.Type().Implements(mapUnmarshalerType) { return mapToStruct(mp, prefix, ptr) } + if stru.Kind() != reflect.Struct { + return errIs(stru.Type(), ErrNoCodec) + } typ := stru.Type() for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) @@ -96,9 +89,6 @@ func unmarshalRecursive(mp map[string]string, prefix string, stru reflect.Value) } if tags.inline { - if kind := value.Kind(); kind != reflect.Struct { - return fmt.Errorf("cannot inline: %w", errIs(value.Type(), ErrNoCodec)) - } err := unmarshalRecursive(mp, tags.name+inlineSep, value) if err != nil { return err From 4e696621f28654f83db541b5ec1e592c030cba17 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:20:01 +0100 Subject: [PATCH 11/13] unmarshal_test.go: Add one more test case in TestInnerMapUnmarshaler --- unmarshal_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/unmarshal_test.go b/unmarshal_test.go index fefb116..6212311 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -285,12 +285,12 @@ func TestInnerMapUnmarshaler(t *testing.T) { Struct stubMapUnmarshaler `redmap:",inline"` }{"regular", stubMapUnmarshaler{Field1: "value1", Field2: "value2"}}, }, - // {In: map[string]string{"RegularField": "regular", "Inner.Field1": "666"}, - // Out: struct { - // RegularField string - // Inner stubIntMapUnmarshaler `redmap:",inline"` - // }{"regular", stubIntMapUnmarshaler(666)}, - // }, + {In: map[string]string{"RegularField": "regular", "Inner.Field1": "666"}, + Out: struct { + RegularField string + Inner stubIntMapUnmarshaler `redmap:",inline"` + }{"regular", stubIntMapUnmarshaler(666)}, + }, } for _, test := range tests { From 2b0d03a03c3a7935bbfe88a6944ccd1c378b8082 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:30:30 +0100 Subject: [PATCH 12/13] marshal.go: Document the use of StringMapMarshaler --- marshal.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/marshal.go b/marshal.go index 67a1d58..f31e9e2 100644 --- a/marshal.go +++ b/marshal.go @@ -14,11 +14,12 @@ type StringMapMarshaler interface { MarshalStringMap() (map[string]string, error) } -// Marshal returns the map[string]string representation of v, which must be a struct, -// by reading every exported field and translating it into a (key, value) pair to be -// added to the resulting map. Interfaces or pointers to struct are also accepted. +// Marshal returns the map[string]string representation of v, which must be a struct +// or implementing StringMapMarshaler. When implementing the interface, the map is returned verbatim +// from its method along with the error value. When not, Marshal reads every exported field and translates it +// into a (key, value) pair to be added to the resulting map. Interfaces or pointers to struct are also accepted. // -// Marshal converts all non-reference built-in types except arrays, plus +// Marshal converts all fields of non-reference built-in types except arrays, plus // structs implementing encoding.TextMarshaler or fmt.Stringer, checked in this exact order. // // The encoding of each struct field can be customized by the format string stored under the "redmap" @@ -47,9 +48,9 @@ type StringMapMarshaler interface { // // Field appears in the map as key "-". // Field int `redmap:"-,"` // -// // Field must be a struct. Field is flattened and its fields -// // are added to the map as (key, value) pairs, where the keys -// // are constructed in the "customName.subFieldName" format. +// // Field must be a struct or implementing StringMapMarshaler. +// // The resulting map is added to the final map with keys flattened, +// // constructed in the "customName.subKeyName" format. // Field int `redmap:"customName,inline"` func Marshal(v interface{}) (map[string]string, error) { val, err := validValue(v) From 34bea507ce4dee4d183c592644b01167a1640216 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Tue, 25 Jan 2022 12:33:54 +0100 Subject: [PATCH 13/13] unmarshal.go: Document Unmarshal better --- unmarshal.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/unmarshal.go b/unmarshal.go index 63e1023..8456a75 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -18,8 +18,7 @@ type StringMapUnmarshaler interface { // v must be a pointer to struct or an interface. Neither data nor v can be nil. // // Unmarshal uses the inverse of the encodings that Marshal uses, so all the types supported -// by it are also supported in Unmarshal, except the interfaces: only encoding.TextUnmarshaler -// can be unmarshaled. +// by it are also supported in Unmarshal, except fmt.Stringer which doesn't have an inverse. // // The decoding of each struct field can be customized by the format string documented in Marshal. func Unmarshal(data map[string]string, v interface{}) error {