Skip to content

Commit eea07a0

Browse files
derickrderickr
committed
Fixed review comments, use COMMAND_* types instead of *ALLOWED constants, and fixed wrong name in error message
1 parent b2e90bb commit eea07a0

File tree

2 files changed

+29
-26
lines changed

2 files changed

+29
-26
lines changed

‎php_phongo.c

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -448,32 +448,30 @@ mongoc_bulk_operation_t *phongo_bulkwrite_init(zend_bool ordered) { /* {{{ */
448448
return mongoc_bulk_operation_new(ordered);
449449
} /* }}} */
450450

451-
#define PHONGO_WRITECONCERN_ALLOWED 0x01
452-
#define PHONGO_READPREFERENCE_ALLOWED 0x02
453-
454451
static bool process_read_concern(zval *option, bson_t *mongoc_opts TSRMLS_DC)
455452
{
456453
if (Z_TYPE_P(option) == IS_OBJECT && instanceof_function(Z_OBJCE_P(option), php_phongo_readconcern_ce TSRMLS_CC)) {
457454
const mongoc_read_concern_t *read_concern = phongo_read_concern_from_zval(option TSRMLS_CC);
458455

459456
if (!mongoc_read_concern_append((mongoc_read_concern_t*)read_concern, mongoc_opts)) {
460-
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"%s\" option", "ReadPreference");
457+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"%s\" option", "ReadConcern");
458+
return false;
461459
}
462460
} else {
463461
phongo_throw_exception(
464462
PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC,
465-
"Expected 'readConcern' option to be 'MongoDB\\Driver\\ReadPreference', %s given",
463+
"Expected 'readConcern' option to be 'MongoDB\\Driver\\ReadConcern', %s given",
466464
zend_get_type_by_const(Z_TYPE_P(option))
467465
);
468466
return false;
469467
}
470468
return true;
471469
}
472470

