Memcached: doContains return true pretty much everytime if the stored value is "false"

[doctrine]

Memcached: doContains return true pretty much everytime if the stored value is "false"

[Tobion]

According to documentation, those return values cannot be returned from a simple get. So please provide a test case to prove something else.

[deeky666]

@Tobion unfortunately the docs are not revealing every implementation detail. There are even tests that cover those constants.

[Tobion]

I don't see a test for get and RES_NOTFOUND.

[riquito]

I can't provide a test, but I suppose that if you try a get after shutting down memcached you won't get RES_NOTFOUND (can't try right now). At present I'm getting in a production server result code 47, MEMCACHED_SERVER_TEMPORARILY_DISABLED, which causes doContains() to return true (don't know how to reproduce it on purpose).

[deeky666]

@Tobion https://github.com/php-memcached-dev/php-memcached/blob/607860c656a3cea22fb164c4eb71d36ec869c598/tests/experimental/getmulti_partial_error.phpt#L23

[Tobion]

That is a multiget. I already said twice that this ticket is about a simple get returning those states. Please read more carefully. And for multiget it is also documented, see docs.

[riquito]

Here are some links to the implementation of get in the php memcached extension.

return_code ranges from BAD_KEY_PROVIDED to RES_PAYLOAD_FAILURE to pretty much anything (see implementation of php_memc_handle_error)

[Tobion]

Does that work?

return false !== $this->memcached->get($id)
            || $this->memcached->getResultCode() !== Memcached::RES_SUCCESS;

[riquito]

In libmemcached a result can be returned only if RES_SUCCESS is true, so it should be correct. I find the first part of the expression misleading because we don't care about the returned value and false is a legitimate result. I'd write it differently because it's easier to reason about, but maybe that's just me.

$this->memcached->get($id);
return $this->memcached->getResultCode() === Memcached::RES_SUCCESS;

edit: in your code you should compare Memcached::RES_SUCCESS for equality, not !==

[JonasHaouzi]

👍

We're faced to the same issue.

[Ocramius]

A fix was provided via #181, but no tests are there, so I cannot merge it.