This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new db936dbb1c [Cherry-pick to branch-1.3] [#14578] fix(rest): validate
malformed requests and translate Iceberg errors (#14584) (#14679)
db936dbb1c is described below
commit db936dbb1c51a70d9f8a26b4f74a44fc3d957fce
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Nov 6 15:22:13 2026 +0800
[Cherry-pick to branch-1.3] [#14578] fix(rest): validate malformed requests
and translate Iceberg errors (#14584) (#14679)
### What changes were proposed in this pull request?
Backport d813da6953af8103688eff88802c09493366396e (#14584) to
`branch-1.3`, preserving malformed-request validation and Iceberg error
translation.
Resolved the cherry-pick conflicts by attaching schema comment
validation to the existing 1.3 `createSchema` interface, retaining both
parameterized REST test imports, and keeping the Iceberg validation
exception import used on this branch. The main-branch-only schema secret
overload and its test invocation are omitted. Test fixtures use the 1.3
table dispatcher, existing tag association API, and real Iceberg catalog
initialization instead of main-only types.
### Why are the changes needed?
The automatic cherry-pick committed conflict markers and referenced an
interface unavailable in 1.3. This resolution preserves the fix while
adapting it to the target branch.
### Does this PR introduce _any_ user-facing change?
Same as #14584: reject malformed requests and unsupported input before
mutation, preserve backend failures from supported table catalogs, and
translate Iceberg validation failures to client errors.
### How was this patch tested?
96 tests passed across 8 selected classes with `-PskipITs`: common (4),
core (9), server (63), iceberg-common (9), and catalog-lakehouse-iceberg
(11). Core execution was limited to `TestSchemaNormalizeDispatcher` and
`TestTableExistence`; no full core suite ran. Scoped Spotless checks and
`git diff --check` passed; conflict-marker scanning found no remaining
markers in Java sources.
---------
Co-authored-by: roryqi <[email protected]>
Co-authored-by: yuqi <[email protected]>
---
.../iceberg/IcebergCatalogOperations.java | 11 +
.../iceberg/TestIcebergTableValidation.java | 221 +++++++++++++++++++++
.../dto/requests/PrivilegeOverrideRequest.java | 2 +
.../dto/requests/TestPrivilegeOverrideRequest.java | 46 +++++
.../catalog/SchemaNormalizeDispatcher.java | 11 +
.../catalog/TableOperationDispatcher.java | 17 ++
.../catalog/TestSchemaNormalizeDispatcher.java | 32 +++
.../gravitino/catalog/TestTableExistence.java | 134 +++++++++++++
.../iceberg/common/ops/IcebergCatalogWrapper.java | 25 +++
.../common/ops/TestIcebergNamespaceProperties.java | 182 +++++++++++++++++
.../web/filter/GravitinoInterceptionService.java | 2 +-
.../gravitino/server/web/filter/ParameterUtil.java | 20 +-
.../filter/TestGravitinoInterceptionService.java | 35 ++++
.../server/web/rest/TestPermissionOperations.java | 18 ++
.../server/web/rest/TestSchemaOperations.java | 19 ++
15 files changed, 767 insertions(+), 8 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
index 732c7f9bad..708edb4d8c 100644
---
a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
+++
b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
@@ -82,6 +82,7 @@ import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.NamespaceNotEmptyException;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.rest.requests.RenameTableRequest;
import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
import org.apache.iceberg.rest.responses.GetNamespaceResponse;
@@ -472,6 +473,8 @@ public class IcebergCatalogOperations
LoadTableResponse loadTableResponse =
icebergCatalogWrapper.updateTable(icebergTableChange);
loadTableResponse.validate();
return IcebergTable.fromIcebergTable(loadTableResponse.tableMetadata(),
tableIdent.name());
+ } catch (ValidationException e) {
+ throw new IllegalArgumentException(e.getMessage(), e);
} catch (org.apache.iceberg.exceptions.NoSuchTableException e) {
throw new NoSuchTableException(e, ICEBERG_TABLE_DOES_NOT_EXIST_MSG,
tableIdent.name());
}
@@ -565,6 +568,12 @@ public class IcebergCatalogOperations
Index[] indexes)
throws NoSuchSchemaException, TableAlreadyExistsException {
Preconditions.checkArgument(indexes.length == 0, "Iceberg-catalog does not
support indexes");
+ for (Column column : columns) {
+ Preconditions.checkArgument(
+ !column.autoIncrement(),
+ "Iceberg does not support auto-increment column: %s",
+ column.name());
+ }
try {
NameIdentifier schemaIdent =
NameIdentifier.of(tableIdent.namespace().levels());
if (!schemaExists(schemaIdent)) {
@@ -616,6 +625,8 @@ public class IcebergCatalogOperations
return createdTable;
} catch (AlreadyExistsException e) {
throw new TableAlreadyExistsException(e, "Table already exists: %s",
tableIdent.name());
+ } catch (ValidationException e) {
+ throw new IllegalArgumentException(e.getMessage(), e);
}
}
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTableValidation.java
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTableValidation.java
new file mode 100644
index 0000000000..282b3a32b2
--- /dev/null
+++
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTableValidation.java
@@ -0,0 +1,221 @@
+/*
+ * 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.lakehouse.iceberg;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.nio.file.Path;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Map;
+import java.util.stream.Stream;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.connector.CatalogInfo;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.sorts.SortOrder;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.rel.expressions.transforms.Transforms;
+import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.server.web.rest.ExceptionHandlers;
+import org.apache.gravitino.server.web.rest.OperationType;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class TestIcebergTableValidation {
+
+ @TempDir Path warehouse;
+
+ private IcebergCatalogOperations operations;
+
+ @BeforeEach
+ void setUp() {
+ CatalogInfo info =
+ new CatalogInfo(
+ 1L,
+ "cat",
+ Catalog.Type.RELATIONAL,
+ "iceberg",
+ "",
+ Map.of(),
+
AuditInfo.builder().withCreator("test").withCreateTime(Instant.EPOCH).build(),
+ Namespace.of("ml"));
+ operations = new IcebergCatalogOperations();
+ operations.initialize(
+ Map.of("catalog-backend", "memory", "warehouse", warehouse.toString()),
+ info,
+ TestIcebergCatalog.ICEBERG_PROPERTIES_METADATA);
+ operations.createSchema(NameIdentifier.of("ml", "cat", "s"), "", Map.of());
+ }
+
+ @AfterEach
+ void tearDown() {
+ operations.close();
+ operations.onCatalogDropped();
+ }
+
+ @Test
+ void testDuplicateColumnsOnCreateAreBadRequest() {
+ IllegalArgumentException error =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ create(
+ "duplicates",
+ new Column[] {
+ Column.of("id", Types.IntegerType.get()),
+ Column.of("id", Types.StringType.get())
+ }));
+ assertTrue(error.getMessage().contains("multiple fields for name id"));
+ assertEquals(400, tableError(OperationType.CREATE, error));
+ assertFalse(operations.tableExists(table("duplicates")));
+ }
+
+ @Test
+ void testRenameColumnConflictLeavesSchemaUnchanged() {
+ createOrdinaryTable("rename_column");
+ Exception error =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ operations.alterTable(
+ table("rename_column"), TableChange.renameColumn(new
String[] {"name"}, "id")));
+ assertEquals(400, tableError(OperationType.ALTER, error));
+ assertOrdinaryColumns("rename_column");
+ }
+
+ @Test
+ void testDuplicateBatchAddLeavesSchemaUnchanged() {
+ createOrdinaryTable("batch_add");
+ Exception error =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ operations.alterTable(
+ table("batch_add"),
+ TableChange.addColumn(new String[] {"batch_a"},
Types.StringType.get()),
+ TableChange.addColumn(new String[] {"batch_a"},
Types.IntegerType.get())));
+ assertEquals(400, tableError(OperationType.ALTER, error));
+ assertOrdinaryColumns("batch_add");
+ }
+
+ @Test
+ void testAutoIncrementIsRejectedBeforeCreate() {
+ Column column = Column.of("id", Types.LongType.get(), null, false, true,
null);
+ Exception error =
+ assertThrows(
+ IllegalArgumentException.class, () -> create("auto_increment", new
Column[] {column}));
+ assertTrue(error.getMessage().contains("auto-increment"));
+ assertEquals(400, tableError(OperationType.CREATE, error));
+ assertFalse(operations.tableExists(table("auto_increment")));
+ }
+
+ @ParameterizedTest
+ @MethodSource("incompatiblePartitionTransforms")
+ void testIncompatiblePartitionTransformIsBadRequest(Type type, Transform
transform) {
+ Exception error =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> create("bad_partition", new Column[] {Column.of("value",
type)}, transform));
+ assertTrue(error.getMessage().contains("Invalid source type"));
+ assertEquals(400, tableError(OperationType.CREATE, error));
+ assertFalse(operations.tableExists(table("bad_partition")));
+ }
+
+ @Test
+ void testValidCreateAndColumnChangesStillSucceed() {
+ create(
+ "valid", new Column[] {Column.of("value", Types.DateType.get())},
Transforms.year("value"));
+ operations.alterTable(
+ table("valid"),
+ TableChange.renameColumn(new String[] {"value"}, "date"),
+ TableChange.addColumn(new String[] {"name"}, Types.StringType.get()));
+ assertArrayEquals(
+ new String[] {"date", "name"},
+ Arrays.stream(operations.loadTable(table("valid")).columns())
+ .map(Column::name)
+ .toArray(String[]::new));
+ }
+
+ private static Stream<Arguments> incompatiblePartitionTransforms() {
+ return Stream.of(
+ Arguments.of(Types.StringType.get(), Transforms.year("value")),
+ Arguments.of(Types.StringType.get(), Transforms.month("value")),
+ Arguments.of(Types.StringType.get(), Transforms.day("value")),
+ Arguments.of(Types.StringType.get(), Transforms.hour("value")),
+ Arguments.of(Types.IntegerType.get(), Transforms.day("value")),
+ Arguments.of(Types.TimestampType.withoutTimeZone(),
Transforms.truncate(3, "value")));
+ }
+
+ private Table create(String name, Column[] columns, Transform...
partitioning) {
+ return operations.createTable(
+ table(name),
+ columns,
+ "",
+ Map.of(),
+ partitioning,
+ Distributions.NONE,
+ new SortOrder[0],
+ new Index[0]);
+ }
+
+ private void createOrdinaryTable(String name) {
+ create(
+ name,
+ new Column[] {
+ Column.of("id", Types.LongType.get()), Column.of("name",
Types.StringType.get())
+ });
+ }
+
+ private void assertOrdinaryColumns(String name) {
+ assertArrayEquals(
+ new String[] {"id", "name"},
+ Arrays.stream(operations.loadTable(table(name)).columns())
+ .map(Column::name)
+ .toArray(String[]::new));
+ }
+
+ private static NameIdentifier table(String name) {
+ return NameIdentifier.of("ml", "cat", "s", name);
+ }
+
+ private static int tableError(OperationType operationType, Exception error) {
+ try (Response response =
+ ExceptionHandlers.handleTableException(operationType, "t", "s",
error)) {
+ return response.getStatus();
+ }
+ }
+}
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeOverrideRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeOverrideRequest.java
index 67aa4dbfc2..dce8cf2926 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeOverrideRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/PrivilegeOverrideRequest.java
@@ -61,7 +61,9 @@ public class PrivilegeOverrideRequest implements RESTRequest {
*/
@Override
public void validate() throws IllegalArgumentException {
+ Preconditions.checkArgument(overrides != null, "\"overrides\" field is
required");
for (SecurableObjectDTO objectDTO : overrides) {
+ Preconditions.checkArgument(objectDTO != null, "\"overrides\" cannot
contain null objects");
Preconditions.checkArgument(
StringUtils.isNotBlank(objectDTO.name()), "\"securable object name\"
cannot be blank");
Preconditions.checkArgument(
diff --git
a/common/src/test/java/org/apache/gravitino/dto/requests/TestPrivilegeOverrideRequest.java
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPrivilegeOverrideRequest.java
new file mode 100644
index 0000000000..41fef7e79a
--- /dev/null
+++
b/common/src/test/java/org/apache/gravitino/dto/requests/TestPrivilegeOverrideRequest.java
@@ -0,0 +1,46 @@
+/*
+ * 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.dto.requests;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import org.apache.gravitino.json.JsonUtils;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class TestPrivilegeOverrideRequest {
+
+ @ParameterizedTest
+ @ValueSource(strings = {"{}", "{\"overrides\":null}",
"{\"overrides\":[null]}"})
+ void testRejectsMissingOrNullOverrides(String json) throws
JsonProcessingException {
+ PrivilegeOverrideRequest request =
+ JsonUtils.objectMapper().readValue(json,
PrivilegeOverrideRequest.class);
+ IllegalArgumentException error =
+ Assertions.assertThrows(IllegalArgumentException.class,
request::validate);
+ Assertions.assertTrue(error.getMessage().contains("overrides"));
+ }
+
+ @Test
+ void testAcceptsEmptyOverrides() throws JsonProcessingException {
+ PrivilegeOverrideRequest request =
+ JsonUtils.objectMapper().readValue("{\"overrides\":[]}",
PrivilegeOverrideRequest.class);
+ Assertions.assertDoesNotThrow(request::validate);
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SchemaNormalizeDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/SchemaNormalizeDispatcher.java
index b4d55e6cb9..3ecaa6d951 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SchemaNormalizeDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SchemaNormalizeDispatcher.java
@@ -23,7 +23,10 @@ import static
org.apache.gravitino.catalog.CapabilityHelpers.applyCaseSensitive;
import static org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
import java.util.Map;
+import javax.annotation.Nullable;
import org.apache.commons.lang3.ArrayUtils;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.Schema;
@@ -73,6 +76,7 @@ public class SchemaNormalizeDispatcher implements
SchemaDispatcher {
@Override
public Schema createSchema(NameIdentifier ident, String comment, Map<String,
String> properties)
throws NoSuchCatalogException, SchemaAlreadyExistsException {
+ checkCommentLength(comment);
return dispatcher.createSchema(normalizeNameIdentifier(ident), comment,
properties);
}
@@ -86,6 +90,8 @@ public class SchemaNormalizeDispatcher implements
SchemaDispatcher {
@Override
public Schema alterSchema(NameIdentifier ident, SchemaChange... changes)
throws NoSuchSchemaException {
+ // SchemaChange supports property changes only; schema comments cannot be
updated.
+ // Comment length is therefore validated on createSchema, the only API
that accepts a comment.
// The constraints of the name spec may be more strict than underlying
catalog,
// and for compatibility reasons, we only apply case-sensitive
capabilities here.
return dispatcher.alterSchema(normalizeCaseSensitive(ident), changes);
@@ -101,6 +107,11 @@ public class SchemaNormalizeDispatcher implements
SchemaDispatcher {
return capabilities.supportsHierarchicalSchema().supported();
}
+ private static void checkCommentLength(@Nullable String comment) {
+ EntityFieldLimits.checkMaxLength(
+ comment, EntityFieldLimits.MAX_COMMENT_LENGTH, "comment",
Entity.EntityType.SCHEMA);
+ }
+
private NameIdentifier normalizeNameIdentifier(NameIdentifier schemaIdent) {
Capability capabilities = getCapability(schemaIdent, catalogManager);
return applyCapabilities(schemaIdent, Capability.Scope.SCHEMA,
capabilities);
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
index 42b5125d87..d3544e560d 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
@@ -67,6 +67,7 @@ import org.apache.gravitino.meta.ColumnEntity;
import org.apache.gravitino.meta.TableEntity;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
@@ -121,6 +122,22 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
schemaDispatcherSupplier, "schemaDispatcherSupplier must not be
null");
}
+ /**
+ * Checks whether a table exists, treating catalogs without table operations
as empty.
+ *
+ * @param ident the table identifier
+ * @return whether the table exists
+ */
+ @Override
+ public boolean tableExists(NameIdentifier ident) {
+ boolean supportsTables =
+ doWithCatalog(
+ getCatalogIdentifier(ident),
+ wrapper -> wrapper.doWithCatalog(catalog -> catalog.ops()
instanceof TableCatalog),
+ RuntimeException.class);
+ return supportsTables && TableDispatcher.super.tableExists(ident);
+ }
+
/**
* Lists the tables within a schema.
*
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaNormalizeDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaNormalizeDispatcher.java
index fb438402b3..e9e98422a2 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaNormalizeDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaNormalizeDispatcher.java
@@ -29,10 +29,42 @@ import org.apache.gravitino.SchemaChange;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
public class TestSchemaNormalizeDispatcher extends TestOperationDispatcher {
private static SchemaNormalizeDispatcher schemaNormalizeDispatcher;
+ @Test
+ void testOversizedCommentRejectedBeforeCatalogMutation() {
+ SchemaDispatcher delegate = Mockito.mock(SchemaDispatcher.class);
+ SchemaNormalizeDispatcher dispatcher = new
SchemaNormalizeDispatcher(delegate, catalogManager);
+ NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"oversized_comment");
+ String comment = "c".repeat(257);
+ IllegalArgumentException error =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> dispatcher.createSchema(ident, comment, ImmutableMap.of()));
+ Assertions.assertTrue(error.getMessage().contains("comment"));
+ Assertions.assertTrue(error.getMessage().contains("256"));
+ Mockito.verifyNoInteractions(delegate);
+ }
+
+ @Test
+ void testCommentLengthBoundary() {
+ NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"comment_boundary");
+ String comment = "c".repeat(256);
+ Schema schema =
+ schemaNormalizeDispatcher.createSchema(
+ ident, comment, ImmutableMap.of("k1", "v1", "k2", "v2"));
+ Assertions.assertEquals(comment, schema.comment());
+ Schema altered =
+ schemaNormalizeDispatcher.alterSchema(ident,
SchemaChange.setProperty("k1", "updated"));
+ Assertions.assertEquals("updated", altered.properties().get("k1"));
+ Assertions.assertEquals(comment, altered.comment());
+ Assertions.assertEquals(comment,
schemaNormalizeDispatcher.loadSchema(ident).comment());
+ Assertions.assertTrue(schemaNormalizeDispatcher.dropSchema(ident, false));
+ }
+
@BeforeAll
public static void initialize() throws IOException, IllegalAccessException {
TestSchemaOperationDispatcher.initialize();
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestTableExistence.java
b/core/src/test/java/org/apache/gravitino/catalog/TestTableExistence.java
new file mode 100644
index 0000000000..279c7f6998
--- /dev/null
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestTableExistence.java
@@ -0,0 +1,134 @@
+/*
+ * 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;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.connector.BaseCatalog;
+import org.apache.gravitino.connector.CatalogOperations;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
+import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.TableCatalog;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.utils.MetadataObjectUtil;
+import org.apache.gravitino.utils.ThrowableFunction;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+
+class TestTableExistence {
+
+ private static final NameIdentifier IDENT = NameIdentifier.of("ml", "cat",
"s", "t");
+
+ private BaseCatalog<?> catalog;
+ private CatalogManager manager;
+ private TableOperationDispatcher dispatcher;
+
+ @BeforeEach
+ @SuppressWarnings("unchecked")
+ void setUp() throws Exception {
+ manager = mock(CatalogManager.class);
+ catalog = mock(BaseCatalog.class);
+ when(catalog.capability()).thenReturn(new Capability() {});
+ CatalogTestUtils.mockDoWithCatalog(manager, catalog);
+ CatalogManager.CatalogWrapper wrapper =
+
CatalogTestUtils.mockDetachConnectorResult(mock(CatalogManager.CatalogWrapper.class));
+ doAnswer(
+ invocation -> {
+ ThrowableFunction<BaseCatalog, Object> operation =
invocation.getArgument(0);
+ return operation.apply(catalog);
+ })
+ .when(wrapper)
+ .doWithCatalog(any());
+ doAnswer(
+ invocation -> {
+ ThrowableFunction<CatalogManager.CatalogWrapper, Object>
operation =
+ invocation.getArgument(1);
+ return operation.apply(wrapper);
+ })
+ .when(manager)
+ .doWithCatalogWrapper(any(), any());
+ dispatcher =
+ spy(
+ new TableOperationDispatcher(
+ manager, mock(EntityStore.class), mock(IdGenerator.class)));
+ }
+
+ @Test
+ void testUnsupportedCatalogHasNoTables() {
+ when(catalog.ops()).thenReturn(mock(CatalogOperations.class));
+ Assertions.assertFalse(dispatcher.tableExists(IDENT));
+ verify(dispatcher, never()).loadTable(any());
+ }
+
+ @Test
+ void testSupportedCatalogStillLoadsAndImportsTables() {
+ when(catalog.ops()).thenReturn(mock(TableCatalogOperations.class));
+ doReturn(mock(Table.class)).when(dispatcher).loadTable(IDENT);
+ Assertions.assertTrue(dispatcher.tableExists(IDENT));
+ verify(dispatcher).loadTable(IDENT);
+
+ doThrow(new
NoSuchTableException("missing")).when(dispatcher).loadTable(IDENT);
+ Assertions.assertFalse(dispatcher.tableExists(IDENT));
+
+ doThrow(new UnsupportedOperationException("backend
failure")).when(dispatcher).loadTable(IDENT);
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () ->
dispatcher.tableExists(IDENT));
+ }
+
+ @Test
+ void testNormalizedMetadataExistenceCheckReturnsNotFound() {
+ when(catalog.ops()).thenReturn(mock(CatalogOperations.class));
+ TableNormalizeDispatcher normalized = new
TableNormalizeDispatcher(dispatcher, manager);
+ GravitinoEnv env = mock(GravitinoEnv.class);
+ when(env.internalTableDispatcher()).thenReturn(normalized);
+ try (MockedStatic<GravitinoEnv> mocked = mockStatic(GravitinoEnv.class)) {
+ mocked.when(GravitinoEnv::getInstance).thenReturn(env);
+ for (MetadataObject object :
+ new MetadataObject[] {
+ MetadataObjects.parse("cat.s.t", MetadataObject.Type.TABLE),
+ MetadataObjects.parse("cat.s.t.c", MetadataObject.Type.COLUMN)
+ }) {
+ Assertions.assertThrows(
+ NoSuchMetadataObjectException.class,
+ () -> MetadataObjectUtil.checkMetadataObject("ml", object));
+ }
+ }
+ verify(dispatcher, never()).loadTable(any());
+ }
+
+ private interface TableCatalogOperations extends CatalogOperations,
TableCatalog {}
+}
diff --git
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
index ca77a66053..9a39703212 100644
---
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
+++
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
@@ -27,6 +27,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import javax.annotation.Nullable;
import lombok.Getter;
import lombok.Setter;
import org.apache.commons.lang3.StringUtils;
@@ -64,6 +65,7 @@ import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.exceptions.NotFoundException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.ResolvingFileIO;
+import org.apache.iceberg.jdbc.JdbcCatalog;
import org.apache.iceberg.jdbc.JdbcCatalogWithMetadataLocationSupport;
import org.apache.iceberg.rest.CatalogHandlers;
import org.apache.iceberg.rest.RESTCatalog;
@@ -102,6 +104,9 @@ public class IcebergCatalogWrapper implements AutoCloseable
{
private static final int MAX_FORMAT_VERSION = 3;
+ // Iceberg JdbcCatalog declares iceberg_namespace_properties.property_value
as VARCHAR(1000).
+ private static final int MAX_JDBC_NAMESPACE_PROPERTY_VALUE_LENGTH = 1000;
+
private final Object initializationLock = new Object();
private volatile Catalog catalog;
private volatile SupportsNamespaces asNamespaceCatalog;
@@ -199,6 +204,7 @@ public class IcebergCatalogWrapper implements AutoCloseable
{
public CreateNamespaceResponse createNamespace(CreateNamespaceRequest
request) {
validateNamespace(Optional.of(request.namespace()));
+ validateNamespaceProperties(request.properties());
return CatalogHandlers.createNamespace(getNamespaceCatalog(), request);
}
@@ -225,6 +231,7 @@ public class IcebergCatalogWrapper implements AutoCloseable
{
public UpdateNamespacePropertiesResponse updateNamespaceProperties(
Namespace namespace, UpdateNamespacePropertiesRequest
updateNamespacePropertiesRequest) {
validateNamespace(Optional.of(namespace));
+ validateNamespaceProperties(updateNamespacePropertiesRequest.updates());
return CatalogHandlers.updateNamespaceProperties(
getNamespaceCatalog(), namespace, updateNamespacePropertiesRequest);
}
@@ -739,4 +746,22 @@ public class IcebergCatalogWrapper implements
AutoCloseable {
}
}
}
+
+ @VisibleForTesting
+ void validateNamespaceProperties(@Nullable Map<String, String> properties) {
+ if (properties == null) {
+ return;
+ }
+ if (getCatalog() instanceof JdbcCatalog) {
+ properties.forEach(
+ (key, value) ->
+ Preconditions.checkArgument(
+ value == null
+ || value.codePointCount(0, value.length())
+ <= MAX_JDBC_NAMESPACE_PROPERTY_VALUE_LENGTH,
+ "Namespace property '%s' exceeds the JDBC catalog limit of
%s characters",
+ key,
+ MAX_JDBC_NAMESPACE_PROPERTY_VALUE_LENGTH));
+ }
+ }
}
diff --git
a/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergNamespaceProperties.java
b/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergNamespaceProperties.java
new file mode 100644
index 0000000000..0fb854308b
--- /dev/null
+++
b/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergNamespaceProperties.java
@@ -0,0 +1,182 @@
+/*
+ * 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.iceberg.common.ops;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableMap;
+import java.nio.file.Path;
+import java.util.Map;
+import org.apache.gravitino.iceberg.common.IcebergConfig;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.rest.RESTSerializers;
+import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class TestIcebergNamespaceProperties {
+
+ private static final ObjectMapper MAPPER = createObjectMapper();
+
+ @TempDir Path warehouse;
+
+ @Test
+ void testNullNamespacePropertiesRequireNoValidation() throws Exception {
+ try (IcebergCatalogWrapper wrapper = jdbcWrapper()) {
+ Assertions.assertDoesNotThrow(() ->
wrapper.validateNamespaceProperties(null));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"", ",\"properties\":null"})
+ void testCreateNamespaceWithoutProperties(String propertiesJson) throws
Exception {
+ try (IcebergCatalogWrapper wrapper = jdbcWrapper()) {
+ CreateNamespaceRequest request =
+ MAPPER.readValue(
+ "{\"namespace\":[\"empty\"]" + propertiesJson + "}",
CreateNamespaceRequest.class);
+ wrapper.createNamespace(request);
+ Assertions.assertTrue(wrapper.namespaceExists(request.namespace()));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"", ",\"updates\":null"})
+ void testRemovalOnlyUpdateWithoutProperties(String updatesJson) throws
Exception {
+ try (IcebergCatalogWrapper wrapper = jdbcWrapper()) {
+ Namespace namespace = Namespace.of("removals");
+ wrapper.createNamespace(
+ CreateNamespaceRequest.builder()
+ .withNamespace(namespace)
+ .setProperties(ImmutableMap.of("remove", "value", "keep",
"original"))
+ .build());
+ UpdateNamespacePropertiesRequest request =
+ MAPPER.readValue(
+ "{\"removals\":[\"remove\"]" + updatesJson + "}",
+ UpdateNamespacePropertiesRequest.class);
+ wrapper.updateNamespaceProperties(namespace, request);
+ Map<String, String> properties =
wrapper.loadNamespace(namespace).properties();
+ Assertions.assertFalse(properties.containsKey("remove"));
+ Assertions.assertEquals("original", properties.get("keep"));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"x", "\uD83D\uDE80"})
+ void testJdbcNamespacePropertyLengthBoundary(String character) throws
Exception {
+ try (IcebergCatalogWrapper wrapper = jdbcWrapper()) {
+ Namespace valid = Namespace.of("valid");
+ String allowedValue = character.repeat(1000);
+ wrapper.createNamespace(
+ CreateNamespaceRequest.builder()
+ .withNamespace(valid)
+ .setProperties(ImmutableMap.of("big", allowedValue))
+ .build());
+ Assertions.assertEquals(allowedValue,
wrapper.loadNamespace(valid).properties().get("big"));
+
+ Namespace invalid = Namespace.of("invalid");
+ IllegalArgumentException error =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ wrapper.createNamespace(
+ CreateNamespaceRequest.builder()
+ .withNamespace(invalid)
+ .setProperties(ImmutableMap.of("big",
character.repeat(1001)))
+ .build()));
+ Assertions.assertTrue(error.getMessage().contains("big"));
+ Assertions.assertTrue(error.getMessage().contains("1000"));
+ Assertions.assertFalse(wrapper.namespaceExists(invalid));
+ }
+ }
+
+ @Test
+ void testInvalidUpdateDoesNotApplyRemovalsOrOtherUpdates() throws Exception {
+ try (IcebergCatalogWrapper wrapper = jdbcWrapper()) {
+ Namespace namespace = Namespace.of("updates");
+ Map<String, String> original = ImmutableMap.of("big", "original",
"keep", "value");
+ wrapper.createNamespace(
+ CreateNamespaceRequest.builder()
+ .withNamespace(namespace)
+ .setProperties(original)
+ .build());
+ Map<String, String> before =
+ ImmutableMap.copyOf(wrapper.loadNamespace(namespace).properties());
+ UpdateNamespacePropertiesRequest invalid =
+ UpdateNamespacePropertiesRequest.builder()
+ .remove("keep")
+ .update("new", "value")
+ .update("big", "x".repeat(1001))
+ .build();
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> wrapper.updateNamespaceProperties(namespace, invalid));
+ Assertions.assertEquals(before,
wrapper.loadNamespace(namespace).properties());
+
+ wrapper.updateNamespaceProperties(
+ namespace,
+ UpdateNamespacePropertiesRequest.builder().update("big",
"x".repeat(1000)).build());
+ Assertions.assertEquals(
+ 1000,
wrapper.loadNamespace(namespace).properties().get("big").length());
+ }
+ }
+
+ @Test
+ void testOtherBackendsRetainLargerProperties() throws Exception {
+ try (IcebergCatalogWrapper wrapper =
+ new IcebergCatalogWrapper(new IcebergConfig(Map.of("catalog-backend",
"memory")))) {
+ Namespace namespace = Namespace.of("large");
+ wrapper.createNamespace(
+ CreateNamespaceRequest.builder()
+ .withNamespace(namespace)
+ .setProperties(ImmutableMap.of("big", "x".repeat(1001)))
+ .build());
+ wrapper.updateNamespaceProperties(
+ namespace,
+ UpdateNamespacePropertiesRequest.builder().update("big",
"x".repeat(2000)).build());
+ Assertions.assertEquals(
+ 2000,
wrapper.loadNamespace(namespace).properties().get("big").length());
+ }
+ }
+
+ private static ObjectMapper createObjectMapper() {
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setVisibility(PropertyAccessor.FIELD,
JsonAutoDetect.Visibility.ANY);
+ RESTSerializers.registerAll(mapper);
+ return mapper;
+ }
+
+ private IcebergCatalogWrapper jdbcWrapper() {
+ return new IcebergCatalogWrapper(
+ new IcebergConfig(
+ Map.of(
+ "catalog-backend",
+ "jdbc",
+ "jdbc-driver",
+ "org.sqlite.JDBC",
+ "uri",
+ "jdbc:sqlite:" + warehouse.resolve("catalog.db"),
+ "warehouse",
+ warehouse.toString())));
+ }
+}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
index 47fa28e110..b72113e33a 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
@@ -281,7 +281,7 @@ public class GravitinoInterceptionService implements
InterceptionService {
LOG.warn("Invalid authorization request", ex);
return Utils.illegalArguments(ex.getMessage(), ex);
} catch (IllegalMetadataObjectException ex) {
- LOG.warn("Invalid metadata object type during authorization", ex);
+ LOG.warn("Invalid metadata object during authorization", ex);
return Utils.illegalArguments(
IllegalArgumentException.class.getSimpleName(), ex.getMessage(),
ex);
} catch (IllegalNameIdentifierException ex) {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
index eede162869..d8d3e1b24f 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
@@ -90,14 +90,20 @@ public class ParameterUtil {
} catch (IllegalArgumentException e) {
throw new IllegalMetadataObjectException(e, "Invalid metadata object
type: %s", rawType);
}
- MetadataObject object =
- RequestValidation.parse(
- () -> MetadataObjects.parse(fullName.get(), type),
- e -> new IllegalMetadataObjectException(e, "%s",
e.getMessage()));
- NameIdentifier nameIdentifier =
MetadataObjectUtil.toEntityIdent(metalake, object);
nameIdentifierMap.putAll(
- NameIdentifierUtil.splitNameIdentifier(
- metalake, MetadataObjectUtil.toEntityType(type),
nameIdentifier));
+ RequestValidation.parse(
+ () -> {
+ NameIdentifier nameIdentifier =
+ MetadataObjectUtil.toEntityIdent(
+ metalake, MetadataObjects.parse(fullName.get(),
type));
+ if (type == MetadataObject.Type.CATALOG) {
+ // MetadataObjects.parse preserves single-level names,
including embedded dots.
+ NameIdentifierUtil.checkCatalog(nameIdentifier);
+ }
+ return NameIdentifierUtil.splitNameIdentifier(
+ metalake, MetadataObjectUtil.toEntityType(type),
nameIdentifier);
+ },
+ e -> new IllegalMetadataObjectException(e, "%s",
e.getMessage())));
}
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
index 63175f93cc..05252a1a29 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
@@ -104,6 +104,7 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
@@ -473,6 +474,40 @@ public class TestGravitinoInterceptionService {
verify(invocation, never()).proceed();
}
+ @ParameterizedTest
+ @CsvSource({
+ "table,a.b",
+ "table,a.b.c.d",
+ "schema,a",
+ "schema,a.b.c",
+ "column,a.b.c",
+ "column,a.b.c.d.e",
+ "catalog,a.b"
+ })
+ void testMalformedMetadataObjectNameReturnsBadRequest(String type, String
fullName)
+ throws Throwable {
+ Method method =
+ TestMetadataObjectTagAssociationOperations.class.getMethod(
+ "associateTagsForObject",
+ String.class,
+ String.class,
+ String.class,
+ TagsAssociateRequest.class);
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments()).thenReturn(new Object[] {"testMetalake",
type, fullName, null});
+ MethodInterceptor interceptor =
+ new
GravitinoInterceptionService().getMethodInterceptors(method).get(0);
+ try (Response response = (Response) interceptor.invoke(invocation)) {
+ assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, error.getCode());
+ assertEquals(IllegalArgumentException.class.getSimpleName(),
error.getType());
+ Assertions.assertFalse(error.getMessage().contains("system internal
error"));
+ }
+ verify(invocation, never()).proceed();
+ }
+
@Test
public void testSystemInternalErrorHandling() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
index 119bb78f8c..6fe50d2b98 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
@@ -713,6 +713,24 @@ public class TestPermissionOperations extends
BaseOperationsTest {
IllegalPrivilegeException.class.getSimpleName(),
wrongPriErrorResp.getType());
}
+ @Test
+ void testOverridePrivilegesRejectsMissingAndNullOverrides() {
+ for (String json : new String[] {"{}", "{\"overrides\":null}",
"{\"overrides\":[null]}"}) {
+ try (Response response =
+ target("/metalakes/metalake1/permissions/roles/role1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(json, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
error.getCode());
+
Assertions.assertEquals(IllegalArgumentException.class.getSimpleName(),
error.getType());
+ Assertions.assertTrue(error.getMessage().contains("overrides"));
+ }
+ }
+ Mockito.verify(manager, Mockito.never()).overridePrivilegesInRole(any(),
any(), any());
+ }
+
@Test
public void testOverridePrivileges() throws IOException {
// Mock metalake with in-use property
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 1cddd755ef..75b0453b3b 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
@@ -72,6 +72,8 @@ import org.glassfish.jersey.test.TestProperties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
public class TestSchemaOperations extends BaseOperationsTest {
@@ -373,6 +375,23 @@ public class TestSchemaOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @ParameterizedTest
+ @ValueSource(ints = {0, 256, 257})
+ void testAlterSchemaRejectsCommentUpdatesBeforeDispatch(int commentLength) {
+ String json =
+ "{\"updates\":[{\"@type\":\"updateComment\",\"newComment\":\""
+ + "c".repeat(commentLength)
+ + "\"}]}";
+ try (Response response =
+ target("/metalakes/" + metalake + "/catalogs/" + catalog +
"/schemas/schema1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(json, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ Mockito.verifyNoInteractions(dispatcher);
+ }
+ }
+
@Test
public void testAlterSchema() {
SchemaUpdateRequest setReq = new
SchemaUpdateRequest.SetSchemaPropertyRequest("key2", "value2");
