-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-1209: Add tutorial to show working with Codecs #1154
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
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.
I think we need an example with a nested document person.city
docs/tutorial/codecs.txt
Outdated
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.
Would performance be better without an intermediate array?
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 BSON document class is currently read-only, so the only way to create one is through fromPHP or the other factory methods. The reason for this is that we only keep a bson_t* internally, which is essentially a byte stream of the BSON. In-place replacement only works for types with the same size, e.g. overwriting an int with another int. As soon as values grow or shrink in size, the BSON needs to be rewritten (i.e. create a new bson_t*, copy over everything except the property being changed, then write the changed property). Since the main motivation for adding the Document and PackedArray classes was to provide a way to access BSON data without decoding it to PHP objects, we refrained from adding a BSON writing mechanism. That said, there is PHPC-2172 which tracks creating an implementation of a BSON writer.
aeec90d to
0037220
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.
LGTM.
87094fe to
36f4c51
Compare
36f4c51 to
d193e7b
Compare
2c7ffb4 to
7123d47
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.
Thinking...
It's 8 times if you call decodeIfPossible as the decode calls canDecode.
Also, we don't know which document will be tested. It's not because this codec works for small documents that this function will not check huge ones.
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.
That is correct. Inspecting large documents will be slow, but since we've removed libraries users are unlikely to run into such scenarios as they'd be using specific codecs for specific fields.
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.
Did you propose to add a method to the trait that wraps this generic 3 lines?
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.
Yes. Will add in a separate PR.
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.
When you get a nested BSON document, does it duplicates the data in memory or is it a reference to the parent data?
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.
It duplicates the data in memory. Each Document and PackedArray instance is tied to its own bson_t* so we don't have to start refcounting the libmongoc BSON structures.
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.
If you set a typemap, does it disable the codec?
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.
Ah, good point. Specifying an operation-level typeMap option also overrides the collection-level codec. I've added a sentence explaining this and updated the example to show both ways.
docs/tutorial/codecs.txt
Outdated
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.
docs/tutorial/codecs.txt
Outdated
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.
Enjoy this gem I came across while googling for "POPOs": http://www.javaleaks.org/open-source/php/plain-old-php-object.html
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.
You'll have an army of lawyers suing you for making POPOs.
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.
Haha. Leaving this unresolved so that someone can eventually stumble upon this while cave diving into old pull requests.
docs/tutorial/codecs.txt
Outdated
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 drops the colon but should explain the entire example you're about to present. I think it's worth describing the encode and decode steps in prose here, as the remainder of the section is pretty light on words.
We still exclude psalm from these examples to avoid littering the code with assertions and annotations
7123d47 to
e1f63ed
Compare
PHPLIB-1209
The tutorial uses the same classes and functionality we show in other BSON tutorials, namely the
Personclass to show serialisation (this is used to show the behaviour ofPersistableandSerializableinterfaces), and our example of storing a date asUTCDateTimewith timezone information.