These commented-out lines should be removed prior to merge.

Only 2.2.x and 3.x are supported. Version 2.1.x causes symbol error in extension when requesting last result code.

See https://travis-ci.org/symfony/symfony/jobs/182877309#L2410 for example of the above described issue. Only applicable to memcached version 2.1.0 in Travis PHP 5.5 environment.

To the unaware reader (like me a few minutes ago), this reads as if 2.1.1 is fine. But there is no 2.1.x, x>0
Could the check be ">=2.2.0"?

btw, this check could be moved to a public static function isSupported() on the adapter (see ApcuAdapter for similar concern)

I was wondering: how does this method play with Memcached::OPT_PREFIX_KEY? Does it ignore it (ie return all keys, even those that don't start with the prefix) or does it filter by prefix?

From the docs:

Memcached::getAllKeys() queries each memcache server and retrieves an array of all keys stored on them at that point in time [...]

So it sounds like it ignores that setting, but I haven't used Memcached::OPT_PREFIX_KEY myself, so I don't think we'd find a conclusive answer without delving into the C code for the extension.

FYI: Remember, I don't have a combination of libmemcached/ext-memcached that actually supports this method so testing this behavior is...difficult. I'll see if I can't compile a working combination using phpenv later tonight.

I think we should drop the use of getAllKeys and reduce doClear to return $this->client->flush();: you can't make it work, and it comes with many notices saying this can kill kittens.

can't we just return $this->client->getMulti($ids);?

since false is a valid value, this can't work, isn't it?
I guess this should be

        $this->client->get($id);

        return $this->client->getResultCode() === \Memcached::RES_SUCCESS;

That is an error on my part: intended for that to be an OR (||). The idea is to avoid the additional call to $this->client->getResultCode() if the $this->client->get($id) isn't false.

This is what I meant to have there (and what I'd recommend we use):

protected function doHave($id)
    {
        return $this->client->get($id) !== false
            || $this->client->getResultCode() !== \Memcached::RES_NOTFOUND;
    }

Kind of a micro-optimization, but worth avoiding an additional function call if unnecessary, IMHO.

Updated per above comment.

Checking against === RES_SUCCESS looks required to me, !== opens for all the other codes

Ah, that change does make sense. I missed the change from !== RES_NOTFOUND to === RES_SUCCESS in your comment. Updated in latest push.

we usualy don't add author on tests (even if test some have, but they could be removed)

btw, this check could be moved to a public static function isSupported() on the adapter (see ApcuAdapter for similar concern)

we don't use groups like this

not sure this test is useful: it tests only test methods/behavior, I'd remove it

should be moved as first method in the class (same as e.g. ApcuAdapter)

ping @robfrawley: :)

    protected function doClear($namespace)
    {
        return $this->client->flush();
    }

Added a commit with https://github.com/symfony/symfony/pull/20858/files#diff-e01499fab5ad7f5e36517d157bc13e2fR96 to check if any Travis environment actually has a functional combination of the extension and library. After checking https://travis-ci.org/symfony/symfony/builds/183319461, if none actually utilize this, then I'm with you 100%.

Nope. None call the extra logic: removed.

Removing that also raises coverage to 100%, so it's a solid move in my book.

so, only one remaining comment here: in which case can $result be true? when the cast is in effect and only then (look at the doc, I'd say yes?)

Shouldn't the implem be this one?

if (true === $results = $this->client->deleteMulti($ids)) {
    return true;
}
$toDelete = count($ids);
foreach ($results as $result) {
    if (\Memcached::RES_SUCCESS === $result || \Memcached::RES_NOTFOUND === $result) {
        --$toDelete;
    }
}

return 0 === $toDelete;

Return of $this->client->deleteMulti($ids) is actually always bool[] now that we have isSupported() require >=2.2.0.

So the cast can be removed now, to simply have

        $toDelete = count($ids);
        foreach ($this->client->deleteMulti($ids) as $result) {
            if (true === $result || \Memcached::RES_NOTFOUND === $result) {
                --$toDelete;
            }
        }

        return 0 === $toDelete;

That was to support <2.2.0 extension versions that would return a single bool

Scratch that, you're right. The check can now simply be:

        $toDelete = count($ids);
        foreach ($this->client->deleteMulti($ids) as $result) {
            if (\Memcached::RES_SUCCESS === $result || \Memcached::RES_NOTFOUND === $result) {
                --$toDelete;
            }
        }

        return 0 === $toDelete;

The situation where it returns bool is behind us with the version requirement in isSupported().