PHPC-635: WriteResult debug handler should return objects #472
Conversation
jmikola
changed the title
| uint32_t len; | ||
| const uint8_t *data = NULL; | ||
|
|
||
| bson_iter_document(&iter, &len, &data); |
There was a problem hiding this comment.
BSON_ITER_HOLDS_DOCUMENT() and bson_iter_document() is consistent with what we were already doing in phongo_writeconcernerror_init() for a very similar routine. This replaces the previous bson_append_iter() code path, which is much less common.
|
|
||
| bson_iter_document(&iter, &len, &data); | ||
|
|
||
| if (!data) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
With BSON_ITER_HOLDS_DOCUMENT() guarding this block, bson_iter_document() will always populate the document's data. This removes an odd case where we might have returned false without throwing. If the data is somehow null after this (I can't imagine how), phongo_bson_to_zval() will catch and throw an error when initializing the BSON reader.
| return false; | ||
| } | ||
|
|
||
| ZVAL_ZVAL(return_value, writeconcernerror, 1, 1); |
There was a problem hiding this comment.
I ran into trouble trying to use zval_ptr_dtor() on the input argument so it seemed more straightforward to create my own zval in this function and copy it into the return value on success. For any failure case, we clean up our internally allocated zval and leave the return value initialized to null.
| @@ -50,6 +50,108 @@ PHONGO_API zend_class_entry *php_phongo_writeresult_ce; | |||
|
|
|||
| zend_object_handlers php_phongo_handler_writeresult; | |||
|
|
|||
| static bool php_phongo_writeresult_get_writeconcernerror(php_phongo_writeresult_t *intern, zval *return_value TSRMLS_DC) | |||
There was a problem hiding this comment.
Although these two functions return a boolean indicating their success, we don't actually check for it later. The return value is always initialized to something usable (e.g. empty array or null), so we add it to the debug info's hash regardless. If an exception was thrown, it will be caught above the debug handler (technically get_properties).
|
@derickr: We didn't want to stuff this into 1.2.0 at the last minute, but I believe it's worth revisiting for inclusion in a future release. Please review when you get a chance. |
https://jira.mongodb.org/browse/PHPC-635