Skip to content
This repository was archived by the owner on Dec 23, 2021. It is now read-only.

HHVM-161, HHVM-191: WriteResult should use libmongoc's public API #76

Merged
merged 2 commits into from
Mar 23, 2016
Merged

HHVM-161, HHVM-191: WriteResult should use libmongoc's public API #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7d5e7d
HHVM-161, HHVM-191: WriteResult should use libmongoc's public API
derickr Mar 17, 2016
896ec02
Address issues that arose from merging #77
derickr Mar 22, 2016
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
28 changes: 21 additions & 7 deletions ext_mongodb.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,31 @@ public function __debugInfo() : array

$ret['nInserted'] = $this->nInserted;
$ret['nMatched'] = $this->nMatched;
if ($this->omit_nModified) {
$ret['nModified'] = null;
} else {
$ret['nModified'] = $this->nModified;
}
$ret['nModified'] = $this->nModified;
$ret['nRemoved'] = $this->nRemoved;
$ret['nUpserted'] = $this->nUpserted;

$ret['upsertedIds'] = (array) $this->upsertedIds;
$ret['writeErrors'] = $this->writeErrors;
$ret['writeConcernError'] = $this->writeConcernError;
$ret['writeErrors'] = array_map(
function($value) {
$a = [
'index' => $value->getIndex(),
'code' => $value->getCode(),
'errmsg' => $value->getMessage(),
];

return $a;
},
$this->writeErrors
);
if (is_object($this->writeConcernError) && $this->writeConcernError instanceof \MongoDB\Driver\WriteConcernError) {
$ret['writeConcernError'] = [
'code' => $this->writeConcernError->getCode(),
'errmsg' => $this->writeConcernError->getMessage(),
];
} else {
$ret['writeConcernError'] = NULL;
}

if ($this->writeConcern) {
$ret['writeConcern'] = $this->writeConcern;
Expand Down
72 changes: 27 additions & 45 deletions src/MongoDB/Driver/WriteResult.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,21 @@ const StaticString
s_nRemoved("nRemoved"),
s_nInserted("nInserted"),
s_nModified("nModified"),
s_omit_nModified("omit_nModified"),
s_writeConcern("writeConcern"),
s_upserted("upserted"),
s_upsertedIds("upsertedIds"),
s_writeErrors("writeErrors"),
s_errmsg("errmsg"),
s_message("message"),
s_code("code"),
s_index("index"),
s_info("info"),
s_writeConcernError("writeConcernError");
s_writeConcernError("writeConcernError"),
s_writeConcernErrors("writeConcernErrors");

Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t *error, mongoc_client_t *client, int server_id, int success, const mongoc_write_concern_t *write_concern)
Object hippo_write_result_init(bson_t *reply, bson_error_t *error, mongoc_client_t *client, int server_id, int success, const mongoc_write_concern_t *write_concern)
{
static Class* c_writeResult;
std::string message;

c_writeResult = Unit::lookupClass(s_MongoDriverWriteResult_className.get());
assert(c_writeResult);
Expand All @@ -88,16 +88,20 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t
wr_data->m_server_id = server_id;
wr_data->m_write_concern = mongoc_write_concern_copy(write_concern);

obj->o_set(s_nUpserted, Variant((int64_t) write_result->nUpserted), s_MongoDriverWriteResult_className);
obj->o_set(s_nMatched, Variant((int64_t) write_result->nMatched), s_MongoDriverWriteResult_className);
obj->o_set(s_nRemoved, Variant((int64_t) write_result->nRemoved), s_MongoDriverWriteResult_className);
obj->o_set(s_nInserted, Variant((int64_t) write_result->nInserted), s_MongoDriverWriteResult_className);
obj->o_set(s_nModified, Variant((int64_t) write_result->nModified), s_MongoDriverWriteResult_className);
obj->o_set(s_omit_nModified, Variant((int64_t) write_result->omit_nModified), s_MongoDriverWriteResult_className);
/* Convert the whole BSON reply into a Variant */
Variant v;
Array a;
hippo_bson_conversion_options_t options = HIPPO_TYPEMAP_DEBUG_INITIALIZER;

if (!success) {
message = "BulkWrite error";
}
BsonToVariantConverter convertor(bson_get_data(reply), reply->len, options);
convertor.convert(&v);
a = v.toArray();

obj->o_set(s_nUpserted, Variant(a[s_nUpserted]), s_MongoDriverWriteResult_className);
obj->o_set(s_nMatched, Variant(a[s_nMatched]), s_MongoDriverWriteResult_className);
obj->o_set(s_nRemoved, Variant(a[s_nRemoved]), s_MongoDriverWriteResult_className);
obj->o_set(s_nInserted, Variant(a[s_nInserted]), s_MongoDriverWriteResult_className);
obj->o_set(s_nModified, Variant(a[s_nModified]), s_MongoDriverWriteResult_className);
Copy link
Member

Choose a reason for hiding this comment

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

Will this fall back to null if the "nModified" field is missing in the BSON document?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I've just tested this by passing in s_info instead of s_nMatched (which doesn't exist), and the value turned into a NULL.


