From 891eb37128eb04411d1405842c5731c216c7e0bd Mon Sep 17 00:00:00 2001 From: Barry Lagerweij Date: Tue, 14 Nov 2023 13:05:29 +0100 Subject: [PATCH] fix issue #41 with MySQL long lock names (#44) --- .../sessionlock/MSSQLLockService.java | 7 ++--- .../sessionlock/MySQLLockService.java | 12 ++++++--- .../sessionlock/OracleLockService.java | 5 ++-- .../sessionlock/util/StringUtils.java | 26 ++++++++++++++++++ .../sessionlock/MySQLLockServiceTest.java | 24 +++++++++++++++-- .../sessionlock/util/StringUtilsTest.java | 27 +++++++++++++++++++ 6 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/github/blagerweij/sessionlock/util/StringUtils.java create mode 100644 src/test/java/com/github/blagerweij/sessionlock/util/StringUtilsTest.java diff --git a/src/main/java/com/github/blagerweij/sessionlock/MSSQLLockService.java b/src/main/java/com/github/blagerweij/sessionlock/MSSQLLockService.java index 9949d95..fe2d66f 100644 --- a/src/main/java/com/github/blagerweij/sessionlock/MSSQLLockService.java +++ b/src/main/java/com/github/blagerweij/sessionlock/MSSQLLockService.java @@ -16,6 +16,8 @@ import java.util.Date; import java.util.Locale; +import static com.github.blagerweij.sessionlock.util.StringUtils.toUpperCase; + /** * Employs MSSQL application resource locks. * @@ -59,8 +61,7 @@ public boolean supports(Database database) { } private String getChangeLogLockName() { - return (database.getDefaultSchemaName() + "." + database.getDatabaseChangeLogLockTableName()) - .toUpperCase(Locale.ROOT); + return toUpperCase(database.getDefaultSchemaName() + "." + database.getDatabaseChangeLogLockTableName()); } private static Integer getIntegerResult(PreparedStatement stmt) throws SQLException { @@ -117,7 +118,7 @@ protected void releaseLock(final Connection con) throws SQLException, LockExcept Integer unlocked = getIntegerResult(stmt); if (!Integer.valueOf(0).equals(unlocked)) { throw new LockException( - "RELEASE_LOCK() returned " + String.valueOf(unlocked).toUpperCase(Locale.ROOT)); + "RELEASE_LOCK() returned " + toUpperCase(String.valueOf(unlocked))); } } } diff --git a/src/main/java/com/github/blagerweij/sessionlock/MySQLLockService.java b/src/main/java/com/github/blagerweij/sessionlock/MySQLLockService.java index 6497b72..6af7bbb 100644 --- a/src/main/java/com/github/blagerweij/sessionlock/MySQLLockService.java +++ b/src/main/java/com/github/blagerweij/sessionlock/MySQLLockService.java @@ -10,11 +10,16 @@ import java.sql.SQLException; import java.util.Date; import java.util.Locale; + +import com.github.blagerweij.sessionlock.util.StringUtils; import liquibase.database.Database; import liquibase.database.core.MySQLDatabase; import liquibase.exception.LockException; import liquibase.lockservice.DatabaseChangeLogLock; +import static com.github.blagerweij.sessionlock.util.StringUtils.toUpperCase; +import static com.github.blagerweij.sessionlock.util.StringUtils.truncate; + /** * Employs MySQL user-level (a.k.a. application-level or advisory) locks. * @@ -47,10 +52,9 @@ public boolean supports(Database database) { return (database instanceof MySQLDatabase); } - private String getChangeLogLockName() { + protected String getChangeLogLockName() { // MySQL 5.7 and later enforces a maximum length on lock names of 64 characters. - return (database.getDefaultSchemaName() + "." + database.getDatabaseChangeLogLockTableName()) - .toUpperCase(Locale.ROOT); + return toUpperCase(truncate((database.getDefaultSchemaName() + "." + database.getDatabaseChangeLogLockTableName()), 64)); } private static Integer getIntegerResult(PreparedStatement stmt) throws SQLException { @@ -96,7 +100,7 @@ protected void releaseLock(Connection con) throws SQLException, LockException { Integer unlocked = getIntegerResult(stmt); if (!Integer.valueOf(1).equals(unlocked)) { throw new LockException( - "RELEASE_LOCK() returned " + String.valueOf(unlocked).toUpperCase(Locale.ROOT)); + "RELEASE_LOCK() returned " + toUpperCase(String.valueOf(unlocked))); } } } diff --git a/src/main/java/com/github/blagerweij/sessionlock/OracleLockService.java b/src/main/java/com/github/blagerweij/sessionlock/OracleLockService.java index e0f74d9..ad6f277 100644 --- a/src/main/java/com/github/blagerweij/sessionlock/OracleLockService.java +++ b/src/main/java/com/github/blagerweij/sessionlock/OracleLockService.java @@ -16,6 +16,8 @@ import liquibase.exception.LockException; import liquibase.lockservice.DatabaseChangeLogLock; +import static com.github.blagerweij.sessionlock.util.StringUtils.toUpperCase; + /** * Employs Oracle user-level (a.k.a. application-level or advisory) locks. * @@ -40,8 +42,7 @@ public boolean supports(Database database) { } private String getChangeLogLockName() { - return (database.getLiquibaseCatalogName() + "." + database.getDatabaseChangeLogLockTableName()) - .toUpperCase(Locale.ROOT); + return toUpperCase(database.getLiquibaseCatalogName() + "." + database.getDatabaseChangeLogLockTableName()); } /** diff --git a/src/main/java/com/github/blagerweij/sessionlock/util/StringUtils.java b/src/main/java/com/github/blagerweij/sessionlock/util/StringUtils.java new file mode 100644 index 0000000..a3f78b5 --- /dev/null +++ b/src/main/java/com/github/blagerweij/sessionlock/util/StringUtils.java @@ -0,0 +1,26 @@ +package com.github.blagerweij.sessionlock.util; + +import java.util.Locale; + +public class StringUtils { + + StringUtils() { + throw new IllegalArgumentException("Utility class"); + } + + public static String truncate(final String str, final int maxlength) { + if (str == null || str.length() <= maxlength) { + return str; + } else { + return str.substring(0, maxlength); + } + } + + public static String toUpperCase(final String str) { + if (str == null) { + return null; + } else { + return str.toUpperCase(Locale.ROOT); + } + } +} diff --git a/src/test/java/com/github/blagerweij/sessionlock/MySQLLockServiceTest.java b/src/test/java/com/github/blagerweij/sessionlock/MySQLLockServiceTest.java index 9766c8f..6786201 100644 --- a/src/test/java/com/github/blagerweij/sessionlock/MySQLLockServiceTest.java +++ b/src/test/java/com/github/blagerweij/sessionlock/MySQLLockServiceTest.java @@ -6,6 +6,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -22,18 +24,20 @@ import liquibase.lockservice.DatabaseChangeLogLock; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; public class MySQLLockServiceTest { private MySQLLockService lockService; private Connection dbCon; + private MySQLDatabase database; - @BeforeEach + @BeforeEach public void setUp() { dbCon = mock(Connection.class); - MySQLDatabase database = new MySQLDatabase(); + database = new MySQLDatabase(); database.setDefaultCatalogName("test_schema"); database = spy(database); when(database.getConnection()).thenReturn(new JdbcConnection(dbCon)); @@ -86,6 +90,22 @@ public void acquireFailure() throws Exception { assertThatThrownBy(() -> lockService.acquireLock()).isInstanceOf(LockException.class); } + @Test + @SuppressWarnings("resource") + public void lockNameTooLongShouldFail() throws Exception { + database.setDefaultCatalogName("this_name_is_too_long_and_should_be_truncated_since_mysql_does_not_like_long_locknames"); + PreparedStatement stmt = mock(PreparedStatement.class); + ResultSet rs = mock(ResultSet.class); + when(stmt.executeQuery()).thenReturn(rs); + when(dbCon.prepareStatement(MySQLLockService.SQL_GET_LOCK)).thenReturn(stmt); + ArgumentCaptor lockNameCaptor = ArgumentCaptor.forClass(String.class); + doNothing().when(stmt).setString(eq(1), lockNameCaptor.capture()); + when(rs.next()).thenReturn(true, false); + when(rs.getObject(1)).thenReturn(1); + assertThat(lockService.acquireLock()).isTrue(); + assertThat(lockNameCaptor.getValue()).isEqualTo("THIS_NAME_IS_TOO_LONG_AND_SHOULD_BE_TRUNCATED_SINCE_MYSQL_DOES_N"); + } + @Test @SuppressWarnings("resource") public void releaseSuccess() throws Exception { diff --git a/src/test/java/com/github/blagerweij/sessionlock/util/StringUtilsTest.java b/src/test/java/com/github/blagerweij/sessionlock/util/StringUtilsTest.java new file mode 100644 index 0000000..a7fb558 --- /dev/null +++ b/src/test/java/com/github/blagerweij/sessionlock/util/StringUtilsTest.java @@ -0,0 +1,27 @@ +package com.github.blagerweij.sessionlock.util; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class StringUtilsTest { + @Test + void constructor() { + assertThatThrownBy(() -> new StringUtils()).isInstanceOf(IllegalArgumentException.class); + } + + @Test + void truncate() { + assertThat(StringUtils.truncate("testing", 4)).isEqualTo("test"); + assertThat(StringUtils.truncate("test", 4)).isEqualTo("test"); + assertThat(StringUtils.truncate("t", 4)).isEqualTo("t"); + assertThat(StringUtils.truncate(null, 4)).isEqualTo(null); + } + + @Test + void toUpperCase() { + assertThat(StringUtils.toUpperCase("testing")).isEqualTo("TESTING"); + assertThat(StringUtils.toUpperCase(null)).isEqualTo(null); + } +}