Skip to content

PHPC-2440: Remove deprecated Query constructor options #1707

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 2 commits into from
Oct 2, 2024
Merged

PHPC-2440: Remove deprecated Query constructor options #1707

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
3bf7dbc
PHPC-2440: Remove deprecated Query constructor options
jmikola Oct 1, 2024
d2018ef
Add upgrade notes
jmikola Oct 1, 2024
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
4 changes: 4 additions & 0 deletions UPGRADE-2.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ UPGRADE FROM 1.x to 2.0
ensure that it is detected by `pkg-config`.
* The `--with-system-ciphers` configure option has been removed. Use
`--enable-mongodb-crypto-system-profile` instead.
* `MongoDB\Driver\Query` removes the following options: `partial` (use
`allowPartialResults` instead), `maxScan`, `modifiers` (use alternative
top-level options instead), `oplogReplay`, and `snapshot`. Support for
negative `limit` values has been removed (use `singleBatch` instead).
Copy link
Member Author

Choose a reason for hiding this comment

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

@alcaeus: I realized I forgot to add upgrade notes before merging. Doing so now and will merge this down once CI comes back green.

160 changes: 31 additions & 129 deletions src/MongoDB/Query.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ static bool php_phongo_query_opts_append_string(bson_t* opts, const char* opts_k
zval* value = php_array_fetch_deref(zarr, zarr_key);

if (Z_TYPE_P(value) != IS_STRING) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"%s\" %s to be string, %s given", zarr_key, zarr_key[0] == '$' ? "modifier" : "option", PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(value));
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"%s\" option to be string, %s given", zarr_key, PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(value));
return false;
}

Expand All @@ -59,7 +59,7 @@ static bool php_phongo_query_opts_append_document(bson_t* opts, const char* opts
bson_t b = BSON_INITIALIZER;

if (Z_TYPE_P(value) != IS_OBJECT && Z_TYPE_P(value) != IS_ARRAY) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"%s\" %s to be array or object, %s given", zarr_key, zarr_key[0] == '$' ? "modifier" : "option", PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(value));
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"%s\" option to be array or object, %s given", zarr_key, PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(value));
return false;
}

Expand All @@ -71,7 +71,7 @@ static bool php_phongo_query_opts_append_document(bson_t* opts, const char* opts
}

if (!bson_validate(&b, BSON_VALIDATE_EMPTY_KEYS, NULL)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Cannot use empty keys in \"%s\" %s", zarr_key, zarr_key[0] == '$' ? "modifier" : "option");
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Cannot use empty keys in \"%s\" option", zarr_key);
bson_destroy(&b);
return false;
}
Expand Down Expand Up @@ -110,20 +110,14 @@ static bool php_phongo_query_opts_append_value(bson_t* opts, const char* opts_ke
return true;
}

#define PHONGO_QUERY_OPT_BOOL_EX(opt, zarr, key, deprecated) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if ((deprecated)) { \
php_error_docref(NULL, E_DEPRECATED, "The \"%s\" option is deprecated and will be removed in a future release", key); \
} \
if (!BSON_APPEND_BOOL(intern->opts, (opt), php_array_fetchc_bool((zarr), (key)))) { \
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"%s\" option", (opt)); \
return false; \
} \
#define PHONGO_QUERY_OPT_BOOL(opt, zarr, key) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if (!BSON_APPEND_BOOL(intern->opts, (opt), php_array_fetchc_bool((zarr), (key)))) { \
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"%s\" option", (opt)); \
return false; \
} \
}

#define PHONGO_QUERY_OPT_BOOL(opt, zarr, key) PHONGO_QUERY_OPT_BOOL_EX((opt), (zarr), (key), 0)
#define PHONGO_QUERY_OPT_BOOL_DEPRECATED(opt, zarr, key) PHONGO_QUERY_OPT_BOOL_EX((opt), (zarr), (key), 1)

#define PHONGO_QUERY_OPT_BSON_VALUE(opt, zarr, key) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if (!php_phongo_query_opts_append_value(intern->opts, (opt), (zarr), (key))) { \
Expand All @@ -140,21 +134,14 @@ static bool php_phongo_query_opts_append_value(bson_t* opts, const char* opts_ke

/* Note: handling of integer options will depend on SIZEOF_ZEND_LONG and we
* are not converting strings to 64-bit integers for 32-bit platforms. */

#define PHONGO_QUERY_OPT_INT64_EX(opt, zarr, key, deprecated) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if ((deprecated)) { \
php_error_docref(NULL, E_DEPRECATED, "The \"%s\" option is deprecated and will be removed in a future release", key); \
} \
if (!BSON_APPEND_INT64(intern->opts, (opt), php_array_fetchc_long((zarr), (key)))) { \
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"%s\" option", (opt)); \
return false; \
} \
#define PHONGO_QUERY_OPT_INT64(opt, zarr, key) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if (!BSON_APPEND_INT64(intern->opts, (opt), php_array_fetchc_long((zarr), (key)))) { \
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"%s\" option", (opt)); \
return false; \
} \
}

#define PHONGO_QUERY_OPT_INT64(opt, zarr, key) PHONGO_QUERY_OPT_INT64_EX((opt), (zarr), (key), 0)
#define PHONGO_QUERY_OPT_INT64_DEPRECATED(opt, zarr, key) PHONGO_QUERY_OPT_INT64_EX((opt), (zarr), (key), 1)

#define PHONGO_QUERY_OPT_STRING(opt, zarr, key) \
if ((zarr) && php_array_existsc((zarr), (key))) { \
if (!php_phongo_query_opts_append_string(intern->opts, (opt), (zarr), (key))) { \
Expand All @@ -165,12 +152,10 @@ static bool php_phongo_query_opts_append_value(bson_t* opts, const char* opts_ke
/* Initialize the "hint" option. Returns true on success; otherwise, false is
* returned and an exception is thrown.
*
* The "hint" option (or "$hint" modifier) must be a string or document. Check
* for both types and merge into BSON options accordingly. */
static bool php_phongo_query_init_hint(php_phongo_query_t* intern, zval* options, zval* modifiers)
* The "hint" option must be a string or document. Check for both types and
* merge into BSON options accordingly. */
static bool php_phongo_query_init_hint(php_phongo_query_t* intern, zval* options)
{
/* The "hint" option (or "$hint" modifier) must be a string or document.
* Check for both types and merge into BSON options accordingly. */
if (php_array_existsc(options, "hint")) {
zend_uchar type = Z_TYPE_P(php_array_fetchc_deref(options, "hint"));

Expand All @@ -182,50 +167,6 @@ static bool php_phongo_query_init_hint(php_phongo_query_t* intern, zval* options
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"hint\" option to be string, array, or object, %s given", zend_get_type_by_const(type));
return false;
}
} else if (modifiers && php_array_existsc(modifiers, "$hint")) {
zend_uchar type = Z_TYPE_P(php_array_fetchc_deref(modifiers, "$hint"));

if (type == IS_STRING) {
PHONGO_QUERY_OPT_STRING("hint", modifiers, "$hint");
} else if (type == IS_OBJECT || type == IS_ARRAY) {
PHONGO_QUERY_OPT_DOCUMENT("hint", modifiers, "$hint");
} else {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"$hint\" modifier to be string, array, or object, %s given", zend_get_type_by_const(type));
return false;
}
}

return true;
}

/* Initialize the "limit" and "singleBatch" options. Returns true on success;
* otherwise, false is returned and an exception is thrown.
*
* mongoc_collection_find_with_opts() requires a non-negative limit. For
* backwards compatibility, a negative limit should be set as a positive value
* and default singleBatch to true. */
static bool php_phongo_query_init_limit_and_singlebatch(php_phongo_query_t* intern, zval* options)
{
if (php_array_fetchc_long(options, "limit") < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Noted that we'll add a deprecation for this in PHPC-2464.

zend_long limit = php_array_fetchc_long(options, "limit");

if (!BSON_APPEND_INT64(intern->opts, "limit", -limit)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"limit\" option");
return false;
}

if (php_array_existsc(options, "singleBatch") && !php_array_fetchc_bool(options, "singleBatch")) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Negative \"limit\" option conflicts with false \"singleBatch\" option");
return false;
} else {
if (!BSON_APPEND_BOOL(intern->opts, "singleBatch", true)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Error appending \"singleBatch\" option");
return false;
}
}
} else {
PHONGO_QUERY_OPT_INT64("limit", options, "limit");
PHONGO_QUERY_OPT_BOOL("singleBatch", options, "singleBatch");
}

return true;
Expand Down Expand Up @@ -287,14 +228,10 @@ static bool php_phongo_query_init_max_await_time_ms(php_phongo_query_t* intern,
}

