Skip to content

Commit ea59c18

Browse files
clameyclamey
committed
Change at least until handling, clean up code, straighten out tests
1 parent 3f05543 commit ea59c18

File tree

2 files changed

+61
-37
lines changed

2 files changed

+61
-37
lines changed

‎shedlock-provider-jedis/src/main/java/net/javacrumbs/shedlock/provider/jedis/JedisLockProvider.java

Lines changed: 47 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424

2525
import java.net.InetAddress;
2626
import java.net.UnknownHostException;
27+
import java.time.Duration;
2728
import java.time.Instant;
29+
import java.time.temporal.ChronoUnit;
2830
import java.util.Optional;
2931

3032
/**
@@ -35,68 +37,80 @@
3537
*/
3638
public class JedisLockProvider implements LockProvider {
3739

38-
private static final String keyPrefix = "job-lock";
40+
private static final String KEY_PREFIX = "job-lock";
41+
private static final String ENV_DEFAULT = "default";
3942

40-
private static final String DEFAULT_ENV = "default";
43+
private static final long AT_MOST_HOURS_DEFAULT = 1;
44+
45+
// Redis Flags
46+
private static final String SET_IF_NOT_EXIST = "NX";
47+
private static final String SET_EXPIRE_TIME_IN_MS = "PX";
4148

4249
private JedisPool jedisPool;
4350
private String environment;
51+
private Duration atMostHoursDefault;
4452

4553
public JedisLockProvider(JedisPool jedisPool) {
46-
this(jedisPool, DEFAULT_ENV);
54+
this(jedisPool, ENV_DEFAULT, AT_MOST_HOURS_DEFAULT);
4755
}
4856

4957
public JedisLockProvider(JedisPool jedisPool, String environment) {
58+
this(jedisPool, environment, AT_MOST_HOURS_DEFAULT);
59+
}
60+
61+
public JedisLockProvider(JedisPool jedisPool, String environment, long atMostHoursDefault) {
5062
this.jedisPool = jedisPool;
5163
this.environment = environment;
64+
this.atMostHoursDefault = Duration.of(atMostHoursDefault, ChronoUnit.HOURS);
5265
}
5366

5467
@Override
5568
public Optional<SimpleLock> lock(LockConfiguration lockConfiguration) {
56-
long difference = getDifference(lockConfiguration);
57-
if (difference > 0) {
58-
String key = buildKey(lockConfiguration.getName(), this.environment);
59-
try (Jedis jedis = jedisPool.getResource()) {
60-
String rez = jedis.set(key, buildValue(), "NX", "PX", difference);
61-
if (rez != null && "OK".equals(rez)) {
62-
return Optional.of(new RedisLock(key, jedisPool));
63-
}
69+
Instant now = Instant.now();
70+
71+
// Get 'furthest out' configured expire time for lock TTL
72+
long expireTime = Math.max(
73+
Duration.between(now, lockConfiguration.getLockAtMostUntil()).toMillis(),
74+
Duration.between(now, lockConfiguration.getLockAtLeastUntil()).toMillis());
75+
76+
String key = buildKey(lockConfiguration.getName(), this.environment);
77+
78+
try (Jedis jedis = jedisPool.getResource()) {
79+
String rez = jedis.set(key,
80+
buildValue(),
81+
SET_IF_NOT_EXIST,
82+
SET_EXPIRE_TIME_IN_MS,
83+
expireTime < 0 ? atMostHoursDefault.toMillis() : expireTime); // And if not set, use default
84+
85+
if (rez != null && "OK".equals(rez)) {
86+
return Optional.of(new RedisLock(key, jedisPool, lockConfiguration));
6487
}
6588
}
6689
return Optional.empty();
6790
}
6891

69-
long getDifference(LockConfiguration lockConfiguration) {
70-
long now = Instant.now().toEpochMilli();
71-
long mostDiff = lockConfiguration.getLockAtMostUntil().toEpochMilli() - now;
72-
long leastDiff = lockConfiguration.getLockAtLeastUntil().toEpochMilli() - now;
73-
74-
long difference = -1;
75-
if (mostDiff > 0 && leastDiff > 0) {
76-
difference = Math.max(mostDiff, leastDiff);
77-
} else if (mostDiff > 0 && leastDiff <= 0) {
78-
difference = mostDiff;
79-
} else if (mostDiff <= 0 && leastDiff > 0) {
80-
difference = leastDiff;
81-
}
82-
return difference;
83-
}
84-
8592
private static final class RedisLock implements SimpleLock {
8693
private final String key;
8794
private final JedisPool jedisPool;
95+
private final LockConfiguration lockConfiguration;
8896

89-
private RedisLock(String key, JedisPool jedisPool) {
97+
private RedisLock(String key, JedisPool jedisPool, LockConfiguration lockConfiguration) {
9098
this.key = key;
9199
this.jedisPool = jedisPool;
100+
this.lockConfiguration = lockConfiguration;
92101
}
93102

94103
@Override
95104
public void unlock() {
96-
try (Jedis jedis = jedisPool.getResource()) {
97-
jedis.del(key);
98-
} catch (Exception e) {
99-
throw new LockException("Can not remove node", e);
105+
Instant now = Instant.now();
106+
Instant atLeastUntil = lockConfiguration.getLockAtLeastUntil();
107+
108+
if (now.equals(atLeastUntil) || now.isAfter(atLeastUntil)) {
109+
try (Jedis jedis = jedisPool.getResource()) {
110+
jedis.del(key);
111+
} catch (Exception e) {
112+
throw new LockException("Can not remove node", e);
113+
}
100114
}
101115
}
102116
}
@@ -110,7 +124,7 @@ protected static String getHostname() {
110124
}
111125

112126
public static String buildKey(String lockName, String env) {
113-
return String.format("%s:%s:%s", keyPrefix, env, lockName);
127+
return String.format("%s:%s:%s", KEY_PREFIX, env, lockName);
114128
}
115129

116130
static String buildValue() {

‎shedlock-provider-jedis/src/test/java/net/javacrumbs/shedlock/provider/jedis/JedisLockProviderIntegrationTest.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ public void shouldTimeout() throws InterruptedException {
7373

7474
sleep(5);
7575

76+
// Get new config with updated timeout
77+
configWithShortTimeout = lockConfig(LOCK_NAME1, 2, Duration.ZERO);
7678
assertUnlocked(configWithShortTimeout.getName());
7779
}
7880

@@ -83,14 +85,22 @@ public void shouldLockAtLeastFor() throws InterruptedException {
8385
assertThat(lock1).isNotEmpty();
8486

8587
// can not acquire lock, grace period did not pass yet
86-
configWithGracePeriod = lockConfig(LOCK_NAME1, 0, LOCK_AT_LEAST_FOR);
8788
Optional<SimpleLock> lock2 = getLockProvider().lock(configWithGracePeriod);
8889
assertThat(lock2).isEmpty();
8990

90-
sleep(LOCK_AT_LEAST_FOR.toMillis());
91+
// can not acquire lock, grace period did not pass yet
9192
configWithGracePeriod = lockConfig(LOCK_NAME1, 0, LOCK_AT_LEAST_FOR);
9293
Optional<SimpleLock> lock3 = getLockProvider().lock(configWithGracePeriod);
93-
assertThat(lock3).isNotEmpty();
94-
lock3.get().unlock();
94+
assertThat(lock3).isEmpty();
95+
96+
sleep(LOCK_AT_LEAST_FOR.toMillis());
97+
// Get with updated timeout
98+
configWithGracePeriod = lockConfig(LOCK_NAME1, 0, LOCK_AT_LEAST_FOR);
99+
Optional<SimpleLock> lock4 = getLockProvider().lock(configWithGracePeriod);
100+
assertThat(lock4).isNotEmpty();
101+
lock4.get().unlock();
102+
103+
// clean up for next test
104+
lock1.get().unlock();
95105
}
96106
}

0 commit comments

Comments
 (0)