Skip to content

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

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
Sep 20, 2024
Merged

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

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
7cbea84
PHPC-2444: Remove support for string arguments in UTCDateTime constru…
alcaeus Sep 17, 2024
4739a6c
Consolidate tests for invalid values
alcaeus Sep 19, 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
3 changes: 3 additions & 0 deletions UPGRADE-2.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ UPGRADE FROM 1.x to 2.0
removed. Use `--with-mongodb-system-libs` instead.
* All classes that previously implemented the `Serializable` interface no
longer implement this interface.
* The constructor of `MongoDB\BSON\UTCDateTime` no longer accepts a `string`
argument. To pass 64-bit integers on 32-bit platforms, use the
`MongoDB\BSON\Int64` class instead.
8 changes: 1 addition & 7 deletions src/BSON/UTCDateTime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,9 @@ static PHP_METHOD(MongoDB_BSON_UTCDateTime, __construct)
case IS_DOUBLE:
php_phongo_utcdatetime_init_from_double(intern, Z_DVAL_P(milliseconds));
return;

case IS_STRING:
php_error_docref(NULL, E_DEPRECATED, "Creating a %s instance with a string is deprecated and will be removed in ext-mongodb 2.0", ZSTR_VAL(php_phongo_utcdatetime_ce->name));

php_phongo_utcdatetime_init_from_string(intern, Z_STRVAL_P(milliseconds), Z_STRLEN_P(milliseconds));
return;
}

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.

}

static PHP_METHOD(MongoDB_BSON_UTCDateTime, __set_state)
Expand Down
2 changes: 1 addition & 1 deletion src/BSON/UTCDateTime.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

final class UTCDateTime implements UTCDateTimeInterface, \JsonSerializable, Type, \Stringable
{
final public function __construct(int|string|float|\DateTimeInterface|Int64|null $milliseconds = null) {}
final public function __construct(int|float|\DateTimeInterface|Int64|null $milliseconds = null) {}

final public function toDateTime(): \DateTime {}

Expand Down
4 changes: 2 additions & 2 deletions src/BSON/UTCDateTime_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 0 additions & 21 deletions tests/bson/bson-utcdatetime-008.phpt
View file
Open in desktop

This file was deleted.

45 changes: 0 additions & 45 deletions tests/bson/bson-utcdatetime_error-003.phpt
View file
Open in desktop

This file was deleted.

23 changes: 17 additions & 6 deletions tests/bson/bson-utcdatetime_error-004.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,31 @@ require_once __DIR__ . '/../utils/basic.inc';
/* UTCDateTime::__construct() internally converts floats to integers, so we will
* not use a float to test for an invalid value. We also don't test an object,
* since that is used for validating a possible DateTimeInterface argument. */
$invalidValues = [true, []];
$invalidValues = [
true,
[],
// Numeric strings are no longer supported as of 2.0
'1416445411987',
'1234.5678',
];

foreach ($invalidValues as $invalidValue) {
echo throws(function() use ($invalidValue) {
new MongoDB\BSON\UTCDateTime($invalidValue);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
echo throws(
fn () => new MongoDB\BSON\UTCDateTime($invalidValue),
MongoDB\Driver\Exception\InvalidArgumentException::class,
), PHP_EOL;
}

?>
===DONE===
<?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).

OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Expected integer or string, array given
Expected integer or object, array given
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Expected integer or object, string given
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Expected integer or object, string given
===DONE===