[5.8] Refactor caching TTL to seconds

[driesvints]

These changes will allow developers more finer granular caching. By allowing caching in seconds, developers can set even more specific caching times.

// This...
Cache::put('foo', 'bar', 60);

// Will change to this...
Cache::put('foo', 'bar', 3600);

This also makes the caching TTL conform with PSR-16 which states that:

Implementing Libraries MUST support at minimum TTL functionality as described below with whole-second granularity.

https://www.php-fig.org/psr/psr-16/

This refactor also simplifies things by removing support for floats and only supporting integers since floats aren't necessary anymore. If you take a look at the refactored tests and internals you'll see that we now do much less calculations.

I've added a separate commit which shows the places in the other components where the new behavior is updated. The changes don't affect other components but we may also consider if other components like auth and especially session could benefit from this.

This will have quite an impact on any app implementing caching and should be indicated in the upgrade guide as a high impact change.

[driesvints]

I should indeed note that this is already possible by division (1/4) because of the floats but the refactor makes it more obvious and intuitive. And more conform to the PSR.

[trevorpan]

Hi,

I'm going through the https://laravel.com/docs/5.8/upgrade#upgrade-5.8.0 process (first time ever) and found #27217 and #27276 were referenced in the cache section of upgrade notes.

The #27217 changed file src/Illuminate/Cache/RedisTaggedCache.php uses $minutes. This page #27276 src/Illuminate/Cache/RedisTaggedCache.php changes that variable to $seconds.

In the future should I disregard older posts such as #27217 and only utilize #27276, in this case? (Still trying to figure out all these tools and processes. Thank you`)

[devcircus]

#27217 didn't change it to minutes. It was already minutes.

[trevorpan]

Hi @devcircus , yes, I saw that thank you. I may be misinterpreting how to use the upgrade guide but it references both. Reading from top to bottom the newest #27276 is referenced first, so when I came to #27217 I decided to clarify here.

Are we supposed to make the changes for each referenced pull request noted in the upgrade guide?

[devcircus]

Not sure I follow. The upgrade guide is just pointing out that ttl has changed from minutes to seconds and that now, psr16 is more closely followed. The two ideas do not oppose each other. If you're digging in to a PR to see exactly how a specific piece of code works, always follow the most recent. Code is always changing.

[trevorpan]

So, e.g. on #27276, I completed all the changes first but when I went down the page and started making the next changes the older #27217 conflicted. I may be doing changes in a bad way. idk

To be sure...we should be updating the changes in Files changed tab at the top of the PR referenced? And we manually go through this each time there's an upgrade?

Or does updating the composer.json to "laravel/framework": "5.8.*" handle all that?

e.g. this line "In the very unlikely event you are implementing this interface, you should add these methods to your implementation." I took that as we manually update our app - just a bit unclear on where the composer.json leaves off and manual updating starts.

[devcircus]

What are you changing using the pull requests? The actual code?

[trevorpan]

ha, man, this sounds badd...

- $this->event(new KeyWritten($key, $value, 0));

+ $this->event(new KeyWritten($key, $value));

e.g. in the above I removed, 0)

I wish the upgrade guide had a blue boxed tip about this.

edit: alright so I just ran composer update and by-joe, the changes are all there.

so e.g., what is meant, in the docs, by this:
The addContextualBinding method was added to the Illuminate\Contracts\Container\Container contract. If you are implementing this interface, you should add this method to your implementation.

I think that's where I got confused. Really appreciate your clarification, as I mentioned this is the first time i've done an upgrade. happy saturday ~

[tegos]

Hello. Is method (Illuminate\Cookie\CookieJar)->make() should receive seconds instead minutes (or ttl)?

[GrahamCampbell]

No, the code looks correct with minutes?

[driesvints]

@tegos the cookie component was left untouched and still uses minutes.

[tegos]

Yeah, it looks correct.
I meant, whether is it necessary to update it as well as the Cache for the same reasons(granular, psr-16)?

[driesvints]

@tegos cookies don't have anything to do with PSR-16. But yeah I suppose it could profit from the same thing. You're free to send in a PR to the master branch for this.

[tegos]

@driesvints ok, sorry. I sent pr to the master.