GODRIVER-2835 Change the `bson.ValueReader` input to an `io.Reader`. #1673

qingyang-hu · 2024-06-12T22:00:46Z

GODRIVER-2835

Summary

Renaming:
- NewValueReader --> NewDocumentReader
- NewValueWriter --> NewDocumentWriter
Change the NewDocumentWriter (formerly bson.NewValueReader) input to an io.Reader.

mongodb-drivers-pr-bot · 2024-06-12T22:13:16Z

API Change Report

./bson

incompatible changes

NewBSONValueReader: removed
NewValueReader: removed
NewValueWriter: removed

compatible changes

NewDocumentReader: added
NewDocumentWriter: added

bson/value_reader.go

matthewdale · 2024-07-02T17:02:28Z

bson/value_reader.go

+func NewValueReader(r io.Reader) ValueReader {
+	return newValueReader(r)
 }

 // NewBSONValueReader returns a ValueReader that starts in the Value mode instead of in top
 // level document mode. This enables the creation of a ValueReader for a single BSON value.
-func NewBSONValueReader(t Type, val []byte) ValueReader {
-	stack := make([]vrState, 1, 5)
-	stack[0] = vrState{
-		mode:  mValue,
-		vType: t,
-	}
-	return &valueReader{
-		d:     val,
-		stack: stack,
+func NewBSONValueReader(t Type, r io.Reader) ValueReader {


The naming of these functions is confusing. I see in #1525, NewBSONDocumentReader was changed to NewValueReader. However, a better name seems to be NewDocumentReader, because it reads BSON documents. Then NewBSONValueReader can be renamed to NewValueReader, because it reads BSON values.

matthewdale · 2024-07-02T17:06:45Z

bson/value_reader_test.go

@@ -19,22 +19,22 @@ import (
 	"go.mongodb.org/mongo-driver/x/bsonx/bsoncore"
 )

+var testcstring = append([]byte("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc hendrerit nibh eu lorem scelerisque iaculis. Mauris tincidunt est quis gravida sollicitudin. Vestibulum ante dolor, semper eget finibus accumsan, finibus id nulla. Nam mauris libero, mattis sed libero at, vestibulum vehicula tellus. Nullam in finibus neque. Ut bibendum blandit condimentum. Etiam tortor mauris, tempus et accumsan a, lobortis cursus lacus.  Pellentesque fermentum gravida aliquam. Donec fermentum nisi ut venenatis convallis. Praesent ultrices, justo ac fringilla eleifend, augue libero tristique quam, imperdiet rutrum lectus odio id purus. Curabitur vel metus ullamcorper massa dapibus tincidunt. Nam mollis feugiat sapien ut sagittis. Cras sed ex ligula. Etiam nec nisi diam. Pellentesque id metus molestie, tempus velit et, malesuada eros. Praesent convallis arcu et semper molestie. Morbi eu orci tempor, blandit dui egestas, faucibus diam. Aenean ac tempor justo.  Donec id lectus tristique, ultrices enim sed, suscipit sapien. In arcu mauris, venenatis sed venenatis id, mollis a risus. Phasellus id massa et magna volutpat elementum in quis libero. Aliquam a aliquam erat. Nunc leo massa, sagittis at egestas vel, iaculis vitae quam. Pellentesque placerat metus id velit lobortis euismod in non sapien. Duis a quam sed elit fringilla maximus at et purus. Nullam pharetra accumsan efficitur. Integer a mattis urna. Suspendisse vehicula, nunc euismod luctus suscipit, mi mauris sollicitudin diam, porta rhoncus libero metus vitae erat. Sed lacus sem, feugiat vitae nisi at, malesuada ultricies ipsum. Quisque hendrerit posuere metus. Donec magna erat, facilisis et dictum at, tempor in leo. Maecenas luctus vestibulum quam, eu ornare ex aliquam vitae. Mauris ac mauris posuere, mattis nisl nec, fringilla libero.  Nulla in ipsum ut arcu condimentum mollis. Donec viverra massa nec lacus condimentum vulputate. Nulla at dictum eros, quis sodales ante. Duis condimentum accumsan consectetur. Aenean sodales at turpis vitae efficitur. Vestibulum in diam faucibus, consequat sem ut, auctor lorem. Duis tincidunt non eros pretium rhoncus. Sed quis eros ligula. Donec vulputate ultrices enim, quis interdum dui rhoncus eu. In at mollis ex.  In sed pulvinar risus. Morbi efficitur leo magna, eget bibendum leo consequat id. Pellentesque ultricies ipsum leo, sit amet cursus est bibendum a. Mauris eget porta felis. Vivamus dignissim pellentesque risus eget interdum. Mauris ultrices metus at blandit tincidunt. Duis tempor sapien vel luctus mattis. Vivamus ac orci nibh. Nam eget tempor neque. Proin lacus nibh, porttitor nec pellentesque id, dignissim et eros. Nullam et libero faucibus, iaculis mi sed, faucibus leo. In mollis sem ac porta suscipit.  Ut rutrum, justo a gravida lobortis, neque nibh tincidunt mi, id eleifend dolor dolor vel arcu. Fusce vel egestas ante, eu commodo eros. Donec augue odio, bibendum ut nulla ultricies, cursus eleifend lacus. Nulla viverra ac massa vel placerat. Duis aliquam, ipsum vitae ultricies imperdiet, tellus nisl venenatis mauris, et congue elit nulla vitae est. Suspendisse volutpat ullamcorper diam, et vehicula leo bibendum at. In hac habitasse platea dictumst.  Donec mattis neque a lorem ullamcorper rutrum. Curabitur mattis auctor velit, vitae iaculis mauris convallis in. Donec vulputate sapien non ex pretium semper. Vestibulum ut ligula sit amet arcu pellentesque aliquam. Pellentesque odio diam, pharetra at nunc varius, pharetra consectetur sem. Integer pretium magna pretium, mattis lectus eget, laoreet libero. Morbi dictum et dolor eu finibus. Etiam vestibulum finibus quam, vel eleifend tortor mattis viverra. Donec porttitor est vitae ligula volutpat lobortis. Donec ac semper diam. Maecenas sapien eros, blandit non velit in, faucibus auctor risus. In quam nibh, congue a nisl sit amet, tempor volutpat tortor. Curabitur dignissim auctor orci a varius. Nulla faucibus lacus libero, vitae fringilla elit facilisis id.  Aliquam id elit dui. Cras convallis ligula ac leo bibendum lacinia. Duis interdum ac lectus sed tristique. Maecenas sem magna, gravida quis sapien sit amet, varius luctus ligula. Curabitur eleifend mi nibh. Suspendisse iaculis commodo justo, vitae pretium risus scelerisque non. Sed pulvinar augue nec fermentum feugiat. Nam et ligula tellus. Vestibulum euismod accumsan nibh, at rutrum est tristique sit amet. Duis porttitor ex felis, quis consectetur nunc tempor ut. Nulla vitae consequat velit, id condimentum orci. Sed lacinia velit urna, nec laoreet est varius ac. Integer dapibus libero vel bibendum posuere. Curabitur cursus est vel ante euismod dapibus. Ut hendrerit odio id rhoncus efficitur.  Nam luctus sem orci, in congue ipsum ultrices at. Morbi sed tortor ut metus elementum ultrices. Cras vehicula ante magna, nec faucibus neque placerat et. Vivamus justo lacus, aliquet sit amet semper ac, porta vehicula nibh. Duis et rutrum elit. In nisi eros, fringilla ut odio eget, vehicula laoreet elit. Suspendisse potenti. Vivamus ut ultricies lacus. Integer pellentesque posuere mauris, eget aliquet purus tincidunt in. Suspendisse potenti. Nam quis purus iaculis, cursus mauris a, tempus mi.  Pellentesque ullamcorper lacus vitae lacus volutpat, quis ultricies metus sagittis. Etiam imperdiet libero vitae ante cursus tempus. Nulla eu mi sodales neque scelerisque eleifend id non nisi. Maecenas blandit vitae turpis nec lacinia. Duis posuere cursus metus. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridaculus mus. Ut ultrices hendrerit quam, sit amet condimentum massa finibus a. Donec et vehicula urna. Pellentesque lorem felis, fermentum vel feugiat et, congue eleifend orci. Suspendisse potenti. Sed dignissim massa at justo tristique, id feugiat neque vulputate.  Pellentesque dui massa, maximus quis consequat quis, fringilla vel turpis. Etiam fermentum ex eget massa varius, a fringilla velit placerat. Sed fringilla convallis urna, ut finibus purus accumsan a. Vestibulum porttitor risus lorem, eu venenatis velit suscipit vel. Ut nec diam egestas, sollicitudin nulla sit amet, porttitor felis. Duis a nisl a ante interdum hendrerit. Integer sollicitudin scelerisque ex, et blandit magna blandit at.  Vivamus a vulputate ante. Nam non tortor a lacus euismod venenatis. Vestibulum libero augue, consequat vitae turpis nec, mattis tristique nibh. Fusce pulvinar dolor vel ipsum eleifend varius. Morbi id ante eget tellus venenatis interdum non sit amet ante. Nulla luctus tempor purus, eget ultrices odio varius eget. Duis commodo eros ac molestie fermentum. Praesent vestibulum est eu massa posuere, et fermentum orci tincidunt. Duis dignissim nunc sit amet elit laoreet mollis. Aenean porttitor et nunc vel venenatis. Nunc viverra ligula nec tincidunt vehicula. Pellentesque in magna volutpat, consequat est eget, varius mauris. Maecenas in tellus eros. Aliquam erat volutpat. Phasellus blandit faucibus velit ac placerat.  Donec luctus hendrerit pretium. Sed mauris purus, lobortis non erat sed, mattis ornare nulla. Fusce eu vulputate lacus. In enim justo, elementum at tortor nec, interdum semper ligula. Donec condimentum erat elit, non luctus augue rhoncus et. Quisque interdum elit dui, in vestibulum lacus aliquet et. Mauris aliquam sed ante id eleifend. Donec velit dolor, blandit et mattis non, bibendum at lorem. Nullam blandit quam sapien. Duis rutrum nunc vitae odio imperdiet condimentum. Nunc vel pellentesque purus. Cras iaculis dui est, quis cursus tortor mattis non. Donec non tincidunt lacus.  Sed eget molestie lacus. Phasellus pharetra ullamcorper purus. Sed sit amet ultricies ligula, aliquam elementum velit. Cras commodo mauris vel sapien rutrum, ac pharetra libero mollis. Donec facilisis sit amet elit ac porttitor. Phasellus rutrum rhoncus sagittis. Interdum et malesuada fames ac ante ipsum primis in faucibus. Etiam iaculis ac odio eu sodales.  Proin blandit fermentum arcu efficitur ornare. Vestibulum pharetra est quis mi lobortis interdum. Proin molestie pretium viverra. Integer pellentesque eros nisi, non rutrum odio interdum ut. Quisque vel ante et mi placerat mollis ut eget eros. Etiam vitae orci lectus. Nulla scelerisque dui in dictum ornare. Aliquam vestibulum fringilla eros, id fermentum dolor euismod eget. Ut vitae massa a augue suscipit bibendum non ac mi. Pellentesque id ligula in sapien fermentum fermentum. In ut sem molestie, consectetur ex tristique, tempor risus.  Maecenas scelerisque, ex eget cursus ornare, dolor nisi condimentum tellus, in venenatis nibh elit rutrum turpis. Sed sed vestibulum ex, molestie sodales leo. Vivamus cursus aliquet consequat. Aliquam et enim eget lorem placerat egestas a at justo. Praesent congue vitae purus vel scelerisque. Praesent faucibus massa felis, non porttitor dolor varius at. Nam fringilla risus sit amet faucibus vestibulum. Aliquam rhoncus ex vel magna blandit, eu dapibus felis tristique. Nam dignissim vestibulum neque vitae suscipit. Nunc a pharetra dui. Etiam nec quam sed mauris pharetra finibus in a tellus. Ut vehicula molestie lectus in pretium. Donec sit amet dui purus.  Nunc in vestibulum sapien. Donec elit quam, mollis luctus gravida ac, ullamcorper quis urna. Vivamus a urna egestas velit tempor interdum non eget dui. Maecenas maximus diam at consequat dictum. Etiam sed metus quis enim faucibus cursus condimentum ut nisi. In et iaculis odio. Curabitur sollicitudin ultrices finibus. Aliquam et nisi porta, vehicula urna id, dictum turpis. Sed id iaculis justo, non semper metus.  Quisque euismod, tellus iaculis sagittis vestibulum, leo magna blandit felis, non pharetra velit lacus sed nunc. Curabitur mollis porttitor odio, sed feugiat leo rhoncus quis. Duis faucibus tellus id venenatis vestibulum. Duis interdum pretium cursus. Integer sed iaculis mi. Phasellus at odio at felis fermentum congue. Morbi at ante ut lacus posuere accumsan quis in orci. Nullam eget sapien eu nibh venenatis malesuada.  Nulla sed ligula et metus mattis placerat sed eget nisl. Nunc cursus et nulla id dictum. Vivamus efficitur aliquam. "), []byte{0x00}...)


Optional: Consider moving this text into a txt file in a testdata directory and loading it using the "embed" package.

matthewdale · 2024-07-02T17:51:54Z

bson/decoder_test.go

We should add a test where we provide a reader that returns a stream of BSON documents, since that's one of the motivations for changing the input to an io.Reader.

Added in e0d6552

matthewdale · 2024-07-02T18:12:08Z

bson/value_reader.go

+func (vr *valueReader) prepload(length int32) (int32, error) {
+	const chunckSize = 512


Optional: There are a some simplifications that would make this method mostly unnecessary:

The functionality of prepload has a lot of overlap with the bufio.Reader type. Consider refactoring valueReader to use methods provided by a bufio.Reader instead.

The current code won't actually reduce memory usage by reading the document incrementally because we're still appending every byte read to the slice vr.d. A simpler approach would be to use ReadDocument to read the entire next BSON document from the input stream anytime we reach the end of vr.d, allowing the rest of the code to mostly stay the same.
For example:

func (vr *valueReader) prepload() error { if offset < len(vr.d) return nil } doc, err := ReadDocument(vr.r) if err != nil { return err } vr.d = doc return nil }

vr.d kept in this PR is primarily for the compatibility of newValueReaderFromSlice, which is only used by three deprecated methods in unmarshal.go. I'm converting this PR to a draft and will update the implementation after merging PR #1698 where all deprecated code is removed.

I will refine the implementation in a following PR.

qingyang-hu · 2024-07-06T15:06:06Z

bson/mgoregistry_test.go

 	if err != nil {
 		return err
 	}
-	vr := bson.NewBSONValueReader(raw.Type, raw.Value)
-	err = decoder.DecodeValue(bson.DecodeContext{Registry: Registry}, vr, rval)
+	vr := newValueReader(raw.Type, bytes.NewReader(raw.Value))


Moved this file to /bson so we don't have to export NewBSONValueReader.

qingyang-hu · 2024-07-25T18:12:00Z

.pre-commit-config.yaml

Skip spell checking for "lorem.txt"

matthewdale

Looks good! 👍

mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Jun 12, 2024

qingyang-hu force-pushed the godriver2835 branch 3 times, most recently from 2d9424f to 1585fdd Compare June 14, 2024 20:52

GODRIVER-2835 Change the bson.ValueReader input to an io.Reader.

40ef227

qingyang-hu force-pushed the godriver2835 branch from 1585fdd to 40ef227 Compare June 15, 2024 02:19

fix

5971ced

qingyang-hu force-pushed the godriver2835 branch from 475c3ea to 5971ced Compare June 15, 2024 04:26

update tests

ccee970

qingyang-hu commented Jun 17, 2024

View reviewed changes

bson/value_reader.go Outdated Show resolved Hide resolved

qingyang-hu marked this pull request as ready for review June 17, 2024 18:52

qingyang-hu requested review from prestonvasquez and matthewdale June 17, 2024 18:52

matthewdale requested changes Jul 2, 2024

View reviewed changes

Merge branch 'master' into godriver2835

cc56c27

qingyang-hu marked this pull request as draft July 5, 2024 21:21

qingyang-hu commented Jul 6, 2024

View reviewed changes

Rename NewValueReader and NewValueWriter.

cd4de94

qingyang-hu force-pushed the godriver2835 branch from d584695 to cd4de94 Compare July 6, 2024 15:14

Merge branch 'master' into godriver2835

a183c57

qingyang-hu force-pushed the godriver2835 branch 2 times, most recently from 971d6e4 to 0ab045a Compare July 19, 2024 19:19

update pre commit config to skip lorem.txt

15201d4

qingyang-hu force-pushed the godriver2835 branch from 0ab045a to 15201d4 Compare July 19, 2024 19:24

qingyang-hu added 3 commits July 19, 2024 16:32

minor fix

19c853e

update stream test

e0d6552

Merge branch 'master' into godriver2835

b7306c5

qingyang-hu commented Jul 25, 2024

View reviewed changes

.pre-commit-config.yaml Outdated

Copy link

Collaborator Author

qingyang-hu Jul 25, 2024

Choose a reason for hiding this comment

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

Skip spell checking for "lorem.txt"

qingyang-hu marked this pull request as ready for review July 25, 2024 18:28

qingyang-hu requested a review from matthewdale July 25, 2024 18:28

qingyang-hu added priority-2-medium Medium Priority PR for Review and removed priority-3-low Low Priority PR for Review labels Jul 25, 2024

matthewdale previously approved these changes Jul 26, 2024

View reviewed changes

Merge branch 'master' into godriver2835

4ffb40f

qingyang-hu dismissed matthewdale’s stale review via 4ffb40f July 26, 2024 15:12

matthewdale approved these changes Jul 26, 2024

View reviewed changes

prestonvasquez approved these changes Jul 26, 2024

View reviewed changes

qingyang-hu merged commit 060b2b2 into mongodb:master Jul 26, 2024
28 of 33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GODRIVER-2835 Change the `bson.ValueReader` input to an `io.Reader`. #1673

GODRIVER-2835 Change the `bson.ValueReader` input to an `io.Reader`. #1673

qingyang-hu commented Jun 12, 2024 •

edited

Loading

mongodb-drivers-pr-bot bot commented Jun 12, 2024 •

edited

Loading

matthewdale Jul 2, 2024

matthewdale Jul 2, 2024

matthewdale Jul 2, 2024

qingyang-hu Jul 25, 2024

matthewdale Jul 2, 2024

qingyang-hu Jul 6, 2024

qingyang-hu Jul 25, 2024

qingyang-hu Jul 6, 2024

qingyang-hu Jul 25, 2024

matthewdale left a comment

		func (vr *valueReader) prepload(length int32) (int32, error) {
		const chunckSize = 512

GODRIVER-2835 Change the bson.ValueReader input to an io.Reader. #1673

GODRIVER-2835 Change the bson.ValueReader input to an io.Reader. #1673

Conversation

qingyang-hu commented Jun 12, 2024 • edited Loading

Summary

mongodb-drivers-pr-bot bot commented Jun 12, 2024 • edited Loading

API Change Report

./bson

incompatible changes

compatible changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewdale left a comment

Choose a reason for hiding this comment

GODRIVER-2835 Change the `bson.ValueReader` input to an `io.Reader`. #1673

GODRIVER-2835 Change the `bson.ValueReader` input to an `io.Reader`. #1673

qingyang-hu commented Jun 12, 2024 •

edited

Loading

mongodb-drivers-pr-bot bot commented Jun 12, 2024 •

edited

Loading