/* Initializes the query from filter and options arguments and returns whether
* an error occurred. If query is undefined, it will be initialized.
*
* This function will fall back to a modifier in the absence of a top-level
* option (where applicable). */
* an error occurred. If query is undefined, it will be initialized. */
bool phongo_query_init(zval* return_value, zval* filter, zval* options)
{
php_phongo_query_t* intern;
zval* modifiers = NULL;

if (Z_ISUNDEF_P(return_value)) {
object_init_ex(return_value, php_phongo_query_ce);
Expand Down Expand Up @@ -330,59 +267,28 @@ bool phongo_query_init(zval* return_value, zval* filter, zval* options)
return true;
}

if (php_array_existsc(options, "modifiers")) {
modifiers = php_array_fetchc_deref(options, "modifiers");

if (Z_TYPE_P(modifiers) != IS_ARRAY) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"modifiers\" option to be array, %s given", PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(modifiers));
return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"modifiers\" option is deprecated and will be removed in a future release");
}

PHONGO_QUERY_OPT_BOOL("allowDiskUse", options, "allowDiskUse")
PHONGO_QUERY_OPT_BOOL("allowPartialResults", options, "allowPartialResults")
else PHONGO_QUERY_OPT_BOOL("allowPartialResults", options, "partial");
PHONGO_QUERY_OPT_BOOL("allowDiskUse", options, "allowDiskUse");
PHONGO_QUERY_OPT_BOOL("allowPartialResults", options, "allowPartialResults");
PHONGO_QUERY_OPT_BOOL("awaitData", options, "awaitData");
PHONGO_QUERY_OPT_INT64("batchSize", options, "batchSize");
PHONGO_QUERY_OPT_DOCUMENT("collation", options, "collation");
PHONGO_QUERY_OPT_BSON_VALUE("comment", options, "comment")
else PHONGO_QUERY_OPT_BSON_VALUE("comment", modifiers, "$comment");
PHONGO_QUERY_OPT_BSON_VALUE("comment", options, "comment");
PHONGO_QUERY_OPT_BOOL("exhaust", options, "exhaust");
PHONGO_QUERY_OPT_DOCUMENT("let", options, "let");
PHONGO_QUERY_OPT_DOCUMENT("max", options, "max")
else PHONGO_QUERY_OPT_DOCUMENT("max", modifiers, "$max");
PHONGO_QUERY_OPT_INT64_DEPRECATED("maxScan", options, "maxScan")
else PHONGO_QUERY_OPT_INT64_DEPRECATED("maxScan", modifiers, "$maxScan");
PHONGO_QUERY_OPT_INT64("maxTimeMS", options, "maxTimeMS")
else PHONGO_QUERY_OPT_INT64("maxTimeMS", modifiers, "$maxTimeMS");
PHONGO_QUERY_OPT_DOCUMENT("min", options, "min")
else PHONGO_QUERY_OPT_DOCUMENT("min", modifiers, "$min");
PHONGO_QUERY_OPT_INT64("limit", options, "limit");
PHONGO_QUERY_OPT_DOCUMENT("max", options, "max");
PHONGO_QUERY_OPT_INT64("maxTimeMS", options, "maxTimeMS");
PHONGO_QUERY_OPT_DOCUMENT("min", options, "min");
PHONGO_QUERY_OPT_BOOL("noCursorTimeout", options, "noCursorTimeout");
PHONGO_QUERY_OPT_BOOL_DEPRECATED("oplogReplay", options, "oplogReplay");
PHONGO_QUERY_OPT_DOCUMENT("projection", options, "projection");
PHONGO_QUERY_OPT_BOOL("returnKey", options, "returnKey")
else PHONGO_QUERY_OPT_BOOL("returnKey", modifiers, "$returnKey");
PHONGO_QUERY_OPT_BOOL("showRecordId", options, "showRecordId")
else PHONGO_QUERY_OPT_BOOL("showRecordId", modifiers, "$showDiskLoc");
PHONGO_QUERY_OPT_BOOL("returnKey", options, "returnKey");
PHONGO_QUERY_OPT_BOOL("showRecordId", options, "showRecordId");
PHONGO_QUERY_OPT_BOOL("singleBatch", options, "singleBatch");
PHONGO_QUERY_OPT_INT64("skip", options, "skip");
PHONGO_QUERY_OPT_DOCUMENT("sort", options, "sort")
else PHONGO_QUERY_OPT_DOCUMENT("sort", modifiers, "$orderby");
PHONGO_QUERY_OPT_BOOL_DEPRECATED("snapshot", options, "snapshot")
else PHONGO_QUERY_OPT_BOOL_DEPRECATED("snapshot", modifiers, "$snapshot");
PHONGO_QUERY_OPT_DOCUMENT("sort", options, "sort");
PHONGO_QUERY_OPT_BOOL("tailable", options, "tailable");

