Skip to content

Commit

Permalink
THRIFT-4260 Go context generation issue. Context is parameter in Inte…
Browse files Browse the repository at this point in the history
…rface not in implementation

Client: Go
Patch: taozle <zhangliyang26@gmail.com>

This closes apache#1312
  • Loading branch information
taozle authored and Jens-G committed Jul 24, 2017
1 parent c0d384a commit 5c302e0
Show file tree
Hide file tree
Showing 18 changed files with 294 additions and 114 deletions.
22 changes: 13 additions & 9 deletions compiler/cpp/src/thrift/generate/t_go_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ class t_go_generator : public t_generator {
std::string function_signature(t_function* tfunction, std::string prefix = "");
std::string function_signature_if(t_function* tfunction,
std::string prefix = "",
bool addError = false,
bool enableContext = false);
bool addError = false);
std::string argument_list(t_struct* tstruct);
std::string type_to_enum(t_type* ttype);
std::string type_to_go_type(t_type* ttype);
Expand Down Expand Up @@ -1838,7 +1837,7 @@ void t_go_generator::generate_service_interface(t_service* tservice) {

for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
generate_go_docstring(f_types_, (*f_iter));
f_types_ << indent() << function_signature_if(*f_iter, "", true, true) << endl;
f_types_ << indent() << function_signature_if(*f_iter, "", true) << endl;
}
}

Expand Down Expand Up @@ -1952,7 +1951,7 @@ void t_go_generator::generate_service_client(t_service* tservice) {
// Open function
generate_go_docstring(f_types_, (*f_iter));
f_types_ << indent() << "func (p *" << serviceName << "Client) "
<< function_signature_if(*f_iter, "", true, false) << " {" << endl;
<< function_signature_if(*f_iter, "", true) << " {" << endl;
indent_up();
/*
f_types_ <<
Expand Down Expand Up @@ -2160,9 +2159,15 @@ void t_go_generator::generate_service_remote(t_service* tservice) {

string unused_protection;

string ctxPackage = "context";
if (legacy_context_) {
ctxPackage = "golang.org/x/net/context";
}

f_remote << go_autogen_comment();
f_remote << indent() << "package main" << endl << endl;
f_remote << indent() << "import (" << endl;
f_remote << indent() << " \"" << ctxPackage << "\"" << endl;
f_remote << indent() << " \"flag\"" << endl;
f_remote << indent() << " \"fmt\"" << endl;
f_remote << indent() << " \"math\"" << endl;
Expand Down Expand Up @@ -2502,9 +2507,11 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
f_remote << indent() << "fmt.Print(client." << pubName << "(";
bool argFirst = true;

f_remote << "context.Background()";
for (std::vector<t_field*>::size_type i = 0; i < num_args; ++i) {
if (argFirst) {
argFirst = false;
f_remote << ", ";
} else {
f_remote << ", ";
}
Expand Down Expand Up @@ -3434,15 +3441,12 @@ string t_go_generator::function_signature(t_function* tfunction, string prefix)
* Renders an interface function signature of the form 'type name(args)'
*
* @param tfunction Function definition
* @param enableContext Client doesn't suppport context for now.
* @return String of rendered function definition
*/
string t_go_generator::function_signature_if(t_function* tfunction, string prefix, bool addError, bool enableContext) {
string t_go_generator::function_signature_if(t_function* tfunction, string prefix, bool addError) {
// TODO(mcslee): Nitpicky, no ',' if argument_list is empty
string signature = publicize(prefix + tfunction->get_name()) + "(";
if (enableContext) {
signature += "ctx context.Context, ";
}
signature += "ctx context.Context, ";
signature += argument_list(tfunction->get_arglist()) + ") (";
t_type* ret = tfunction->get_returntype();
t_struct* exceptions = tfunction->get_xceptions();
Expand Down
9 changes: 9 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ if test "$with_go" = "yes"; then
if [[ -x "$GO" ]] ; then
AS_IF([test -n "$GO"],[
ax_go_version="1.4"
ax_go17_version="1.7"
AC_MSG_CHECKING([for Go version])
golang_version=`$GO version 2>&1 | $SED -e 's/\(go \)\(version \)\(go\)\(@<:@0-9@:>@.@<:@0-9@:>@.@<:@0-9@:>@\)\(@<:@\*@:>@*\).*/\4/'`
Expand All @@ -410,13 +411,21 @@ if test "$with_go" = "yes"; then
:
have_go="no"
])
AX_COMPARE_VERSION([$golang_version],[lt],[$ax_go17_version],[
:
go_version_lt_17="yes"
],[
:
go_version_lt_17="no"
])
],[
AC_MSG_WARN([could not find Go ])
have_go="no"
])
fi
fi
AM_CONDITIONAL(WITH_GO, [test "$have_go" = "yes"])
AM_CONDITIONAL([GOVERSION_LT_17], [test "$go_version_lt_17" = "yes"])

AX_THRIFT_LIB(rs, [Rust], yes)
have_rs="no"
Expand Down
6 changes: 5 additions & 1 deletion lib/go/test/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
# under the License.
#

if GOVERSION_LT_17
COMPILER_EXTRAFLAG=",legacy_context"
endif

THRIFT = $(top_builddir)/compiler/cpp/thrift
THRIFTARGS = -out gopath/src/ --gen go:thrift_import=thrift,legacy_context
THRIFTARGS = -out gopath/src/ --gen go:thrift_import=thrift$(COMPILER_EXTRAFLAG)
THRIFTTEST = $(top_srcdir)/test/ThriftTest.thrift

# Thrift for GO has problems with complex map keys: THRIFT-2063
Expand Down
12 changes: 6 additions & 6 deletions lib/go/test/tests/client_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func TestClientReportTTransportErrors(t *testing.T) {
return
}
client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, retErr := client.TestStruct(thing)
_, retErr := client.TestStruct(defaultCtx, thing)
mockCtrl.Finish()
err2, ok := retErr.(thrift.TTransportException)
if !ok {
Expand Down Expand Up @@ -444,7 +444,7 @@ func TestClientReportTProtocolErrors(t *testing.T) {
return
}
client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, retErr := client.TestStruct(thing)
_, retErr := client.TestStruct(defaultCtx, thing)
mockCtrl.Finish()
err2, ok := retErr.(thrift.TProtocolException)
if !ok {
Expand Down Expand Up @@ -565,7 +565,7 @@ func TestClientCallException(t *testing.T) {
willComplete := !prepareClientCallException(protocol, i, err)

client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, retErr := client.TestString("test")
_, retErr := client.TestString(defaultCtx, "test")
mockCtrl.Finish()

if !willComplete {
Expand Down Expand Up @@ -608,7 +608,7 @@ func TestClientSeqIdMismatch(t *testing.T) {
)

client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, err := client.TestString("test")
_, err := client.TestString(defaultCtx, "test")
mockCtrl.Finish()
appErr, ok := err.(thrift.TApplicationException)
if !ok {
Expand Down Expand Up @@ -638,7 +638,7 @@ func TestClientWrongMethodName(t *testing.T) {
)

client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, err := client.TestString("test")
_, err := client.TestString(defaultCtx, "test")
mockCtrl.Finish()
appErr, ok := err.(thrift.TApplicationException)
if !ok {
Expand Down Expand Up @@ -668,7 +668,7 @@ func TestClientWrongMessageType(t *testing.T) {
)

client := errortest.NewErrorTestClientProtocol(transport, protocol, protocol)
_, err := client.TestString("test")
_, err := client.TestString(defaultCtx, "test")
mockCtrl.Finish()
appErr, ok := err.(thrift.TApplicationException)
if !ok {
Expand Down
8 changes: 4 additions & 4 deletions lib/go/test/tests/multiplexed_protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func createLegacyClient(t *testing.T) *multiplexedprotocoltest.SecondClient {
}

func TestCallFirst(t *testing.T) {
ret, err := firstClient.ReturnOne()
ret, err := firstClient.ReturnOne(defaultCtx)
if err != nil {
t.Fatal("Unable to call first server:", err)
}
Expand All @@ -113,7 +113,7 @@ func TestCallFirst(t *testing.T) {
}

func TestCallSecond(t *testing.T) {
ret, err := secondClient.ReturnTwo()
ret, err := secondClient.ReturnTwo(defaultCtx)
if err != nil {
t.Fatal("Unable to call second server:", err)
}
Expand All @@ -124,15 +124,15 @@ func TestCallSecond(t *testing.T) {

func TestCallLegacy(t *testing.T) {
legacyClient := createLegacyClient(t)
ret, err := legacyClient.ReturnTwo()
ret, err := legacyClient.ReturnTwo(defaultCtx)
//expect error since default processor is not registered
if err == nil {
t.Fatal("Expecting error")
}
//register default processor and call again
processor.RegisterDefault(multiplexedprotocoltest.NewSecondProcessor(&SecondImpl{}))
legacyClient = createLegacyClient(t)
ret, err = legacyClient.ReturnTwo()
ret, err = legacyClient.ReturnTwo(defaultCtx)
if err != nil {
t.Fatal("Unable to call legacy server:", err)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/go/test/tests/one_way_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func TestInitOnewayClient(t *testing.T) {

func TestCallOnewayServer(t *testing.T) {
//call oneway function
err := client.Hi(1, "")
err := client.Hi(defaultCtx, 1, "")
if err != nil {
t.Fatal("Unexpected error: ", err)
}
//There is no way to detect protocol problems with single oneway call so we call it second time
i, err := client.EchoInt(42)
i, err := client.EchoInt(defaultCtx, 42)
if err != nil {
t.Fatal("Unexpected error: ", err)
}
Expand Down
Loading

0 comments on commit 5c302e0

Please sign in to comment.