Repository: thrift Updated Branches: refs/heads/master bc0082e02 -> 43fb34df2
THRIFT-4011 Sets of Thrift structs generate Go code that can't be serialized to JSON Client: Go Patch: D. Can Celasun <[email protected]> This closes #1156 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/43fb34df Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/43fb34df Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/43fb34df Branch: refs/heads/master Commit: 43fb34df2871b69c2f34dc3bb353e65cbc9f8692 Parents: bc0082e Author: D. Can Celasun <[email protected]> Authored: Sun Jan 15 10:53:19 2017 +0100 Committer: Jens Geyer <[email protected]> Committed: Tue Feb 21 22:32:49 2017 +0100 ---------------------------------------------------------------------- .../cpp/src/thrift/generate/t_go_generator.cc | 27 ++++++++++++-------- lib/go/test/tests/client_error_test.go | 4 +-- lib/go/test/tests/thrifttest_driver.go | 2 +- lib/go/test/tests/thrifttest_handler.go | 2 +- test/go/src/bin/testclient/main.go | 15 ++++++++--- test/go/src/common/clientserver_test.go | 17 ++++++++---- test/go/src/common/mock_handler.go | 4 +-- test/go/src/common/printing_handler.go | 2 +- test/go/src/common/simple_handler.go | 2 +- 9 files changed, 48 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/compiler/cpp/src/thrift/generate/t_go_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc index bb5a609..14b3597 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc @@ -882,6 +882,7 @@ string t_go_generator::go_imports_begin(bool consts) { return string( "import (\n" "\t\"bytes\"\n" + "\t\"reflect\"\n" + extra + "\t\"fmt\"\n" "\t\"" + gen_thrift_import_ + "\"\n"); @@ -890,7 +891,7 @@ string t_go_generator::go_imports_begin(bool consts) { /** * End the import statement, include undscore-assignments * - * These "_ =" prevent the go compiler complaining about used imports. + * These "_ =" prevent the go compiler complaining about unused imports. * This will have to do in lieu of more intelligent import statement construction */ string t_go_generator::go_imports_end() { @@ -899,6 +900,7 @@ string t_go_generator::go_imports_end() { "// (needed to ensure safety because of naive import list construction.)\n" "var _ = thrift.ZERO\n" "var _ = fmt.Printf\n" + "var _ = reflect.DeepEqual\n" "var _ = bytes.Equal\n\n"); } @@ -1155,12 +1157,12 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co } else if (type->is_set()) { t_type* etype = ((t_set*)type)->get_elem_type(); const vector<t_const_value*>& val = value->get_list(); - out << "map[" << type_to_go_key_type(etype) << "]struct{}{" << endl; + out << "[]" << type_to_go_key_type(etype) << "{" << endl; indent_up(); vector<t_const_value*>::const_iterator v_iter; for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) { - out << indent() << render_const_value(etype, *v_iter, name) << ": struct{}{}," << endl; + out << indent() << render_const_value(etype, *v_iter, name) << ", "; } indent_down(); @@ -2980,13 +2982,11 @@ void t_go_generator::generate_deserialize_container(ofstream& out, out << indent() << "tMap := make(" << type_to_go_type(orig_type) << ", size)" << endl; out << indent() << prefix << eq << " " << (pointer_field ? "&" : "") << "tMap" << endl; } else if (ttype->is_set()) { - t_set* t = (t_set*)ttype; out << indent() << "_, size, err := iprot.ReadSetBegin()" << endl; out << indent() << "if err != nil {" << endl; out << indent() << " return thrift.PrependError(\"error reading set begin: \", err)" << endl; out << indent() << "}" << endl; - out << indent() << "tSet := make(map[" - << type_to_go_key_type(t->get_elem_type()->get_true_type()) << "]struct{}, size)" << endl; + out << indent() << "tSet := make(" << type_to_go_type(orig_type) << ", 0, size)" << endl; out << indent() << prefix << eq << " " << (pointer_field ? "&" : "") << "tSet" << endl; } else if (ttype->is_list()) { out << indent() << "_, size, err := iprot.ReadListBegin()" << endl; @@ -3065,7 +3065,7 @@ void t_go_generator::generate_deserialize_set_element(ofstream& out, t_field felem(tset->get_elem_type(), elem); felem.set_req(t_field::T_OPT_IN_REQ_OUT); generate_deserialize_field(out, &felem, true, "", false, false, false, true, true); - indent(out) << prefix << "[" << elem << "] = struct{}{}" << endl; + indent(out) << prefix << " = append(" << prefix << ", " << elem << ")" << endl; } /** @@ -3224,7 +3224,14 @@ void t_go_generator::generate_serialize_container(ofstream& out, indent(out) << "}" << endl; } else if (ttype->is_set()) { t_set* tset = (t_set*)ttype; - out << indent() << "for v, _ := range " << prefix << " {" << endl; + out << indent() << "for i := 0; i<len(" << prefix << "); i++ {" << endl; + out << indent() << " for j := i+1; j<len(" << prefix << "); j++ {" << endl; + out << indent() << " if reflect.DeepEqual(" << prefix << "[i]," << prefix << "[j]) { " << endl; + out << indent() << " return thrift.PrependError(\"\", fmt.Errorf(\"%T error writing set field: slice is not unique\", " << prefix << "[i]))" << endl; + out << indent() << " }" << endl; + out << indent() << " }" << endl; + out << indent() << "}" << endl; + out << indent() << "for _, v := range " << prefix << " {" << endl; indent_up(); generate_serialize_set_element(out, tset, "v"); indent_down(); @@ -3627,8 +3634,8 @@ string t_go_generator::type_to_go_type_with_opt(t_type* type, return maybe_pointer + string("map[") + keyType + "]" + valueType; } else if (type->is_set()) { t_set* t = (t_set*)type; - string elemType = type_to_go_key_type(t->get_elem_type()); - return maybe_pointer + string("map[") + elemType + string("]struct{}"); + string elemType = type_to_go_type(t->get_elem_type()); + return maybe_pointer + string("[]") + elemType; } else if (type->is_list()) { t_list* t = (t_list*)type; string elemType = type_to_go_type(t->get_elem_type()); http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/lib/go/test/tests/client_error_test.go ---------------------------------------------------------------------- diff --git a/lib/go/test/tests/client_error_test.go b/lib/go/test/tests/client_error_test.go index 838883d..0810be6 100644 --- a/lib/go/test/tests/client_error_test.go +++ b/lib/go/test/tests/client_error_test.go @@ -402,7 +402,7 @@ func TestClientReportTTransportErrors(t *testing.T) { thing := errortest.NewTestStruct() thing.M = make(map[string]string) thing.L = make([]string, 0) - thing.S = make(map[string]struct{}) + thing.S = make([]string, 0) thing.I = 3 err := thrift.NewTTransportException(thrift.TIMED_OUT, "test") @@ -434,7 +434,7 @@ func TestClientReportTProtocolErrors(t *testing.T) { thing := errortest.NewTestStruct() thing.M = make(map[string]string) thing.L = make([]string, 0) - thing.S = make(map[string]struct{}) + thing.S = make([]string, 0) thing.I = 3 err := thrift.NewTProtocolExceptionWithType(thrift.INVALID_DATA, errors.New("test")) http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/lib/go/test/tests/thrifttest_driver.go ---------------------------------------------------------------------- diff --git a/lib/go/test/tests/thrifttest_driver.go b/lib/go/test/tests/thrifttest_driver.go index 1e0cf86..a1e6917 100644 --- a/lib/go/test/tests/thrifttest_driver.go +++ b/lib/go/test/tests/thrifttest_driver.go @@ -162,7 +162,7 @@ func (p *ThriftTestDriver) Start() { t.Fatal("TestStringMap failed") } - setTestInput := map[int32]struct{}{1: {}, 2: {}, 3: {}} + setTestInput := []int32{1, 2, 3} if r, err := client.TestSet(setTestInput); !reflect.DeepEqual(r, setTestInput) || err != nil { t.Fatal("TestSet failed") } http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/lib/go/test/tests/thrifttest_handler.go ---------------------------------------------------------------------- diff --git a/lib/go/test/tests/thrifttest_handler.go b/lib/go/test/tests/thrifttest_handler.go index 822a6c7..5b76066 100644 --- a/lib/go/test/tests/thrifttest_handler.go +++ b/lib/go/test/tests/thrifttest_handler.go @@ -96,7 +96,7 @@ func (p *ThriftTestHandler) TestStringMap(thing map[string]string) (r map[string return thing, nil } -func (p *ThriftTestHandler) TestSet(thing map[int32]struct{}) (r map[int32]struct{}, err error) { +func (p *ThriftTestHandler) TestSet(thing []int32) (r []int32, err error) { return thing, nil } http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/test/go/src/bin/testclient/main.go ---------------------------------------------------------------------- diff --git a/test/go/src/bin/testclient/main.go b/test/go/src/bin/testclient/main.go index f4a19dd..228120b 100644 --- a/test/go/src/bin/testclient/main.go +++ b/test/go/src/bin/testclient/main.go @@ -174,13 +174,20 @@ func callEverything(client *thrifttest.ThriftTestClient) { t.Fatalf("Unexpected TestStringMap() result expected %#v, got %#v ", sm, smret) } - s := map[int32]struct{}{1: struct{}{}, 2: struct{}{}, 42: struct{}{}} + s := []int32{1, 2, 42} sret, err := client.TestSet(s) if err != nil { t.Fatalf("Unexpected error in TestSet() call: ", err) } - if !reflect.DeepEqual(s, sret) { - t.Fatalf("Unexpected TestSet() result expected %#v, got %#v ", s, sret) + // Sets can be in any order, but Go slices are ordered, so reflect.DeepEqual won't work. + stemp := map[int32]struct{}{} + for _, val := range s { + stemp[val] = struct{}{} + } + for _, val := range sret { + if _, ok := stemp[val]; !ok { + t.Fatalf("Unexpected TestSet() result expected %#v, got %#v ", s, sret) + } } l := []int32{1, 2, 42} @@ -189,7 +196,7 @@ func callEverything(client *thrifttest.ThriftTestClient) { t.Fatalf("Unexpected error in TestList() call: ", err) } if !reflect.DeepEqual(l, lret) { - t.Fatalf("Unexpected TestSet() result expected %#v, got %#v ", l, lret) + t.Fatalf("Unexpected TestList() result expected %#v, got %#v ", l, lret) } eret, err := client.TestEnum(thrifttest.Numberz_TWO) http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/test/go/src/common/clientserver_test.go ---------------------------------------------------------------------- diff --git a/test/go/src/common/clientserver_test.go b/test/go/src/common/clientserver_test.go index 549e8d1..9f490ea 100644 --- a/test/go/src/common/clientserver_test.go +++ b/test/go/src/common/clientserver_test.go @@ -105,7 +105,7 @@ func callEverythingWithMock(t *testing.T, client *thrifttest.ThriftTestClient, h handler.EXPECT().TestNest(&thrifttest.Xtruct2{StructThing: &thrifttest.Xtruct{StringThing: "thing", ByteThing: 42, I32Thing: 4242, I64Thing: 424242}}).Return(&thrifttest.Xtruct2{StructThing: &thrifttest.Xtruct{StringThing: "thing", ByteThing: 42, I32Thing: 4242, I64Thing: 424242}}, nil), handler.EXPECT().TestMap(map[int32]int32{1: 2, 3: 4, 5: 42}).Return(map[int32]int32{1: 2, 3: 4, 5: 42}, nil), handler.EXPECT().TestStringMap(map[string]string{"a": "2", "b": "blah", "some": "thing"}).Return(map[string]string{"a": "2", "b": "blah", "some": "thing"}, nil), - handler.EXPECT().TestSet(map[int32]struct{}{1: struct{}{}, 2: struct{}{}, 42: struct{}{}}).Return(map[int32]struct{}{1: struct{}{}, 2: struct{}{}, 42: struct{}{}}, nil), + handler.EXPECT().TestSet([]int32{1, 2, 42}).Return([]int32{1, 2, 42}, nil), handler.EXPECT().TestList([]int32{1, 2, 42}).Return([]int32{1, 2, 42}, nil), handler.EXPECT().TestEnum(thrifttest.Numberz_TWO).Return(thrifttest.Numberz_TWO, nil), handler.EXPECT().TestTypedef(thrifttest.UserId(42)).Return(thrifttest.UserId(42), nil), @@ -222,13 +222,20 @@ func callEverythingWithMock(t *testing.T, client *thrifttest.ThriftTestClient, h t.Errorf("Unexpected TestStringMap() result expected %#v, got %#v ", sm, smret) } - s := map[int32]struct{}{1: struct{}{}, 2: struct{}{}, 42: struct{}{}} + s := []int32{1, 2, 42} sret, err := client.TestSet(s) if err != nil { t.Errorf("Unexpected error in TestSet() call: ", err) } - if !reflect.DeepEqual(s, sret) { - t.Errorf("Unexpected TestSet() result expected %#v, got %#v ", s, sret) + // Sets can be in any order, but Go slices are ordered, so reflect.DeepEqual won't work. + stemp := map[int32]struct{}{} + for _, val := range s { + stemp[val] = struct{}{} + } + for _, val := range sret { + if _, ok := stemp[val]; !ok { + t.Fatalf("Unexpected TestSet() result expected %#v, got %#v ", s, sret) + } } l := []int32{1, 2, 42} @@ -237,7 +244,7 @@ func callEverythingWithMock(t *testing.T, client *thrifttest.ThriftTestClient, h t.Errorf("Unexpected error in TestList() call: ", err) } if !reflect.DeepEqual(l, lret) { - t.Errorf("Unexpected TestSet() result expected %#v, got %#v ", l, lret) + t.Errorf("Unexpected TestList() result expected %#v, got %#v ", l, lret) } eret, err := client.TestEnum(thrifttest.Numberz_TWO) http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/test/go/src/common/mock_handler.go ---------------------------------------------------------------------- diff --git a/test/go/src/common/mock_handler.go b/test/go/src/common/mock_handler.go index 6ae8130..b6738ee 100644 --- a/test/go/src/common/mock_handler.go +++ b/test/go/src/common/mock_handler.go @@ -223,9 +223,9 @@ func (_mr *_MockThriftTestRecorder) TestOneway(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "TestOneway", arg0) } -func (_m *MockThriftTest) TestSet(_param0 map[int32]struct{}) (map[int32]struct{}, error) { +func (_m *MockThriftTest) TestSet(_param0 []int32) ([]int32, error) { ret := _m.ctrl.Call(_m, "TestSet", _param0) - ret0, _ := ret[0].(map[int32]struct{}) + ret0, _ := ret[0].([]int32) ret1, _ := ret[1].(error) return ret0, ret1 } http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/test/go/src/common/printing_handler.go ---------------------------------------------------------------------- diff --git a/test/go/src/common/printing_handler.go b/test/go/src/common/printing_handler.go index afee8da..d4e2be9 100644 --- a/test/go/src/common/printing_handler.go +++ b/test/go/src/common/printing_handler.go @@ -188,7 +188,7 @@ func (p *printingHandler) TestStringMap(thing map[string]string) (r map[string]s // // Parameters: // - Thing -func (p *printingHandler) TestSet(thing map[int32]struct{}) (r map[int32]struct{}, err error) { +func (p *printingHandler) TestSet(thing []int32) (r []int32, err error) { fmt.Printf("testSet({") first := true for k, _ := range thing { http://git-wip-us.apache.org/repos/asf/thrift/blob/43fb34df/test/go/src/common/simple_handler.go ---------------------------------------------------------------------- diff --git a/test/go/src/common/simple_handler.go b/test/go/src/common/simple_handler.go index 7bd3a30..0c9463d 100644 --- a/test/go/src/common/simple_handler.go +++ b/test/go/src/common/simple_handler.go @@ -77,7 +77,7 @@ func (p *simpleHandler) TestStringMap(thing map[string]string) (r map[string]str return thing, nil } -func (p *simpleHandler) TestSet(thing map[int32]struct{}) (r map[int32]struct{}, err error) { +func (p *simpleHandler) TestSet(thing []int32) (r []int32, err error) { return thing, nil }
