-
Notifications
You must be signed in to change notification settings - Fork 909
GODRIVER-3303 Remove redundant code; Add test cases. #1753
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
Conversation
API Change Report./x/mongo/driver/wiremessageincompatible changesReadCompressedCompressedMessage: removed |
@@ -1199,18 +1199,12 @@ func (Operation) decompressWireMessage(wm []byte) (wiremessage.OpCode, []byte, e | |||
if !ok { | |||
return 0, nil, errors.New("malformed OP_COMPRESSED: missing compressor ID") | |||
} | |||
compressedSize := len(wm) - 9 // original opcode (4) + uncompressed size (4) + compressor ID (1) |
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.
We don't have to calculate the length as long as the above ReadCompressedOriginalOpCode
, ReadCompressedUncompressedSize
, and ReadCompressedCompressorID
are correct. compressedSize
must be strictly equal to len(rem)
because it is where rem
comes from. Therefore, the ReadCompressedCompressedMessage
is unnecessary as well.
if !ok { | ||
return "", nil, rem, false | ||
return "", nil, ret, false |
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.
Why return ret
here instead of rem
? Should we always return either src
or []byte{}
in the !ok
condition instead?
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.
The ret
returned by ReadMsgSectionRawDocumentSequence
is the rem
in the original logic. I just kept the returned variable names of ReadMsgSectionRawDocumentSequence
to avoid confusion there. However, you are right that we only need to return an empty slice or even nil on false cases including those underneath.
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.
Looks good! 👍
Co-authored-by: Kobrin Ilay <ilayko3110@yandex.ru> (cherry picked from commit 87ae788)
GODRIVER-3303
Summary
As a follow-up of PR #1735
Background & Motivation