Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add missing hasOneof method to check presence of oneof fields #8003

Merged
merged 2 commits into from Nov 4, 2020
Merged

add missing hasOneof method to check presence of oneof fields #8003

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
913e1ed
add missing has<Field>() method for oneof fields
anight Nov 3, 2020
52d3be3
added tests for hasOneof<Field>() php methods
anight Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions php/ext/google/protobuf/message.c
View file
Open in desktop
Expand Up @@ -904,6 +904,39 @@ PHP_METHOD(Message, whichOneof) {
RETURN_STRING(field ? upb_fielddef_name(field) : "");
}

/**
* Message::hasOneof()
*
* Returns the presense of the given oneof field, given a field number. Called
anight marked this conversation as resolved.
Show resolved Hide resolved
* from generated code methods such as:
*
* public function hasDoubleValueOneof()
* {
* return $this->hasOneof(10);
* }
*
* @return boolean
*/
PHP_METHOD(Message, hasOneof) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test for this?

Copy link
Member

Choose a reason for hiding this comment

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

Test looks great, but it might be better to move it into one of the existing test files (I think GeneratedClassTest.php would be a good option). Adding new files is a bit more involved because then the new file has to get added to the Makefile:

https://github.com/protocolbuffers/protobuf/blob/master/Makefile.am#L767-L961

Message* intern = (Message*)Z_OBJ_P(getThis());
zend_long field_num;
const upb_fielddef* f;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &field_num) == FAILURE) {
return;
}

f = upb_msgdef_itof(intern->desc->msgdef, field_num);

if (!f || !upb_fielddef_realcontainingoneof(f)) {
php_error_docref(NULL, E_USER_ERROR,
"Internal error, no such oneof field %d\n",
(int)field_num);
}

RETVAL_BOOL(upb_msg_has(intern->msg, f));
}

/**
* Message::readOneof()
*
Expand Down Expand Up @@ -1014,6 +1047,7 @@ static zend_function_entry Message_methods[] = {
PHP_ME(Message, mergeFrom, arginfo_mergeFrom, ZEND_ACC_PUBLIC)
PHP_ME(Message, readWrapperValue, arginfo_read, ZEND_ACC_PROTECTED)
PHP_ME(Message, writeWrapperValue, arginfo_write, ZEND_ACC_PROTECTED)
PHP_ME(Message, hasOneof, arginfo_read, ZEND_ACC_PROTECTED)
PHP_ME(Message, readOneof, arginfo_read, ZEND_ACC_PROTECTED)
PHP_ME(Message, writeOneof, arginfo_write, ZEND_ACC_PROTECTED)
PHP_ME(Message, whichOneof, arginfo_read, ZEND_ACC_PROTECTED)
Expand Down