diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #39298 [Cache] Fixed incorrect usage of UNLINK with PHPRedis with Redis < 4.0 (wickex)

Browse Source 
This PR was merged into the 5.2 branch.

Discussion
----------

[Cache] Fixed incorrect usage of UNLINK with PHPRedis with Redis < 4.0

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #39280
| License       | MIT

Currently, deleting cache keys is broken for users using PhpRedis with Redis-server < 4.0.0. The current implementation expects PhpRedis to throw an exception if the 'unlink'-function is used but unavailable, after which it's supposed to switch to using the 'del'-function. Using the 'unlink'-function on lower Redis-server versions seems to not throw an exception, but instead it appears to silently fail.

This pull request changes this behavior and checks the Redis-server version instead. If the version is 4.0 or higher, it uses the unlink function. If not, it uses the del-function.

Also see https://redis.io/commands/unlink > "Available since 4.0.0".

(Footnote: this is one of my first times contributing to an open-source project and my first time contributing to Symfony. I've tried following the guidelines, but please let me know if I missed anything. I'm unsure how I would go about unit testing this specific bugfix due to it being dependent on the Redis version, so I omitted it. Please let me know if a unit test is indeed required for this and if so, let me know if you have any suggestions on how to go about that.)

Commits
-------

9363f3b973 [Cache] Fixed incorrect usage of UNLINK with PHPRedis with Redis < 4.0
This commit is contained in:
Nicolas Grekas 2020-12-08 12:05:08 +01:00
commit 5f08c617cd
3 changed files with 12 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/Symfony/Component/Cache/Traits/RedisTrait.php
View File

@ -408,15 +408,15 @@ trait RedisTrait
if ($unlink) {
try {
$this->redis->unlink($ids);
return true;
$unlink = false !== $this->redis->unlink($ids);
} catch (\Throwable $e) {
$unlink = false;
}
}
$this->redis->del($ids);
if (!$unlink) {
$this->redis->del($ids);
}
}
return true;

8
src/Symfony/Component/HttpFoundation/Session/Storage/Handler/RedisSessionHandler.php
View File

@ -93,15 +93,15 @@ class RedisSessionHandler extends AbstractSessionHandler
if ($unlink) {
try {
$this->redis->unlink($this->prefix.$sessionId);
return true;
$unlink = false !== $this->redis->unlink($this->prefix.$sessionId);
} catch (\Throwable $e) {
$unlink = false;
}
}
$this->redis->del($this->prefix.$sessionId);
if (!$unlink) {
$this->redis->del($this->prefix.$sessionId);
}
return true;
}

10
src/Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.php
View File

@ -485,17 +485,15 @@ class Connection
if ($unlink) {
try {
$this->connection->unlink($this->stream);
$this->connection->unlink($this->queue);
return;
$unlink = false !== $this->connection->unlink($this->stream, $this->queue);
} catch (\Throwable $e) {
$unlink = false;
}
}
$this->connection->del($this->stream);
$this->connection->del($this->queue);
if (!$unlink) {
$this->connection->del($this->stream, $this->queue);
}
}
}
class_alias(Connection::class, \Symfony\Component\Messenger\Transport\RedisExt\Connection::class);