This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-b748d734-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 3a54b3453cefad534e69c70e08ffef52d1e2be35 Author: Qi Yu <[email protected]> AuthorDate: Wed Oct 21 17:36:05 2026 +0800 [#14050] fix(jdbc): report non-empty schema drops consistently (#14051) ### What changes were proposed in this pull request? Throw `NonEmptySchemaException` when a non-cascading schema drop finds tables in JDBC common, Doris, StarRocks and ClickHouse. MySQL and OceanBase inherit the shared JDBC fix. Keep the existing SQL and cascade behavior. ### Why are the changes needed? The schema drop contract and REST error handler expect `NonEmptySchemaException`. Throwing generic state/argument exceptions currently produces HTTP 500 or 400 for a normal non-empty-schema rejection. Fix: #14050 ### Does this PR introduce _any_ user-facing change? Affected catalog clients receive `NonEmptySchemaException` and REST clients receive HTTP 409 when dropping a non-empty schema without cascade, instead of generic errors. No API signatures or configuration changes. ### How was this patch tested? - Four pre-fix non-empty-drop regression cases failed with the incorrect exception types. - After the fix: all 438 non-Docker tests in JDBC common, Doris, StarRocks and ClickHouse passed, plus all 11 `TestSchemaOperations` REST tests (449 total). - New coverage checks rejection before DROP, successful empty-schema drops, cascade behavior, JDBC resource closure, and the HTTP 409 response/error type. - Module Spotless checks and `git diff --check` passed. - Live database/Docker integration tests were not run locally. --- .../operations/ClickHouseDatabaseOperations.java | 7 +- .../operations/TestClickHouseDatabaseDrop.java | 78 ++++++++++++++++++++++ catalogs/catalog-jdbc-common/build.gradle.kts | 1 + .../jdbc/operation/JdbcDatabaseOperations.java | 8 +-- .../jdbc/operation/TestJdbcDatabaseDrop.java | 70 +++++++++++++++++++ .../doris/operation/DorisDatabaseOperations.java | 6 +- .../doris/operation/TestDorisDatabaseDrop.java | 69 +++++++++++++++++++ catalogs/catalog-jdbc-starrocks/build.gradle.kts | 1 + .../operations/StarRocksDatabaseOperations.java | 6 +- .../operations/TestStarRocksDatabaseDrop.java | 69 +++++++++++++++++++ .../server/web/rest/TestSchemaOperations.java | 21 ++++++ 11 files changed, 322 insertions(+), 14 deletions(-) diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java index e35946364d..c59924d0c6 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java @@ -43,6 +43,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcSchema; import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations; import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils; import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NonEmptySchemaException; import org.apache.gravitino.meta.AuditInfo; public class ClickHouseDatabaseOperations extends JdbcDatabaseOperations { @@ -196,10 +197,8 @@ public class ClickHouseDatabaseOperations extends JdbcDatabaseOperations { checkStmt.setString(1, databaseName); try (ResultSet rs = checkStmt.executeQuery()) { if (rs.next()) { - throw new IllegalArgumentException( - String.format( - "Database %s is not empty, you can drop it with CASCADE option.", - databaseName)); + throw new NonEmptySchemaException( + "Database %s is not empty, you can drop it with CASCADE option.", databaseName); } } } diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseDrop.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseDrop.java new file mode 100644 index 0000000000..a37b94882a --- /dev/null +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseDrop.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.operations; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.Collections; +import javax.sql.DataSource; +import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; +import org.apache.gravitino.exceptions.NonEmptySchemaException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; + +class TestClickHouseDatabaseDrop { + + @ParameterizedTest + @CsvSource({"true, false", "false, false", "true, true"}) + void testDropRespectsNonEmptySchemaContract(boolean nonEmpty, boolean cascade) throws Exception { + DataSource dataSource = Mockito.mock(DataSource.class); + Connection connection = Mockito.mock(Connection.class); + PreparedStatement check = Mockito.mock(PreparedStatement.class); + PreparedStatement comment = Mockito.mock(PreparedStatement.class); + Statement statement = Mockito.mock(Statement.class); + ResultSet tables = Mockito.mock(ResultSet.class); + ResultSet comments = Mockito.mock(ResultSet.class); + String checkSql = "SELECT 1 FROM system.tables WHERE database = ? LIMIT 1"; + String commentSql = "SELECT comment FROM system.databases WHERE name = ?"; + Mockito.when(dataSource.getConnection()).thenReturn(connection); + Mockito.when(connection.prepareStatement(checkSql)).thenReturn(check); + Mockito.when(connection.prepareStatement(commentSql)).thenReturn(comment); + Mockito.when(connection.createStatement()).thenReturn(statement); + Mockito.when(check.executeQuery()).thenReturn(tables); + Mockito.when(comment.executeQuery()).thenReturn(comments); + Mockito.when(tables.next()).thenReturn(nonEmpty); + ClickHouseDatabaseOperations operations = new ClickHouseDatabaseOperations(); + operations.initialize(dataSource, new JdbcExceptionConverter(), Collections.emptyMap()); + + if (nonEmpty && !cascade) { + NonEmptySchemaException failure = + Assertions.assertThrowsExactly( + NonEmptySchemaException.class, () -> operations.delete("nonempty_schema", false)); + Assertions.assertTrue(failure.getMessage().contains("nonempty_schema")); + Mockito.verify(statement, Mockito.never()).executeUpdate(Mockito.anyString()); + Mockito.verify(connection, Mockito.never()).prepareStatement(commentSql); + } else { + Assertions.assertTrue(operations.delete("nonempty_schema", cascade)); + Mockito.verify(statement).executeUpdate("DROP DATABASE `nonempty_schema`"); + } + if (cascade) { + Mockito.verify(connection, Mockito.never()).prepareStatement(checkSql); + } else { + Mockito.verify(check).setString(1, "nonempty_schema"); + Mockito.verify(tables).close(); + Mockito.verify(check).close(); + } + Mockito.verify(connection).close(); + } +} diff --git a/catalogs/catalog-jdbc-common/build.gradle.kts b/catalogs/catalog-jdbc-common/build.gradle.kts index ce772b8afb..b29ae7786f 100644 --- a/catalogs/catalog-jdbc-common/build.gradle.kts +++ b/catalogs/catalog-jdbc-common/build.gradle.kts @@ -65,6 +65,7 @@ dependencies { testImplementation(libs.commons.io) testImplementation(libs.junit.jupiter.api) testImplementation(libs.junit.jupiter.params) + testImplementation(libs.mockito.core) testImplementation(libs.sqlite.jdbc) testImplementation(libs.testcontainers) testImplementation(libs.testcontainers.mysql) diff --git a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcDatabaseOperations.java b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcDatabaseOperations.java index 68f9d97bae..a6d2e9ce18 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcDatabaseOperations.java +++ b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcDatabaseOperations.java @@ -38,6 +38,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcSchema; import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils; import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NonEmptySchemaException; import org.apache.gravitino.exceptions.SchemaAlreadyExistsException; import org.apache.gravitino.meta.AuditInfo; import org.slf4j.Logger; @@ -164,6 +165,7 @@ public abstract class JdbcDatabaseOperations implements DatabaseOperation { * @param databaseName The name of the database. * @param cascade cascade If set to true, drops all the tables in the schema as well. * @return the SQL statement to drop a database with the given name. + * @throws NonEmptySchemaException if the database contains tables and cascade is false */ protected String generateDropDatabaseSql(String databaseName, boolean cascade) { final String dropDatabaseSql = String.format("DROP DATABASE `%s`", databaseName); @@ -177,10 +179,8 @@ public abstract class JdbcDatabaseOperations implements DatabaseOperation { // Execute the query and check if there exists any tables in the database try (ResultSet resultSet = statement.executeQuery(query)) { if (resultSet.next()) { - throw new IllegalStateException( - String.format( - "Database %s is not empty, the value of cascade should be true.", - databaseName)); + throw new NonEmptySchemaException( + "Database %s is not empty, the value of cascade should be true.", databaseName); } } } diff --git a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcDatabaseDrop.java b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcDatabaseDrop.java new file mode 100644 index 0000000000..98032f742b --- /dev/null +++ b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcDatabaseDrop.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.jdbc.operation; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.Collections; +import javax.sql.DataSource; +import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; +import org.apache.gravitino.exceptions.NonEmptySchemaException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; + +class TestJdbcDatabaseDrop { + + @ParameterizedTest + @CsvSource({"true, false", "false, false", "true, true"}) + void testDropRespectsNonEmptySchemaContract(boolean nonEmpty, boolean cascade) throws Exception { + DataSource dataSource = Mockito.mock(DataSource.class); + Connection connection = Mockito.mock(Connection.class); + Statement statement = Mockito.mock(Statement.class); + ResultSet tables = Mockito.mock(ResultSet.class); + Mockito.when(dataSource.getConnection()).thenReturn(connection); + Mockito.when(connection.createStatement()).thenReturn(statement); + Mockito.when(statement.executeQuery("SHOW TABLES IN `nonempty_schema`")).thenReturn(tables); + Mockito.when(tables.next()).thenReturn(nonEmpty); + JdbcDatabaseOperations operations = + Mockito.mock(JdbcDatabaseOperations.class, Mockito.CALLS_REAL_METHODS); + operations.initialize(dataSource, new JdbcExceptionConverter(), Collections.emptyMap()); + + if (nonEmpty && !cascade) { + NonEmptySchemaException failure = + Assertions.assertThrowsExactly( + NonEmptySchemaException.class, () -> operations.delete("nonempty_schema", false)); + Assertions.assertTrue(failure.getMessage().contains("nonempty_schema")); + Mockito.verify(statement, Mockito.never()).executeUpdate(Mockito.anyString()); + } else { + Assertions.assertTrue(operations.delete("nonempty_schema", cascade)); + Mockito.verify(statement) + .executeUpdate("DROP DATABASE `nonempty_schema`" + (cascade ? "" : "")); + } + if (cascade) { + Mockito.verify(statement, Mockito.never()).executeQuery(Mockito.anyString()); + } else { + Mockito.verify(tables).close(); + } + int connections = !nonEmpty && !cascade ? 2 : 1; + Mockito.verify(statement, Mockito.times(connections)).close(); + Mockito.verify(connection, Mockito.times(connections)).close(); + } +} diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java index 1f0a37c153..1a6fbd2b10 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java @@ -34,6 +34,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcSchema; import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations; import org.apache.gravitino.exceptions.NoSuchSchemaException; import org.apache.gravitino.exceptions.NoSuchTableException; +import org.apache.gravitino.exceptions.NonEmptySchemaException; import org.apache.gravitino.meta.AuditInfo; /** Database operations for Apache Doris. */ @@ -76,9 +77,8 @@ public class DorisDatabaseOperations extends JdbcDatabaseOperations { Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery(query)) { if (resultSet.next()) { - throw new IllegalStateException( - String.format( - "Database %s is not empty, the value of cascade should be true.", databaseName)); + throw new NonEmptySchemaException( + "Database %s is not empty, the value of cascade should be true.", databaseName); } } catch (SQLException sqlException) { throw this.exceptionMapper.toGravitinoException(sqlException); diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseDrop.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseDrop.java new file mode 100644 index 0000000000..07f3f0c29e --- /dev/null +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseDrop.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.doris.operation; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.Collections; +import javax.sql.DataSource; +import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; +import org.apache.gravitino.exceptions.NonEmptySchemaException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; + +class TestDorisDatabaseDrop { + + @ParameterizedTest + @CsvSource({"true, false", "false, false", "true, true"}) + void testDropRespectsNonEmptySchemaContract(boolean nonEmpty, boolean cascade) throws Exception { + DataSource dataSource = Mockito.mock(DataSource.class); + Connection connection = Mockito.mock(Connection.class); + Statement statement = Mockito.mock(Statement.class); + ResultSet tables = Mockito.mock(ResultSet.class); + Mockito.when(dataSource.getConnection()).thenReturn(connection); + Mockito.when(connection.createStatement()).thenReturn(statement); + Mockito.when(statement.executeQuery("SHOW TABLES IN `nonempty_schema`")).thenReturn(tables); + Mockito.when(tables.next()).thenReturn(nonEmpty); + DorisDatabaseOperations operations = new DorisDatabaseOperations(); + operations.initialize(dataSource, new JdbcExceptionConverter(), Collections.emptyMap()); + + if (nonEmpty && !cascade) { + NonEmptySchemaException failure = + Assertions.assertThrowsExactly( + NonEmptySchemaException.class, () -> operations.delete("nonempty_schema", false)); + Assertions.assertTrue(failure.getMessage().contains("nonempty_schema")); + Mockito.verify(statement, Mockito.never()).executeUpdate(Mockito.anyString()); + } else { + Assertions.assertTrue(operations.delete("nonempty_schema", cascade)); + Mockito.verify(statement) + .executeUpdate("DROP DATABASE `nonempty_schema`" + (cascade ? " FORCE" : "")); + } + if (cascade) { + Mockito.verify(statement, Mockito.never()).executeQuery(Mockito.anyString()); + } else { + Mockito.verify(tables).close(); + } + int connections = !nonEmpty && !cascade ? 2 : 1; + Mockito.verify(statement, Mockito.times(connections)).close(); + Mockito.verify(connection, Mockito.times(connections)).close(); + } +} diff --git a/catalogs/catalog-jdbc-starrocks/build.gradle.kts b/catalogs/catalog-jdbc-starrocks/build.gradle.kts index 03b7d260bb..60a45284da 100644 --- a/catalogs/catalog-jdbc-starrocks/build.gradle.kts +++ b/catalogs/catalog-jdbc-starrocks/build.gradle.kts @@ -50,6 +50,7 @@ dependencies { testImplementation(libs.awaitility) testImplementation(libs.junit.jupiter.api) testImplementation(libs.junit.jupiter.params) + testImplementation(libs.mockito.core) testImplementation(libs.mysql.driver) testImplementation(libs.postgresql.driver) testImplementation(libs.testcontainers) diff --git a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksDatabaseOperations.java b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksDatabaseOperations.java index 4e7eda3b63..14cfd10859 100644 --- a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksDatabaseOperations.java +++ b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksDatabaseOperations.java @@ -31,6 +31,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcSchema; import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations; import org.apache.gravitino.catalog.starrocks.utils.StarRocksUtils; import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NonEmptySchemaException; import org.apache.gravitino.meta.AuditInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -68,9 +69,8 @@ public class StarRocksDatabaseOperations extends JdbcDatabaseOperations { Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery(query)) { if (resultSet.next()) { - throw new IllegalStateException( - String.format( - "Database %s is not empty, the value of cascade should be true.", databaseName)); + throw new NonEmptySchemaException( + "Database %s is not empty, the value of cascade should be true.", databaseName); } } catch (SQLException sqlException) { throw this.exceptionMapper.toGravitinoException(sqlException); diff --git a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operations/TestStarRocksDatabaseDrop.java b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operations/TestStarRocksDatabaseDrop.java new file mode 100644 index 0000000000..a1ce629ba7 --- /dev/null +++ b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operations/TestStarRocksDatabaseDrop.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.starrocks.operations; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.Collections; +import javax.sql.DataSource; +import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; +import org.apache.gravitino.exceptions.NonEmptySchemaException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mockito; + +class TestStarRocksDatabaseDrop { + + @ParameterizedTest + @CsvSource({"true, false", "false, false", "true, true"}) + void testDropRespectsNonEmptySchemaContract(boolean nonEmpty, boolean cascade) throws Exception { + DataSource dataSource = Mockito.mock(DataSource.class); + Connection connection = Mockito.mock(Connection.class); + Statement statement = Mockito.mock(Statement.class); + ResultSet tables = Mockito.mock(ResultSet.class); + Mockito.when(dataSource.getConnection()).thenReturn(connection); + Mockito.when(connection.createStatement()).thenReturn(statement); + Mockito.when(statement.executeQuery("SHOW TABLES IN `nonempty_schema`")).thenReturn(tables); + Mockito.when(tables.next()).thenReturn(nonEmpty); + StarRocksDatabaseOperations operations = new StarRocksDatabaseOperations(); + operations.initialize(dataSource, new JdbcExceptionConverter(), Collections.emptyMap()); + + if (nonEmpty && !cascade) { + NonEmptySchemaException failure = + Assertions.assertThrowsExactly( + NonEmptySchemaException.class, () -> operations.delete("nonempty_schema", false)); + Assertions.assertTrue(failure.getMessage().contains("nonempty_schema")); + Mockito.verify(statement, Mockito.never()).executeUpdate(Mockito.anyString()); + } else { + Assertions.assertTrue(operations.delete("nonempty_schema", cascade)); + Mockito.verify(statement) + .executeUpdate("DROP DATABASE `nonempty_schema`" + (cascade ? " FORCE" : "")); + } + if (cascade) { + Mockito.verify(statement, Mockito.never()).executeQuery(Mockito.anyString()); + } else { + Mockito.verify(tables).close(); + } + int connections = !nonEmpty && !cascade ? 2 : 1; + Mockito.verify(statement, Mockito.times(connections)).close(); + Mockito.verify(connection, Mockito.times(connections)).close(); + } +} diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java index 94469ba1c1..1cddd755ef 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java @@ -455,6 +455,27 @@ public class TestSchemaOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp3.getType()); } + @Test + void testDropNonEmptySchemaWithoutCascadeReturnsConflict() { + NameIdentifier identifier = NameIdentifier.of(metalake, catalog, "schema1"); + doThrow(new NonEmptySchemaException("Schema schema1 is not empty")) + .when(dispatcher) + .dropSchema(identifier, false); + + try (Response response = + target("/metalakes/" + metalake + "/catalogs/" + catalog + "/schemas/schema1") + .queryParam("cascade", false) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .delete()) { + Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(), response.getStatus()); + ErrorResponse error = response.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.NON_EMPTY_CODE, error.getCode()); + Assertions.assertEquals(NonEmptySchemaException.class.getSimpleName(), error.getType()); + verify(dispatcher).dropSchema(identifier, false); + } + } + @Test public void testDropSchema() { when(dispatcher.dropSchema(any(), eq(false))).thenReturn(true);
