From d4b5ff29cf8925ae4640bf2074c562429cd1b78f Mon Sep 17 00:00:00 2001
From: Mark Rushakoff <mark@influxdata.com>
Date: Wed, 10 Apr 2019 11:25:00 -0700
Subject: [PATCH] fix: encode IDs as JSON map keys properly

The encoding/json docs explain that you need to provide a MarshalText
method to encode integer types as map keys, otherwise they will be
formatted as a string version of the decimal number.

Providing MarshalText and UnmarshalText also uses those methods as a
fallback for MarshalJSON and UnmarshalJSON, so we no longer need
explicit versions of those latter methods.

Apparently Sources were using IDs as map keys and were providing the
,string attribute on the JSON tag on the struct. This was not correct so
that attribute has been removed. Existing sources will no longer be
readable as a result of this change.

Fixes #13277.
---
 id.go      | 30 +++++++++++-------------------
 id_test.go | 23 +++++++++++++++++++++++
 source.go  |  2 +-
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/id.go b/id.go
index 5bf262ab9d..307a6870f3 100644
--- a/id.go
+++ b/id.go
@@ -3,7 +3,6 @@ package influxdb
 import (
 	"encoding/binary"
 	"encoding/hex"
-	"encoding/json"
 	"errors"
 	"reflect"
 	"strconv"
@@ -119,24 +118,17 @@ func (i ID) GoString() string {
 	return `"` + i.String() + `"`
 }
 
-// UnmarshalJSON implements JSON unmarshaller for IDs.
-func (i *ID) UnmarshalJSON(b []byte) error {
-	if b[0] == '"' {
-		b = b[1:]
-	}
-
-	if b[len(b)-1] == '"' {
-		b = b[:len(b)-1]
-	}
+// MarshalText encodes i as text.
+// Providing this method is a fallback for json.Marshal,
+// with the added benefit that IDs encoded as map keys will be the expected string encoding,
+// rather than the effective fmt.Sprintf("%d", i) that json.Marshal uses by default for integer types.
+func (i ID) MarshalText() ([]byte, error) {
+	return i.Encode()
+}
 
+// UnmarshalText decodes i from a byte slice.
+// Providing this method is also a fallback for json.Unmarshal,
+// also relevant when IDs are used as map keys.
+func (i *ID) UnmarshalText(b []byte) error {
 	return i.Decode(b)
 }
-
-// MarshalJSON implements JSON marshaller for IDs.
-func (i ID) MarshalJSON() ([]byte, error) {
-	enc, err := i.Encode()
-	if err != nil {
-		return nil, err
-	}
-	return json.Marshal(string(enc))
-}
diff --git a/id_test.go b/id_test.go
index 13bf7ce173..58f25ff6d5 100644
--- a/id_test.go
+++ b/id_test.go
@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/influxdata/influxdb"
 	platform "github.com/influxdata/influxdb"
 	platformtesting "github.com/influxdata/influxdb/testing"
 )
@@ -181,6 +182,28 @@ func TestMarshalling(t *testing.T) {
 	if !bytes.Equal(bytes1, bytes2) {
 		t.Errorf("error marshalling/unmarshalling ID")
 	}
+
+	// When used as a map key, IDs must use their string encoding.
+	// If you only implement json.Marshaller, they will be encoded with Go's default integer encoding.
+	b, err := json.Marshal(map[influxdb.ID]int{0x1234: 5678})
+	if err != nil {
+		t.Error(err)
+	}
+	const exp = `{"0000000000001234":5678}`
+	if string(b) != exp {
+		t.Errorf("expected map to json.Marshal as %s; got %s", exp, string(b))
+	}
+
+	var idMap map[influxdb.ID]int
+	if err := json.Unmarshal(b, &idMap); err != nil {
+		t.Error(err)
+	}
+	if len(idMap) != 1 {
+		t.Errorf("expected length 1, got %d", len(idMap))
+	}
+	if idMap[0x1234] != 5678 {
+		t.Errorf("unmarshalled incorrectly; exp 0x1234:5678, got %v", idMap)
+	}
 }
 
 func TestValid(t *testing.T) {
diff --git a/source.go b/source.go
index e64189ff3d..c62ded99ce 100644
--- a/source.go
+++ b/source.go
@@ -23,7 +23,7 @@ const (
 // TODO(desa): do we still need default?
 // TODO(desa): do sources belong
 type Source struct {
-	ID                 ID         `json:"id,string,omitempty"`          // ID is the unique ID of the source
+	ID                 ID         `json:"id,omitempty"`                 // ID is the unique ID of the source
 	OrganizationID     ID         `json:"orgID"`                        // OrganizationID is the organization ID that resource belongs to
 	Default            bool       `json:"default"`                      // Default specifies the default source for the application
 	Name               string     `json:"name"`                         // Name is the user-defined name for the source