Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

PHPC-1021: Remove support for ReadPreference integer modes #1666

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.

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 3 commits into from
Sep 19, 2024

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Sep 18, 2024

https://jira.mongodb.org/browse/PHPC-1021

This removes the deprecated integer constants, constructor support, and getMode().

Error-checking in php_phongo_readpreference_get_mode_string() has also been removed. ReadPreference only uses mongoc_read_prefs_new() internally, and we should not need to worry about libmongoc feeding us mongoc_read_prefs_t with an invalid mode.

@@ -526,16 +477,12 @@ static PHP_METHOD(MongoDB_Driver_ReadPreference, serialize)
}

tags = mongoc_read_prefs_get_tags(intern->read_preference);
mode = mongoc_read_prefs_get_mode(intern->read_preference);
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this will conflict with #1663, depending on which gets merged first.

phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Invalid mode: '%s'", Z_STRVAL_P(mode));
return;
}
if (zend_string_equals_literal_ci(mode, PHONGO_READ_PRIMARY)) {
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 was delighted to find this in zend_string.h.

@@ -153,9 +153,9 @@ static bool php_phongo_readpreference_init_from_hash(php_phongo_readpreference_t
return false;
}

static const char* php_phongo_readpreference_get_mode_string(mongoc_read_mode_t mode)
static const char* php_phongo_readpreference_get_mode_string(const mongoc_read_prefs_t* read_prefs)
Copy link
Member Author

Choose a reason for hiding this comment

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

This lets us eliminate some mongoc_read_prefs_get_mode() calls in parent contexts.

Additionally, I changed this function to return an empty string instead of throwing in the very unlikely event that the RP mode is invalid. _mongoc_read_mode_as_str() does the same, and libmongoc uses that for appending read preferences to command payloads. In our case, this is used for serialization and debugging, and the change allowed us to clean up the parent contexts.

@@ -1,18 +0,0 @@
--TEST--
MongoDB\Driver\ReadPreference construction (invalid string mode)
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was redundant as readpreference-ctor_error-001.phpt uses an invalid string value.

require_once __DIR__ . '/../utils/basic.inc';

echo throws(function() {
new MongoDB\Driver\ReadPreference(3.14);
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we replaced string|int with string in the constructor signature, type coercion is now applied outside of strict mode and this would be implicitly cast to '3.14'. I didn't see any reason to use a different type (or declare strict types) just to assert a TypeError.

This removes the deprecated integer constants, constructor support, and getMode().

Error-checking in php_phongo_readpreference_get_mode_string() has also been removed. ReadPreference only uses mongoc_read_prefs_new() internally, and we should not need to worry about libmongoc feeding us mongoc_read_prefs_t with an invalid mode.
@alcaeus alcaeus enabled auto-merge (squash) September 19, 2024 10:17
@alcaeus alcaeus merged commit d20de7e into mongodb:v2.x Sep 19, 2024
62 checks passed
@jmikola jmikola deleted the 2.0-phpc-1021 branch September 20, 2024 20:04
alcaeus added a commit that referenced this pull request Oct 29, 2024
* v2.x: (22 commits)
  PHPC-2441: Remove deprecated Manager constructor options (#1719)
  PHPC-990: Strict type validation for boolean URI options (#1713)
  PHPC-2440: Remove deprecated Query constructor options (#1707)
  PHPC-2459: Remove support for float arg in UTCDateTime ctor (#1709)
  Remove obsolete test
  PHPC-2344 Remove SSLConnectionException (#1696)
  PHPC-2144 Throw a LogicException when getting info from unacknowledged write result (#1687)
  PHPC-2454: Remove --enable-system-ciphers configure option (#1681)
  PHPC-2348 Remove `WriteException` and move `getWriteResult` to `BulkWriteException` (#1685)
  PHPC-2417 Add UTCDateTimeInterface::toDateTimeImmutable() (#1684)
  PHPC-2309: Remove --with-openssl-dir configure option (#1676)
  PHPC-2444: Remove support for string arguments in UTCDateTime constructor (#1662)
  PHPC-2248: Remove Serializable implementations (#1663)
  Update version for 2.x branch (#1672)
  PHPC-1021: Remove support for ReadPreference integer modes (#1666)
  PHPC-2342: Remove --with-libbson and --with-libmongoc configure options (#1667)
  PHPC-2351: Remove CursorId class (#1664)
  PHPC-2140: Make tentative return types definitive (#1658)
  PHPC-2402: Remove range_preview constants (#1665)
  PHPC-2346: Remove deprecated BSON functions (#1653)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.