/* The "$explain" modifier should be converted to an "explain" option, which
* libmongoc will later convert back to a modifier for the OP_QUERY code
* path. This modifier will be ignored for the find command code path. */
PHONGO_QUERY_OPT_BOOL("explain", modifiers, "$explain");

if (!php_phongo_query_init_hint(intern, options, modifiers)) {
return false;
}

if (!php_phongo_query_init_limit_and_singlebatch(intern, options)) {
if (!php_phongo_query_init_hint(intern, options)) {
return false;
}

Expand All @@ -397,14 +303,10 @@ bool phongo_query_init(zval* return_value, zval* filter, zval* options)
return true;
}

#undef PHONGO_QUERY_OPT_BOOL_EX
#undef PHONGO_QUERY_OPT_BOOL
#undef PHONGO_QUERY_OPT_BOOL_DEPRECATED
#undef PHONGO_QUERY_OPT_BSON_VALUE
#undef PHONGO_QUERY_OPT_DOCUMENT
#undef PHONGO_QUERY_OPT_INT64_EX
#undef PHONGO_QUERY_OPT_INT64
#undef PHONGO_QUERY_OPT_INT64_DEPRECATED
#undef PHONGO_QUERY_OPT_STRING

/* Constructs a new Query */
Expand Down
22 changes: 4 additions & 18 deletions tests/query/query-ctor-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,15 @@ var_dump(new MongoDB\Driver\Query(
'exhaust' => false,
'limit' => 20,
'max' => ['y' => 100],
'maxScan' => 50,
'maxTimeMS' => 1000,
'min' => ['y' => 1],
'noCursorTimeout' => false,
'oplogReplay' => false,
'projection' => ['x' => 1, 'y' => 1],
'returnKey' => false,
'showRecordId' => false,
'singleBatch' => false,
'skip' => 5,
'sort' => ['y' => -1],
'snapshot' => false,
'tailable' => false,
]
));
Expand All @@ -50,11 +47,6 @@ var_dump(new MongoDB\Driver\Query(
===DONE===
<?php exit(0); ?>
--EXPECTF--
Deprecated: MongoDB\Driver\Query::__construct(): The "maxScan" option is deprecated and will be removed in a future release in %s on line %d

Deprecated: MongoDB\Driver\Query::__construct(): The "oplogReplay" option is deprecated and will be removed in a future release in %s on line %d

Deprecated: MongoDB\Driver\Query::__construct(): The "snapshot" option is deprecated and will be removed in a future release in %s on line %d
object(MongoDB\Driver\Query)#%d (%d) {
["filter"]=>
object(stdClass)#%d (%d) {
Expand All @@ -80,13 +72,13 @@ object(MongoDB\Driver\Query)#%d (%d) {
string(3) "foo"
["exhaust"]=>
bool(false)
["limit"]=>
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed php_phongo_query_init_limit_and_singlebatch(), so these options are now parsed alphabetically alongside other options. Since those parsing macros are responsible for appending them to the cursor's BSON options, it impacted the debug output order (where we dump the options document as-is).

int(20)
["max"]=>
object(stdClass)#%d (%d) {
["y"]=>
int(100)
}
["maxScan"]=>
int(50)
["maxTimeMS"]=>
int(1000)
["min"]=>
Expand All @@ -96,8 +88,6 @@ object(MongoDB\Driver\Query)#%d (%d) {
}
["noCursorTimeout"]=>
bool(false)
["oplogReplay"]=>
bool(false)
["projection"]=>
object(stdClass)#%d (%d) {
["x"]=>
Expand All @@ -109,21 +99,17 @@ object(MongoDB\Driver\Query)#%d (%d) {
bool(false)
["showRecordId"]=>
bool(false)
["singleBatch"]=>
bool(false)
["skip"]=>
int(5)
["sort"]=>
object(stdClass)#%d (%d) {
["y"]=>
int(-1)
}
["snapshot"]=>
bool(false)
["tailable"]=>
bool(false)
["limit"]=>
int(20)
["singleBatch"]=>
bool(false)
}
["readConcern"]=>
NULL
Expand Down
Loading