Skip to content

Conversation

@timacdonald
Copy link
Member

@timacdonald timacdonald commented Nov 6, 2024

This PR addresses a rounding bug when datetime or intervals are used with the cache.

Cache::put($key, $value, now()->addSecond());

Surprisingly, this will not put the value into the cache. It will instead call Cache::forget under the hood.

This is because by the time we do a diff between now() and $ttl = now()->addSecond() some time has passed during PHP execution, even if only microseconds. That means the diff is ~0.999 seconds, which we cast to an int resulting in 0.

This also impacts date time intervals, because we convert those to datetime instances.

I have a gut feeling this also impacts other parts of the framework where we pull the seconds off a datetime, such as the queue system. Also anything that uses InteractsWithTime may also be impacted. Some of those values are likely off by a second. I haven't investigated this to confirm, though.

Comment on lines +627 to +629
$duration = (int) ceil(
Carbon::now()->diffInMilliseconds($duration, false) / 1000
);
Copy link
Member Author

@timacdonald timacdonald Nov 6, 2024

Choose a reason for hiding this comment

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

I've changed this from diffInSeconds to diffInMilliseconds / 1000 because Carbon 2 would always return an int while carbon 3 returns a float.

This allows us to ensure we get the correct value on both versions.

@timacdonald timacdonald marked this pull request as ready for review November 6, 2024 23:33
@taylorotwell taylorotwell merged commit 091d4d4 into laravel:11.x Nov 7, 2024
31 checks passed
@timacdonald timacdonald deleted the cache-rouding branch November 7, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants