You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2017/02/21 21:33:20 UTC
thrift git commit: THRIFT-4011 Sets of Thrift structs generate Go
code that can't be serialized to JSON Client: Go Patch: D. Can Celasun
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 <dc...@gmail.com>
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 <dc...@gmail.com>
Authored: Sun Jan 15 10:53:19 2017 +0100
Committer: Jens Geyer <je...@apache.org>
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
}