optimize: undo_log table check optimize (#6031)

changes/en-us/2.x.md

@@ -9,7 +9,7 @@ Add changes here for all PR submitted to the 2.x branch.
 - [[#PR_NO](https://github.com/seata/seata/pull/PR_NO)] A brief and accurate description of PR
 
 ### optimize:
-- [[#PR_NO](https://github.com/seata/seata/pull/PR_NO)] A brief and accurate description of PR
+- [[#6031](https://github.com/seata/seata/pull/6031)] add a check for the existence of the undolog table
 
 ### security:
 - [[#6069](https://github.com/seata/seata/pull/6069)] Upgrade Guava dependencies to fix security vulnerabilities
@@ -21,6 +21,8 @@ Thanks to these contributors for their code commits. Please report an unintended
 
 <!-- Please make sure your Github ID is in the list below -->
 - [slievrly](https://github.com/slievrly)
+- [laywin](https://github.com/laywin)
 - [imcmai](https://github.com/imcmai)
 
+
 Also, we receive many valuable issues, questions and advices from our community. Thanks for you all.

changes/zh-cn/2.x.md

@@ -9,7 +9,7 @@
 - [[#PR_NO](https://github.com/seata/seata/pull/PR_NO)] 准确简要的PR描述
 
 ### optimize:
-- [[#PR_NO](https://github.com/seata/seata/pull/PR_NO)] 准确简要的PR描述
+- [[#6031](https://github.com/seata/seata/pull/6031)] 添加undo_log表的存在性校验
 
 ### security:
 - [[#6069](https://github.com/seata/seata/pull/6069)] 升级Guava依赖版本，修复安全漏洞
@@ -21,6 +21,7 @@
 
 <!-- 请确保您的 GitHub ID 在以下列表中 -->
 - [slievrly](https://github.com/slievrly)
+- [laywin](https://github.com/laywin)
 - [imcmai](https://github.com/imcmai)
 
 同时，我们收到了社区反馈的很多有价值的issue和建议，非常感谢大家。

config/seata-config-core/src/test/java/io/seata/config/ConfigFutureTest.java

@@ -21,7 +21,6 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
-import org.mockito.internal.util.reflection.FieldSetter;
 
 import java.lang.reflect.Field;
 import java.util.concurrent.CompletableFuture;
@@ -44,7 +43,8 @@ void testGet() throws NoSuchFieldException, IllegalAccessException, ExecutionExc
         // mock field
         origin = Mockito.spy(origin);
         // set mocked field to object
-        FieldSetter.setField(configFuture, originField, origin);
+        originField.setAccessible(true);
+        originField.set(configFuture, origin);
 
         Mockito.doThrow(ExecutionException.class).when(origin).get(Mockito.anyLong(), Mockito.any());
         Assertions.assertThrows(ShouldNeverHappenException.class, configFuture::get);

core/src/main/java/io/seata/core/constants/DBType.java

@@ -208,5 +208,4 @@ public static DBType valueof(String dbType) {
         }
         throw new IllegalArgumentException("unknown dbtype:" + dbType);
     }
-
 }

dependencies/pom.xml

@@ -118,7 +118,7 @@
         <kotlin-coroutines.version>1.4.3</kotlin-coroutines.version>
 
         <!-- # for test -->
-        <mockito.version>2.23.4</mockito.version>
+        <mockito.version>4.5.1</mockito.version>
         <assertj-core.version>3.12.2</assertj-core.version>
         <jetty-version>9.4.38.v20210224</jetty-version>
         <janino-version>3.1.7</janino-version>
@@ -701,6 +701,11 @@
                 <artifactId>mockito-junit-jupiter</artifactId>
                 <version>${mockito.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.mockito</groupId>
+                <artifactId>mockito-inline</artifactId>
+                <version>${mockito.version}</version>
+            </dependency>
             <dependency>
                 <groupId>org.assertj</groupId>
                 <artifactId>assertj-core</artifactId>

pom.xml

@@ -92,6 +92,11 @@
             <artifactId>mockito-junit-jupiter</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-inline</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.assertj</groupId>
             <artifactId>assertj-core</artifactId>

rm-datasource/src/main/java/io/seata/rm/RMHandlerAT.java

@@ -19,8 +19,6 @@
 import java.sql.SQLException;
 import java.text.ParseException;
 import java.util.Date;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 import io.seata.common.util.DateUtil;
 import io.seata.core.model.BranchType;
@@ -44,8 +42,6 @@ public class RMHandlerAT extends AbstractRMHandler {
 
     private static final int LIMIT_ROWS = 3000;
 
-    private final Map<String, Boolean> undoLogTableExistRecord = new ConcurrentHashMap<>();
-
     @Override
     public void handle(UndoLogDeleteRequest request) {
         String resourceId = request.getResourceId();
@@ -56,12 +52,6 @@ public void handle(UndoLogDeleteRequest request) {
             return;
         }
 
-        boolean hasUndoLogTable = undoLogTableExistRecord.computeIfAbsent(resourceId, id -> checkUndoLogTableExist(dataSourceProxy));
-        if (!hasUndoLogTable) {
-            LOGGER.debug("resource({}) has no undo_log table, UndoLogDeleteRequest will be ignored", resourceId);
-            return;
-        }
-
         Date division = getLogCreated(request.getSaveDays());
 
         UndoLogManager manager = getUndoLogManager(dataSourceProxy);
@@ -80,19 +70,6 @@ public void handle(UndoLogDeleteRequest request) {
         }
     }
 
-    boolean checkUndoLogTableExist(DataSourceProxy dataSourceProxy) {
-        UndoLogManager manager = getUndoLogManager(dataSourceProxy);
-        try (Connection connection = getConnection(dataSourceProxy)) {
-            if (connection == null) {
-                return false;
-            }
-            return manager.hasUndoLogTable(connection);
-        } catch (Exception e) {
-            // should never happen, hasUndoLogTable method had catch all Exception
-            return false;
-        }
-    }
-
     Connection getConnection(DataSourceProxy dataSourceProxy) {
         try {
             return dataSourceProxy.getPlainConnection();

rm-datasource/src/main/java/io/seata/rm/datasource/DataSourceProxy.java

@@ -22,18 +22,25 @@
 
 import javax.sql.DataSource;
 
+import io.seata.common.ConfigurationKeys;
 import io.seata.common.Constants;
+import io.seata.common.loader.EnhancedServiceNotFoundException;
+import io.seata.config.ConfigurationFactory;
 import io.seata.core.context.RootContext;
 import io.seata.core.model.BranchType;
 import io.seata.core.model.Resource;
 import io.seata.rm.DefaultResourceManager;
 import io.seata.rm.datasource.sql.struct.TableMetaCacheFactory;
+import io.seata.rm.datasource.undo.UndoLogManager;
+import io.seata.rm.datasource.undo.UndoLogManagerFactory;
 import io.seata.rm.datasource.util.JdbcUtils;
 import io.seata.sqlparser.util.JdbcConstants;
 import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static io.seata.common.DefaultValues.DEFAULT_TRANSACTION_UNDO_LOG_TABLE;
+
 /**
  * The type Data source proxy.
  *
@@ -101,6 +108,8 @@ private void init(DataSource dataSource, String resourceGroupId) {
                 validMySQLVersion(connection);
                 checkDerivativeProduct();
             }
+            checkUndoLogTableExist(connection);
+
         } catch (SQLException e) {
             throw new IllegalStateException("can not init dataSource", e);
         }
@@ -143,6 +152,31 @@ private boolean isPolardbXProduct() {
         return false;
     }
 
+    /**
+     * check existence of undolog table
+     *
+     * if the table not exist fast fail, or else keep silence
+     *
+     * @param conn db connection
+     */
+    private void checkUndoLogTableExist(Connection conn) {
+        UndoLogManager undoLogManager;
+        try {
+            undoLogManager = UndoLogManagerFactory.getUndoLogManager(dbType);
+        } catch (EnhancedServiceNotFoundException e) {
+            String errMsg = String.format("AT mode don't support the the dbtype: %s", dbType);
+            throw new IllegalStateException(errMsg, e);
+        }
+
+        boolean undoLogTableExist = undoLogManager.hasUndoLogTable(conn);
+        if (!undoLogTableExist) {
+            String undoLogTableName = ConfigurationFactory.getInstance()
+                    .getConfig(ConfigurationKeys.TRANSACTION_UNDO_LOG_TABLE, DEFAULT_TRANSACTION_UNDO_LOG_TABLE);
+            String errMsg = String.format("in AT mode, %s table not exist", undoLogTableName);
+            throw new IllegalStateException(errMsg);
+        }
+    }
+
     /**
      * publish tableMeta refresh event
      */

rm-datasource/src/test/java/io/seata/rm/RMHandlerATTest.java

@@ -22,7 +22,10 @@
 import org.junit.jupiter.api.Test;
 
 import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Date;
 
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.times;
@@ -41,30 +44,26 @@
 class RMHandlerATTest {
 
     @Test
-    void hasUndoLogTableTest() {
-        RMHandlerAT handler = buildHandler(true);
+    void testNormalDeleteUndoLogTable() throws SQLException {
+        RMHandlerAT handler = buildHandler(false);
         UndoLogDeleteRequest request = buildRequest();
         int testTimes = 5;
         for (int i = 0; i < testTimes; i++) {
             handler.handle(request);
         }
-        verify(handler, times(1)).checkUndoLogTableExist(any());
         verify(handler, times(testTimes)).deleteUndoLog(any(), any(), any());
     }
 
     @Test
-    void noUndoLogTableTest() {
-        RMHandlerAT handler = buildHandler(false);
+    void testErrorDeleteUndoLogTable() throws SQLException {
+        RMHandlerAT handler = buildHandler(true);
         UndoLogDeleteRequest request = buildRequest();
-        int testTimes = 5;
-        for (int i = 0; i < testTimes; i++) {
-            handler.handle(request);
-        }
-        verify(handler, times(1)).checkUndoLogTableExist(any());
-        verify(handler, never()).deleteUndoLog(any(), any(), any());
+        request.setSaveDays((short) -1);
+        handler.handle(request);
+        verify(handler, times(1)).deleteUndoLog(any(), any(), any());
     }
 
-    private RMHandlerAT buildHandler(boolean hasUndoLogTable) {
+    private RMHandlerAT buildHandler(boolean errorDeleteUndologTable) throws SQLException {
         RMHandlerAT handler = spy(new RMHandlerAT());
         DataSourceManager dataSourceManager = mock(DataSourceManager.class);
         doReturn(dataSourceManager).when(handler).getResourceManager();
@@ -78,9 +77,14 @@ private RMHandlerAT buildHandler(boolean hasUndoLogTable) {
         });
 
         UndoLogManager manager = mock(UndoLogManager.class);
-        when(manager.hasUndoLogTable(any())).thenReturn(hasUndoLogTable);
+        when(manager.hasUndoLogTable(any())).thenReturn(true);
         doReturn(manager).when(handler).getUndoLogManager(any());
 
+        if (errorDeleteUndologTable) {
+            when(manager.deleteUndoLogByLogCreated(any(Date.class), anyInt(), any(Connection.class)))
+                    .thenThrow(new SQLException());
+        }
+
         return handler;
     }
 

rm-datasource/src/test/java/io/seata/rm/datasource/DataSourceProxyTest.java

@@ -16,15 +16,25 @@
 package io.seata.rm.datasource;
 
 import java.lang.reflect.Field;
+import java.sql.Connection;
 import java.sql.SQLException;
 import javax.sql.DataSource;
 
 import com.alibaba.druid.pool.DruidDataSource;
 import io.seata.rm.datasource.mock.MockDataSource;
 import io.seata.rm.datasource.mock.MockDriver;
 import io.seata.rm.datasource.sql.struct.TableMetaCacheFactory;
+import io.seata.rm.datasource.undo.UndoLogManagerFactory;
+import io.seata.rm.datasource.undo.mysql.MySQLUndoLogManager;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.*;
 
 /**
  * @author ph3636
@@ -34,14 +44,49 @@ public class DataSourceProxyTest {
     @Test
     public void test_constructor() {
         DataSource dataSource = new MockDataSource();
-
         DataSourceProxy dataSourceProxy = new DataSourceProxy(dataSource);
         Assertions.assertEquals(dataSourceProxy.getTargetDataSource(), dataSource);
 
         DataSourceProxy dataSourceProxy2 = new DataSourceProxy(dataSourceProxy);
         Assertions.assertEquals(dataSourceProxy2.getTargetDataSource(), dataSource);
     }
 
+    @Test
+    public void testNotSupportDb() {
+        final MockDriver mockDriver = new MockDriver();
+        final String username = "username";
+        final String jdbcUrl = "jdbc:mock:xxx";
+
+        // create data source
+        final DruidDataSource dataSource = new DruidDataSource();
+        dataSource.setUrl(jdbcUrl);
+        dataSource.setDriver(mockDriver);
+        dataSource.setUsername(username);
+        dataSource.setPassword("password");
+
+        Throwable throwable = Assertions.assertThrows(IllegalStateException.class, () -> new DataSourceProxy(dataSource));
+        assertThat(throwable).hasMessageContaining("AT mode don't support the the dbtype");
+    }
+
+
+    @Test
+    public void testUndologTableNotExist() {
+        DataSource dataSource = new MockDataSource();
+
+        MockedStatic<UndoLogManagerFactory> undoLogManagerFactoryMockedStatic = Mockito.mockStatic(UndoLogManagerFactory.class);
+
+        MySQLUndoLogManager mysqlUndoLogManager = mock(MySQLUndoLogManager.class);
+        undoLogManagerFactoryMockedStatic.when(()->UndoLogManagerFactory.getUndoLogManager(anyString()))
+                .thenReturn(mysqlUndoLogManager);
+
+        doReturn(false).when(mysqlUndoLogManager).hasUndoLogTable(any(Connection.class));
+
+        Throwable throwable = Assertions.assertThrows(IllegalStateException.class, () -> new DataSourceProxy(dataSource));
+        undoLogManagerFactoryMockedStatic.close();
+
+        assertThat(throwable).hasMessageContaining("table not exist");
+    }
+
     @Test
     public void getResourceIdTest() throws SQLException, NoSuchFieldException, IllegalAccessException {
         // Disable 'DataSourceProxy.tableMetaExecutor' to prevent unit tests from being affected
@@ -62,7 +107,7 @@ public void getResourceIdTest() throws SQLException, NoSuchFieldException, Illeg
         dataSource.setPassword("password");
 
         // create data source proxy
-        final DataSourceProxy proxy = new DataSourceProxy(dataSource);
+        final DataSourceProxy proxy = getDataSourceProxy(dataSource);
 
         // get fields
         Field resourceIdField = proxy.getClass().getDeclaredField("resourceId");
@@ -124,4 +169,17 @@ public void getResourceIdTest() throws SQLException, NoSuchFieldException, Illeg
             jdbcUrlField.set(proxy, jdbcUrl);
         }
     }
+
+    // to skip the db & undolog table check
+    public static DataSourceProxy getDataSourceProxy(DataSource dataSource) {
+        try (MockedStatic<UndoLogManagerFactory> undoLogManagerFactoryMockedStatic = Mockito.mockStatic(UndoLogManagerFactory.class)) {
+            MySQLUndoLogManager mysqlUndoLogManager = mock(MySQLUndoLogManager.class);
+            undoLogManagerFactoryMockedStatic.when(() -> UndoLogManagerFactory.getUndoLogManager(anyString())).thenReturn(mysqlUndoLogManager);
+
+            doReturn(true).when(mysqlUndoLogManager).hasUndoLogTable(any(Connection.class));
+
+            // create data source proxy
+            return new DataSourceProxy(dataSource);
+        }
+    }
 }