Skip to content
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

PHPC-2444: Remove support for string arguments in UTCDateTime constructor #1662

Merged

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Sep 18, 2024

}

phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected integer or string, %s given", PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(milliseconds));
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected integer or object, %s given", PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(milliseconds));
Copy link
Member

Choose a reason for hiding this comment

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

Odd that "object" was never included in the original message. Looks like that dates back to 8d7e358.

tests/bson/bson-utcdatetime-008.phpt Outdated Show resolved Hide resolved
@@ -26,20 +26,13 @@ echo throws(function() {
?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
Deprecated: MongoDB\BSON\UTCDateTime::__construct(): Creating a MongoDB\BSON\UTCDateTime instance with a string is deprecated and will be removed in ext-mongodb 2.0 in %s
--EXPECT--
Copy link
Member

Choose a reason for hiding this comment

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

This test appears entirely redundant in light of bson-utcdatetime-008.phpt, but I think both can be deleted in favor of bson-utcdatetime_error-004.phpt

@@ -21,7 +21,7 @@ foreach ($invalidValues as $invalidValue) {
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Expected integer or string, bool given
Expected integer or object, bool given
Copy link
Member

Choose a reason for hiding this comment

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

This test file looks perfectly serviceable to add a case for a numeric string. I don't think we need to bother with testing a non-numeric string since it'd just trigger the same code path in the constructor.

I'd suggest removing both of the other test files and adding to the comment about $invalidValues above to explain why you're using a numeric string (previously supported but now we require Int64, yada yada).

@alcaeus alcaeus force-pushed the phpc-2444-remove-utcdatetime-string-argument branch from 5d8643a to 2057018 Compare September 19, 2024 06:20
@alcaeus alcaeus requested review from jmikola and removed request for GromNaN September 19, 2024 06:20
@alcaeus alcaeus force-pushed the phpc-2444-remove-utcdatetime-string-argument branch 2 times, most recently from 3a8a430 to b47023a Compare September 19, 2024 11:24
@alcaeus alcaeus enabled auto-merge (squash) September 19, 2024 11:24
@jmikola
Copy link
Member

jmikola commented Sep 19, 2024

@alcaeus: I assume this conflicts after merging the Serializable PR (#1663). I'll leave it to you to resolve and merge down.

@alcaeus alcaeus force-pushed the phpc-2444-remove-utcdatetime-string-argument branch from b47023a to 4739a6c Compare September 20, 2024 06:45
@alcaeus alcaeus merged commit 9cce39d into mongodb:v2.x Sep 20, 2024
61 checks passed
@alcaeus alcaeus deleted the phpc-2444-remove-utcdatetime-string-argument branch September 20, 2024 07:55
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