-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Add PSQLBackendMessageEncoder #175
Add PSQLBackendMessageEncoder #175
Conversation
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.
I'm neither approving nor requesting changes for now on this one - I'm not convinced any of my comments are really worth spending effort on, but I'd like to get the sense of your thoughts before moving on.
func encode(data message: PSQLBackendMessage, out buffer: inout ByteBuffer) throws { | ||
switch message { | ||
case .authentication(let authentication): | ||
buffer.writeBackendMessageID(message.id) |
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.
There is nothing specifically wrong with having it this way, but it nags at me to have the same line repeated in (almost) every single case
like this over and over. Is there any not-horrible way to factor it out? (I can't think of something off the top of my head, and it's certainly not worth spending any amount of time on.)
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.
Transformed into one liners.
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.
LGTM!
Motivation
To test a
PSQLChannelHandler
, that uses an internalNIOSingleStepByteToMessageDecoder
, in anEmbeddedChannel
we need to writeInbound bytes. To make this easier, this PR introduces aPSQLBackendMessageEncoder
as a test utilChanges
PSQLBackendMessageEncoder
PSQLBackendMessageEncoder
in authentication testsNote
This pr only changes test code