[5.4] Laravel cache does not support false as a cached value anymore

[halaei]

• Laravel Version: 5.4

Description:

I think #15160 has an unnecessary change that should be undone, unless there is a reason for that:

Illuminate/Cache/RedisStore and Illuminate/Cache/Repository now check key existence using null or false.

In the code, all the checks for if (is_null($value)) are converted to if (is_null($value) || $value === false). IMHO, it adds an unnecessary limitation to the Laravel Cache.

@tillkruss do you agree?

Steps To Reproduce:

\Cache::put('x', false, 60);
var_dump(\Cache::has('x'));

Expected output:

true

Actual output:

false

[tillkruss]

Predis returns null when a cache key doesn't exists. PhpRedis and HHVM usually return false. Since 5.4 now fully supports PhpRedis you'ldo need to use something like "false" or 0 or "no" from 5.4 onwards.

[GrahamCampbell]

To get around this, we could get each connector to throw an exception for a miss?

[halaei]

@tillkruss
I think laravel is not supposed to support HHVM anymore? I have no idea about returning false instead of null by PhpRedis. If it is true, then maybe the redis driver should handle false itself. Moreove, maybe Redis Queue driver needs fixes for that as well?

    public function get($key)
    {
        $value = $this->connection()->get($this->prefix.$key);
        if (! is_null($value) && $value !== false) {
            return $this->unserialize($value);
        }
    }

[GrahamCampbell]

Indeed, HHVM support has been dropped.

[taylorotwell]

It's probably indeed true we need a PR to normalize this across the drivers so that null is returned for miss from both.

[tillkruss]

I don't think storing PHP types like booleans in Redis is a good idea in general.

• true is stored as 1
• false is stored as an empty string

For both, Predis and PhpRedis.

However the root problem seems to be that Illuminate\Cache\Repository::has() is using a GET call to check that a keys exists and it should really be using an EXISTS call. That would solve the problem no matter what client is used and what values users store.

[halaei]

@tillkruss true is not stored as 1 and false is not stored as empty string in the current code. true and false (as well as everything except numbers) are php-serialized into strings before being stored.

[tillkruss]

Right, when using Laravel's cache, not when using Redis directly.

[tillkruss]

See #17196 (comment)

[casperbakker]

It's probably indeed true we need a PR to normalize this across the drivers so that null is returned for miss from both.

That's it. I am done with Laravel. What is this for a reason? Cache::has() should only check if the key exists and nothing more. Why even check the inner results?

[tillkruss]

@casperbakker: Feel free to open a discussion on https://github.com/laravel/internals about this.