From 4440c3b8306d28f4af5833bdf12ac54866dc1423 Mon Sep 17 00:00:00 2001
From: Menghan Li <menghanl@google.com>
Date: Tue, 22 Jun 2021 14:57:05 -0700
Subject: [PATCH] cluster_resolver: fix DiscoveryMechanismType marshal JSON
 (#4532)

---
 .../clusterresolver/balancerconfig/type.go    | 11 ++-
 .../balancerconfig/type_test.go               | 88 +++++++++++++++++++
 2 files changed, 95 insertions(+), 4 deletions(-)
 create mode 100644 xds/internal/balancer/clusterresolver/balancerconfig/type_test.go

diff --git a/xds/internal/balancer/clusterresolver/balancerconfig/type.go b/xds/internal/balancer/clusterresolver/balancerconfig/type.go
index eb149cd384d..2f9ba68fe59 100644
--- a/xds/internal/balancer/clusterresolver/balancerconfig/type.go
+++ b/xds/internal/balancer/clusterresolver/balancerconfig/type.go
@@ -29,17 +29,20 @@ type DiscoveryMechanismType int
 
 const (
 	// DiscoveryMechanismTypeEDS is eds.
-	DiscoveryMechanismTypeEDS DiscoveryMechanismType = iota // `json:EDS`
+	DiscoveryMechanismTypeEDS DiscoveryMechanismType = iota // `json:"EDS"`
 	// DiscoveryMechanismTypeLogicalDNS is DNS.
-	DiscoveryMechanismTypeLogicalDNS // `json:LOGICAL_DNS`
+	DiscoveryMechanismTypeLogicalDNS // `json:"LOGICAL_DNS"`
 )
 
 // MarshalJSON marshals a DiscoveryMechanismType to a quoted json string.
 //
 // This is necessary to handle enum (as strings) from JSON.
-func (t *DiscoveryMechanismType) MarshalJSON() ([]byte, error) {
+//
+// Note that this needs to be defined on the type not pointer, otherwise the
+// variables of this type will marshal to int not string.
+func (t DiscoveryMechanismType) MarshalJSON() ([]byte, error) {
 	buffer := bytes.NewBufferString(`"`)
-	switch *t {
+	switch t {
 	case DiscoveryMechanismTypeEDS:
 		buffer.WriteString("EDS")
 	case DiscoveryMechanismTypeLogicalDNS:
diff --git a/xds/internal/balancer/clusterresolver/balancerconfig/type_test.go b/xds/internal/balancer/clusterresolver/balancerconfig/type_test.go
new file mode 100644
index 00000000000..1adbc7c5d23
--- /dev/null
+++ b/xds/internal/balancer/clusterresolver/balancerconfig/type_test.go
@@ -0,0 +1,88 @@
+/*
+ *
+ * Copyright 2021 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package balancerconfig
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestDiscoveryMechanismTypeMarshalJSON(t *testing.T) {
+	tests := []struct {
+		name string
+		typ  DiscoveryMechanismType
+		want string
+	}{
+		{
+			name: "eds",
+			typ:  DiscoveryMechanismTypeEDS,
+			want: `"EDS"`,
+		},
+		{
+			name: "dns",
+			typ:  DiscoveryMechanismTypeLogicalDNS,
+			want: `"LOGICAL_DNS"`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got, err := json.Marshal(tt.typ); err != nil || string(got) != tt.want {
+				t.Fatalf("DiscoveryMechanismTypeEDS.MarshalJSON() = (%v, %v), want (%s, nil)", string(got), err, tt.want)
+			}
+		})
+	}
+}
+func TestDiscoveryMechanismTypeUnmarshalJSON(t *testing.T) {
+	tests := []struct {
+		name    string
+		js      string
+		want    DiscoveryMechanismType
+		wantErr bool
+	}{
+		{
+			name: "eds",
+			js:   `"EDS"`,
+			want: DiscoveryMechanismTypeEDS,
+		},
+		{
+			name: "dns",
+			js:   `"LOGICAL_DNS"`,
+			want: DiscoveryMechanismTypeLogicalDNS,
+		},
+		{
+			name:    "error",
+			js:      `"1234"`,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var got DiscoveryMechanismType
+			err := json.Unmarshal([]byte(tt.js), &got)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("DiscoveryMechanismTypeEDS.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if diff := cmp.Diff(got, tt.want); diff != "" {
+				t.Fatalf("DiscoveryMechanismTypeEDS.UnmarshalJSON() got unexpected output, diff (-got +want): %v", diff)
+			}
+		})
+	}
+}