Skip to content

Allowing for encoding of values that implement compatible interfaces #1

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
27 changes: 27 additions & 0 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,27 @@ func (d *decoder) value(val reflect.Value) error {
if elemField.CanAddr() {
err = d.value(elemField.Addr())
}
} else if typField.Type.Kind() == reflect.Ptr {
recQuery := make(url.Values)
pref := qstring + "."
for k, v := range d.data {
if strings.HasPrefix(k, pref) {
key := strings.Replace(k, pref, "", 1)
recQuery[key] = v
}
}
if len(recQuery) > 0 {
if elemField.IsNil() {
elemField.Set(reflect.New(typField.Type.Elem()))
elemField = elemField.Elem()
}
temp := d.data
d.data = recQuery
if elemField.CanAddr() {
err = d.value(elemField.Addr())
}
d.data = temp
}
}
if err != nil {
return err
Expand Down Expand Up @@ -152,6 +173,12 @@ func (d *decoder) coerce(query string, target reflect.Kind, field reflect.Value)
if err == nil {
field.Set(reflect.ValueOf(t))
}
case ComparativeString:
s := ComparativeString{}
err = s.Parse(query)
if err == nil {
field.Set(reflect.ValueOf(s))
}
default:
d.value(field)
}
Expand Down
33 changes: 33 additions & 0 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,36 @@ func TestUnmarshaller(t *testing.T) {
}
}
}

func TestUnmarshalEmbeddedStruct(t *testing.T) {
testIO := []struct {
inp url.Values
err interface{}
expected *RecursiveStruct
}{
{
url.Values{"object.value": []string{"embedded-example"}, "value": []string{"example"},},
nil,
&RecursiveStruct{
Object: &RecursiveStruct{
Value: "embedded-example",
},
Value: "example",
},
},
}
s := &RecursiveStruct{}
for _, test := range testIO {
err := Unmarshal(test.inp, s)
if err != test.err {
t.Errorf("Expected Unmarshaller to return %s, but got %s instead", test.err, err)
}
if !(test.expected.Value == s.Value) {
t.Errorf("Expected Unmarshaller to return %s, but got %s instead", test.expected.Value, s.Value)
}
if !(test.expected.Object.Value == s.Object.Value) {
t.Errorf("Expected Unmarshaller to return %s, but got %s instead", test.expected.Object.Value, s.Object.Value)
}
}

}
16 changes: 7 additions & 9 deletions doc_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package qstring_test
package qstring

import (
"fmt"
"net/url"
"os"
"time"

"github.com/dyninc/qstring"
)

func ExampleUnmarshal() {
Expand All @@ -19,7 +17,7 @@ func ExampleUnmarshal() {

query := &Query{}
qValues, _ := url.ParseQuery("names=foo&names=bar&limit=50&page=1")
err := qstring.Unmarshal(qValues, query)
err := Unmarshal(qValues, query)
if err != nil {
panic("Unable to Parse Query String")
}
Expand All @@ -41,7 +39,7 @@ func ExampleMarshalString() {
Limit: 50,
Page: 1,
}
q, _ := qstring.MarshalString(query)
q, _ := MarshalString(query)
os.Stdout.Write([]byte(q))
// Output: limit=50&names=foo&names=bar&page=1
}
Expand All @@ -62,21 +60,21 @@ func ExampleUnmarshal_complex() {
}
query := &Query{}
qValues, _ := url.ParseQuery("names=foo&names=bar&limit=50&page=1&ids=1&ids=2&created=2006-01-02T15:04:05Z")
err := qstring.Unmarshal(qValues, query)
err := Unmarshal(qValues, query)
if err != nil {
panic("Unable to Parse Query String")
}
}

