Skip to content

Commit

Permalink
THRIFT-4914: Make TClient.Call to return the response meta
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fishy committed Jan 22, 2021
1 parent d9fcdd3 commit b4096fc
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 34 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 two values: THeaderMap and error

### Java

Expand All @@ -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, LastResponseHeaders_(), to get the response headers from the last client call (if the transport/protocol used was THeader).

## 0.13.0

Expand Down
34 changes: 27 additions & 7 deletions compiler/cpp/src/thrift/generate/t_go_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2132,8 +2152,8 @@ void t_go_generator::generate_service_client(t_service* tservice) {
}
} else {
// 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;
Expand Down
24 changes: 19 additions & 5 deletions lib/go/thrift/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
6 changes: 3 additions & 3 deletions lib/go/thrift/example_client_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
}
Expand Down
14 changes: 0 additions & 14 deletions lib/go/thrift/header_protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/go/thrift/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions lib/go/thrift/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit b4096fc

Please sign in to comment.