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
src/ClientContextInterface.php
Outdated
| @@ -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?
| /** | ||
| * 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?
There was a problem hiding this comment.
It's fine. I think my original point was to split the class in two, but on second thought I'm not sure it's worth it.
| */ | ||
| 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
| @@ -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.
There's already a PR with this changes, I'm waiting for approval and then rebase this branch
| @@ -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.
There's already a PR with this changes, I'm waiting for approval and then rebase this branch
vladvildanov
force-pushed
the
vv-hash-field-expiration
branch
from
82eab7a
to
c522417
Compare
This PR adds new commands operates on hash field expiration functionality