[5.7] Honor PSR-16 SimpleCache interface

[plehatron]

Cache repository methods set and setMultiple, defined in the SimpleCache interface do not return any value, but they should according to PSR-16. This PR aims to solve this and fix issue #26674.

Cache stores Apc, Array, Database, File, Memcached, Null, and Redis are affected. Their respective unit tests were updated to check for return values.

[X-Coder264]

You've changed the docblocks from void to bool on the concrete Repository implementation but you left them to be void on the interface. Also there was no need to change the forever method return type to honor the PSR-16 interface as that method is not on that interface to begin with. Generally changing a method signature is considered a breaking change (especially now that it happened on a contract) so this PR should probably target master (as all Store implementations out there will not honor the interface anymore after this gets merged).

btw, pozdrav iz Trikodera 😄

[taylorotwell]

What are we accomplishing for users (and what percentage of users) for using PSR-16 at all? Do you have a particular use case in mind?

[X-Coder264]

@taylorotwell I'm using some framework-agnostic packages which depend on PSR-16 on a couple of Laravel projects and having the ability to inject the Illuminate Cache Repository there is a great thing. Since we already claim to implement that interface I don't see the harm in actually fixing this so that we are compliant with it, especially since it's not a huge change.

[plehatron]

@taylorotwell In my case, I'm working on a project where we use the jeremykendall/php-domain-parser package. It has a dependency for any PSR-16 compatible cache library. As reported in issue #26674, caching with Laravel's cache component doesn't work because the package strictly expects the return values for certain methods to be boolean (as defined in the PSR-16 contract). What I had to do is create a wrapper around the cache component and basically fix the return values to make it work, which kind of defeats the purpose of having a cache component that tries to be PSR-16 complaint.

With this PR we're aiming to make the cache component fully compliant with PSR-16, so there are no surprises, and workarounds have to be made for packages that expect strict return types.

I can't say exactly what percentage of users will benefit from this, but I can say that the adoption of PSR-16 is growing. When SimpleCache interface was implemented with PR #20194, there were 120 packages depending on psr/simple-cache. Today there are 413 packages, that's a 244% increase in almost 1.5 years.

[plehatron]

You've changed the docblocks from void to bool on the concrete Repository implementation but you left them to be void on the interface. Also there was no need to change the forever method return type to honor the PSR-16 interface as that method is not on that interface to begin with. Generally changing a method signature is considered a breaking change (especially now that it happened on a contract) so this PR should probably target master (as all Store implementations out there will not honor the interface anymore after this gets merged).

btw, pozdrav iz Trikodera 😄

@X-Coder264 Good catch, thanks. Fixed it with last changes. I figured since forever internally uses put it can just use it's return value since it's now available. Also, wanted to make it a bit cleaner, now when you look at the Illuminate\Contracts\Cache\Store, none of the methods return void.

ps, pozdrav s ex3k strane 👋 😃

[taylorotwell]

Thanks for the info.

[taylorotwell]

Do we actually implement the PSR-16 interfaces anywhere?

[deleugpn]

I think the only PR regarding psr-16 was #20194. Laravel contract extends the PSR-16
https://github.com/laravel/framework/pull/20194/files#diff-8a46e94db2940c5a3253b58255641436R8

[taylorotwell]

There's a lot of changes here to the extent I would probably prefer this be sent to master branch.