if (write_concern) {
Array debugInfoResult = Array::Create();
Expand All @@ -108,27 +112,17 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t
obj->o_set(s_writeConcern, Variant(), s_MongoDriverWriteConcern_className);
}

if (!bson_empty0(&write_result->upserted)) {
Variant v;
hippo_bson_conversion_options_t options = HIPPO_TYPEMAP_DEBUG_INITIALIZER;

BsonToVariantConverter convertor(bson_get_data(&write_result->upserted), write_result->upserted.len, options);
convertor.convert(&v);
obj->o_set(s_upsertedIds, v.toArray(), s_MongoDriverWriteResult_className);
if (a.exists(s_upserted)) {
obj->o_set(s_upsertedIds, a[s_upserted], s_MongoDriverWriteResult_className);
} else {
Array a = Array::Create();
obj->o_set(s_upsertedIds, a, s_MongoDriverWriteResult_className);
}

if (!bson_empty0(&write_result->writeErrors)) {
Variant v;
hippo_bson_conversion_options_t options = HIPPO_TYPEMAP_DEBUG_INITIALIZER;
if (a.exists(s_writeErrors)) {
Array writeErrors = Array::Create();

BsonToVariantConverter convertor(bson_get_data(&write_result->writeErrors), write_result->writeErrors.len, options);
convertor.convert(&v);

for (ArrayIter iter(v.toArray()); iter; ++iter) {
for (ArrayIter iter(a[s_writeErrors].toArray()); iter; ++iter) {
const Variant& value = iter.second();
static Class* c_writeError;

Expand All @@ -140,8 +134,6 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t

if (a_we.exists(s_errmsg)) {
we_obj->o_set(s_message, a_we[s_errmsg], s_MongoDriverWriteError_className);
message += " :: ";
message.append(a_we[s_errmsg].toString().c_str());
}
if (a_we.exists(s_code)) {
we_obj->o_set(s_code, a_we[s_code], s_MongoDriverWriteError_className);
Expand All @@ -159,15 +151,10 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t
obj->o_set(s_writeErrors, writeErrors, s_MongoDriverWriteResult_className);
}

if (!bson_empty0(&write_result->writeConcernErrors)) {
Variant v;
hippo_bson_conversion_options_t options = HIPPO_TYPEMAP_DEBUG_INITIALIZER;
if (a.exists(s_writeConcernErrors)) {
Array a_v;

BsonToVariantConverter convertor(bson_get_data(&write_result->writeConcernErrors), write_result->writeConcernErrors.len, options);
convertor.convert(&v);

a_v = v.toArray();
a_v = a[s_writeConcernErrors].toArray();

static Class* c_writeConcernError;

Expand All @@ -180,8 +167,6 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t

if (first_item.exists(s_errmsg)) {
wce_obj->o_set(s_message, first_item[s_errmsg], s_MongoDriverWriteConcernError_className);
message += " :: ";
message.append(first_item[s_errmsg].toString().c_str());
}
if (first_item.exists(s_code)) {
wce_obj->o_set(s_code, first_item[s_code], s_MongoDriverWriteConcernError_className);
Expand All @@ -197,16 +182,13 @@ Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t
}

if (success == 0) {
if (
bson_empty0(&write_result->writeErrors) &&
bson_empty0(&write_result->writeConcernErrors)
) {
throw MongoDriver::Utils::throwExceptionFromBsonError(error);
} else {
auto bw_exception = MongoDriver::Utils::throwBulkWriteException(message);
if ((error->domain == MONGOC_ERROR_COMMAND && error->code != MONGOC_ERROR_COMMAND_INVALID_ARG) || error->domain == MONGOC_ERROR_WRITE_CONCERN) {
auto bw_exception = MongoDriver::Utils::throwBulkWriteException(error->message);
Copy link
Member

Choose a reason for hiding this comment

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

How does this work with #77? If the BulkWrite is empty, success is obviously 0 and the error domain will be MONGOC_ERROR_COMMAND with code MONGOC_ERROR_COMMAND_INVALID_ARG. Will we end up throwing a BulkWriteException with an empty WriteResult attached?

Or will that InvalidArgumentException be thrown before we even get to this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the else case below will take care of it:

throw MongoDriver::Utils::throwExceptionFromBsonError(error);

bw_exception->o_set(s_MongoDriverExceptionBulkWriteException_writeResult, obj, MongoDriver::s_MongoDriverExceptionBulkWriteException_className);

throw bw_exception;
} else {
throw MongoDriver::Utils::throwExceptionFromBsonError(error);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/MongoDB/Driver/WriteResult.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class MongoDBDriverWriteResultData
Object HHVM_METHOD(MongoDBDriverWriteResult, getServer);
bool HHVM_METHOD(MongoDBDriverWriteResult, isAcknowledged);

Object hippo_write_result_init(mongoc_write_result_t *write_result, bson_error_t *error, mongoc_client_t *client, int server_id, int success, const mongoc_write_concern_t *write_concern);
Object hippo_write_result_init(bson_t *reply, bson_error_t *error, mongoc_client_t *client, int server_id, int success, const mongoc_write_concern_t *write_concern);

}
#endif
2 changes: 1 addition & 1 deletion tests/MongoDBDriverBulkWrite_error-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ try {
}
?>
--EXPECTF--
BulkWriteException: BulkWrite error :: E11000 duplicate key error index: demo.test.$_id_ dup key: { : 1 }
BulkWriteException: E11000 duplicate key error index: demo.test.$_id_ dup key: { : 1 }
6 changes: 4 additions & 2 deletions utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ HPHP::Object Utils::doExecuteBulkWrite(const HPHP::String ns, mongoc_client_t *c
char *database;
char *collection;
int success;
bson_t reply = BSON_INITIALIZER;

/* Prepare */
if (!MongoDriver::Utils::splitNamespace(ns, &database, &collection)) {
Expand All @@ -211,10 +212,11 @@ HPHP::Object Utils::doExecuteBulkWrite(const HPHP::String ns, mongoc_client_t *c
}

/* Run operation */
success = mongoc_bulk_operation_execute(bulk_data->m_bulk, NULL, &error);
success = mongoc_bulk_operation_execute(bulk_data->m_bulk, &reply, &error);

/* Prepare result */
HPHP::Object obj = HPHP::hippo_write_result_init(&bulk_data->m_bulk->result, &error, client, bulk_data->m_bulk->hint, success, write_concern);
HPHP::Object obj = HPHP::hippo_write_result_init(&reply, &error, client, bulk_data->m_bulk->hint, success, write_concern);
bson_destroy(&reply);

return obj;
}
Expand Down