Skip to content

PHPC-2165: Expose server error replies in BulkWriteResult #1385

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

Merged
merged 1 commit into from
Dec 6, 2022
Merged

PHPC-2165: Expose server error replies in BulkWriteResult #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion src/MongoDB/WriteResult.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,32 @@ static bool php_phongo_writeresult_get_writeerrors(php_phongo_writeresult_t* int
return true;
}

static bool php_phongo_writeresult_get_error_replies(php_phongo_writeresult_t* intern, zval* return_value)
{
bson_iter_t iter, child;

array_init(return_value);

if (bson_iter_init_find(&iter, intern->reply, "errorReplies") && BSON_ITER_HOLDS_ARRAY(&iter) && bson_iter_recurse(&iter, &child)) {
while (bson_iter_next(&child)) {
uint32_t len;
const uint8_t* data;
zval error_reply;

if (!BSON_ITER_HOLDS_DOCUMENT(&child)) {
continue;
}

bson_iter_document(&child, &len, &data);
php_phongo_bson_data_to_zval(data, len, &error_reply);

add_next_index_zval(return_value, &error_reply);
}
}

return true;
}

PHONGO_DISABLED_CONSTRUCTOR(MongoDB_Driver_WriteResult)
PHONGO_DISABLED_WAKEUP(MongoDB_Driver_WriteResult)

Expand Down Expand Up @@ -273,6 +299,17 @@ static PHP_METHOD(MongoDB_Driver_WriteResult, getWriteErrors)
php_phongo_writeresult_get_writeerrors(intern, return_value);
}

static PHP_METHOD(MongoDB_Driver_WriteResult, getErrorReplies)
{
php_phongo_writeresult_t* intern;

intern = Z_WRITERESULT_OBJ_P(getThis());

PHONGO_PARSE_PARAMETERS_NONE();

php_phongo_writeresult_get_error_replies(intern, return_value);
}

/* Returns whether the write operation was acknowledged (based on the write
concern). */
static PHP_METHOD(MongoDB_Driver_WriteResult, isAcknowledged)
Expand Down Expand Up @@ -328,7 +365,7 @@ static HashTable* php_phongo_writeresult_get_debug_info(phongo_compat_object_han

intern = Z_OBJ_WRITERESULT(PHONGO_COMPAT_GET_OBJ(object));
*is_temp = 1;
array_init_size(&retval, 9);
array_init_size(&retval, 10);

#define PHONGO_WRITERESULT_SCP(field) \
if (bson_iter_init_find(&iter, intern->reply, (field)) && BSON_ITER_HOLDS_INT32(&iter)) { \
Expand Down Expand Up @@ -386,6 +423,13 @@ static HashTable* php_phongo_writeresult_get_debug_info(phongo_compat_object_han
ADD_ASSOC_NULL_EX(&retval, "writeConcern");
}

{
zval error_replies;

php_phongo_writeresult_get_error_replies(intern, &error_replies);
ADD_ASSOC_ZVAL_EX(&retval, "errorReplies", &error_replies);
}

done:
return Z_ARRVAL(retval);
}
Expand Down
2 changes: 2 additions & 0 deletions src/MongoDB/WriteResult.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ final public function getWriteConcernError(): ?WriteConcernError {}

final public function getWriteErrors(): array {}

final public function getErrorReplies(): array {}

final public function isAcknowledged(): bool {}

final public function __wakeup(): void {}
Expand Down
6 changes: 5 additions & 1 deletion src/MongoDB/WriteResult_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: eae27215b994b4941296f69331abd6d1a3173df7 */
* Stub hash: 279a9a00d54bf67c310f7b8802a5868bf1d507eb */

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_MongoDB_Driver_WriteResult___construct, 0, 0, 0)
ZEND_END_ARG_INFO()
Expand All @@ -26,6 +26,8 @@ ZEND_END_ARG_INFO()

#define arginfo_class_MongoDB_Driver_WriteResult_getWriteErrors arginfo_class_MongoDB_Driver_WriteResult_getUpsertedIds

#define arginfo_class_MongoDB_Driver_WriteResult_getErrorReplies arginfo_class_MongoDB_Driver_WriteResult_getUpsertedIds
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize gen_stub.php re-used arginfo definitions but I just came across this code:

/* If there already is an equivalent arginfo structure, only emit a #define */
if ($generatedFuncInfo = findEquivalentFuncInfo($generatedFuncInfos, $funcInfo)) { ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was occasionally confused by this as well. TBH, I stopped reviewing the generated files and instead review the stubs, relying on CI to catch any outdated arginfo files.


ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_MongoDB_Driver_WriteResult_isAcknowledged, 0, 0, _IS_BOOL, 0)
ZEND_END_ARG_INFO()

