Skip to content

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

Merged
merged 5 commits into from
Aug 23, 2024

Conversation

qingyang-hu
Copy link
Collaborator

GODRIVER-3303

Summary

As a follow-up of PR #1735

  • Remove redundant code
  • Add test cases

Background & Motivation

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Aug 14, 2024
Copy link
Contributor

mongodb-drivers-pr-bot bot commented Aug 14, 2024

API Change Report

./x/mongo/driver/wiremessage

incompatible changes

ReadCompressedCompressedMessage: 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)
Copy link
Collaborator Author

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.

@qingyang-hu qingyang-hu marked this pull request as ready for review August 14, 2024 14:59
if !ok {
return "", nil, rem, false
return "", nil, ret, false
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@qingyang-hu qingyang-hu merged commit 87ae788 into mongodb:v1 Aug 23, 2024
31 of 33 checks passed
blink1073 pushed a commit to blink1073/mongo-go-driver that referenced this pull request Aug 23, 2024
Co-authored-by: Kobrin Ilay <ilayko3110@yandex.ru>
(cherry picked from commit 87ae788)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low Low Priority PR for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants