Fix time/date and deepObject binding issues

This fixes #247. DeepObject had various bugs in binding to
time and dates, and we explicitly need to handle aliases
via reflection instead of type converting to time.Time and
types.Date

Added a pile of tests for these cases.

pkg/runtime/bindparam.go

@@ -395,22 +395,23 @@ func BindQueryParameter(style string, explode bool, required bool, paramName str
 // We don't try to be smart here, if the field exists as a query argument,
 // set its value.
 func bindParamsToExplodedObject(paramName string, values url.Values, dest interface{}) error {
- // special handling for custom types
- switch dest.(type) {
- case *types.Date:
+ // Dereference pointers to their destination values
+ v := reflect.Indirect(reflect.ValueOf(dest))
+ t := v.Type()
+ // special handling for custom types which might look like an object. We
+ // don't want to use object binding on them, but rather treat them as
+ // primitive types.
+ if t.ConvertibleTo(reflect.TypeOf(time.Time{})) {
  return BindStringToObject(values.Get(paramName), dest)
- case *time.Time:
+ }
+ if t.ConvertibleTo(reflect.TypeOf(types.Date{})) {
  return BindStringToObject(values.Get(paramName), dest)
-
  }
 
- v := reflect.Indirect(reflect.ValueOf(dest))
- if v.Type().Kind() != reflect.Struct {
+ if t.Kind() != reflect.Struct {
  return fmt.Errorf("unmarshaling query arg '%s' into wrong type", paramName)
  }
 
- t := v.Type()
-
  for i := 0; i < t.NumField(); i++ {
  fieldT := t.Field(i)
 

pkg/runtime/bindparam_test.go

@@ -18,8 +18,10 @@ import (
  "testing"
  "time"
 
- "github.com/deepmap/oapi-codegen/pkg/types"
  "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/deepmap/oapi-codegen/pkg/types"
 )
 
 func TestSplitParameter(t *testing.T) {
@@ -271,3 +273,111 @@ func TestBindQueryParameter(t *testing.T) {
  assert.Equal(t, expected, birthday)
  })
 }
+
+func TestBindParameterViaAlias(t *testing.T) {
+ // We don't need to check every parameter format type here, since the binding
+ // code is identical irrespective of parameter type, buy we do want to test
+ // a bunch of types.
+ type AString string
+ type Aint int
+ type Afloat float64
+ type Atime time.Time
+ type Adate types.Date
+
+ type AliasTortureTest struct {
+ S AString `json:"s"`
+ Sp *AString `json:"sp,omitempty"`
+ I Aint `json:"i"`
+ Ip *Aint `json:"ip,omitempty"`
+ F Afloat `json:"f"`
+ Fp *Afloat `json:"fp,omitempty"`
+ T Atime `json:"t"`
+ Tp *Atime `json:"tp,omitempty"`
+ D Adate `json:"d"`
+ Dp *Adate `json:"dp,omitempty"`
+ }
+
+ now := time.Now().UTC()
+ later := now.Add(time.Hour)
+
+ queryParams := url.Values{
+ "alias[s]": {"str"},
+ "alias[sp]": {"strp"},
+ "alias[i]": {"1"},
+ "alias[ip]": {"2"},
+ "alias[f]": {"3.5"},
+ "alias[fp]": {"4.5"},
+ "alias[t]": {now.Format(time.RFC3339Nano)},
+ "alias[tp]": {later.Format(time.RFC3339Nano)},
+ "alias[d]": {"2020-11-06"},
+ "alias[dp]": {"2020-11-07"},
+ }
+
+ dst := new(AliasTortureTest)
+
+ err := BindQueryParameter("deepObject", true, false, "alias", queryParams, &dst)
+ require.NoError(t, err)
+
+ var sp AString = "strp"
+ var ip Aint = 2
+ var fp Afloat = 4.5
+ dp := Adate{Time: time.Date(2020, 11, 7, 0, 0, 0, 0, time.UTC)}
+
+ expected := AliasTortureTest{
+ S: "str",
+ Sp: &sp,
+ I: 1,
+ Ip: &ip,
+ F: 3.5,
+ Fp: &fp,
+ T: Atime(now),
+ Tp: (*Atime)(&later),
+ D: Adate{Time: time.Date(2020, 11, 6, 0, 0, 0, 0, time.UTC)},
+ Dp: &dp,
+ }
+
+ // Compare field by field, makes errors easier to track.
+ assert.EqualValues(t, expected.S, dst.S)
+ assert.EqualValues(t, expected.Sp, dst.Sp)
+ assert.EqualValues(t, expected.I, dst.I)
+ assert.EqualValues(t, expected.Ip, dst.Ip)
+ assert.EqualValues(t, expected.F, dst.F)
+ assert.EqualValues(t, expected.Fp, dst.Fp)
+ assert.EqualValues(t, expected.T, dst.T)
+ assert.EqualValues(t, expected.Tp, dst.Tp)
+ assert.EqualValues(t, expected.D, dst.D)
+ assert.EqualValues(t, expected.Dp, dst.Dp)
+}
+
+// bindParamsToExplodedObject has to special case some types. Make sure that
+// these non-object types are handled correctly. The other parts of the functionality
+// are tested via more generic code above.
+func TestBindParamsToExplodedObject(t *testing.T) {
+ now := time.Now().UTC()
+ values := url.Values{
+ "time": {now.Format(time.RFC3339Nano)},
+ "date": {"2020-11-06"},
+ }
+
+ var dstTime time.Time
+ err := bindParamsToExplodedObject("time", values, &dstTime)
+ assert.NoError(t, err)
+ assert.EqualValues(t, now, dstTime)
+
+ type AliasedTime time.Time
+ var aDstTime AliasedTime
+ err = bindParamsToExplodedObject("time", values, &aDstTime)
+ assert.NoError(t, err)
+ assert.EqualValues(t, now, aDstTime)
+
+ var dstDate types.Date
+ expectedDate := types.Date{time.Date(2020, 11, 6, 0, 0, 0, 0, time.UTC)}
+ err = bindParamsToExplodedObject("date", values, &dstDate)
+ assert.EqualValues(t, expectedDate, dstDate)
+
+ type AliasedDate types.Date
+ var aDstDate AliasedDate
+ err = bindParamsToExplodedObject("date", values, &aDstDate)
+ assert.EqualValues(t, expectedDate, aDstDate)
+
+}

pkg/runtime/bindstring.go

@@ -76,8 +76,7 @@ func BindStringToObject(src string, dst interface{}) error {
  v.SetBool(val)
  }
  case reflect.Struct:
- switch dstType := dst.(type) {
- case *time.Time:
+ if t.ConvertibleTo(reflect.TypeOf(time.Time{})) {
  // Don't fail on empty string.
  if src == "" {
  return nil
@@ -90,9 +89,20 @@ func BindStringToObject(src string, dst interface{}) error {
  return fmt.Errorf("error parsing '%s' as RFC3339 or 2006-01-02 time: %s", src, err)
  }
  }
- *dstType = parsedTime
+ // So, assigning this gets a little fun. We have a value to the
+ // dereferenced destination. We can't do a conversion to
+ // time.Time because the result isn't assignable, so we need to
+ // convert pointers.
+ if t != reflect.TypeOf(time.Time{}) {
+ vPtr := v.Addr()
+ vtPtr := vPtr.Convert(reflect.TypeOf(&time.Time{}))
+ v = reflect.Indirect(vtPtr)
+ }
+ v.Set(reflect.ValueOf(parsedTime))
  return nil
- case *types.Date:
+ }
+
+ if t.ConvertibleTo(reflect.TypeOf(types.Date{})) {
  // Don't fail on empty string.
  if src == "" {
  return nil
@@ -101,9 +111,21 @@ func BindStringToObject(src string, dst interface{}) error {
  if err != nil {
  return fmt.Errorf("error parsing '%s' as date: %s", src, err)
  }
- dstType.Time = parsedTime
+ parsedDate := types.Date{Time: parsedTime}
+
+ // We have to do the same dance here to assign, just like with times
+ // above.
+ if t != reflect.TypeOf(types.Date{}) {
+ vPtr := v.Addr()
+ vtPtr := vPtr.Convert(reflect.TypeOf(&types.Date{}))
+ v = reflect.Indirect(vtPtr)
+ }
+ v.Set(reflect.ValueOf(parsedDate))
  return nil
  }
+
+ // We fall through to the error case below if we haven't handled the
+ // destination type above.
  fallthrough
  default:
  // We've got a bunch of types unimplemented, don't fail silently.

pkg/runtime/bindstring_test.go

@@ -18,6 +18,8 @@ import (
  "time"
 
  "github.com/stretchr/testify/assert"
+
+ "github.com/deepmap/oapi-codegen/pkg/types"
 )
 
 func TestBindStringToObject(t *testing.T) {
@@ -145,4 +147,18 @@ func TestBindStringToObject(t *testing.T) {
  assert.NoError(t, BindStringToObject(strTime, &parsedTime))
  parsedTime = parsedTime.UTC()
  assert.EqualValues(t, now, parsedTime)
+
+ // Checks whether time binding works through a type alias.
+ type AliasedTime time.Time
+ var aliasedTime AliasedTime
+ assert.NoError(t, BindStringToObject(strTime, &aliasedTime))
+ assert.EqualValues(t, now, aliasedTime)
+
+ // Checks whether date binding works directly and through an alias.
+ dateString := "2020-11-05"
+ var dstDate types.Date
+ assert.NoError(t, BindStringToObject(dateString, &dstDate))
+ type AliasedDate types.Date
+ var dstAliasedDate AliasedDate
+ assert.NoError(t, BindStringToObject(dateString, &dstAliasedDate))
 }

pkg/runtime/deepobject.go

@@ -212,24 +212,46 @@ func assignPathValues(dst interface{}, pathValues fieldOrValue) error {
  return nil
  case reflect.Struct:
  // Some special types we care about are structs. Handle them
- // here.
- if _, isDate := iv.Interface().(types.Date); isDate {
+ // here. They may be aliased, so we need to do some hoop
+ // jumping. If the types are aliased, we need to type convert
+ // the pointer, then set the value of the dereferenced pointer.
+ if it.ConvertibleTo(reflect.TypeOf(types.Date{})) {
  var date types.Date
  var err error
  date.Time, err = time.Parse(types.DateFormat, pathValues.value)
  if err != nil {
  return errors.Wrap(err, "invalid date format")
  }
- iv.Set(reflect.ValueOf(date))
+ dst := iv
+ if it != reflect.TypeOf(types.Date{}) {
+ // Types are aliased, convert the pointers.
+ ivPtr := iv.Addr()
+ aPtr := ivPtr.Convert(reflect.TypeOf(&types.Date{}))
+ dst = reflect.Indirect(aPtr)
+ }
+ dst.Set(reflect.ValueOf(date))
  }
- if _, isTime := iv.Interface().(time.Time); isTime {
+
+ if it.ConvertibleTo(reflect.TypeOf(time.Time{})) {
  var tm time.Time
  var err error
- tm, err = time.Parse(types.DateFormat, pathValues.value)
+ tm, err = time.Parse(time.RFC3339Nano, pathValues.value)
  if err != nil {
+ // Fall back to parsing it as a date.
+ tm, err = time.Parse(types.DateFormat, pathValues.value)
+ if err != nil {
+ return fmt.Errorf("error parsing tim as RFC3339 or 2006-01-02 time: %s", err)
+ }
  return errors.Wrap(err, "invalid date format")
  }
- iv.Set(reflect.ValueOf(tm))
+ dst := iv
+ if it != reflect.TypeOf(time.Time{}) {
+ // Types are aliased, convert the pointers.
+ ivPtr := iv.Addr()
+ aPtr := ivPtr.Convert(reflect.TypeOf(&time.Time{}))
+ dst = reflect.Indirect(aPtr)
+ }
+ dst.Set(reflect.ValueOf(tm))
  }
 
  fieldMap, err := fieldIndicesByJsonTag(iv.Interface())