Added support for hash field expiration commands #1456
Conversation
|
PR still on draft, until:
|
vladvildanov
force-pushed
the
vv-hash-field-expiration
branch
from
cbc69b4
to
b59f5fb
Compare
| @@ -165,7 +175,10 @@ | |||
| * @method $this hrandfield(string $key, int $count = 1, bool $withValues = false) | |||
| * @method $this hscan($key, $cursor, array $options = null) | |||
| * @method $this hset($key, $field, $value) | |||
| * @method $this hsetf(string $key, array $keyValuePairs, HSetFArguments $arguments = null) | |||
There was a problem hiding this comment.
Call keyValuePairs dictionary, like in hmset?
| @@ -174,7 +184,10 @@ | |||
| * @method array hrandfield(string $key, int $count = 1, bool $withValues = false) | |||
| * @method array hscan(string $key, $cursor, array $options = null) | |||
| * @method int hset(string $key, string $field, string $value) | |||
| * @method mixed hsetf(string $key, array $keyValuePairs, HSetFArguments $arguments = null) | |||
There was a problem hiding this comment.
Same here, use same naming like in hmset, i.e. dictionary?
| /** | ||
| * Set the modifier that defines a behaviour on expiration. | ||
| * | ||
| * NX for each specified field set expiration only when the field has no expiration |
There was a problem hiding this comment.
Being picky here, missing end dot at two sentences. And some more at the other method below.
| /** | ||
| * @var array | ||
| */ | ||
| protected $expireModifierEnum = [ |
There was a problem hiding this comment.
In my mind these two don't belong together. The {NX,XX,GT,LT} set is for overwriting rules, while {EX,PX,EXAT,PXAT} are basically just parameter name in compact form. For example {NX,XX,GT,LT} applies to other hash expiration commands, while the other set does not.
There was a problem hiding this comment.
So something like overrideModifierEnum sounds better?
| */ | ||
| public function setPersist(): self | ||
| { | ||
| if (!empty(array_intersect($this->ttlModifierEnum, $this->arguments))) { |
There was a problem hiding this comment.
What if someone first calls setPersist and then calls setTtlModifier?
There was a problem hiding this comment.
Okay, let's add the same kind of check on the other side
| if (!in_array(strtoupper($modifier), $this->fieldModifierEnum, true)) { | ||
| throw new UnexpectedValueException('Incorrect field modifier value'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Check that at most one modifier from the group is used.
| if (!in_array(strtoupper($modifier), $this->getModifierEnum, true)) { | ||
| throw new UnexpectedValueException('Incorrect get modifier value'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Check that at most one modifier from the group is used.
| */ | ||
| public function enableKeepTTL(): self | ||
| { | ||
| if (!empty(array_intersect($this->ttlModifierEnum, $this->arguments))) { |
There was a problem hiding this comment.
Same like for HGETF, what if someone first enables keepTtl and then calls setTtlModifierEnum? There should be a check on the other side too?
|
|
||
| use Predis\Command\Command as RedisCommand; | ||
|
|
||
| class HEXPIRETIME extends RedisCommand |
There was a problem hiding this comment.
I think this one too has the {NX,XX,LT,GT} flags. Maybe it should extend HEXPIRE?
| @@ -72,7 +72,7 @@ class MasterSlaveReplication implements ReplicationInterface | |||
| /** | |||
| * {@inheritdoc} | |||
| */ | |||
| public function __construct(ReplicationStrategy $strategy = null) | |||
| public function __construct(?ReplicationStrategy $strategy = null) | |||
There was a problem hiding this comment.
An idea: put these changes not strictly related to hash field expiration in a separate PR, to avoid noise?
There was a problem hiding this comment.
There's already a PR with this changes, I'm waiting for approval and then rebase this branch
This PR adds new commands operates on hash field expiration functionality