This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new c2ddaf0 THRIFT-4914: Make TClient.Call to return the response meta
c2ddaf0 is described below
commit c2ddaf0766499ab522cb7c0ca011d579707fcb5f
Author: Yuxuan 'fishy' Wang <[email protected]>
AuthorDate: Fri Jan 22 09:37:18 2021 -0800
THRIFT-4914: Make TClient.Call to return the response meta
Client: go
Make a breaking change so that TClient.Call returns the response
meta, currently only contains headers but could be expanded in the
future, and make a compiler change to compiler generated clients to take
advantage of that and provide access to response metadata to users.
---
CHANGES.md | 2 ++
compiler/cpp/src/thrift/generate/t_go_generator.cc | 38 ++++++++++++++++++----
lib/go/thrift/client.go | 24 +++++++++++---
lib/go/thrift/example_client_middleware_test.go | 6 ++--
lib/go/thrift/header_protocol.go | 14 --------
lib/go/thrift/middleware.go | 4 +--
lib/go/thrift/middleware_test.go | 6 ++--
7 files changed, 60 insertions(+), 34 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 8e4d08e..c6c08f2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -18,6 +18,7 @@
- [THRIFT-5152](https://issues.apache.org/jira/browse/THRIFT-5152) - go:
TSocket and TSSLSocket now have separated connect timeout and socket timeout
- c++: dropped support for Windows XP
- [THRIFT-5326](https://issues.apache.org/jira/browse/THRIFT-5326) - go:
TException interface now has a new function: TExceptionType
+- [THRIFT-4914](https://issues.apache.org/jira/browse/THRIFT-4914) - go:
TClient.Call now returns ResponseMeta in addition to error
### Java
@@ -31,6 +32,7 @@
- [THRIFT-5233](https://issues.apache.org/jira/browse/THRIFT-5233) - Add
context deadline check to ReadMessageBegin in TBinaryProtocol,
TCompactProtocol, and THeaderProtocol.
- [THRIFT-5240](https://issues.apache.org/jira/browse/THRIFT-5240) - The
context passed into server handler implementations will be canceled when we
detected that the client closed the connection.
- [THRIFT-5322](https://issues.apache.org/jira/browse/THRIFT-5322) - Add
support to TConfiguration, and also fix a bug that could cause excessive memory
usage when reading malformed messages from TCompactProtocol.
+- [THRIFT-4914](https://issues.apache.org/jira/browse/THRIFT-4914) - Compiler
generated service clients now provide a new function, LastResponseMeta_(), to
get the response metadata (e.g. headers from THeader) from the last client call.
## 0.13.0
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc
b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index 49d8bc1..33cd12d 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -1493,11 +1493,15 @@ void
t_go_generator::generate_go_struct_definition(ostream& out,
if (is_exception) {
out << indent() << "func (p *" << tstruct_name << ") Error() string {" <<
endl;
- out << indent() << indent() << "return p.String()" << endl;
+ indent_up();
+ out << indent() << "return p.String()" << endl;
+ indent_down();
out << indent() << "}" << endl << endl;
out << indent() << "func (" << tstruct_name << ") TExceptionType()
thrift.TExceptionType {" << endl;
- out << indent() << indent() << "return thrift.TExceptionTypeCompiled" <<
endl;
+ indent_up();
+ out << indent() << "return thrift.TExceptionTypeCompiled" << endl;
+ indent_down();
out << indent() << "}" << endl << endl;
out << indent() << "var _ thrift.TException = (*" << tstruct_name <<
")(nil)"
@@ -1990,6 +1994,7 @@ void t_go_generator::generate_service_client(t_service*
tservice) {
f_types_ << indent() << "*" << extends_client << endl;
} else {
f_types_ << indent() << "c thrift.TClient" << endl;
+ f_types_ << indent() << "meta thrift.ResponseMeta" << endl;
}
indent_down();
@@ -2059,7 +2064,19 @@ void t_go_generator::generate_service_client(t_service*
tservice) {
indent_up();
f_types_ << indent() << "return p.c" << endl;
indent_down();
- f_types_ << indent() << "}" << endl;
+ f_types_ << indent() << "}" << endl << endl;
+
+ f_types_ << indent() << "func (p *" << serviceName << "Client)
LastResponseMeta_() thrift.ResponseMeta {" << endl;
+ indent_up();
+ f_types_ << indent() << "return p.meta" << endl;
+ indent_down();
+ f_types_ << indent() << "}" << endl << endl;
+
+ f_types_ << indent() << "func (p *" << serviceName << "Client)
SetLastResponseMeta_(meta thrift.ResponseMeta) {" << endl;
+ indent_up();
+ f_types_ << indent() << "p.meta = meta" << endl;
+ indent_down();
+ f_types_ << indent() << "}" << endl << endl;
}
// Generate client method implementations
@@ -2091,8 +2108,11 @@ void t_go_generator::generate_service_client(t_service*
tservice) {
std::string resultName = tmp("_result");
std::string resultType = publicize(method + "_result", true);
f_types_ << indent() << "var " << resultName << " " << resultType <<
endl;
- f_types_ << indent() << "if err = p.Client_().Call(ctx, \""
- << method << "\", &" << argsName << ", &" << resultName << "); err !=
nil {" << endl;
+ f_types_ << indent() << "var meta thrift.ResponseMeta" << endl;
+ f_types_ << indent() << "meta, err = p.Client_().Call(ctx, \""
+ << method << "\", &" << argsName << ", &" << resultName << ")" << endl;
+ f_types_ << indent() << "p.SetLastResponseMeta_(meta)" << endl;
+ f_types_ << indent() << "if err != nil {" << endl;
indent_up();
f_types_ << indent() << "return" << endl;
@@ -2131,9 +2151,13 @@ void t_go_generator::generate_service_client(t_service*
tservice) {
f_types_ << indent() << "return nil" << endl;
}
} else {
+ // Since we don't have response meta for oneway calls, overwrite it with
+ // an empty one to avoid users getting the meta from last call and
+ // mistaken it as from the oneway call.
+ f_types_ << indent() << "p.SetLastResponseMeta_(thrift.ResponseMeta{})"
<< endl;
// TODO: would be nice to not to duplicate the call generation
- f_types_ << indent() << "if err := p.Client_().Call(ctx, \""
- << method << "\", &"<< argsName << ", nil); err != nil {" << endl;
+ f_types_ << indent() << "if _, err := p.Client_().Call(ctx, \""
+ << method << "\", &"<< argsName << ", nil); err != nil {" << endl;
indent_up();
f_types_ << indent() << "return err" << endl;
diff --git a/lib/go/thrift/client.go b/lib/go/thrift/client.go
index 1c5705d..ea2c01f 100644
--- a/lib/go/thrift/client.go
+++ b/lib/go/thrift/client.go
@@ -5,8 +5,15 @@ import (
"fmt"
)
+// ResponseMeta represents the metadata attached to the response.
+type ResponseMeta struct {
+ // The headers in the response, if any.
+ // If the underlying transport/protocol is not THeader, this will
always be nil.
+ Headers THeaderMap
+}
+
type TClient interface {
- Call(ctx context.Context, method string, args, result TStruct) error
+ Call(ctx context.Context, method string, args, result TStruct)
(ResponseMeta, error)
}
type TStandardClient struct {
@@ -78,18 +85,25 @@ func (p *TStandardClient) Recv(ctx context.Context, iprot
TProtocol, seqId int32
return iprot.ReadMessageEnd(ctx)
}
-func (p *TStandardClient) Call(ctx context.Context, method string, args,
result TStruct) error {
+func (p *TStandardClient) Call(ctx context.Context, method string, args,
result TStruct) (ResponseMeta, error) {
p.seqId++
seqId := p.seqId
if err := p.Send(ctx, p.oprot, seqId, method, args); err != nil {
- return err
+ return ResponseMeta{}, err
}
// method is oneway
if result == nil {
- return nil
+ return ResponseMeta{}, nil
}
- return p.Recv(ctx, p.iprot, seqId, method, result)
+ err := p.Recv(ctx, p.iprot, seqId, method, result)
+ var headers THeaderMap
+ if hp, ok := p.iprot.(*THeaderProtocol); ok {
+ headers = hp.transport.readHeaders
+ }
+ return ResponseMeta{
+ Headers: headers,
+ }, err
}
diff --git a/lib/go/thrift/example_client_middleware_test.go
b/lib/go/thrift/example_client_middleware_test.go
index 8a29083..a68cdbc 100644
--- a/lib/go/thrift/example_client_middleware_test.go
+++ b/lib/go/thrift/example_client_middleware_test.go
@@ -45,16 +45,16 @@ func NewMyServiceClient(_ TClient) MyService {
func simpleClientLoggingMiddleware(next TClient) TClient {
return WrappedTClient{
- Wrapped: func(ctx context.Context, method string, args, result
TStruct) error {
+ Wrapped: func(ctx context.Context, method string, args, result
TStruct) (ResponseMeta, error) {
log.Printf("Before: %q", method)
log.Printf("Args: %#v", args)
- err := next.Call(ctx, method, args, result)
+ headers, err := next.Call(ctx, method, args, result)
log.Printf("After: %q", method)
log.Printf("Result: %#v", result)
if err != nil {
log.Printf("Error: %v", err)
}
- return err
+ return headers, err
},
}
}
diff --git a/lib/go/thrift/header_protocol.go b/lib/go/thrift/header_protocol.go
index 35e0458..878041f 100644
--- a/lib/go/thrift/header_protocol.go
+++ b/lib/go/thrift/header_protocol.go
@@ -345,20 +345,6 @@ func (p *THeaderProtocol) SetTConfiguration(cfg
*TConfiguration) {
p.cfg = cfg
}
-// GetResponseHeadersFromClient is a helper function to get the read THeaderMap
-// from the last response received from the given client.
-//
-// If the last response was not sent over THeader protocol,
-// a nil map will be returned.
-func GetResponseHeadersFromClient(c TClient) THeaderMap {
- if sc, ok := c.(*TStandardClient); ok {
- if hp, ok := sc.iprot.(*THeaderProtocol); ok {
- return hp.transport.readHeaders
- }
- }
- return nil
-}
-
var (
_ TConfigurationSetter = (*tHeaderProtocolFactory)(nil)
_ TConfigurationSetter = (*THeaderProtocol)(nil)
diff --git a/lib/go/thrift/middleware.go b/lib/go/thrift/middleware.go
index b575e16..8a788df 100644
--- a/lib/go/thrift/middleware.go
+++ b/lib/go/thrift/middleware.go
@@ -78,11 +78,11 @@ type ClientMiddleware func(TClient) TClient
//
// This is provided to aid in developing ClientMiddleware.
type WrappedTClient struct {
- Wrapped func(ctx context.Context, method string, args, result TStruct)
error
+ Wrapped func(ctx context.Context, method string, args, result TStruct)
(ResponseMeta, error)
}
// Call implements the TClient interface by calling and returning c.Wrapped.
-func (c WrappedTClient) Call(ctx context.Context, method string, args, result
TStruct) error {
+func (c WrappedTClient) Call(ctx context.Context, method string, args, result
TStruct) (ResponseMeta, error) {
return c.Wrapped(ctx, method, args, result)
}
diff --git a/lib/go/thrift/middleware_test.go b/lib/go/thrift/middleware_test.go
index 2a4d1f9..6c4058f 100644
--- a/lib/go/thrift/middleware_test.go
+++ b/lib/go/thrift/middleware_test.go
@@ -54,7 +54,7 @@ func testProcessorMiddleware(c *counter) ProcessorMiddleware {
func testClientMiddleware(c *counter) ClientMiddleware {
return func(next TClient) TClient {
return WrappedTClient{
- Wrapped: func(ctx context.Context, method string, args,
result TStruct) error {
+ Wrapped: func(ctx context.Context, method string, args,
result TStruct) (ResponseMeta, error) {
c.incr()
return next.Call(ctx, method, args, result)
},
@@ -122,8 +122,8 @@ func TestWrapTMultiplexedProcessor(t *testing.T) {
func TestWrapClient(t *testing.T) {
client := WrappedTClient{
- Wrapped: func(ctx context.Context, method string, args, result
TStruct) error {
- return nil
+ Wrapped: func(ctx context.Context, method string, args, result
TStruct) (ResponseMeta, error) {
+ return ResponseMeta{}, nil
},
}
c := newCounter(t)
