From 5ba2fcab73a264ac1785e1bfa562d27d1af18f74 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Sun, 9 Mar 2025 21:15:29 +0800
Subject: [PATCH] [#6630] fix(jdbc-catalog): jdbc.pool.test-on-borrow does not
 work when connecting to JDBC catalog (#6643)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

jdbc.pool.test-on-borrow does not work when connecting to JDBC catalog

### Why are the changes needed?

Fix: #6630

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

N/A

Co-authored-by: Xiaojian Sun <sunxiaojian926@163.com>
---
 .../jdbc/JdbcCatalogPropertiesMetadata.java   | 12 +++++++++-
 .../jdbc/TestJdbcCatalogOperations.java       | 23 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java
index d65bdfd7c1d..0499ab221cb 100644
--- a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java
+++ b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java
@@ -18,6 +18,7 @@
  */
 package org.apache.gravitino.catalog.jdbc;
 
+import static org.apache.gravitino.connector.PropertyEntry.booleanPropertyEntry;
 import static org.apache.gravitino.connector.PropertyEntry.integerPropertyEntry;
 import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry;
 import static org.apache.gravitino.connector.PropertyEntry.stringPropertyEntry;
@@ -42,7 +43,8 @@ public class JdbcCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata
           JdbcConfig.USERNAME.getKey(),
           JdbcConfig.PASSWORD.getKey(),
           JdbcConfig.POOL_MIN_SIZE.getKey(),
-          JdbcConfig.POOL_MAX_SIZE.getKey());
+          JdbcConfig.POOL_MAX_SIZE.getKey(),
+          JdbcConfig.TEST_ON_BORROW.getKey());
 
   static {
     List<PropertyEntry<?>> propertyEntries =
@@ -100,6 +102,14 @@ public class JdbcCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata
                 false /* immutable */,
                 JdbcConfig.POOL_MAX_SIZE.getDefaultValue(),
                 true /* hidden */,
+                false /* reserved */),
+            booleanPropertyEntry(
+                JdbcConfig.TEST_ON_BORROW.getKey(),
+                JdbcConfig.TEST_ON_BORROW.getDoc(),
+                false /* required */,
+                false /* immutable */,
+                JdbcConfig.TEST_ON_BORROW.getDefaultValue(),
+                true /* hidden */,
                 false /* reserved */));
     PROPERTIES_METADATA =
         ImmutableMap.<String, PropertyEntry<?>>builder()
diff --git a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
index aab5760f993..39512ff9a6c 100644
--- a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
+++ b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
@@ -19,13 +19,20 @@
 package org.apache.gravitino.catalog.jdbc;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import java.sql.SQLException;
+import java.util.HashMap;
+import javax.sql.DataSource;
+import org.apache.commons.dbcp2.BasicDataSource;
 import org.apache.gravitino.Catalog;
 import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
 import org.apache.gravitino.catalog.jdbc.converter.SqliteColumnDefaultValueConverter;
 import org.apache.gravitino.catalog.jdbc.converter.SqliteExceptionConverter;
 import org.apache.gravitino.catalog.jdbc.converter.SqliteTypeConverter;
 import org.apache.gravitino.catalog.jdbc.operation.SqliteDatabaseOperations;
 import org.apache.gravitino.catalog.jdbc.operation.SqliteTableOperations;
+import org.apache.gravitino.catalog.jdbc.utils.DataSourceUtils;
 import org.apache.gravitino.exceptions.GravitinoRuntimeException;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -51,4 +58,20 @@ public void testTestConnection() {
                 "comment",
                 ImmutableMap.of()));
   }
+
+  @Test
+  public void testConfigTestOnBorrow() throws SQLException {
+    HashMap<String, String> properties = Maps.newHashMap();
+    properties.put(JdbcConfig.JDBC_DRIVER.getKey(), "org.sqlite.JDBC");
+    properties.put(JdbcConfig.JDBC_URL.getKey(), "jdbc:sqlite::memory:");
+    properties.put(JdbcConfig.USERNAME.getKey(), "test");
+    properties.put(JdbcConfig.PASSWORD.getKey(), "test");
+    properties.put(JdbcConfig.TEST_ON_BORROW.getKey(), "false");
+
+    DataSource dataSource =
+        Assertions.assertDoesNotThrow(() -> DataSourceUtils.createDataSource(properties));
+    Assertions.assertTrue(dataSource instanceof org.apache.commons.dbcp2.BasicDataSource);
+    Assertions.assertTrue(((BasicDataSource) dataSource).getTestOnBorrow() == false);
+    ((BasicDataSource) dataSource).close();
+  }
 }