PHPC-245: executeUpdate() converts objects to arrays

bjori · bjori · commit e147fd3f7fb9 · 2015-04-15T11:27:19.000-07:00
Multiple things here -- we shouldn't be injecting __pclass every time
we serialize an object -- only when we are preparing it to be inserted.
Otherwise, for example when executing an update, it will be injected
into the update document too -- which b0rks the update statement.

We also need to bail out when we don't get an array from the serialize
callback -- and rather then pretend the return array from the callback
is the data we got passed in in the get-go, keep the original zval
intact so we don't accidentally convert it to array
diff --git a/php_bson.h b/php_bson.h
@@ -30,9 +30,10 @@
 #include "php_phongo.h"
 
 typedef enum {
-	PHONGO_BSON_NONE   = 0x00,
-	PHONGO_BSON_ADD_ID = 0x01,
+	PHONGO_BSON_NONE      = 0x00,
+	PHONGO_BSON_ADD_ID    = 0x01,
 	PHONGO_BSON_RETURN_ID = 0x02,
+	PHONGO_BSON_ADD_ODS   = 0x04,
 } phongo_bson_flags_t;
 
 PHONGO_API void zval_to_bson(zval *data, phongo_bson_flags_t flags, bson_t *bson, bson_t **bson_out TSRMLS_DC);
diff --git a/src/MongoDB/BulkWrite.c b/src/MongoDB/BulkWrite.c
@@ -97,6 +97,7 @@ PHP_METHOD(BulkWrite, insert)
 	if (return_value_used) {
 		bson_flags |= PHONGO_BSON_RETURN_ID;
 	}
+	bson_flags |= PHONGO_BSON_ADD_ODS;
 
 	bson = bson_new();
 	zval_to_bson(document, bson_flags, bson, &bson_out TSRMLS_CC);
diff --git a/src/MongoDB/Manager.c b/src/MongoDB/Manager.c
@@ -172,7 +172,7 @@ PHP_METHOD(Manager, executeInsert)
 
 
 	bson = bson_new();
-	zval_to_bson(document, PHONGO_BSON_NONE, bson, NULL TSRMLS_CC);
+	zval_to_bson(document, PHONGO_BSON_ADD_ODS, bson, NULL TSRMLS_CC);
 	phongo_execute_single_insert(intern->client, namespace, bson, phongo_write_concern_from_zval(zwrite_concern TSRMLS_CC), -1, return_value, return_value_used TSRMLS_CC);
 	bson_clear(&bson);
 }
diff --git a/src/bson.c b/src/bson.c
@@ -561,14 +561,14 @@ int php_phongo_is_array_or_document(zval **val TSRMLS_DC) /* {{{ */
 	return IS_ARRAY;
 }
 /* }}} */
-void object_to_bson(zval *object, const char *key, long key_len, bson_t *bson TSRMLS_DC)
+void object_to_bson(zval *object, phongo_bson_flags_t flags, const char *key, long key_len, bson_t *bson TSRMLS_DC)
 {
 	bson_t child;
 
 	if (Z_TYPE_P(object) != IS_OBJECT || instanceof_function(Z_OBJCE_P(object), zend_standard_class_def TSRMLS_CC)) {
 		mongoc_log(MONGOC_LOG_LEVEL_TRACE, MONGOC_LOG_DOMAIN, "encoding as-if was stdclass");
 		bson_append_document_begin(bson, key, key_len, &child);
-		zval_to_bson(object, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);
+		zval_to_bson(object, flags, &child, NULL TSRMLS_CC);
 		bson_append_document_end(bson, &child);
 		return;
 	}
@@ -591,9 +591,11 @@ void object_to_bson(zval *object, const char *key, long key_len, bson_t *bson TS
 
 				bson_append_document_begin(bson, key, key_len, &child);
 				if (instanceof_function(Z_OBJCE_P(object), php_phongo_persistable_ce TSRMLS_CC)) {
-					bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));
+					if (flags & PHONGO_BSON_ADD_ODS) {
+						bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));
+					}
 				}
-				zval_to_bson(retval, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);
+				zval_to_bson(retval, flags, &child, NULL TSRMLS_CC);
 				bson_append_document_end(bson, &child);
 
 				if (tmp_ht) {
@@ -665,7 +667,7 @@ void object_to_bson(zval *object, const char *key, long key_len, bson_t *bson TS
 		}
 	}
 }
-void phongo_bson_append(bson_t *bson, const char *key, long key_len, int entry_type, zval *entry TSRMLS_DC)
+void phongo_bson_append(bson_t *bson, phongo_bson_flags_t flags, const char *key, long key_len, int entry_type, zval *entry TSRMLS_DC)
 {
 	switch (entry_type)
 	{
@@ -704,7 +706,7 @@ void phongo_bson_append(bson_t *bson, const char *key, long key_len, int entry_t
 				}
 
 				bson_append_array_begin(bson, key, key_len, &child);
-				zval_to_bson(entry, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);
+				zval_to_bson(entry, flags, &child, NULL TSRMLS_CC);
 				bson_append_array_end(bson, &child);
 
 				if (tmp_ht) {
@@ -714,7 +716,7 @@ void phongo_bson_append(bson_t *bson, const char *key, long key_len, int entry_t
 			}
 			/* break intentionally omitted */
 		case IS_OBJECT:
-			object_to_bson(entry, key, key_len, bson TSRMLS_CC);
+			object_to_bson(entry, flags, key, key_len, bson TSRMLS_CC);
 			break;
 
 		default:
@@ -727,18 +729,26 @@ PHONGO_API void zval_to_bson(zval *data, phongo_bson_flags_t flags, bson_t *bson
 {
 	HashPosition pos;
 	HashTable   *ht_data;
+	zval        *obj_data = NULL;
 
 	switch(Z_TYPE_P(data)) {
 		case IS_OBJECT:
 			if (instanceof_function(Z_OBJCE_P(data), php_phongo_persistable_ce TSRMLS_CC)) {
-				zval *retval;
 
-				bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));
+				if (flags & PHONGO_BSON_ADD_ODS) {
+					bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));
+				}
+
+				zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);
+				if(obj_data) {
+					if (Z_TYPE_P(obj_data) == IS_ARRAY) {
+						ht_data = HASH_OF(obj_data);
+					} else {
+						phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "%s", "Return value expected to be array");
+						zval_ptr_dtor(&obj_data);
+					}
 
-				zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &retval);
-				if(retval) {
-					ZVAL_ZVAL(data, retval, 0, 1);
-					convert_to_array(data);
+					break;
 				}
 			}
 			/* break intentionally omitted */
@@ -752,6 +762,9 @@ PHONGO_API void zval_to_bson(zval *data, phongo_bson_flags_t flags, bson_t *bson
 	}
 
 	if (ht_data && ht_data->nApplyCount > 1) {
+		if (obj_data) {
+			zval_ptr_dtor(&obj_data);
+		}
 		return;
 	}
 
@@ -794,7 +807,7 @@ PHONGO_API void zval_to_bson(zval *data, phongo_bson_flags_t flags, bson_t *bson
 			key_len = bson_uint32_to_string(index, (const char **)&key, numbuf, sizeof(numbuf));
 		}
 
-		phongo_bson_append(bson, key, key_len, Z_TYPE_PP(entry), *entry TSRMLS_CC);
+		phongo_bson_append(bson, flags, key, key_len, Z_TYPE_PP(entry), *entry TSRMLS_CC);
 	}
 
 	if (flags & PHONGO_BSON_ADD_ID) {
@@ -810,6 +823,9 @@ PHONGO_API void zval_to_bson(zval *data, phongo_bson_flags_t flags, bson_t *bson
 			}
 		}
 	}
+	if (obj_data) {
+		zval_ptr_dtor(&obj_data);
+	}
 }
 
 /* }}} */
@@ -873,7 +889,7 @@ PHP_FUNCTION(fromArray)
 	}
 
 	bson = bson_new();
-	zval_to_bson(data, PHONGO_BSON_NONE, bson, NULL TSRMLS_CC);
+	zval_to_bson(data, PHONGO_BSON_ADD_ODS, bson, NULL TSRMLS_CC);
 
 	RETVAL_STRINGL((const char *) bson_get_data(bson), bson->len, 1);
 	bson_destroy(bson);
diff --git a/tests/bson/bson-ods-001.phpt b/tests/bson/bson-ods-001.phpt
@@ -81,6 +81,7 @@ class Person implements BSON\Persistable {
                 $updated['$set'] = array($k => $v);
             }
         }
+        return $updated;
     }
  
     function getName() {
diff --git a/tests/bson/bson-ods-002.phpt b/tests/bson/bson-ods-002.phpt

Original file line number	Diff line number	Diff line change
`@@ -97,6 +97,7 @@ PHP_METHOD(BulkWrite, insert)`
`97`	`97`	`if (return_value_used) {`
`98`	`98`	`bson_flags \|= PHONGO_BSON_RETURN_ID;`
`99`	`99`	`}`
	`100`	`+ bson_flags \|= PHONGO_BSON_ADD_ODS;`
`100`	`101`
`101`	`102`	`bson = bson_new();`
`102`	`103`	`zval_to_bson(document, bson_flags, bson, &bson_out TSRMLS_CC);`
Original file line number	Diff line number	Diff line change
`@@ -172,7 +172,7 @@ PHP_METHOD(Manager, executeInsert)`
`172`	`172`
`173`	`173`
`174`	`174`	`bson = bson_new();`
`175`		`- zval_to_bson(document, PHONGO_BSON_NONE, bson, NULL TSRMLS_CC);`
	`175`	`+ zval_to_bson(document, PHONGO_BSON_ADD_ODS, bson, NULL TSRMLS_CC);`
`176`	`176`	`phongo_execute_single_insert(intern->client, namespace, bson, phongo_write_concern_from_zval(zwrite_concern TSRMLS_CC), -1, return_value, return_value_used TSRMLS_CC);`
`177`	`177`	`bson_clear(&bson);`
`178`	`178`	`}`
Original file line number	Diff line number	Diff line change
`@@ -561,14 +561,14 @@ int php_phongo_is_array_or_document(zval *val TSRMLS_DC) / {{{ */`
`561`	`561`	`return IS_ARRAY;`
`562`	`562`	`}`
`563`	`563`	`/* }}} */`
`564`		`-void object_to_bson(zval object, const char key, long key_len, bson_t *bson TSRMLS_DC)`
	`564`	`+void object_to_bson(zval object, phongo_bson_flags_t flags, const char key, long key_len, bson_t *bson TSRMLS_DC)`
`565`	`565`	`{`
`566`	`566`	`bson_t child;`
`567`	`567`
`568`	`568`	`if (Z_TYPE_P(object) != IS_OBJECT \|\| instanceof_function(Z_OBJCE_P(object), zend_standard_class_def TSRMLS_CC)) {`
`569`	`569`	`mongoc_log(MONGOC_LOG_LEVEL_TRACE, MONGOC_LOG_DOMAIN, "encoding as-if was stdclass");`
`570`	`570`	`bson_append_document_begin(bson, key, key_len, &child);`
`571`		`- zval_to_bson(object, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);`
	`571`	`+ zval_to_bson(object, flags, &child, NULL TSRMLS_CC);`
`572`	`572`	`bson_append_document_end(bson, &child);`
`573`	`573`	`return;`
`574`	`574`	`}`
`@@ -591,9 +591,11 @@ void object_to_bson(zval object, const char key, long key_len, bson_t *bson TS`
`591`	`591`
`592`	`592`	`bson_append_document_begin(bson, key, key_len, &child);`
`593`	`593`	`if (instanceof_function(Z_OBJCE_P(object), php_phongo_persistable_ce TSRMLS_CC)) {`
`594`		`- bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));`
	`594`	`+ if (flags & PHONGO_BSON_ADD_ODS) {`
	`595`	`+ bson_append_binary(&child, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(object)->name, strlen(Z_OBJCE_P(object)->name));`
	`596`	`+ }`
`595`	`597`	`}`
`596`		`- zval_to_bson(retval, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);`
	`598`	`+ zval_to_bson(retval, flags, &child, NULL TSRMLS_CC);`
`597`	`599`	`bson_append_document_end(bson, &child);`
`598`	`600`
`599`	`601`	`if (tmp_ht) {`
`@@ -665,7 +667,7 @@ void object_to_bson(zval object, const char key, long key_len, bson_t *bson TS`
`665`	`667`	`}`
`666`	`668`	`}`
`667`	`669`	`}`
`668`		`-void phongo_bson_append(bson_t bson, const char key, long key_len, int entry_type, zval *entry TSRMLS_DC)`
	`670`	`+void phongo_bson_append(bson_t bson, phongo_bson_flags_t flags, const char key, long key_len, int entry_type, zval *entry TSRMLS_DC)`
`669`	`671`	`{`
`670`	`672`	`switch (entry_type)`
`671`	`673`	`{`
`@@ -704,7 +706,7 @@ void phongo_bson_append(bson_t bson, const char key, long key_len, int entry_t`
`704`	`706`	`}`
`705`	`707`
`706`	`708`	`bson_append_array_begin(bson, key, key_len, &child);`
`707`		`- zval_to_bson(entry, PHONGO_BSON_NONE, &child, NULL TSRMLS_CC);`
	`709`	`+ zval_to_bson(entry, flags, &child, NULL TSRMLS_CC);`
`708`	`710`	`bson_append_array_end(bson, &child);`
`709`	`711`
`710`	`712`	`if (tmp_ht) {`
`@@ -714,7 +716,7 @@ void phongo_bson_append(bson_t bson, const char key, long key_len, int entry_t`
`714`	`716`	`}`
`715`	`717`	`/* break intentionally omitted */`
`716`	`718`	`case IS_OBJECT:`
`717`		`- object_to_bson(entry, key, key_len, bson TSRMLS_CC);`
	`719`	`+ object_to_bson(entry, flags, key, key_len, bson TSRMLS_CC);`
`718`	`720`	`break;`
`719`	`721`
`720`	`722`	`default:`
`@@ -727,18 +729,26 @@ PHONGO_API void zval_to_bson(zval data, phongo_bson_flags_t flags, bson_t bson`
`727`	`729`	`{`
`728`	`730`	`HashPosition pos;`
`729`	`731`	`HashTable *ht_data;`
	`732`	`+ zval *obj_data = NULL;`
`730`	`733`
`731`	`734`	`switch(Z_TYPE_P(data)) {`
`732`	`735`	`case IS_OBJECT:`
`733`	`736`	`if (instanceof_function(Z_OBJCE_P(data), php_phongo_persistable_ce TSRMLS_CC)) {`
`734`		`- zval *retval;`
`735`	`737`
`736`		`- bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));`
	`738`	`+ if (flags & PHONGO_BSON_ADD_ODS) {`
	`739`	`+ bson_append_binary(bson, PHONGO_ODM_FIELD_NAME, -1, 0x80, (const uint8_t *)Z_OBJCE_P(data)->name, strlen(Z_OBJCE_P(data)->name));`
	`740`	`+ }`
	`741`	`+`
	`742`	`+ zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &obj_data);`
	`743`	`+ if(obj_data) {`
	`744`	`+ if (Z_TYPE_P(obj_data) == IS_ARRAY) {`
	`745`	`+ ht_data = HASH_OF(obj_data);`
	`746`	`+ } else {`
	`747`	`+ phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "%s", "Return value expected to be array");`
	`748`	`+ zval_ptr_dtor(&obj_data);`
	`749`	`+ }`
`737`	`750`
`738`		`- zend_call_method_with_0_params(&data, NULL, NULL, BSON_SERIALIZE_FUNC_NAME, &retval);`
`739`		`- if(retval) {`
`740`		`- ZVAL_ZVAL(data, retval, 0, 1);`
`741`		`- convert_to_array(data);`
	`751`	`+ break;`
`742`	`752`	`}`
`743`	`753`	`}`
`744`	`754`	`/* break intentionally omitted */`
`@@ -752,6 +762,9 @@ PHONGO_API void zval_to_bson(zval data, phongo_bson_flags_t flags, bson_t bson`
`752`	`762`	`}`
`753`	`763`
`754`	`764`	`if (ht_data && ht_data->nApplyCount > 1) {`
	`765`	`+ if (obj_data) {`
	`766`	`+ zval_ptr_dtor(&obj_data);`
	`767`	`+ }`
`755`	`768`	`return;`
`756`	`769`	`}`
`757`	`770`
`@@ -794,7 +807,7 @@ PHONGO_API void zval_to_bson(zval data, phongo_bson_flags_t flags, bson_t bson`
`794`	`807`	`key_len = bson_uint32_to_string(index, (const char **)&key, numbuf, sizeof(numbuf));`
`795`	`808`	`}`
`796`	`809`
`797`		`- phongo_bson_append(bson, key, key_len, Z_TYPE_PP(entry), *entry TSRMLS_CC);`
	`810`	`+ phongo_bson_append(bson, flags, key, key_len, Z_TYPE_PP(entry), *entry TSRMLS_CC);`
`798`	`811`	`}`
`799`	`812`
`800`	`813`	`if (flags & PHONGO_BSON_ADD_ID) {`
`@@ -810,6 +823,9 @@ PHONGO_API void zval_to_bson(zval data, phongo_bson_flags_t flags, bson_t bson`
`810`	`823`	`}`
`811`	`824`	`}`
`812`	`825`	`}`
	`826`	`+ if (obj_data) {`
	`827`	`+ zval_ptr_dtor(&obj_data);`
	`828`	`+ }`
`813`	`829`	`}`
`814`	`830`
`815`	`831`	`/* }}} */`
`@@ -873,7 +889,7 @@ PHP_FUNCTION(fromArray)`
`873`	`889`	`}`
`874`	`890`
`875`	`891`	`bson = bson_new();`
`876`		`- zval_to_bson(data, PHONGO_BSON_NONE, bson, NULL TSRMLS_CC);`
	`892`	`+ zval_to_bson(data, PHONGO_BSON_ADD_ODS, bson, NULL TSRMLS_CC);`
`877`	`893`
`878`	`894`	`RETVAL_STRINGL((const char *) bson_get_data(bson), bson->len, 1);`
`879`	`895`	`bson_destroy(bson);`
Original file line number	Diff line number	Diff line change
`@@ -81,6 +81,7 @@ class Person implements BSON\Persistable {`
`81`	`81`	`$updated['$set'] = array($k => $v);`
`82`	`82`	`}`
`83`	`83`	`}`
	`84`	`+ return $updated;`
`84`	`85`	`}`
`85`	`86`
`86`	`87`	`function getName() {`