SET IFEQ command implemented in node + tests #2909
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Maayan Shani <[email protected]> Signed-off-by: Maayan Shani <[email protected]>
Signed-off-by: Maayan Shani <[email protected]> Signed-off-by: Maayan Shani <[email protected]>
Maayanshani25
force-pushed
the
maayans-ifeq
branch
from
3ac0f80 to
a7300c5
Compare
Signed-off-by: Maayan Shani <[email protected]>
Signed-off-by: Maayan Shani <[email protected]>
Maayanshani25
force-pushed
the
maayans-ifeq
branch
from
041a662 to
febef71
Compare
| @@ -1473,7 +1473,7 @@ export class BaseClient { | |||
| * @param value - The value to store with the given key. | |||
| * @param options - (Optional) See {@link SetOptions} and {@link DecoderOption}. | |||
| * @returns - If the value is successfully set, return OK. | |||
| * If value isn't set because of `onlyIfExists` or `onlyIfDoesNotExist` conditions, return null. | |||
| * If value isn't set because of `onlyIfExists` or `onlyIfDoesNotExist` or `onlyIfEqual`conditions, return null. | |||
There was a problem hiding this comment.
Suggested change
| * If value isn't set because of `onlyIfExists` or `onlyIfDoesNotExist` or `onlyIfEqual`conditions, return null. | |
| * If value isn't set because of `onlyIfExists` or `onlyIfDoesNotExist` or `onlyIfEqual` conditions, return `null`. |
| * it already exist. Equivalent to `EX` in the Valkey API. if `conditional` is | ||
| * not set the value will be set regardless of prior value existence. If value | ||
| * isn't set because of the condition, return null. | ||
| * it already exist. Equivalent to `EX` in the Valkey API. `onlyIfEqual` - Only set the |
There was a problem hiding this comment.
Suggested change
| * it already exist. Equivalent to `EX` in the Valkey API. `onlyIfEqual` - Only set the | |
| * it already exists. Equivalent to `EX` in the Valkey API. `onlyIfEqual` - Only set the |
| } else if (options.conditionalSet === "onlyIfEqual") { | ||
| args.push("IFEQ"); | ||
|
|
||
| if (options.comparisonValue != undefined) { |
There was a problem hiding this comment.
Suggested change
| if (options.comparisonValue != undefined) { | |
| if (options.comparisonValue) { |
| conditionalSet?: "onlyIfExists" | "onlyIfDoesNotExist"; | ||
| conditionalSet?: "onlyIfExists" | "onlyIfDoesNotExist" | "onlyIfEqual"; | ||
| /** | ||
| * If onlyIfEqual is set, the value to compare the existing value with. |
There was a problem hiding this comment.
Suggested change
| * If onlyIfEqual is set, the value to compare the existing value with. | |
| * If `onlyIfEqual` is set, the value to compare the existing value with. |
Comment on lines
+133
to
+134
| * if `conditional` is not set the value will be set regardless of prior value existence. | ||
| * If value isn't set because of the condition, return null. |
There was a problem hiding this comment.
I propose to add line breaks for better readability.
onlyIfDoesNotExist- ...
onlyIfExists- ...
And move last two lines to set docs.
| /** | ||
| * If onlyIfEqual is set, the value to compare the existing value with. | ||
| */ | ||
| comparisonValue?: GlideString; |
There was a problem hiding this comment.
You can do
SetOptions = ({
conditionalSet?: "onlyIfExists" | "onlyIfDoesNotExist";
} | {
conditionalSet: "onlyIfEqual";
comparisonValue: GlideString;
} & {
...
}
)and then remove value check on lines 179-184
| @@ -8260,18 +8319,55 @@ export function runBaseTests(config: { | |||
|
|
|||
| expect(setRes).toBeDefined(); | |||
| } | |||
|
|
|||
| // onlyIfEqual tests | |||
| if (!cluster.checkIfServerVersionLessThan("8.1.0")) { | |||
There was a problem hiding this comment.
You already have a version check on line 8365. This one is always true.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue link
This Pull Request is linked to issue (URL): [/issues/2811]
Description
This pull Request is implemented the
IFEQoption in theSETcommamnd. Tests were added to the shared tests, and docstring to the command and the related methods and structures.Checklist
Before submitting the PR make sure the following are checked: