Merge pull request #244 from gjorgievskivlatko/servertimestatementsou…

…rce-updatestatements-fix

Bugfix - ServerTimeStatementsSource strategies

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/Db2ServerTimeStatementsSource.java

@@ -21,7 +21,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/H2ServerTimeStatementsSource.java

@@ -21,7 +21,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/HsqlServerTimeStatementsSource.java

@@ -21,7 +21,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/MsSqlServerTimeStatementsSource.java

@@ -21,7 +21,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " +  lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " +  lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/MySqlServerTimeStatementsSource.java

@@ -21,7 +21,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/OracleServerTimeStatementsSource.java

@@ -24,7 +24,7 @@ String getInsertStatement() {
 
     @Override
     public String getUpdateStatement() {
-        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + lockUntil() + " <= " + now;
+        return "UPDATE " + tableName() + " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + name() + " = :name AND " + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/main/java/net/javacrumbs/shedlock/provider/jdbctemplate/PostgresSqlServerTimeStatementsSource.java

@@ -22,7 +22,7 @@ String getInsertStatement() {
 
     @NonNull
     private String updateClause() {
-        return " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + tableName() + "." + lockUntil() + " <= " + now;
+        return " SET " + lockUntil() + " = " + lockAtMostFor + ", " + lockedAt() + " = " + now + ", " + lockedBy() + " = :lockedBy WHERE " + tableName() + "." + name() + " = :name AND " + tableName() + "." + lockUntil() + " <= " + now;
     }
 
     @Override

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/AbstractJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,108 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.core.LockConfiguration;
+import net.javacrumbs.shedlock.support.annotation.NonNull;
+import net.javacrumbs.shedlock.test.support.jdbc.DbConfig;
+import net.javacrumbs.shedlock.test.support.jdbc.JdbcTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import java.sql.Timestamp;
+import java.time.Duration;
+
+import static java.lang.Thread.sleep;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public abstract class AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final String MY_LOCK = "my-lock";
+    private static final String OTHER_LOCK = "other-lock";
+
+    private final JdbcTestUtils testUtils;
+
+    protected AbstractJdbcTemplateStorageAccessorTest(DbConfig dbConfig) {
+        this.testUtils = new JdbcTestUtils(dbConfig);
+    }
+
+    @AfterEach
+    public void cleanup() {
+        testUtils.clean();
+    }
+
+    @Test
+    void shouldNotUpdateOnInsertIfPreviousDidNotEndWhenNotUsingDbTime() {
+        shouldNotUpdateOnInsertIfPreviousDidNotEnd(false);
+    }
+
+    @Test
+    void shouldNotUpdateOnInsertIfPreviousDidNotEndWhenUsingDbTime() {
+        shouldNotUpdateOnInsertIfPreviousDidNotEnd(true);
+    }
+
+    private void shouldNotUpdateOnInsertIfPreviousDidNotEnd(boolean usingDbTime) {
+        JdbcTemplateStorageAccessor accessor = getAccessor(usingDbTime);
+
+        assertThat(
+            accessor.insertRecord(new LockConfiguration(MY_LOCK, Duration.ofSeconds(10), Duration.ZERO))
+        ).isEqualTo(true);
+
+        Timestamp originalLockValidity = testUtils.getLockedUntil(MY_LOCK);
+
+        assertThat(
+            accessor.insertRecord(new LockConfiguration(MY_LOCK, Duration.ofSeconds(10), Duration.ZERO))
+        ).isEqualTo(false);
+
+        assertThat(testUtils.getLockedUntil(MY_LOCK)).isEqualTo(originalLockValidity);
+    }
+
+    @Test
+    void shouldNotUpdateOtherLockConfigurationsWhenNotUsingDbTime() throws InterruptedException {
+        shouldNotUpdateOtherLockConfigurations(false);
+    }
+
+    @Test
+    void shouldNotUpdateOtherLockConfigurationsWhenUsingDbTime() throws InterruptedException {
+        shouldNotUpdateOtherLockConfigurations(true);
+    }
+
+    private void shouldNotUpdateOtherLockConfigurations(boolean usingDbTime) throws InterruptedException {
+        JdbcTemplateStorageAccessor accessor = getAccessor(usingDbTime);
+
+        Duration lockAtMostFor = Duration.ofMillis(10);
+        assertThat(accessor.insertRecord(new LockConfiguration(MY_LOCK, lockAtMostFor, Duration.ZERO))).isEqualTo(true);
+        assertThat(accessor.insertRecord(new LockConfiguration(OTHER_LOCK, lockAtMostFor, Duration.ZERO))).isEqualTo(true);
+
+        Timestamp myLockLockedUntil = testUtils.getLockedUntil(MY_LOCK);
+        Timestamp otherLockLockedUntil = testUtils.getLockedUntil(OTHER_LOCK);
+
+        // wait for a while so there will be a difference in the timestamp
+        // when system time is used seems there is no milliseconds in the timestamp so to make a difference we have to wait for at least a second
+        sleep(1000);
+
+
+        // act
+        assertThat(accessor.updateRecord(new LockConfiguration(MY_LOCK, lockAtMostFor, Duration.ZERO))).isEqualTo(true);
+
+
+        // assert
+        assertThat(testUtils.getLockedUntil(MY_LOCK)).isAfter(myLockLockedUntil);
+        // check that the other lock has not been affected by "my-lock" update
+        assertThat(testUtils.getLockedUntil(OTHER_LOCK)).isEqualTo(otherLockLockedUntil);
+    }
+
+    @NonNull
+    protected JdbcTemplateStorageAccessor getAccessor(boolean usingDbTime) {
+        JdbcTemplateLockProvider.Configuration.Builder builder = JdbcTemplateLockProvider
+            .Configuration.builder()
+            .withJdbcTemplate(testUtils.getJdbcTemplate());
+        if (usingDbTime) {
+            builder.usingDbTime();
+        }
+
+        return new JdbcTemplateStorageAccessor(builder.build());
+    }
+
+    protected JdbcTestUtils getTestUtils() {
+        return testUtils;
+    }
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/Db2JdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.Db2ServerConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class Db2JdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final Db2ServerConfig dbConfig = new Db2ServerConfig();
+
+    protected Db2JdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/H2JdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.H2Config;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class H2JdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final H2Config dbConfig = new H2Config();
+
+    protected H2JdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/HsqlJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.HsqlConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class HsqlJdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final HsqlConfig dbConfig = new HsqlConfig();
+
+    protected HsqlJdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/MariaDbJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.MariaDbConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class MariaDbJdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final MariaDbConfig dbConfig = new MariaDbConfig();
+
+    protected MariaDbJdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/MsSqlJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.MsSqlServerConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class MsSqlJdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final MsSqlServerConfig dbConfig = new MsSqlServerConfig();
+
+    protected MsSqlJdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/MySqlJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.MySqlConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class MySqlJdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final MySqlConfig dbConfig = new MySqlConfig();
+
+    protected MySqlJdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}

providers/jdbc/shedlock-provider-jdbc-template/src/test/java/net/javacrumbs/shedlock/provider/jdbctemplate/OracleJdbcTemplateStorageAccessorTest.java

@@ -0,0 +1,25 @@
+package net.javacrumbs.shedlock.provider.jdbctemplate;
+
+import net.javacrumbs.shedlock.test.support.jdbc.OracleServerConfig;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+class OracleJdbcTemplateStorageAccessorTest extends AbstractJdbcTemplateStorageAccessorTest {
+
+    private static final OracleServerConfig dbConfig = new OracleServerConfig();
+
+    protected OracleJdbcTemplateStorageAccessorTest() {
+        super(dbConfig);
+    }
+
+    @BeforeAll
+    public static void startDb() {
+        dbConfig.startDb();
+    }
+
+    @AfterAll
+    public static void shutdownDb() {
+        dbConfig.shutdownDb();
+    }
+
+}