Skip to content

GODRIVER-3303 Remove redundant code; Add test cases.#1753

Merged
qingyang-hu merged 5 commits intomongodb:v1from
qingyang-hu:godriver3303
Aug 23, 2024
Merged

GODRIVER-3303 Remove redundant code; Add test cases.#1753
qingyang-hu merged 5 commits intomongodb:v1from
qingyang-hu:godriver3303

Conversation

@qingyang-hu
Copy link
Copy Markdown
Contributor

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 review-priority-low Low Priority PR for Review: within 3 business days label Aug 14, 2024
@mongodb-drivers-pr-bot
Copy link
Copy Markdown
Contributor

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

API Change Report

./x/mongo/driver/wiremessage

incompatible changes

ReadCompressedCompressedMessage: removed

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
Copy Markdown
Contributor 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
identifier, rem, ret, ok := ReadMsgSectionRawDocumentSequence(src)
if !ok {
return "", nil, rem, false
return "", nil, ret, false
Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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
Copy Markdown
Contributor

@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
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

review-priority-low Low Priority PR for Review: within 3 business days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants