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

@@ -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))
-}

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) {

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