-
Notifications
You must be signed in to change notification settings - Fork 211
BSON class implementations #1387
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
BSON class implementations #1387
Conversation
c4c86f6 to
fa126cb
Compare
89fc705 to
32f9a56
Compare
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.
This completes my first pass through all of the PR. Two additional notes that I couldn't leave as inline comments follow.
I saw that tests/bson/bson-document-getIterator-001.phpt was rendered as a binary file because you included the raw debug output for a Document (string(18) "��foo�bar") instead of using a wildcard pattern. That was likely an oversight you can fix.
Separately, this PR reminded me of PHPC-889. PHPC previously decoded a Javascript scope lazily when it was accessed, which deferred decoding errors. With that change (in 1.7.0), the scope bson_t was converted to a zval when the Javascript instance was created. I think that's fine to leave as-is because Javascript scopes are deprecated and aren't a performance concern -- but I wanted to make a note of it since it's somewhat related to the work we're doing in this PR re: lazy BSON access.
The previous method relied on the type system of PHP to throw an exception when accessing the key of an exhausted iterator, but this does not work on non-debug versions of PHP.
afd3feb to
73f444f
Compare
73f444f to
52c6c72
Compare
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.
Uh oh!
There was an error while loading. Please reload this page.
PHPC-326, PHPC-324
This PR adds new classes to represent BSON arrays and documents to the
MongoDB\BSONnamespace. Both classes are backed by abson_tand are currently read-only. The interfaces for both classes differ a bit, mainly due to constraints in how libbson handlesbson_t:Documentcan be constructed from a JSON string, butArrayListcan't as libbson doesn't offer any functionality for that.Documenthas accessors to get canonical and extended JSON strings for a document, butArrayListdoesn't as libbson doesn't support that.To iterate a BSON structure, the new
Iteratorclass was introduced. Again, this class can't be instantiated directly (mainly due to ZPP not handling union types properly), but will be returned as part of the above classes' implementation ofIteratorAggregate.To receive BSON instances, the new typeMap option
bsonwas introduced. It can be used on the root level, but also for thedocumentandarraytypes, as well as in field path mappings for specific fields. The most common use case will be forrootto be a BSON structure. AnIteratorinstance will always return a literal value (for strings, numbers, etc.), a BSON type (for object IDs, binary objects, etc.), or anotherDocumentorArrayListinstance depending when iterating.Note that the introduction of the
Documentclass makes theMongoDB\BSON\toPHPfunctions (and everything related to them) obsolete. I would suggest deprecating those in a separate pull request once this is merged.Todo:
bson_twhile there's still anIteratorinstance around)public function get(string $key)in theDocumentclass and equivalent inArrayList)get()in BSON classes to throw on non-existent keys (OutOfBoundsExceptionvs.RuntimeException)ArrayList::get()Document::get()has()/get()callsIterator::key(): split tests between PHP 7 (returnsnull) and PHP 8 (throwsTypeError)bson_validate_utf8()before returning themroottype map key to bearrayinArrayList::toPHP()lengthfield in serialised outputodmfield toodm_ceinphp_phongo_bson_stateclassnamevariable totypeinphp_phongo_bson_state_parse_typeArrayListDocumentstdClassObjectIdDocument::fromBSON()tests with better explanations__set_state(),__unserialize(), andunserialize()Iteratorinterface)