Skip to content

PHPC-274: Fix zval_to_bson() encoding of BSON\Serializable objects #40

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

Merged
merged 3 commits into from
May 4, 2015

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Apr 25, 2015


Previously, only BSON\Persistable objects were handled.

@bjori
Copy link
Contributor

bjori commented Apr 26, 2015

It is really difficult to understand the effect of this change in the modified test case.
Can you create a test case that currently fails, and this patch fixes, for clarity?

}
zval_to_bson(retval, flags, &child, NULL TSRMLS_CC);
bson_append_document_end(bson, &child);
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

For PHPC-275: this conditional and the preceding (!obj_data) conditional (when zend_call_method() fails) are the relevant changes for the bson-encode_error-002.phpt test case.

@jmikola
Copy link
Member Author

jmikola commented Apr 26, 2015

The attached bug0274.phpt test is an isolated regression test (extracted from bson-encode-002.phpt).

@jmikola jmikola force-pushed the phpc-274 branch 2 times, most recently from 13bd5ba to 2cc2ecc Compare April 30, 2015 18:13
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2cc2ecc on jmikola:phpc-274 into * on 10gen-labs:master*.

tmp_ht->nApplyCount++;
}
if (Z_TYPE_P(obj_data) != IS_ARRAY) {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "Expected %s() to return an array, %s given", BSON_SERIALIZE_FUNC_NAME, zend_get_type_by_const(Z_TYPE_P(obj_data)));
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be an UnexpectedValueException.

Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to UnexpectedValueException please

Copy link
Member Author

Choose a reason for hiding this comment

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

#43

@bjori
Copy link
Contributor

bjori commented May 4, 2015

lgtm

jmikola added a commit that referenced this pull request May 4, 2015
PHPC-274: Fix zval_to_bson() encoding of BSON\Serializable objects
@jmikola jmikola merged commit ca8ce77 into mongodb:master May 4, 2015
@jmikola jmikola deleted the phpc-274 branch May 4, 2015 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants