Skip to content

Commit b29c423

Browse files
jmikolajmikola
committed
PHPC-1057: Refactor option parsing for execute methods
Execute options are now parsed via php_array functions, which makes the options case-sensitive. A helper function is now used to convert legacy options for the original three execute methods into an array. This simplifies option parsing in php_phongo.c. Additionally, Manager methods now perform their own server selection, which means that execute functions in php_phongo.c can rely on a server_id being provided. Tests have been updated to expect a serverId option when relevant (e.g. query opts). Additionally, error tests for execute methods have been improved so that invalid values for all known options are tested.
1 parent 140bf28 commit b29c423

20 files changed

+694
-356
lines changed

‎php_phongo.c

Lines changed: 195 additions & 174 deletions
Large diffs are not rendered by default.

‎php_phongo.h

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,18 @@ void phongo_throw_exception(php_phongo_error_domain_t domain TSRMLS_DC, const ch
112112
;
113113
void phongo_throw_exception_from_bson_error_t(bson_error_t *error TSRMLS_DC);
114114

115-
/* This enum is used for libmongoc function selection for the
116-
* phongo_execute_command types. The values are important, as the READ and
117-
* WRITE fields are also used as a bit field to see whether ReadPreference,
118-
* ReadConcern, and WriteConcern are supported for each type. */
115+
/* This enum is used for processing options in phongo_execute_parse_options and
116+
* selecting a libmongoc function to use in phongo_execute_command. The values
117+
* are important, as READ and WRITE are also used as a bit field to determine
118+
* whether readPreference, readConcern, and writeConcern options are parsed. */
119119
typedef enum {
120-
PHONGO_COMMAND_RAW = 0x13,
121-
PHONGO_COMMAND_READ = 0x01,
122-
PHONGO_COMMAND_WRITE = 0x02,
123-
PHONGO_COMMAND_READ_WRITE = 0x03
120+
PHONGO_OPTION_READ_CONCERN = 0x01,
121+
PHONGO_OPTION_READ_PREFERENCE = 0x02,
122+
PHONGO_OPTION_WRITE_CONCERN = 0x04,
123+
PHONGO_COMMAND_RAW = 0x07,
124+
PHONGO_COMMAND_READ = 0x03,
125+
PHONGO_COMMAND_WRITE = 0x04,
126+
PHONGO_COMMAND_READ_WRITE = 0x05,
124127
} php_phongo_command_type_t;
125128

126129
zend_object_handlers *phongo_get_std_object_handlers(void);
@@ -141,6 +144,11 @@ const mongoc_write_concern_t* phongo_write_concern_from_zval (zval *zwrite_conc
141144

142145
php_phongo_server_description_type_t php_phongo_server_description_type(mongoc_server_description_t *sd);
143146

147+
bool phongo_parse_read_preference(zval *options, zval **zreadPreference TSRMLS_DC);
148+
149+
zval* php_phongo_prep_legacy_option(zval *options, const char *key, bool *allocated TSRMLS_DC);
150+
void php_phongo_prep_legacy_option_free(zval *options TSRMLS_DC);
151+
144152
void php_phongo_read_preference_prep_tagsets(zval *tagSets TSRMLS_DC);
145153
bool php_phongo_read_preference_tags_are_valid(const bson_t *tags);
146154

‎src/MongoDB/Manager.c

Lines changed: 125 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,40 @@ static void php_phongo_manager_prep_uri_options(zval *options TSRMLS_DC) /* {{{
242242
return;
243243
} /* }}} */
244244

245+
/* Selects a server for an execute method. If "for_writes" is true, a primary
246+
* will be selected. Otherwise, a read preference will be used to select the
247+
* server. If zreadPreference is NULL, the client's read preference will be
248+
* used.
249+
*
250+
* On success, server_id will be set and the function will return true;
251+
* otherwise, false is returned and an exception is thrown. */
252+
static bool php_phongo_manager_select_server(bool for_writes, zval *zreadPreference, mongoc_client_t *client, uint32_t *server_id TSRMLS_DC) /* {{{ */
253+
{
254+
const mongoc_read_prefs_t *read_preference = NULL;
255+
mongoc_server_description_t *selected_server;
256+
bson_error_t error;
257+
258+
if (!for_writes) {
259+
read_preference = zreadPreference ? phongo_read_preference_from_zval(zreadPreference TSRMLS_CC) : mongoc_client_get_read_prefs(client);
260+
}
261+
262+
selected_server = mongoc_client_select_server(client, for_writes, read_preference, &error);
263+
264+
if (selected_server) {
265+
*server_id = mongoc_server_description_id(selected_server);
266+
mongoc_server_description_destroy(selected_server);
267+
268+
return true;
269+
}
270+
271+
/* Check for connection related exceptions */
272+
if (!EG(exception)) {
273+
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
274+
}
275+
276+
return false;
277+
} /* }}} */
278+
245279
/* {{{ proto void MongoDB\Driver\Manager::__construct([string $uri = "mongodb://127.0.0.1/"[, array $options = array()[, array $driverOptions = array()]]])
246280
Constructs a new Manager */
247281
static PHP_METHOD(Manager, __construct)
@@ -292,6 +326,9 @@ static PHP_METHOD(Manager, executeCommand)
292326
phongo_zpp_char_len db_len;
293327
zval *command;
294328
zval *options = NULL;
329+
bool free_options = false;
330+
zval *zreadPreference = NULL;
331+
uint32_t server_id = 0;
295332
DECLARE_RETURN_VALUE_USED
296333
SUPPRESS_UNUSED_WARNING(return_value_ptr)
297334

@@ -301,7 +338,24 @@ static PHP_METHOD(Manager, executeCommand)
301338

302339
intern = Z_MANAGER_OBJ_P(getThis());
303340

304-
phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, -1, return_value, return_value_used TSRMLS_CC);
341+
options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);
342+
343+
if (!phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
344+
/* Exception should already have been thrown */
345+
goto cleanup;
346+
}
347+
348+
if (!php_phongo_manager_select_server(false, zreadPreference, intern->client, &server_id TSRMLS_CC)) {
349+
/* Exception should already have been thrown */
350+
goto cleanup;
351+
}
352+
353+
phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
354+
355+
cleanup:
356+
if (free_options) {
357+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
358+
}
305359
} /* }}} */
306360

307361
/* {{{ proto MongoDB\Driver\Cursor MongoDB\Driver\Manager::executeReadCommand(string $db, MongoDB\Driver\Command $command[, array $options = null])
@@ -313,6 +367,8 @@ static PHP_METHOD(Manager, executeReadCommand)
313367
phongo_zpp_char_len db_len;
314368
zval *command;
315369
zval *options = NULL;
370+
zval *zreadPreference = NULL;
371+
uint32_t server_id = 0;
316372
DECLARE_RETURN_VALUE_USED
317373
SUPPRESS_UNUSED_WARNING(return_value_ptr)
318374

@@ -322,7 +378,17 @@ static PHP_METHOD(Manager, executeReadCommand)
322378

323379
intern = Z_MANAGER_OBJ_P(getThis());
324380

325-
phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, -1, return_value, return_value_used TSRMLS_CC);
381+
if (!phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
382+
/* Exception should already have been thrown */
383+
return;
384+
}
385+
386+
if (!php_phongo_manager_select_server(false, zreadPreference, intern->client, &server_id TSRMLS_CC)) {
387+
/* Exception should already have been thrown */
388+
return;
389+
}
390+
391+
phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
326392
} /* }}} */
327393

328394
/* {{{ proto MongoDB\Driver\Cursor MongoDB\Driver\Manager::executeWriteCommand(string $db, MongoDB\Driver\Command $command[, array $options = null])
@@ -334,6 +400,7 @@ static PHP_METHOD(Manager, executeWriteCommand)
334400
phongo_zpp_char_len db_len;
335401
zval *command;
336402
zval *options = NULL;
403+
uint32_t server_id = 0;
337404
DECLARE_RETURN_VALUE_USED
338405
SUPPRESS_UNUSED_WARNING(return_value_ptr)
339406

@@ -343,7 +410,12 @@ static PHP_METHOD(Manager, executeWriteCommand)
343410

344411
intern = Z_MANAGER_OBJ_P(getThis());
345412

346-
phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, -1, return_value, return_value_used TSRMLS_CC);
413+
if (!php_phongo_manager_select_server(true, NULL, intern->client, &server_id TSRMLS_CC)) {
414+
/* Exception should already have been thrown */
415+
return;
416+
}
417+
418+
phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
347419
} /* }}} */
348420

349421
/* {{{ proto MongoDB\Driver\Cursor MongoDB\Driver\Manager::executeReadWriteCommand(string $db, MongoDB\Driver\Command $command[, array $options = null])
@@ -355,6 +427,7 @@ static PHP_METHOD(Manager, executeReadWriteCommand)
355427
phongo_zpp_char_len db_len;
356428
zval *command;
357429
zval *options = NULL;
430+
uint32_t server_id = 0;
358431
DECLARE_RETURN_VALUE_USED
359432
SUPPRESS_UNUSED_WARNING(return_value_ptr)
360433

@@ -364,7 +437,12 @@ static PHP_METHOD(Manager, executeReadWriteCommand)
364437

365438
intern = Z_MANAGER_OBJ_P(getThis());
366439

367-
phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, -1, return_value, return_value_used TSRMLS_CC);
440+
if (!php_phongo_manager_select_server(true, NULL, intern->client, &server_id TSRMLS_CC)) {
441+
/* Exception should already have been thrown */
442+
return;
443+
}
444+
445+
phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
368446
} /* }}} */
369447

370448
/* {{{ proto MongoDB\Driver\Cursor MongoDB\Driver\Manager::executeQuery(string $namespace, MongoDB\Driver\Query $query[, array $options = null])
@@ -376,6 +454,9 @@ static PHP_METHOD(Manager, executeQuery)
376454
phongo_zpp_char_len namespace_len;
377455
zval *query;
378456
zval *options = NULL;
457+
bool free_options = false;
458+
zval *zreadPreference = NULL;
459+
uint32_t server_id = 0;
379460
DECLARE_RETURN_VALUE_USED
380461
SUPPRESS_UNUSED_WARNING(return_value_ptr)
381462

@@ -385,7 +466,24 @@ static PHP_METHOD(Manager, executeQuery)
385466

386467
intern = Z_MANAGER_OBJ_P(getThis());
387468

388-
phongo_execute_query(intern->client, namespace, query, options, -1, return_value, return_value_used TSRMLS_CC);
469+
options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);
470+
471+
if (!phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
472+
/* Exception should already have been thrown */
473+
goto cleanup;
474+
}
475+
476+
if (!php_phongo_manager_select_server(false, zreadPreference, intern->client, &server_id TSRMLS_CC)) {
477+
/* Exception should already have been thrown */
478+
goto cleanup;
479+
}
480+
481+
phongo_execute_query(intern->client, namespace, query, options, server_id, return_value, return_value_used TSRMLS_CC);
482+
483+
cleanup:
484+
if (free_options) {
485+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
486+
}
389487
} /* }}} */
390488

391489
/* {{{ proto MongoDB\Driver\WriteResult MongoDB\Driver\Manager::executeBulkWrite(string $namespace, MongoDB\Driver\BulkWrite $zbulk[, array $options = null])
@@ -396,8 +494,10 @@ static PHP_METHOD(Manager, executeBulkWrite)
396494
char *namespace;
397495
phongo_zpp_char_len namespace_len;
398496
zval *zbulk;
399-
zval *options = NULL;
400497
php_phongo_bulkwrite_t *bulk;
498+
zval *options = NULL;
499+
bool free_options = false;
500+
uint32_t server_id = 0;
401501
DECLARE_RETURN_VALUE_USED
402502
SUPPRESS_UNUSED_WARNING(return_value_ptr)
403503

@@ -408,7 +508,19 @@ static PHP_METHOD(Manager, executeBulkWrite)
408508
intern = Z_MANAGER_OBJ_P(getThis());
409509
bulk = Z_BULKWRITE_OBJ_P(zbulk);
410510

411-
phongo_execute_bulk_write(intern->client, namespace, bulk, options, -1, return_value, return_value_used TSRMLS_CC);
511+
options = php_phongo_prep_legacy_option(options, "writeConcern", &free_options TSRMLS_CC);
512+
513+
if (!php_phongo_manager_select_server(true, NULL, intern->client, &server_id TSRMLS_CC)) {
514+
/* Exception should already have been thrown */
515+
goto cleanup;
516+
}
517+
518+
phongo_execute_bulk_write(intern->client, namespace, bulk, options, server_id, return_value, return_value_used TSRMLS_CC);
519+
520+
cleanup:
521+
if (free_options) {
522+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
523+
}
412524
} /* }}} */
413525

414526
/* {{{ proto MongoDB\Driver\ReadConcern MongoDB\Driver\Manager::getReadConcern()
@@ -511,9 +623,7 @@ static PHP_METHOD(Manager, selectServer)
511623
{
512624
php_phongo_manager_t *intern;
513625
zval *zreadPreference = NULL;
514-
const mongoc_read_prefs_t *readPreference;
515-
bson_error_t error;
516-
mongoc_server_description_t *selected_server = NULL;
626+
uint32_t server_id = 0;
517627
SUPPRESS_UNUSED_WARNING(return_value_ptr) SUPPRESS_UNUSED_WARNING(return_value_used)
518628

519629

@@ -523,19 +633,12 @@ static PHP_METHOD(Manager, selectServer)
523633
return;
524634
}
525635

526-
readPreference = phongo_read_preference_from_zval(zreadPreference TSRMLS_CC);
527-
selected_server = mongoc_client_select_server(intern->client, false, readPreference, &error);
528-
if (selected_server) {
529-
phongo_server_init(return_value, intern->client, mongoc_server_description_id(selected_server) TSRMLS_CC);
530-
mongoc_server_description_destroy(selected_server);
531-
} else {
532-
/* Check for connection related exceptions */
533-
if (EG(exception)) {
534-
return;
535-
}
536-
537-
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
636+
if (!php_phongo_manager_select_server(false, zreadPreference, intern->client, &server_id TSRMLS_CC)) {
637+
/* Exception should already have been thrown */
638+
return;
538639
}
640+
641+
phongo_server_init(return_value, intern->client, server_id TSRMLS_CC);
539642
} /* }}} */
540643

541644
/* {{{ proto MongoDB\Driver\Session MongoDB\Driver\Manager::startSession([array $options = null])

‎src/MongoDB/Server.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ static PHP_METHOD(Server, executeCommand)
3636
phongo_zpp_char_len db_len;
3737
zval *command;
3838
zval *options = NULL;
39+
bool free_options = false;
3940
DECLARE_RETURN_VALUE_USED
4041
SUPPRESS_UNUSED_WARNING(return_value_ptr)
4142

@@ -46,7 +47,13 @@ static PHP_METHOD(Server, executeCommand)
4647
return;
4748
}
4849

50+
options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);
51+
4952
phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
53+
54+
if (free_options) {
55+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
56+
}
5057
} /* }}} */
5158

5259
/* {{{ proto MongoDB\Driver\Cursor MongoDB\Driver\Server::executeReadCommand(string $db, MongoDB\Driver\Command $command[, array $options = null]))
@@ -124,6 +131,7 @@ static PHP_METHOD(Server, executeQuery)
124131
phongo_zpp_char_len namespace_len;
125132
zval *query;
126133
zval *options = NULL;
134+
bool free_options = false;
127135
DECLARE_RETURN_VALUE_USED
128136
SUPPRESS_UNUSED_WARNING(return_value_ptr)
129137

@@ -134,7 +142,13 @@ static PHP_METHOD(Server, executeQuery)
134142
return;
135143
}
136144

145+
options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);
146+
137147
phongo_execute_query(intern->client, namespace, query, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
148+
149+
if (free_options) {
150+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
151+
}
138152
} /* }}} */
139153

140154
/* {{{ proto MongoDB\Driver\WriteResult MongoDB\Driver\Server::executeBulkWrite(string $namespace, MongoDB\Driver\BulkWrite $zbulk[, array $options = null])
@@ -146,8 +160,9 @@ static PHP_METHOD(Server, executeBulkWrite)
146160
char *namespace;
147161
phongo_zpp_char_len namespace_len;
148162
zval *zbulk;
149-
zval *options = NULL;
150163
php_phongo_bulkwrite_t *bulk;
164+
zval *options = NULL;
165+
bool free_options = false;
151166
DECLARE_RETURN_VALUE_USED
152167
SUPPRESS_UNUSED_WARNING(return_value_ptr)
153168

@@ -161,7 +176,13 @@ static PHP_METHOD(Server, executeBulkWrite)
161176

162177
bulk = Z_BULKWRITE_OBJ_P(zbulk);
163178

179+
options = php_phongo_prep_legacy_option(options, "writeConcern", &free_options TSRMLS_CC);
180+
164181
phongo_execute_bulk_write(intern->client, namespace, bulk, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
182+
183+
if (free_options) {
184+
php_phongo_prep_legacy_option_free(options TSRMLS_CC);
185+
}
165186
} /* }}} */
166187

167188
/* {{{ proto string MongoDB\Driver\Server::getHost()

0 commit comments

Comments
 (0)