-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2/4 tchannel prep] Extracted response writer to dedicated file. #2271
[2/4 tchannel prep] Extracted response writer to dedicated file. #2271
Conversation
…ype and simplified interface
// | ||
// It allows us to control handlerWriter during testing. | ||
// responseWriter enhances transport.ResponseWriter interface with transport specific | ||
// methods. | ||
type responseWriter interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Struct that implements this interface was implicitly used as transport.ResponseWriter.
I'm making this obvious: implementation if used as transport.ResponseWriter in a first place. Other additional functions exist only for make it easier to write unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AddHeaders, SetApplicationError and Write are part of the transport.ResponseWriter interface
type responseWriter interface { | ||
AddHeaders(h transport.Headers) | ||
AddHeader(key string, value string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to AddSystemHeader to increase visibility and stress difference between "AddHeaders" and "AddHeader": AddSystemHeader doesn't have names validation.
transport.ResponseWriter | ||
|
||
AddSystemHeader(key string, value string) | ||
Send() error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed Close to Send, because it's very surprising to find data sending logic in "Close" function.
@@ -251,119 +246,6 @@ func (h handler) callHandler(ctx context.Context, call inboundCall, responseWrit | |||
} | |||
} | |||
|
|||
type handlerWriter struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted to dedicated file.
|
||
type responseWriterConstructor func(inboundCallResponse, tchannel.Format, headerCase) responseWriter | ||
|
||
type responseWriterImpl struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed "handlerWriter" to "responseWriter" as this name better reflects the point of its existence.
Extracted response writer to dedicated file, introduced constructor type and simplified interface.
This PR is part of header handling changes (#2265). This refactoring was extracted from tchannel PR to make review easier.