Skip to content

Commit 6acdd98

Browse files
lukas-krecanlukas-krecan
committed
Small post-merge fixes
1 parent 56bfe4b commit 6acdd98

File tree

3 files changed

+30
-32
lines changed

3 files changed

+30
-32
lines changed

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

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.net.UnknownHostException;
2727
import java.time.Duration;
2828
import java.time.Instant;
29-
import java.time.temporal.ChronoUnit;
3029
import java.util.Optional;
3130

3231
/**
@@ -40,38 +39,26 @@ public class JedisLockProvider implements LockProvider {
4039
private static final String KEY_PREFIX = "job-lock";
4140
private static final String ENV_DEFAULT = "default";
4241

43-
private static final long AT_MOST_HOURS_DEFAULT = 1;
44-
4542
// Redis Flags
4643
private static final String SET_IF_NOT_EXIST = "NX";
44+
private static final String SET_IF_EXIST = "XX";
4745
private static final String SET_EXPIRE_TIME_IN_MS = "PX";
4846

4947
private JedisPool jedisPool;
5048
private String environment;
51-
private Duration atMostHoursDefault;
5249

5350
public JedisLockProvider(JedisPool jedisPool) {
54-
this(jedisPool, ENV_DEFAULT, AT_MOST_HOURS_DEFAULT);
51+
this(jedisPool, ENV_DEFAULT);
5552
}
5653

5754
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) {
6255
this.jedisPool = jedisPool;
6356
this.environment = environment;
64-
this.atMostHoursDefault = Duration.of(atMostHoursDefault, ChronoUnit.HOURS);
6557
}
6658

6759
@Override
6860
public Optional<SimpleLock> lock(LockConfiguration lockConfiguration) {
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());
61+
long expireTime = getMsUntil(lockConfiguration.getLockAtMostUntil());
7562

7663
String key = buildKey(lockConfiguration.getName(), this.environment);
7764

@@ -80,7 +67,7 @@ public Optional<SimpleLock> lock(LockConfiguration lockConfiguration) {
8067
buildValue(),
8168
SET_IF_NOT_EXIST,
8269
SET_EXPIRE_TIME_IN_MS,
83-
expireTime < 0 ? atMostHoursDefault.toMillis() : expireTime); // And if not set, use default
70+
expireTime);
8471

8572
if (rez != null && "OK".equals(rez)) {
8673
return Optional.of(new RedisLock(key, jedisPool, lockConfiguration));
@@ -102,32 +89,44 @@ private RedisLock(String key, JedisPool jedisPool, LockConfiguration lockConfigu
10289

10390
@Override
10491
public void unlock() {
105-
Instant now = Instant.now();
106-
Instant atLeastUntil = lockConfiguration.getLockAtLeastUntil();
92+
long keepLockFor = getMsUntil(lockConfiguration.getLockAtLeastUntil());
10793

108-
if (now.equals(atLeastUntil) || now.isAfter(atLeastUntil)) {
94+
// lock at least until is in the past
95+
if (keepLockFor <= 0) {
10996
try (Jedis jedis = jedisPool.getResource()) {
11097
jedis.del(key);
11198
} catch (Exception e) {
11299
throw new LockException("Can not remove node", e);
113100
}
101+
} else {
102+
try (Jedis jedis = jedisPool.getResource()) {
103+
jedis.set(key,
104+
buildValue(),
105+
SET_IF_EXIST,
106+
SET_EXPIRE_TIME_IN_MS,
107+
keepLockFor);
108+
}
114109
}
115110
}
116111
}
117112

118-
protected static String getHostname() {
113+
private static long getMsUntil(Instant instant) {
114+
return Duration.between(Instant.now(), instant).toMillis();
115+
}
116+
117+
private static String getHostname() {
119118
try {
120119
return InetAddress.getLocalHost().getHostName();
121120
} catch (UnknownHostException e) {
122121
return "unknown host";
123122
}
124123
}
125124

126-
public static String buildKey(String lockName, String env) {
125+
static String buildKey(String lockName, String env) {
127126
return String.format("%s:%s:%s", KEY_PREFIX, env, lockName);
128127
}
129128

130-
static String buildValue() {
129+
private static String buildValue() {
131130
return String.format("ADDED:%s@%s", Instant.now().toString(),getHostname());
132131
}
133132
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,14 @@ protected void assertLocked(String lockName) {
8282

8383
@Override
8484
public void shouldTimeout() throws InterruptedException {
85-
LockConfiguration configWithShortTimeout = lockConfig(LOCK_NAME1, 2, Duration.ZERO);
85+
LockConfiguration configWithShortTimeout = lockConfig(LOCK_NAME1, Duration.ofMillis(2), Duration.ZERO);
8686
Optional<SimpleLock> lock1 = getLockProvider().lock(configWithShortTimeout);
8787
assertThat(lock1).isNotEmpty();
8888

8989
sleep(5);
9090

9191
// Get new config with updated timeout
92-
configWithShortTimeout = lockConfig(LOCK_NAME1, 2, Duration.ZERO);
92+
configWithShortTimeout = lockConfig(LOCK_NAME1, Duration.ofMillis(2), Duration.ZERO);
9393
assertUnlocked(configWithShortTimeout.getName());
9494
}
9595
}

‎shedlock-test-support/src/main/java/net/javacrumbs/shedlock/test/support/AbstractLockProviderIntegrationTest.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
public abstract class AbstractLockProviderIntegrationTest {
3434
protected static final String LOCK_NAME1 = "name";
35-
public static final Duration LOCK_AT_LEAST_FOR = Duration.of(5, SECONDS);
35+
public static final Duration LOCK_AT_LEAST_FOR = Duration.of(2, SECONDS);
3636

3737
protected abstract LockProvider getLockProvider();
3838

@@ -114,17 +114,16 @@ public void fuzzTestShouldPass() throws ExecutionException, InterruptedException
114114

115115
@Test
116116
public void shouldLockAtLeastFor() throws InterruptedException {
117-
LockConfiguration configWithGracePeriod = lockConfig(LOCK_NAME1, LOCK_AT_LEAST_FOR, LOCK_AT_LEAST_FOR);
118-
Optional<SimpleLock> lock1 = getLockProvider().lock(configWithGracePeriod);
117+
Optional<SimpleLock> lock1 = getLockProvider().lock(lockConfig(LOCK_NAME1, LOCK_AT_LEAST_FOR.multipliedBy(2), LOCK_AT_LEAST_FOR));
119118
assertThat(lock1).isNotEmpty();
120119
lock1.get().unlock();
121120

122121
// can not acquire lock, grace period did not pass yet
123-
Optional<SimpleLock> lock2 = getLockProvider().lock(configWithGracePeriod);
124-
assertThat(lock2).isEmpty();
125-
122+
assertThat(getLockProvider().lock(lockConfig(LOCK_NAME1))).isEmpty();
126123
sleep(LOCK_AT_LEAST_FOR.toMillis());
127-
Optional<SimpleLock> lock3 = getLockProvider().lock(configWithGracePeriod);
124+
125+
// can acquire the lock
126+
Optional<SimpleLock> lock3 = getLockProvider().lock(lockConfig(LOCK_NAME1));
128127
assertThat(lock3).isNotEmpty();
129128
lock3.get().unlock();
130129

0 commit comments

Comments
 (0)