Skip to content

Backport to branch(3.14) : Add JDBC database permission test (Part 2) #2942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2025
Merged

Backport to branch(3.14) : Add JDBC database permission test (Part 2) #2942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e9e026
Add JDBC database permission test (Part 2) (#2923)
KodaiD Aug 5, 2025
afe75ec
Remove Db2 handling
KodaiD Aug 5, 2025
02dd5be
Fix to specify mysql in JDBC URL when testing MariaDB
KodaiD Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions .github/workflows/permission-check.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,3 +619,93 @@ jobs:
with:
name: sqlserver_2022_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-mariadb-10-11:
name: MariaDB 10.11 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run MariaDB 10.11
run: |
docker run -e MYSQL_ROOT_PASSWORD=mysql -p 3306:3306 -d mariadb:10.11 --character-set-server=utf8mb4 --collation-server=utf8mb4_bin

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:mysql://localhost:3306 -Dscalardb.jdbc.username=root -Dscalardb.jdbc.password=mysql

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: mariadb_10.11_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-mariadb-11-4:
name: MariaDB 11.4 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run MariaDB 11.4
run: |
docker run -e MYSQL_ROOT_PASSWORD=mysql -p 3306:3306 -d mariadb:11.4 --character-set-server=utf8mb4 --collation-server=utf8mb4_bin

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:mysql://localhost:3306 -Dscalardb.jdbc.username=root -Dscalardb.jdbc.password=mysql

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: mariadb_11.4_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission

integration-test-permission-jdbc-yugabytedb-2:
name: YugabyteDB 2 Permission Integration Test
runs-on: ubuntu-latest

steps:
- name: Run YugabyteDB 2
run: |
docker run -p 5433:5433 -e YSQL_USER=yugabyte -e YSQL_PASSWORD=yugabyte -d yugabytedb/yugabyte:2.20.4.0-b50 bin/yugabyted start --background=false --master_flag="ysql_enable_db_catalog_version_mode=false" --tserver_flags="ysql_enable_db_catalog_version_mode=false"

- uses: actions/checkout@v4

- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
java-version: ${{ env.JAVA_VERSION }}
distribution: ${{ env.JAVA_VENDOR }}

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbcPermission' task
run: ./gradlew integrationTestJdbcPermission -Dscalardb.jdbc.url=jdbc:yugabytedb://localhost:5433/yugabyte?load-balance=any -Dscalardb.jdbc.username=yugabyte -Dscalardb.jdbc.password=yugabyte -Dscalar.db.jdbc.connection_pool.max_total=12 -Dscalar.db.jdbc.table_metadata.connection_pool.max_total=4 -Dscalar.db.jdbc.admin.connection_pool.max_total=4

- name: Upload Gradle test reports
if: always()
uses: actions/upload-artifact@v4
with:
name: yugabytedb_2_permission_integration_test_reports
path: core/build/reports/tests/integrationTestJdbcPermission
2 changes: 1 addition & 1 deletion ...gration-test/java/com/scalar/db/storage/cassandra/CassandraPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected Map<String, String> getCreationOptions() {
}

@Override
protected void waitForTableCreation() {
protected void waitForDdlCompletion() {
try {
AdminTestUtils utils = getAdminTestUtils(TEST_NAME);
int retryCount = 0;
Expand Down
44 changes: 43 additions & 1 deletion .../integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
package com.scalar.db.storage.jdbc;

import static com.scalar.db.storage.jdbc.JdbcPermissionTestUtils.DDL_WAIT_SECONDS;

import com.google.common.util.concurrent.Uninterruptibles;
import com.scalar.db.api.DistributedStorageAdminPermissionIntegrationTestBase;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.util.AdminTestUtils;
import com.scalar.db.util.PermissionTestUtils;
import java.util.Properties;
import java.util.concurrent.TimeUnit;

public class JdbcAdminPermissionIntegrationTest
extends DistributedStorageAdminPermissionIntegrationTestBase {
private RdbEngineStrategy rdbEngine;

@Override
protected Properties getProperties(String testName) {
return JdbcEnv.getProperties(testName);
Properties properties = JdbcEnv.getProperties(testName);
rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
return properties;
}
Comment on lines +15 to 22
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is significant code duplication between this class (JdbcAdminPermissionIntegrationTest) and JdbcPermissionIntegrationTest. Both classes introduce an rdbEngine field, initialize it within getProperties, and implement identical logic to wait for DDL operations on YugabyteDB.

To improve maintainability and reduce redundancy, consider creating a common abstract base class for JDBC permission tests that would contain this shared logic.

For example, an abstract base class could manage the rdbEngine and provide a shared waitForDdlCompletionForYugabyte() method. The concrete test classes would then extend this base class and use the shared functionality.


@Override
Expand All @@ -26,4 +35,37 @@ protected AdminTestUtils getAdminTestUtils(String testName) {
protected PermissionTestUtils getPermissionTestUtils(String testName) {
return new JdbcPermissionTestUtils(getProperties(testName));
}

@Override
protected void waitForTableCreation() {
waitForDdlCompletion();
}

@Override
protected void waitForNamespaceCreation() {
waitForDdlCompletion();
}

@Override
protected void waitForTableDeletion() {
waitForDdlCompletion();
}

@Override
protected void waitForNamespaceDeletion() {
waitForDdlCompletion();
}

@Override
protected void sleepBetweenTests() {
// Sleep to ensure the DDL operations executed as ACT are completed before the next setup.
waitForDdlCompletion();
}

private void waitForDdlCompletion() {
if (JdbcTestUtils.isYugabyte(rdbEngine)) {
// This is needed to avoid schema or catalog version mismatch database errors.
Uninterruptibles.sleepUninterruptibly(DDL_WAIT_SECONDS, TimeUnit.SECONDS);
}
}
}
19 changes: 18 additions & 1 deletion core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
package com.scalar.db.storage.jdbc;

import static com.scalar.db.storage.jdbc.JdbcPermissionTestUtils.DDL_WAIT_SECONDS;

import com.google.common.util.concurrent.Uninterruptibles;
import com.scalar.db.api.DistributedStoragePermissionIntegrationTestBase;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.util.AdminTestUtils;
import com.scalar.db.util.PermissionTestUtils;
import java.util.Properties;
import java.util.concurrent.TimeUnit;

public class JdbcPermissionIntegrationTest extends DistributedStoragePermissionIntegrationTestBase {
private RdbEngineStrategy rdbEngine;

@Override
protected Properties getProperties(String testName) {
return JdbcEnv.getProperties(testName);
Properties properties = JdbcEnv.getProperties(testName);
rdbEngine = RdbEngineFactory.create(new JdbcConfig(new DatabaseConfig(properties)));
return properties;
}

@Override
Expand All @@ -25,4 +34,12 @@ protected PermissionTestUtils getPermissionTestUtils(String testName) {
protected AdminTestUtils getAdminTestUtils(String testName) {
return new JdbcAdminTestUtils(getProperties(testName));
}

@Override
protected void waitForDdlCompletion() {
if (JdbcTestUtils.isYugabyte(rdbEngine)) {
// This is needed to avoid schema or catalog version mismatch database errors.
Uninterruptibles.sleepUninterruptibly(DDL_WAIT_SECONDS, TimeUnit.SECONDS);
}
}
}
1 change: 1 addition & 0 deletions core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.apache.commons.dbcp2.BasicDataSource;

public class JdbcPermissionTestUtils implements PermissionTestUtils {
public static final int DDL_WAIT_SECONDS = 1;
private final RdbEngineStrategy rdbEngine;
private final BasicDataSource dataSource;

Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/com/scalar/db/api/DistributedStorageAdminPermissionIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public abstract class DistributedStorageAdminPermissionIntegrationTestBase {
TableMetadata.newBuilder()
.addColumn(COL_NAME1, DataType.INT)
.addColumn(COL_NAME2, DataType.TEXT)
.addColumn(COL_NAME3, DataType.TEXT)
.addColumn(COL_NAME3, DataType.INT)
.addColumn(COL_NAME4, DataType.INT)
.addPartitionKey(COL_NAME1)
.addClusteringKey(COL_NAME2, Scan.Ordering.Order.ASC)
Expand Down
5 changes: 3 additions & 2 deletions ...test/src/main/java/com/scalar/db/api/DistributedStoragePermissionIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void beforeAll() throws Exception {

namespace = getNamespace();
createTable();
waitForTableCreation();
waitForDdlCompletion();
} catch (Exception e) {
logger.error("Failed to set up the test environment", e);
throw e;
Expand All @@ -78,6 +78,7 @@ public void beforeAll() throws Exception {
@BeforeEach
public void setUp() throws Exception {
truncateTable();
waitForDdlCompletion();
}

@AfterAll
Expand Down Expand Up @@ -296,7 +297,7 @@ protected Map<String, String> getCreationOptions() {
return Collections.emptyMap();
}

protected void waitForTableCreation() {
protected void waitForDdlCompletion() {
// Default do nothing
}

Expand Down
Loading