Expand All @@ -43,6 +45,7 @@ static ZEND_METHOD(MongoDB_Driver_WriteResult, getServer);
static ZEND_METHOD(MongoDB_Driver_WriteResult, getUpsertedIds);
static ZEND_METHOD(MongoDB_Driver_WriteResult, getWriteConcernError);
static ZEND_METHOD(MongoDB_Driver_WriteResult, getWriteErrors);
static ZEND_METHOD(MongoDB_Driver_WriteResult, getErrorReplies);
static ZEND_METHOD(MongoDB_Driver_WriteResult, isAcknowledged);
static ZEND_METHOD(MongoDB_Driver_WriteResult, __wakeup);

Expand All @@ -58,6 +61,7 @@ static const zend_function_entry class_MongoDB_Driver_WriteResult_methods[] = {
ZEND_ME(MongoDB_Driver_WriteResult, getUpsertedIds, arginfo_class_MongoDB_Driver_WriteResult_getUpsertedIds, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(MongoDB_Driver_WriteResult, getWriteConcernError, arginfo_class_MongoDB_Driver_WriteResult_getWriteConcernError, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(MongoDB_Driver_WriteResult, getWriteErrors, arginfo_class_MongoDB_Driver_WriteResult_getWriteErrors, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(MongoDB_Driver_WriteResult, getErrorReplies, arginfo_class_MongoDB_Driver_WriteResult_getErrorReplies, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(MongoDB_Driver_WriteResult, isAcknowledged, arginfo_class_MongoDB_Driver_WriteResult_isAcknowledged, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(MongoDB_Driver_WriteResult, __wakeup, arginfo_class_MongoDB_Driver_WriteResult___wakeup, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_FE_END
Expand Down
3 changes: 3 additions & 0 deletions tests/exception/bulkwriteexception-getwriteresult-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["writeConcern"]=>
object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
7 changes: 5 additions & 2 deletions tests/manager/manager-executeBulkWrite_error-005.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ try {
--EXPECTF--
MongoDB\Driver\Exception\BulkWriteException(0): Bulk write failed due to previous MongoDB\Driver\Exception\ConnectionTimeoutException: Failed to send "delete" command with database "%s": Failed to read 4 bytes: socket error or timeout
MongoDB\Driver\Exception\ConnectionTimeoutException(%d): Failed to send "delete" command with database "%s": Failed to read 4 bytes: socket error or timeout
object(MongoDB\Driver\WriteResult)#%d (9) {
object(MongoDB\Driver\WriteResult)#%d (%d) {
["nInserted"]=>
int(1)
["nMatched"]=>
Expand All @@ -59,7 +59,10 @@ object(MongoDB\Driver\WriteResult)#%d (9) {
["writeConcernError"]=>
NULL
["writeConcern"]=>
object(MongoDB\Driver\WriteConcern)#%d (0) {
object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
3 changes: 3 additions & 0 deletions tests/replicaset/writeresult-getserver-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["w"]=>
int(1)
}
["errorReplies"]=>
array(0) {
}
}
string(%d) "%s"
int(%d)
Expand Down
3 changes: 3 additions & 0 deletions tests/server/server-executeBulkWrite-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["writeConcern"]=>
object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
["errorReplies"]=>
array(0) {
}
}

===> Collection
Expand Down
3 changes: 3 additions & 0 deletions tests/standalone/writeresult-isacknowledged-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["writeConcern"]=>
object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
3 changes: 3 additions & 0 deletions tests/standalone/writeresult-isacknowledged-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["w"]=>
int(0)
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
3 changes: 3 additions & 0 deletions tests/standalone/writeresult-isacknowledged-003.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["w"]=>
int(0)
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
3 changes: 3 additions & 0 deletions tests/writeResult/writeresult-debug-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["writeConcern"]=>
object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
3 changes: 3 additions & 0 deletions tests/writeResult/writeresult-debug-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,8 @@ object(MongoDB\Driver\WriteResult)#%d (%d) {
["w"]=>
int(30)
}
["errorReplies"]=>
array(0) {
}
}
===DONE===
40 changes: 40 additions & 0 deletions tests/writeResult/writeresult-getErrorReplies-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
--TEST--
MongoDB\Driver\WriteResult::getErrorReplies()
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php skip_if_not_live(); ?>
<?php skip_if_not_clean(); ?>
<?php skip_if_no_failcommand_failpoint(); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

$manager = create_test_manager();
$server = $manager->selectServer();

configureTargetedFailPoint(
$server,
'failCommand',
['times' => 1] ,
['errorCode' => 8, 'failCommands' => ['insert']]
);

$errors = [];
try {
$bulk = new MongoDB\Driver\BulkWrite;
$bulk->insert(['_id' => 1, 'x' => 'bar']);
$server->executeBulkWrite(NS, $bulk);
} catch (MongoDB\Driver\Exception\BulkWriteException $e) {
$errors = $e->getWriteResult()->getErrorReplies();
}

var_dump(count($errors));
var_dump($errors[0]->code);

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
int(1)
int(8)
===DONE===