473-
static int phongo_do_select_server(mongoc_client_t *client, bson_t *opts, zval *zreadPreference, int server_id TSRMLS_DC)
471+
static uint32_t phongo_do_select_server(mongoc_client_t *client, bson_t *opts, zval *zreadPreference, int server_id TSRMLS_DC)
474472
{
475473
bson_error_t error;
476-
uint32_t selected_server_id;
474+
uint32_t selected_server_id = 0;
477475

478476
if (server_id > 0) {
479477
bson_append_int32(opts, "serverId", -1, server_id);
@@ -491,8 +489,6 @@ static int phongo_do_select_server(mongoc_client_t *client, bson_t *opts, zval *
491489
if (!EG(exception)) {
492490
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
493491
}
494-
495-
return false;
496492
}
497493
}
498494

@@ -504,7 +500,9 @@ static bool process_read_preference(zval *option, bson_t *mongoc_opts, zval **zr
504500
if (Z_TYPE_P(option) == IS_OBJECT && instanceof_function(Z_OBJCE_P(option), php_phongo_readpreference_ce TSRMLS_CC)) {
505501
int selected_server_id;
506502

507-
*zreadPreference = option;
503+
if (zreadPreference) {
504+
*zreadPreference = option;
505+
}
508506

509507
selected_server_id = phongo_do_select_server(client, mongoc_opts, *zreadPreference, server_id TSRMLS_CC);
510508
if (!selected_server_id) {
@@ -526,7 +524,9 @@ static bool process_write_concern(zval *option, bson_t *mongoc_opts, zval **zwri
526524
if (Z_TYPE_P(option) == IS_OBJECT && instanceof_function(Z_OBJCE_P(option), php_phongo_writeconcern_ce TSRMLS_CC)) {
527525
const mongoc_write_concern_t *write_concern = phongo_write_concern_from_zval(option TSRMLS_CC);
528526

529-
*zwriteConcern = option;
527+
if (zwriteConcern) {
528+
*zwriteConcern = option;
529+
}
530530

531531
if (!mongoc_write_concern_append((mongoc_write_concern_t*) write_concern, mongoc_opts)) {
532532
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"%s\" option", "WriteConcern");
@@ -542,7 +542,7 @@ static bool process_write_concern(zval *option, bson_t *mongoc_opts, zval **zwri
542542
return true;
543543
}
544544

545-
static int phongo_execute_parse_options(mongoc_client_t* client, int server_id, zval *driver_options, int flags, bson_t *mongoc_opts, zval **zreadPreference, zval **zwriteConcern TSRMLS_DC)
545+
static int phongo_execute_parse_options(mongoc_client_t* client, int server_id, zval *driver_options, int type, bson_t *mongoc_opts, zval **zreadPreference, zval **zwriteConcern TSRMLS_DC)
546546
{
547547
if (driver_options && Z_TYPE_P(driver_options) == IS_ARRAY) {
548548
HashTable *ht_data = HASH_OF(driver_options);
@@ -557,15 +557,15 @@ static int phongo_execute_parse_options(mongoc_client_t* client, int server_id,
557557
}
558558

559559
/* URI options are case-insensitive */
560-
if (!strcasecmp(ZSTR_VAL(string_key), "readConcern")) {
560+
if ((!strcasecmp(ZSTR_VAL(string_key), "readConcern")) && (type & PHONGO_COMMAND_READ)) {
561561
if (!process_read_concern(driver_option, mongoc_opts)) {
562562
return false;
563563
}
564-
} else if ((!strcasecmp(ZSTR_VAL(string_key), "readPreference")) && (flags & PHONGO_READPREFERENCE_ALLOWED)) {
564+
} else if ((!strcasecmp(ZSTR_VAL(string_key), "readPreference")) && (type == PHONGO_COMMAND_READ || type == PHONGO_COMMAND_RAW)) {
565565
if (!process_read_preference(driver_option, mongoc_opts, zreadPreference, client, server_id)) {
566566
return false;
567567
}
568-
} else if ((!strcasecmp(ZSTR_VAL(string_key), "writeConcern")) && (flags & PHONGO_WRITECONCERN_ALLOWED)) {
568+
} else if ((!strcasecmp(ZSTR_VAL(string_key), "writeConcern")) && (type & PHONGO_COMMAND_WRITE)) {
569569
if (!process_write_concern(driver_option, mongoc_opts, zwriteConcern)) {
570570
return false;
571571
}
@@ -590,15 +590,15 @@ static int phongo_execute_parse_options(mongoc_client_t* client, int server_id,
590590
}
591591

592592
/* URI options are case-insensitive */
593-
if (!strcasecmp(string_key, "readConcern")) {
593+
if ((!strcasecmp(string_key, "readConcern")) && (type & PHONGO_COMMAND_READ)) {
594594
if (!process_read_concern(*driver_option, mongoc_opts)) {
595595
return false;
596596
}
597-
} else if ((!strcasecmp(string_key, "readPreference")) && (flags & PHONGO_READPREFERENCE_ALLOWED)) {
597+
} else if ((!strcasecmp(string_key, "readPreference")) && (type == PHONGO_COMMAND_READ)) {
598598
if (!process_read_preference(*driver_option, mongoc_opts, zreadPreference, client, server_id TSRMLS_CC)) {
599599
return false;
600600
}
601-
} else if ((!strcasecmp(string_key, "writeConcern")) && (flags & PHONGO_WRITECONCERN_ALLOWED)) {
601+
} else if ((!strcasecmp(string_key, "writeConcern")) && (type & PHONGO_COMMAND_WRITE)) {
602602
if (!process_write_concern(*driver_option, mongoc_opts, zwriteConcern TSRMLS_CC)) {
603603
return false;
604604
}
@@ -635,7 +635,7 @@ bool phongo_execute_write(mongoc_client_t *client, const char *namespace, php_ph
635635
/* FIXME: Legacy way of specifying the writeConcern option into this function */
636636
if (options && Z_TYPE_P(options) == IS_OBJECT && instanceof_function(Z_OBJCE_P(options), php_phongo_writeconcern_ce TSRMLS_CC)) {
637637
zwriteConcern = options;
638-
} else if (!phongo_execute_parse_options(client, server_id, options, PHONGO_WRITECONCERN_ALLOWED, NULL, NULL, &zwriteConcern TSRMLS_CC)) {
638+
} else if (!phongo_execute_parse_options(client, server_id, options, PHONGO_COMMAND_WRITE, NULL, NULL, &zwriteConcern TSRMLS_CC)) {
639639
return false;
640640
}
641641

@@ -742,7 +742,7 @@ int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *z
742742
/* FIXME: Legacy way of specifying the readPreference option into this function */
743743
if (options && Z_TYPE_P(options) == IS_OBJECT && instanceof_function(Z_OBJCE_P(options), php_phongo_readpreference_ce TSRMLS_CC)) {
744744
zreadPreference = options;
745-
} else if (!phongo_execute_parse_options(client, server_id, options, PHONGO_READPREFERENCE_ALLOWED, NULL, &zreadPreference, NULL TSRMLS_CC)) {
745+
} else if (!phongo_execute_parse_options(client, server_id, options, PHONGO_COMMAND_READ, NULL, &zreadPreference, NULL TSRMLS_CC)) {
746746
return false;
747747
}
748748

@@ -804,7 +804,7 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
804804
/* FIXME: Legacy way of specifying the readPreference option into this function */
805805
if (options && Z_TYPE_P(options) == IS_OBJECT && instanceof_function(Z_OBJCE_P(options), php_phongo_readpreference_ce TSRMLS_CC)) {
806806
zreadPreference = options;
807-
} else if (!phongo_execute_parse_options(client, server_id, options, PHONGO_READPREFERENCE_ALLOWED, opts, &zreadPreference, NULL TSRMLS_CC)) {
807+
} else if (!phongo_execute_parse_options(client, server_id, options, type, opts, &zreadPreference, NULL TSRMLS_CC)) {
808808
return false;
809809
}
810810

‎php_phongo.h

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

114-
/* This enum is used for libmongoc function selection for the phongo_execute_command types */
114+
/* This enum is used for libmongoc function selection for the
115+
* phongo_execute_command types. The values are important, as the READ and
116+
* WRITE fields are also used as a bit field to see whether ReadPreference,
117+
* ReadConcern, and WriteConcern are supported for each type. */
115118
typedef enum {
116-
PHONGO_COMMAND_RAW = 1,
117-
PHONGO_COMMAND_READ = 2,
118-
PHONGO_COMMAND_WRITE = 3,
119-
PHONGO_COMMAND_READ_WRITE = 4
119+
PHONGO_COMMAND_RAW = 0x13,
120+
PHONGO_COMMAND_READ = 0x01,
121+
PHONGO_COMMAND_WRITE = 0x02,
122+
PHONGO_COMMAND_READ_WRITE = 0x03
120123
} php_phongo_command_type_t;
121124

122125
zend_object_handlers *phongo_get_std_object_handlers(void);

0 commit comments

Comments
 (0)