Use int as the PHP representation of long fields in generated code

[stof]

No description provided.

[nicolas-grekas]

Just a quick note about this: if portability on x86 is desired, int is a much smaller range. It allows only 2^31 integers, while float allows 2^53, and can handle even more if loss of accuracy is acceptable.

[GrahamCampbell]

This is a huge breaking change, also.

[GrahamCampbell]

In terms of the comment about loss of precision, this is already an issue in the code integer fields in the JSON get approximated, then cast to a string. This PR casts them back to an int. That is, unless AWS is encoding those as strings on their side already (like they do with 128-bit integers in dynamodb).

[stof]

The behavior used here is the same than in the official SDK: https://github.com/aws/aws-sdk-php/blob/1d3e7ae37f31dfe1d4e9d9e361fd8e6702074fbe/src/Api/Parser/AbstractRestParser.php#L109-L110

when looking at the diff, the places using long in the API should be OK for the limit on int size:

• some of them are timestamps (for expiration of some objects), for some services that don't use the timestamp type for some reason. For those, things will indeed break for dates after 2038 (which gives you a few years before the AWS API keys get an expiration timestamp after that threshold). By that time, using a 64bits build of PHP will become required to be able to use DateTimeImmutable anyway due to timestamp sizes.
• many of them are TTLs (in seconds or milliseconds), which are fine
• the CodeBuild build number (if you reaches 2^31 builds there, you might probably afford updating your PHP runtime)
• Byte sizes of some storages
• Item count of some storages

[stof]

@GrahamCampbell regarding the BC break, this actually help fixing issues too. Receiving a count as a string would force you to cast it to do something useful with it anyway (or to rely on PHP type coercion to do it for you, which would then not break when we return an integer directly).

[GrahamCampbell]

People have the option to use bigint implementations on 32 bit systems to do something useful with the string without losing data.

[stof]

Given that the JSON contains a number, they will not get a string with the value from AWS. They will currently get a string cast of the numeric value decoded from JSON. On 32 bit systems, that would already lose information.

[GrahamCampbell]

We should work around that using the JSON_BIGINT_AS_STRING option.

[stof]

@jderusse @Nyholm how do you want to handle that case ?

[jderusse]

👍 for merging this for consistency with the SDK. They cast to int and don't use any option for json_decode/json_encode

But it's a BC break for end users we should add an entry in the changelog and I will release a major version for these components.

And we should probably release an intermediate version with a deprecation notice 🤔

[stof]

@jderusse a deprecation notice won't be possible. There is no way to opt-in a different PHP return type.
Note that the BC break should happen only in case they use the return value of one of our getter and pass it to a string-typed location (parameter or return type) in strict-mode code, or if they call one of our few impacted setters (there are only 5 such setters in the diff) in strict-mode code.
For non-strict code, the PHP type coercion will happen anyway so nothing will break. And for our array shapes in constructor, we perform the proper type casting in the method anyway so passing a numeric string will not break (I'm not counting the case of passing a non-numeric string as that would break on the AWS side when submitting an alphabetic string in a long field anyway).

[jderusse]

That's a BC break for people using strict type... I agree, it's easy to fix, and they are already in trouble if they send alpha char in the string.

But if they use it properly, this PR will break code like this one.

declare(strict_types=1);

$request = new PutObjectRequest([
  'Bucket' => 'my_bucket',
  'Key' => 'image/cat.jpg',
]);

$request->setContentLength((string) filesize($file)); // we have to cast to string because of `strict_types=1`

$result = $s3->putObject($request);

I'm still 👍 for merging it, but we should at least add an entry in the changelog and release a major version.

[stof]

@jderusse I added all the relevant changelog entries.

[jderusse]

thanks 🙏