func ExampleComparativeTime() {
type DateQuery struct {
Created qstring.ComparativeTime
Modified qstring.ComparativeTime
Created ComparativeTime
Modified ComparativeTime
}

var query DateQuery
qValues, _ := url.ParseQuery("created=>=2006-01-02T15:04:05Z&modified=<=2016-01-01T15:04Z")
err := qstring.Unmarshal(qValues, &query)
err := Unmarshal(qValues, &query)
if err != nil {
panic("Unable to Parse Query String")
}
Expand Down
34 changes: 32 additions & 2 deletions encode.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package qstring

import (
"encoding"
"fmt"
"net/url"
"reflect"
"strconv"
Expand Down Expand Up @@ -71,6 +73,9 @@ func (e *encoder) marshal() (url.Values, error) {
}
}

var textMarshallerElem = reflect.TypeOf(new(encoding.TextMarshaler)).Elem()
var stringerElem = reflect.TypeOf(new(fmt.Stringer)).Elem()

func (e *encoder) value(val reflect.Value) (url.Values, error) {
elem := val.Elem()
typ := elem.Type()
Expand All @@ -93,16 +98,21 @@ func (e *encoder) value(val reflect.Value) (url.Values, error) {
continue
}

// verify if the element type implements compatible interfaces
if val, ok := compatibleInterfaceValue(elemField); ok {
output.Set(qstring, val)
continue
}
// only do work if the current fields query string parameter was provided
switch k := typField.Type.Kind(); k {
default:
output.Set(qstring, marshalValue(elemField, k))
case reflect.Slice:
output[qstring] = marshalSlice(elemField)
case reflect.Ptr:
marshalStruct(output, qstring, reflect.Indirect(elemField), k)
case reflect.Struct:
marshalStruct(output, qstring, elemField, k)
default:
output.Set(qstring, marshalValue(elemField, k))
}
}
return output, err
Expand All @@ -116,7 +126,21 @@ func marshalSlice(field reflect.Value) []string {
return out
}

func compatibleInterfaceValue(field reflect.Value) (string, bool) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end I think relying on encoding.TextMarshaler and fmt.Stringer interfaces for compatible types is a mistake, as using their methods implicitly might be a problem for structs that have a different logic for them than what qstring would expect.

I think a better idea would be to create a qstring.Marshaler interface which types would be required to implement explicitly.

A concrete example would be a named type for a slice. A qstring specific implementation would have to prepend the property name (or qstring tag) for every value, but a plain Stringer/TextMarshaler method does not have the complete information for it, and as such would return a broken value.

if field.Type().Implements(textMarshallerElem) {
byt, _ := field.Interface().(encoding.TextMarshaler).MarshalText()
return string(byt), true
}
if field.Type().Implements(stringerElem) {
return field.Interface().(fmt.Stringer).String(), true
}
return "", false
}

func marshalValue(field reflect.Value, source reflect.Kind) string {
if val, ok := compatibleInterfaceValue(field); ok {
return val
}
switch source {
case reflect.String:
return field.String()
Expand All @@ -130,6 +154,9 @@ func marshalValue(field reflect.Value, source reflect.Kind) string {
return strconv.FormatFloat(field.Float(), 'G', -1, 64)
case reflect.Struct:
switch field.Interface().(type) {
case encoding.TextMarshaler:
byt, _ := field.Interface().(encoding.TextMarshaler).MarshalText()
return string(byt)
case time.Time:
return field.Interface().(time.Time).Format(time.RFC3339)
case ComparativeTime:
Expand All @@ -154,6 +181,9 @@ func marshalStruct(output url.Values, qstring string, field reflect.Value, sourc
return err
}
for key, list := range vals {
if qstring != "" {
key = qstring + "." + key
}
output[key] = list
}
}
Expand Down
71 changes: 71 additions & 0 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,3 +294,74 @@ func TestMarshaller(t *testing.T) {
}
}
}

type MyText []byte
type MyOtherText []byte

type MyStruct struct {
Text MyText
Other MyOtherText
}

func (m MyText) MarshalText() ([]byte, error) {
return []byte(m), nil
}
func (m MyOtherText) String() string {
return string(m)
}

func TestMarshalTextMarshalType(t *testing.T) {
el := MyStruct{Text: MyText("example string"), Other: MyOtherText("second example")}

result, err := MarshalString(&el)
if err != nil {
t.Fatalf("Unable to marshal type %T: %s", el, err.Error())
}

var unescaped string
unescaped, err = url.QueryUnescape(result)
if err != nil {
t.Fatalf("Unable to unescape query string %q: %q", result, err.Error())
}

// ensure fields we expect to be present are
expected := []string{"text=example string", "other=second example"}
for _, q := range expected {
if !strings.Contains(unescaped, q) {
t.Errorf("Expected query string %s to contain %s", unescaped, q)
}
}
}

type RecursiveStruct struct {
Object *RecursiveStruct `qstring:"object,omitempty"`
Value string `qstring:"value"`
}

func TestMarshalEmbeddedStruct(t *testing.T) {
rec := RecursiveStruct{
Value: "example",
Object: &RecursiveStruct{
Value: "embedded-example",
},
}

result, err := MarshalString(&rec)
if err != nil {
t.Fatalf("Unable to marshal type %T: %s", rec, err.Error())
}

var unescaped string
unescaped, err = url.QueryUnescape(result)
if err != nil {
t.Fatalf("Unable to unescape query string %q: %q", result, err.Error())
}

// ensure fields we expect to be present are
expected := []string{"value=example", "object.value=embedded-example"}
for _, q := range expected {
if !strings.Contains(unescaped, q) {
t.Errorf("Expected query string %s to contain %s", unescaped, q)
}
}
}
Loading