Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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;
}

@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);
}
}
Comment on lines +65 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic within waitForDdlCompletion is identical to the implementation in JdbcPermissionIntegrationTest.java. To improve maintainability and avoid code duplication, consider extracting this logic into a shared utility method. A new static method in an existing utility class like JdbcPermissionTestUtils would be a suitable place for it.

}
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);
}
}
Comment on lines +39 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This waitForDdlCompletion method is identical to the one in JdbcAdminPermissionIntegrationTest.java. To avoid code duplication and improve maintainability, consider extracting this logic into a common utility method. For example, a new static method in JdbcPermissionTestUtils could hold the shared logic, which can then be called from both test classes.

}
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
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The column COL_NAME3 is being changed from TEXT to INT. Ensure that this change is intentional and that it does not break any existing functionality or data compatibility.

.addColumn(COL_NAME4, DataType.INT)
.addPartitionKey(COL_NAME1)
.addClusteringKey(COL_NAME2, Scan.Ordering.Order.ASC)
Expand Down
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