Repository: ably/ably-go
Pull request #174 (2020-07-24 15:00:37)

proto: Decode ms durations as a time.Duration wrapper.
This way, we don't have to worry about conversions, units, etc. when
using those values.


Patch:
diff --git a/ably/internal/ablyutil/msgpack.go b/ably/internal/ablyutil/msgpack.go
index b91bdae..98dd66c 100644
--- a/ably/internal/ablyutil/msgpack.go
+++ b/ably/internal/ablyutil/msgpack.go
@@ -15,9 +15,9 @@ func init() {
 	handle.RawToString = true
 }
 
-// Unmarshal decodes the MessagePack-encoded data and stores the result in the
+// UnmarshalMsgpack decodes the MessagePack-encoded data and stores the result in the
 // value pointed to by v.
-func Unmarshal(data []byte, v interface{}) error {
+func UnmarshalMsgpack(data []byte, v interface{}) error {
 	return decodeMsg(bytes.NewReader(data), v)
 }
 
@@ -27,8 +27,8 @@ func decodeMsg(r io.Reader, v interface{}) error {
 	return dec.Decode(v)
 }
 
-// Marshal returns msgpack encoding of v
-func Marshal(v interface{}) ([]byte, error) {
+// MarshalMsgpack returns msgpack encoding of v
+func MarshalMsgpack(v interface{}) ([]byte, error) {
 	var buf bytes.Buffer
 	err := encodeMsg(&buf, v)
 	if err != nil {
diff --git a/ably/internal/ablyutil/websocket.go b/ably/internal/ablyutil/websocket.go
index f6c2dc3..7159372 100644
--- a/ably/internal/ablyutil/websocket.go
+++ b/ably/internal/ablyutil/websocket.go
@@ -58,10 +58,10 @@ func DialWebsocket(proto string, u *url.URL) (*WebsocketConn, error) {
 
 var msgpackCodec = websocket.Codec{
 	Marshal: func(v interface{}) ([]byte, byte, error) {
-		p, err := Marshal(v)
+		p, err := MarshalMsgpack(v)
 		return p, websocket.BinaryFrame, err
 	},
 	Unmarshal: func(p []byte, _ byte, v interface{}) error {
-		return Unmarshal(p, v)
+		return UnmarshalMsgpack(p, v)
 	},
 }
diff --git a/ably/proto/protocol_message.go b/ably/proto/protocol_message.go
index 55953ec..cee4e76 100644
--- a/ably/proto/protocol_message.go
+++ b/ably/proto/protocol_message.go
@@ -18,13 +18,13 @@ func (f Flag) Has(flag Flag) bool {
 }
 
 type ConnectionDetails struct {
-	ClientID           string `json:"clientId,omitempty" codec:"clientId,omitempty"`
-	ConnectionKey      string `json:"connectionKey,omitempty" codec:"connectionKey,omitempty"`
-	MaxMessageSize     int64  `json:"maxMessageSize,omitempty" codec:"maxMessageSize,omitempty"`
-	MaxFrameSize       int64  `json:"maxFrameSize,omitempty" codec:"maxFrameSize,omitempty"`
-	MaxInboundRate     int64  `json:"maxInboundRate,omitempty" codec:"maxInboundRate,omitempty"`
-	ConnectionStateTTL int64  `json:"connectionStateTtl,omitempty" codec:"connectionStateTtl,omitempty"`
-	MaxIdleInterval    int64  `json:"maxIdleInterval,omitempty" codec:"maxIdleInterval,omitempty"`
+	ClientID           string            `json:"clientId,omitempty" codec:"clientId,omitempty"`
+	ConnectionKey      string            `json:"connectionKey,omitempty" codec:"connectionKey,omitempty"`
+	MaxMessageSize     int64             `json:"maxMessageSize,omitempty" codec:"maxMessageSize,omitempty"`
+	MaxFrameSize       int64             `json:"maxFrameSize,omitempty" codec:"maxFrameSize,omitempty"`
+	MaxInboundRate     int64             `json:"maxInboundRate,omitempty" codec:"maxInboundRate,omitempty"`
+	ConnectionStateTTL DurationFromMsecs `json:"connectionStateTtl,omitempty" codec:"connectionStateTtl,omitempty"`
+	MaxIdleInterval    DurationFromMsecs `json:"maxIdleInterval,omitempty" codec:"maxIdleInterval,omitempty"`
 }
 
 func (c *ConnectionDetails) FromMap(ctx map[string]interface{}) {
@@ -44,7 +44,7 @@ func (c *ConnectionDetails) FromMap(ctx map[string]interface{}) {
 		c.MaxInboundRate = coerceInt64(v)
 	}
 	if v, ok := ctx["connectionStateTtl"]; ok {
-		c.ConnectionStateTTL = coerceInt64(v)
+		c.ConnectionStateTTL = DurationFromMsecs(coerceInt64(v) * int64(time.Millisecond))
 	}
 }
 
diff --git a/ably/proto/types.go b/ably/proto/types.go
index e0a2aad..cabc355 100644
--- a/ably/proto/types.go
+++ b/ably/proto/types.go
@@ -1,5 +1,12 @@
 package proto
 
+import (
+	"encoding/json"
+	"time"
+
+	"github.com/ugorji/go/codec"
+)
+
 type Type int64
 
 const (
@@ -24,3 +31,45 @@ type Data struct {
 	BinaryData []byte  `json:"binaryData" codec:"binaryData"`
 	CipherData []byte  `json:"cipherData" codec:"cipherData"`
 }
+
+// DurationFromMsecs is a time.Duration that is marshaled as a JSON whole number
+// of milliseconds.
+type DurationFromMsecs time.Duration
+
+var _ interface {
+	json.Marshaler
+	json.Unmarshaler
+	codec.Selfer
+} = (*DurationFromMsecs)(nil)
+
+func (t DurationFromMsecs) asMsecs() int64 {
+	return time.Duration(t).Milliseconds()
+}
+
+func (t *DurationFromMsecs) setFromMsecs(ms int64) {
+	*t = DurationFromMsecs(time.Duration(ms) * time.Millisecond)
+}
+
+func (t DurationFromMsecs) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.asMsecs())
+}
+
+func (t *DurationFromMsecs) UnmarshalJSON(js []byte) error {
+	var ms int64
+	err := json.Unmarshal(js, &ms)
+	if err != nil {
+		return err
+	}
+	t.setFromMsecs(ms)
+	return nil
+}
+
+func (t DurationFromMsecs) CodecEncodeSelf(encoder *codec.Encoder) {
+	encoder.MustEncode(t.asMsecs())
+}
+
+func (t *DurationFromMsecs) CodecDecodeSelf(decoder *codec.Decoder) {
+	var ms int64
+	decoder.MustDecode(&ms)
+	t.setFromMsecs(ms)
+}
diff --git a/ably/rest_client.go b/ably/rest_client.go
index 4218f25..5f9c217 100644
--- a/ably/rest_client.go
+++ b/ably/rest_client.go
@@ -478,7 +478,7 @@ func encode(typ string, in interface{}) ([]byte, error) {
 	case "application/json":
 		return json.Marshal(in)
 	case "application/x-msgpack":
-		return ablyutil.Marshal(in)
+		return ablyutil.MarshalMsgpack(in)
 	case "text/plain":
 		return []byte(fmt.Sprintf("%v", in)), nil
 	default:
@@ -495,7 +495,7 @@ func decode(typ string, r io.Reader, out interface{}) error {
 		if err != nil {
 			return err
 		}
-		return ablyutil.Unmarshal(b, out)
+		return ablyutil.UnmarshalMsgpack(b, out)
 	case "text/plain":
 		p, err := ioutil.ReadAll(r)
 		if err != nil {


Test patch:
diff --git a/ably/ablytest/ablytest.go b/ably/ablytest/ablytest.go
index b262da8..5ed19e9 100644
--- a/ably/ablytest/ablytest.go
+++ b/ably/ablytest/ablytest.go
@@ -56,7 +56,7 @@ func encode(typ string, in interface{}) ([]byte, error) {
 	case "application/json":
 		return json.Marshal(in)
 	case "application/x-msgpack":
-		return ablyutil.Marshal(in)
+		return ablyutil.MarshalMsgpack(in)
 	case "text/plain":
 		return []byte(fmt.Sprintf("%v", in)), nil
 	default:
diff --git a/ably/proto/presence_message_test.go b/ably/proto/presence_message_test.go
index c544d32..b533131 100644
--- a/ably/proto/presence_message_test.go
+++ b/ably/proto/presence_message_test.go
@@ -46,12 +46,12 @@ func TestPresenceMessage(t *testing.T) {
 			}
 		})
 		t.Run("msgpack", func(ts *testing.T) {
-			b, err := ablyutil.Marshal(m)
+			b, err := ablyutil.MarshalMsgpack(m)
 			if err != nil {
 				ts.Fatal(err)
 			}
 			msg := proto.PresenceMessage{}
-			err = ablyutil.Unmarshal(b, &msg)
+			err = ablyutil.UnmarshalMsgpack(b, &msg)
 			if err != nil {
 				ts.Fatal(err)
 			}
diff --git a/ably/proto/types_test.go b/ably/proto/types_test.go
new file mode 100644
index 0000000..4d8bc7a
--- /dev/null
+++ b/ably/proto/types_test.go
@@ -0,0 +1,53 @@
+package proto_test
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/ably/ably-go/ably/internal/ablyutil"
+	"github.com/ably/ably-go/ably/proto"
+)
+
+func TestDurationFromMsecsMarshal(t *testing.T) {
+	t.Parallel()
+
+	d := 123456 * time.Millisecond
+
+	for _, codec := range []struct {
+		name      string
+		marshal   func(interface{}) ([]byte, error)
+		unmarshal func([]byte, interface{}) error
+	}{
+		{"JSON", json.Marshal, json.Unmarshal},
+		{"Msgpack", ablyutil.MarshalMsgpack, ablyutil.UnmarshalMsgpack},
+	} {
+		t.Run(codec.name, func(t *testing.T) {
+			t.Parallel()
+
+			js, err := codec.marshal(proto.DurationFromMsecs(d))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var msecs int64
+			err = codec.unmarshal(js, &msecs)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if expected, got := int64(123456), msecs; expected != got {
+				t.Fatalf("expected marshaling as JSON number of milliseconds; got %d (JSON: %q)", got, js)
+			}
+
+			var decoded proto.DurationFromMsecs
+			err = codec.unmarshal(js, &decoded)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if expected, got := d, time.Duration(decoded); expected != got {
+				t.Fatalf("expected json.Unmarshal after Marshal to produce the same duration; got %v", got)
+			}
+		})
+	}
+}
diff --git a/ably/realtime_conn_spec_test.go b/ably/realtime_conn_spec_test.go
index b4bbe2b..2321ea9 100644
--- a/ably/realtime_conn_spec_test.go
+++ b/ably/realtime_conn_spec_test.go
@@ -1121,8 +1121,7 @@ func TestRealtimeConn_RTN23(t *testing.T) {
 		t.Fatal(err)
 	}
 	msg := <-ok // consume the connect message
-	maxIdleInterval := time.Duration(msg.ConnectionDetails.MaxIdleInterval) * time.Millisecond
-	receiveTimeout := timeout + maxIdleInterval
+	receiveTimeout := timeout + time.Duration(msg.ConnectionDetails.MaxIdleInterval)
 
 	{ // RTN23b
 		h := query.Get("heartbeats")
diff --git a/ably/rest_client_test.go b/ably/rest_client_test.go
index 8af88c6..ad4d443 100644
--- a/ably/rest_client_test.go
+++ b/ably/rest_client_test.go
@@ -100,7 +100,7 @@ func TestRestClient(t *testing.T) {
 				ts.Fatal(err)
 			}
 			var anyMsgPack []map[string]interface{}
-			err = ablyutil.Unmarshal(buffer, &anyMsgPack)
+			err = ablyutil.UnmarshalMsgpack(buffer, &anyMsgPack)
 			if err != nil {
 				ts.Fatal(err)
 			}