Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java: Add expire, expireAt, pexpire, pexpireAt and ttl commands. #1033

Merged

Conversation

SanHalacogluImproving
Copy link
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@SanHalacogluImproving SanHalacogluImproving added the java issues and fixes related to the java client label Feb 23, 2024
@SanHalacogluImproving SanHalacogluImproving self-assigned this Feb 23, 2024
@SanHalacogluImproving SanHalacogluImproving requested a review from a team as a code owner February 23, 2024 20:00
@SanHalacogluImproving SanHalacogluImproving changed the title Java: Add expire, expireAt, pexpire, pexpireAt and ttl comm… Java: Add expire, expireAt, pexpire, pexpireAt and ttl commands. Feb 23, 2024
Copy link
Collaborator

@shohamazon shohamazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very large pr - 5 commands where 4 of them are duplicated, please split such big pr's in the next time 🙂

* @param key The key to set timeout on it.
* @param seconds The timeout in seconds.
* @return <code>true</code> if the timeout was set. <code>false</code> if the timeout was not
* set. e.g. <code>key</code> doesn't exist, or operation skipped due to the provided
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or operation skipped due to the provided arguments -> not the right place for it, its for the function with the expireOptions (for example setting a timeout on an existing key with expiry with the NX expire option)

* @param key The key to set timeout on it.
* @param unixSeconds The timeout in an absolute Unix timestamp.
* @return <code>true</code> if the timeout was set. <code>false</code> if the timeout was not
* set. e.g. <code>key</code> doesn't exist, or operation skipped due to the provided
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with provided arguments

* arguments.
* @example
* <pre>
* Boolean isSet = client.expireAt("my_key", 1672531200).get()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets change that to: Instant.now().getEpochSecond() + 10L

java/integTest/src/test/java/glide/SharedCommandTests.java Outdated Show resolved Hide resolved
String key = UUID.randomUUID().toString();

assertFalse(client.expireAt(key, Instant.now().getEpochSecond() + 10L).get());
assertFalse(client.pexpireAt(key, Instant.now().getEpochSecond() + 10000L).get());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

CompletableFuture<Boolean> response = service.expire(key, seconds);

// verify
assertNotNull(response);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it different than other tests?
(assertEquals(testResponse, response);)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

public void pexpire_returns_success() {
// setup
String key = "testKey";
long milliseconds = 5L;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: change it to something with more meaning, maybe 50000

public void pexpire_with_expireOptions_returns_success() {
// setup
String key = "testKey";
long milliseconds = 5L;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@Yury-Fridlyand
Copy link
Collaborator

Please resolve the conflicts

acarbonetto and others added 3 commits February 29, 2024 11:27
…ands (#96)

* Java: Add expire and ttl commands

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

* Update commands for review comments

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

* Minor javadoc fix

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

* Spotless

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>

* Added examples in documentation. Minor format RedisClientTest.

* Spotless

* Update java/client/src/main/java/glide/api/commands/GenericBaseCommands.java

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Spotless

---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: SanHalacogluImproving <san.halacoglu@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
@SanHalacogluImproving SanHalacogluImproving force-pushed the java/integ_SanH_add_expire_ttl_commands branch from 7b81569 to 13d48b1 Compare February 29, 2024 19:58
@acarbonetto acarbonetto merged commit 4639a9b into valkey-io:main Feb 29, 2024
11 checks passed
@acarbonetto acarbonetto deleted the java/integ_SanH_add_expire_ttl_commands branch February 29, 2024 21:02
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
…ands. (valkey-io#1033)

* Java: Add `expire`, `expireAt`, `pexpire`, `pexpireAt` and `ttl` commands
---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: SanHalacogluImproving <san.halacoglu@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Updated based on PR comments.

* Spotless and fixed ordering in TransactionTests.

---------

Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java issues and fixes related to the java client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants