Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
witriew committed Aug 25, 2021
1 parent 185111e commit e391075
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 25 deletions.
8 changes: 4 additions & 4 deletions encoding/thrift/envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type disableEnvelopingNoWireProtocol struct {
}

func (evsp disableEnvelopingNoWireProtocol) Reader(r io.Reader) stream.Reader {
return disableEnvelopingReader{
return disableEnvelopingNoWireReader{
Reader: evsp.Protocol.Reader(r),
Type: evsp.Type,
}
Expand All @@ -76,21 +76,21 @@ func (evsp disableEnvelopingNoWireProtocol) Writer(w io.Writer) stream.Writer {
}
}

type disableEnvelopingReader struct {
type disableEnvelopingNoWireReader struct {
stream.Reader

Type wire.EnvelopeType
}

func (evr disableEnvelopingReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
func (evr disableEnvelopingNoWireReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
return stream.EnvelopeHeader{
Name: "",
Type: evr.Type,
SeqID: 1,
}, nil
}

func (evr disableEnvelopingReader) ReadEnvelopeEnd() error { return nil }
func (evr disableEnvelopingNoWireReader) ReadEnvelopeEnd() error { return nil }

type disableEnvelopingWriter struct {
stream.Writer
Expand Down
12 changes: 6 additions & 6 deletions encoding/thrift/multiplex.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,37 +57,37 @@ type multiplexedOutboundNoWireProtocol struct {
}

func (m multiplexedOutboundNoWireProtocol) Writer(w io.Writer) stream.Writer {
return multiplexedWriter{
return multiplexedNoWireWriter{
Writer: m.Protocol.Writer(w),
Service: m.Service,
}
}

func (m multiplexedOutboundNoWireProtocol) Reader(r io.Reader) stream.Reader {
return multiplexedReader{
return multiplexedNoWireReader{
Reader: m.Protocol.Reader(r),
Service: m.Service,
}
}

type multiplexedWriter struct {
type multiplexedNoWireWriter struct {
stream.Writer

Service string
}

func (w multiplexedWriter) WriteEnvelopeBegin(eh stream.EnvelopeHeader) error {
func (w multiplexedNoWireWriter) WriteEnvelopeBegin(eh stream.EnvelopeHeader) error {
eh.Name = w.Service + ":" + eh.Name
return w.Writer.WriteEnvelopeBegin(eh)
}

type multiplexedReader struct {
type multiplexedNoWireReader struct {
stream.Reader

Service string
}

func (r multiplexedReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
func (r multiplexedNoWireReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
eh, err := r.Reader.ReadEnvelopeBegin()
eh.Name = strings.TrimPrefix(eh.Name, r.Service+":")
return eh, err
Expand Down
20 changes: 11 additions & 9 deletions encoding/thrift/outbound_nowire.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import (
)

// NoWireClient is a generic Thrift client for encoding/decoding using
// ThriftRW's "streaming" mechanisms.
// ThriftRW's "streaming" mechanisms. The body of the request is expected to be
// written out of through a `reqBody` (its `Encode`), while the response is
// expected to be read out through `resBody` (its `Decode`).
// It speaks in raw Thrift payloads.
//
// Users should use the client generated by the code generator rather than
Expand Down Expand Up @@ -127,7 +129,7 @@ func (c noWireThriftClient) Call(ctx context.Context, reqBody stream.Enveloper,

out := c.cc.GetUnaryOutbound()

treq, proto, err := c.writeTransportRequest(reqBody)
treq, proto, err := c.buildTransportRequest(reqBody)
if err != nil {
return err
}
Expand All @@ -144,27 +146,27 @@ func (c noWireThriftClient) Call(ctx context.Context, reqBody stream.Enveloper,
}
defer tres.Body.Close()

if _, err = call.ReadFromResponse(ctx, tres); err != nil {
if _, err := call.ReadFromResponse(ctx, tres); err != nil {
return err
}

sr := proto.Reader(tres.Body)
defer sr.Close()

env, err := sr.ReadEnvelopeBegin()
envelope, err := sr.ReadEnvelopeBegin()
if err != nil {
return errors.ResponseBodyDecodeError(treq, err)
}

switch env.Type {
switch envelope.Type {
case wire.Reply:
if err := resBody.Decode(sr); err != nil {
return err
}
return sr.ReadEnvelopeEnd()
case wire.Exception:
var exc internal.TApplicationException
if err = exc.Decode(sr); err != nil {
if err := exc.Decode(sr); err != nil {
return errors.ResponseBodyDecodeError(treq, err)
}
return thriftException{
Expand All @@ -174,14 +176,14 @@ func (c noWireThriftClient) Call(ctx context.Context, reqBody stream.Enveloper,
}
default:
return errors.ResponseBodyDecodeError(
treq, errUnexpectedEnvelopeType(env.Type))
treq, errUnexpectedEnvelopeType(envelope.Type))
}
}

func (c noWireThriftClient) CallOneway(ctx context.Context, reqBody stream.Enveloper, opts ...yarpc.CallOption) (transport.Ack, error) {
out := c.cc.GetOnewayOutbound()

treq, _, err := c.writeTransportRequest(reqBody)
treq, _, err := c.buildTransportRequest(reqBody)
if err != nil {
return nil, err
}
Expand All @@ -199,7 +201,7 @@ func (c noWireThriftClient) Enabled() bool {
return c.NoWire
}

func (c noWireThriftClient) writeTransportRequest(reqBody stream.Enveloper) (*transport.Request, stream.Protocol, error) {
func (c noWireThriftClient) buildTransportRequest(reqBody stream.Enveloper) (*transport.Request, stream.Protocol, error) {
proto := c.p
if !c.Enveloping {
proto = disableEnvelopingNoWireProtocol{
Expand Down
6 changes: 0 additions & 6 deletions encoding/thrift/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ type UnaryHandler func(context.Context, wire.Value) (Response, error)
// OnewayHandler is a convenience type alias for functions that act as OnewayHandlers.
type OnewayHandler func(context.Context, wire.Value) error

// UnaryStreamHandler is a convenience type alias for functions that act as StreamHandlers.
type UnaryStreamHandler func(context.Context, stream.Reader, stream.Writer) (Response, error)

// OnewayStreamHandler is a convenience type alias for functions that act as OnewayStreamHandlers.
type OnewayStreamHandler func(context.Context, stream.Reader) error

// HandlerSpec represents the handler behind a Thrift service method.
type HandlerSpec struct {
Type transport.Type
Expand Down

0 comments on commit e391075

Please sign in to comment.