From edb127ec03492599716b0a319c7181b1cb8b30d2 Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Fri, 17 May 2024 12:56:32 -0400 Subject: [PATCH] Handle protojson formatting on server side Signed-off-by: Matt Lord --- go/cmd/vtctldclient/cli/json.go | 66 ---------- go/cmd/vtctldclient/cli/json_test.go | 129 -------------------- go/cmd/vtctldclient/command/topology.go | 16 +-- go/vt/topo/decode.go | 9 +- go/vt/vtctl/grpcvtctldserver/server_test.go | 2 +- 5 files changed, 10 insertions(+), 212 deletions(-) delete mode 100644 go/cmd/vtctldclient/cli/json_test.go diff --git a/go/cmd/vtctldclient/cli/json.go b/go/cmd/vtctldclient/cli/json.go index 67d87dc568d..09bd98c3031 100644 --- a/go/cmd/vtctldclient/cli/json.go +++ b/go/cmd/vtctldclient/cli/json.go @@ -22,8 +22,6 @@ import ( "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" - - "vitess.io/vitess/go/stats" ) const ( @@ -79,67 +77,3 @@ func MarshalJSONPretty(obj any) ([]byte, error) { marshalOptions.UseEnumNumbers = false return MarshalJSON(obj, marshalOptions) } - -// ConvertToSnakeCase converts strings to snake_case. This is useful when converting -// generic JSON data marshalled to a map[string]interface{} for printing so that we -// retain the snake_case used with protobufs and protojson. -func ConvertToSnakeCase(val any) (any, error) { - switch val := val.(type) { - case string: - skval := stats.GetSnakeName(val) - return skval, nil - case map[string]any: - for k, v := range val { - sk := stats.GetSnakeName(k) - // We need to recurse into the map to convert nested maps - // to snake_case. - sv, err := ConvertToSnakeCase(v) - if err != nil { - return nil, err - } - delete(val, k) - val[sk] = sv - } - return val, nil - case map[any]any: - for k, v := range val { - // We need to recurse into the key to support more complex - // key types. - sk, err := ConvertToSnakeCase(k) - if err != nil { - return nil, err - } - // We need to recurse into the map to convert nested types - // to snake_case. - sv, err := ConvertToSnakeCase(v) - if err != nil { - return nil, err - } - delete(val, k) - val[sk] = sv - } - return val, nil - case []string: - for i, v := range val { - // We need to recurse into the slice to convert nested maps - // to snake_case. - sk := stats.GetSnakeName(v) - val[i] = sk - } - return val, nil - case []any: - for i, v := range val { - // We need to recurse into the slice to convert complex types - // to snake_case. - sv, err := ConvertToSnakeCase(v) - if err != nil { - return nil, err - } - val[i] = sv - } - return val, nil - default: - // No need to do any conversion for things like bool. - return val, nil - } -} diff --git a/go/cmd/vtctldclient/cli/json_test.go b/go/cmd/vtctldclient/cli/json_test.go deleted file mode 100644 index defeb4bd645..00000000000 --- a/go/cmd/vtctldclient/cli/json_test.go +++ /dev/null @@ -1,129 +0,0 @@ -/* -Copyright 2024 The Vitess 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 cli - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestConvertToSnakeCase(t *testing.T) { - tests := []struct { - name string - val any - want any - }{ - { - name: "string", - val: "MyValIsNotCool", - want: "my_val_is_not_cool", - }, - { - name: "map[string]bool", - val: map[string]any{"MyValIsNotCool": true}, - want: map[string]any{"my_val_is_not_cool": true}, - }, - { - name: "[]string", - val: []string{ - "MyValIsNotCool", - "NeitherIsYours", - }, - want: []string{ - "my_val_is_not_cool", - "neither_is_yours", - }, - }, - { - name: "map[string][any]", - val: map[string]any{ - "MyValIsNotCool": "val1", - "NeitherIsYours": "val2", - }, - want: map[string]any{ - "my_val_is_not_cool": "val1", - "neither_is_yours": "val2", - }, - }, - { - name: "map[any]any", - val: map[any]any{ - "MyValIsNotCool": []any{ - 0: map[any]any{ - "SubKey1": "val1", - "SubKey2": "val2", - }, - 1: map[any]any{ - "SubKey3": "val3", - "SubKey4": "val4", - }, - }, - "NeitherIsYours": []any{ - 0: map[any]any{ - "SubKey5": "val5", - "SubKey6": "val6", - }, - 1: map[any]any{ - "SubKey7": "val7", - "SubKey8": "val8", - }, - }, - }, - want: map[any]any{ - "my_val_is_not_cool": []any{ - 0: map[any]any{ - "sub_key1": "val1", - "sub_key2": "val2", - }, - 1: map[any]any{ - "sub_key3": "val3", - "sub_key4": "val4", - }, - }, - "neither_is_yours": []any{ - 0: map[any]any{ - "sub_key5": "val5", - "sub_key6": "val6", - }, - 1: map[any]any{ - "sub_key7": "val7", - "sub_key8": "val8", - }, - }, - }, - }, - { - name: "map[any][]any", - val: map[any]any{ - "MyValIsNotCool": []any{"val1", "val2"}, - "NeitherIsYours": []any{"val3", "val4"}, - }, - want: map[any]any{ - "my_val_is_not_cool": []any{"val1", "val2"}, - "neither_is_yours": []any{"val3", "val4"}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ConvertToSnakeCase(tt.val) - require.NoError(t, err, "ConvertToSnakeCase() error = %v", err) - require.EqualValues(t, tt.want, got, "ConvertToSnakeCase() = %v, want %v", got, tt.want) - }) - } -} diff --git a/go/cmd/vtctldclient/command/topology.go b/go/cmd/vtctldclient/command/topology.go index 6a19e7ec4fd..6aa6949341c 100644 --- a/go/cmd/vtctldclient/command/topology.go +++ b/go/cmd/vtctldclient/command/topology.go @@ -17,10 +17,8 @@ limitations under the License. package command import ( - "encoding/json" "fmt" - "github.com/pkg/errors" "github.com/spf13/cobra" "vitess.io/vitess/go/cmd/vtctldclient/cli" @@ -63,19 +61,7 @@ func commandGetTopologyPath(cmd *cobra.Command, args []string) error { if resp.GetCell() == nil || resp.GetCell().GetData() == "" { return fmt.Errorf("no data found for path %s", path) } - m := make(map[string]any) - if err := json.Unmarshal([]byte(resp.GetCell().GetData()), &m); err != nil { - return errors.Wrap(err, "failed to unmarshal node data as JSON") - } - skm, err := cli.ConvertToSnakeCase(m) - if err != nil { - return errors.Wrap(err, "failed to convert names to snake case") - } - js, err := json.MarshalIndent(skm, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal JSON data") - } - fmt.Println(string(js)) + fmt.Println(resp.GetCell().GetData()) return nil } diff --git a/go/vt/topo/decode.go b/go/vt/topo/decode.go index 0268c0a871f..9c13893777b 100644 --- a/go/vt/topo/decode.go +++ b/go/vt/topo/decode.go @@ -79,7 +79,14 @@ func DecodeContent(filename string, data []byte, json bool) (string, error) { var marshalled []byte var err error if json { - marshalled, err = protojson.Marshal(p) + // Maintain snake_case for the JSON output as this keeps the output consistent across + // vtctldclient commands and it is needed if the returned value is used as input to + // vtctldclient, e.g. for ApplyRoutingRules. + pm := protojson.MarshalOptions{ + Indent: " ", + UseProtoNames: true, + } + marshalled, err = pm.Marshal(p) } else { marshalled, err = prototext.Marshal(p) } diff --git a/go/vt/vtctl/grpcvtctldserver/server_test.go b/go/vt/vtctl/grpcvtctldserver/server_test.go index 0eea4e1d093..f72b92479a1 100644 --- a/go/vt/vtctl/grpcvtctldserver/server_test.go +++ b/go/vt/vtctl/grpcvtctldserver/server_test.go @@ -7859,7 +7859,7 @@ func TestGetTopologyPath(t *testing.T) { Cell: &vtctldatapb.TopologyCell{ Name: "Tablet", Path: "/cell1/tablets/cell1-0000000100/Tablet", - Data: `{"alias":{"cell":"cell1","uid":100},"hostname":"localhost","keyspace":"keyspace1","mysqlHostname":"localhost","mysqlPort":17100}`, + Data: "{\n \"alias\": {\n \"cell\": \"cell1\",\n \"uid\": 100\n },\n \"hostname\": \"localhost\",\n \"keyspace\": \"keyspace1\",\n \"mysql_hostname\": \"localhost\",\n \"mysql_port\": 17100\n}", }, }, },