[5.4] insist on not considering false as cache miss

[halaei]

I also note a minor bug: there is a difference in the way values are unserialized in RedisStore::get() and RedisStore::many().

This PR also fix that. I think I should send another PR for laravel 5.1 with the related tests if you give me a day.

[GrahamCampbell]

Just keeping this on 5.4 is probably fine.

[tillkruss]

This needs to be reverted.

@taylorotwell: Not checking for false in Cache/Repository::has() will lead to false-negatives when using PhpRedis. No matter if the cache key exists, it will always return true.

If you want to allow false as a cache value, #17196 needs to be merged.

[taylorotwell]

Sigh. All this stuff is driving me batty :|

[halaei]

@tillkruss I don't get it! Sounds like you wrong! Please take a look at changes in RedisStore::get() and RedisStore::unserialize() first!

[tillkruss]

@halaei Did you test your PR against Cache::has() using PhpRedis?

[halaei]

@tillkruss Ok. The communication between us is a little bit messed up, and I am really confused.

[tillkruss]

PhpRedis returns false if a cache key doesn't exists. Predis returns null. We need to support both cases. Taylor will probably merge #17196 to fix this issue.

[halaei]

    protected function unserialize($value)
    {
        if (! is_null($value) && $value !== false) {
            return is_numeric($value) ? $value : unserialize($value);
        }
    }

This was the trick!
@tillkruss

[halaei]

@tillkruss tests pass under phpredis

[tillkruss]

Can you test it manually, I'm not sure if we have a test case for that yet.

[halaei]

I think you mean RedisCacheIntegrationTest::testRedisCacheAddTwice()` for example. That passes when I change InteractsWithRedis to use phpredis instead of predis.

:( My RIP PR is reverted by mistake ;)

[tillkruss]

You're right. However it's nicer to solve this on the driver level with #17196.

[halaei]

Thanks. That will be nice to remove the limitation and make upgrade safer and easier one way or the other. Please also take a look at the changes I made to RedisStore::unserialize() and the calls to it. Maybe you can add that to your PR as well if you find it helpful.

[tillkruss]

That's not gonna be necessary anymore.