This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 c30a0e55fa [Cherry-pick to branch-1.3] [#14419][#14425][#14426] fix:
classify Iceberg and Lance client errors (#14556) (#14624)
c30a0e55fa is described below
commit c30a0e55fad9678432550503ccb0afba251102c9
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Nov 4 22:53:41 2026 +0800
[Cherry-pick to branch-1.3] [#14419][#14425][#14426] fix: classify Iceberg
and Lance client errors (#14556) (#14624)
### What changes were proposed in this pull request?
Backport #14556 (`5f4a64e77b946d2e81bc6203371a12f04d70ab5b`) to
`branch-1.3` and resolve the conflicts in the Lance implementation,
tests, and documentation.
Preserve Iceberg rename-conflict classification, Lance request/error
envelopes and protocol headers, JSON/body validation, and ordered
column-name preflight. Adapt the preflight to 1.3's existing
drop/rename/index operations, retaining its OOM health checks and
configuration behavior.
### Why are the changes needed?
The automatic cherry-pick left conflict markers and references to newer
main-branch functionality. This resolution makes the client-error fixes
usable on 1.3.
### Does this PR introduce _any_ user-facing change?
Iceberg rename conflicts return 409. Invalid Lance column names and
request bodies return 400; optional missing-column deletes are skipped.
Unsupported Lance routes return code 0 / HTTP 406, typed entity misses
remain 404, and throttling returns 429. Expected backend client errors
use INFO logs. Server failures retain the original exception in ERROR
logs; unexpected failures and framework 5xx return a generic error body.
Details explicitly supplied by native Lance exceptions are preserved.
Lance drop/rename operations are validated as top-level changes before
mutation. Adding columns remains unsupported on 1.3. Preflight prevents
partial edits caused by invalid names, while backend failures and
concurrent external changes retain the existing non-atomic behavior.
This backport uses the existing 1.3 error-detail behavior and adds no
stack-trace configuration option.
### How was this patch tested?
- 155 filtered cases passed with no failures/errors/skips:
`TestLanceTableOperations` (51), `TestIcebergTable` (11),
`TestLanceExceptionMapper` (45), `TestLanceNamespaceOperations` (27),
`TestLanceRESTService` (1), `TestLanceAuxiliaryAuthentication` (3), and
`LanceRESTServiceIT` (17).
- The final Lance run includes embedded HTTP integration coverage for
invalid JSON/bodies, unsupported operations, and column-name errors.
Database/Spark integration matrices outside these classes were not run.
- On the unmodified 1.3 base, 9 of 11 selected column-preflight
regression cases fail; all 11 pass with the backport.
- Scoped Spotless and conflict-marker/diff checks passed. No full core
suite or unfiltered aggregate test task was run.
---------
Co-authored-by: Qi Yu <[email protected]>
---
.../lakehouse/lance/LanceTableOperations.java | 60 +++-
.../lakehouse/lance/TestLanceTableOperations.java | 237 ++++++++++++
.../iceberg/IcebergCatalogOperations.java | 7 +-
.../lakehouse/iceberg/TestIcebergTable.java | 56 +++
docs/lance-rest-service.md | 17 +
.../apache/gravitino/lance/LanceRESTService.java | 4 +-
.../lance/service/LanceExceptionMapper.java | 121 +++++--
.../service/LanceRequestBodyReaderInterceptor.java | 51 +++
.../service/rest/LanceNamespaceOperations.java | 3 +
.../lance/service/rest/LanceTableOperations.java | 13 +-
.../lance/integration/test/LanceRESTServiceIT.java | 115 +++++-
.../lance/service/TestLanceExceptionMapper.java | 397 ++++++++++++++++++++-
.../service/rest/TestLanceNamespaceOperations.java | 106 +++++-
13 files changed, 1120 insertions(+), 67 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
index 344fcefe44..30aae6f356 100644
---
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
@@ -26,11 +26,13 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.time.Instant;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ThreadLocalRandom;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -800,13 +802,20 @@ public class LanceTableOperations extends
ManagedTableOperations {
* @return the new version id of the Lance dataset after applying the changes
*/
long handleLanceTableChange(Table table, TableChange[] changes) {
+ // Validate the whole request before any dataset mutation. The name
preflight below uses
+ // top-level fields, matching the columns Gravitino stores for Lance
tables.
+ validateLanceTableChanges(changes);
String location = table.properties().get(Table.PROPERTY_LOCATION);
Map<String, String> storageOptions =
LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties());
try (Dataset dataset = openDataset(location, storageOptions)) {
- for (TableChange change : changes) {
+ List<TableChange> changesToApply =
+ Arrays.stream(changes).anyMatch(change -> change instanceof
TableChange.ColumnChange)
+ ? validateLanceColumnNames(dataset, changes)
+ : Arrays.asList(changes);
+ for (TableChange change : changesToApply) {
if (change instanceof TableChange.DeleteColumn deleteColumn) {
- dataset.dropColumns(List.of(String.join(".",
deleteColumn.fieldName())));
+ dataset.dropColumns(List.of(deleteColumn.fieldName()[0]));
} else if (change instanceof TableChange.AddIndex addIndex) {
IndexType indexType = IndexType.valueOf(addIndex.getType().name());
IndexParams indexParams = getIndexParamsByIndexType(indexType);
@@ -843,6 +852,53 @@ public class LanceTableOperations extends
ManagedTableOperations {
}
}
+ private static List<TableChange> validateLanceColumnNames(
+ Dataset dataset, TableChange[] changes) {
+ // Use the dataset schema, including out-of-band changes, and validate the
whole ordered
+ // request before applying any changes. A late name conflict must not
leave earlier edits.
+ Set<String> columnNames =
+
dataset.getSchema().getFields().stream().map(Field::getName).collect(Collectors.toSet());
+ List<TableChange> changesToApply = new ArrayList<>(changes.length);
+ for (TableChange change : changes) {
+ if (change instanceof TableChange.DeleteColumn deleteColumn) {
+ String name = deleteColumn.fieldName()[0];
+ boolean exists = columnNames.remove(name);
+ Preconditions.checkArgument(
+ exists || Boolean.TRUE.equals(deleteColumn.getIfExists()),
+ "Column does not exist: %s",
+ name);
+ if (!exists) {
+ // Honor ifExists before reaching Lance, whose dropColumns rejects
missing fields.
+ continue;
+ }
+ } else if (change instanceof TableChange.RenameColumn renameColumn) {
+ String oldName = renameColumn.fieldName()[0];
+ String newName = renameColumn.getNewName();
+ Preconditions.checkArgument(
+ columnNames.remove(oldName), "Column does not exist: %s", oldName);
+ Preconditions.checkArgument(columnNames.add(newName), "Column already
exists: %s", newName);
+ }
+ changesToApply.add(change);
+ }
+ return changesToApply;
+ }
+
+ private static void validateLanceTableChanges(TableChange[] changes) {
+ for (TableChange change : changes) {
+ if (change instanceof TableChange.DeleteColumn
+ || change instanceof TableChange.RenameColumn) {
+ String[] fieldName = ((TableChange.ColumnChange) change).fieldName();
+ Preconditions.checkArgument(
+ fieldName.length == 1,
+ "Lance only supports dropping or renaming top-level columns: %s",
+ String.join(".", fieldName));
+ } else if (!(change instanceof TableChange.AddIndex)) {
+ throw new UnsupportedOperationException(
+ "Unsupported changes to lance table: " +
change.getClass().getSimpleName());
+ }
+ }
+ }
+
Dataset openDataset(String location) {
return openDataset(location, Map.of());
}
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
index 530cfe9bd2..1aec39b551 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
@@ -39,10 +39,13 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
+import java.util.stream.Collectors;
import org.apache.arrow.vector.types.pojo.ArrowType;
import org.apache.arrow.vector.types.pojo.Field;
import org.apache.arrow.vector.types.pojo.Schema;
@@ -52,6 +55,7 @@ import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.catalog.ManagedSchemaOperations;
+import org.apache.gravitino.lance.common.ops.gravitino.LanceDataTypeConverter;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.ColumnEntity;
import org.apache.gravitino.meta.TableEntity;
@@ -676,6 +680,8 @@ public class TestLanceTableOperations {
});
Dataset dataset = mock(Dataset.class);
+ when(dataset.getSchema())
+ .thenReturn(new Schema(List.of(Field.nullable("id", new
ArrowType.Int(32, true)))));
Version version = mock(Version.class);
when(dataset.getVersion()).thenReturn(version);
when(version.getId()).thenReturn(9L);
@@ -797,12 +803,221 @@ public class TestLanceTableOperations {
.update(eq(ident), eq(TableEntity.class), eq(Entity.EntityType.TABLE),
any());
}
+ @Test
+ public void testHandleLanceTableChangeValidatesBeforeChangingDataset() {
+ Table table = mock(Table.class);
+ when(table.properties()).thenReturn(Map.of(Table.PROPERTY_LOCATION,
"location"));
+ Dataset dataset = mock(Dataset.class);
+ Mockito.doReturn(dataset).when(lanceTableOps).openDataset("location",
Map.of());
+
+ // Each request starts with a valid change, which must not reach the
dataset when a later
+ // change in the same request is rejected.
+ List<TableChange> rejectedChanges =
+ List.of(
+ TableChange.renameColumn(new String[] {"parent", "nested"},
"renamed"),
+ TableChange.deleteColumn(new String[] {"parent", "nested"}, false),
+ TableChange.addColumn(new String[] {"required"},
Types.StringType.get(), false),
+ TableChange.updateColumnComment(new String[] {"col1"}, "comment"));
+ for (TableChange rejected : rejectedChanges) {
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ lanceTableOps.handleLanceTableChange(
+ table,
+ new TableChange[] {
+ TableChange.deleteColumn(new String[] {"col1"}, false),
rejected
+ }));
+ }
+ verify(lanceTableOps, never()).openDataset("location", Map.of());
+ verify(dataset, never()).dropColumns(anyList());
+ verify(dataset, never()).alterColumns(anyList());
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "rename",
+ "deleteThenRenameMissing",
+ "renameThenRename",
+ "renameThenDeleteMissing",
+ "deleteTwice",
+ "deleteMissingDefault"
+ })
+ void testInvalidColumnNamesRejectWholeRequest(String path) throws Exception {
+ NameIdentifier ident = NameIdentifier.of("schema", "conflict");
+ String location = tempDir.resolve("conflict").toString();
+ Column[] columns = {
+ Column.of("id", Types.LongType.get()), Column.of("name",
Types.StringType.get())
+ };
+ long version;
+ try (Dataset dataset = createDatasetForColumnNameValidation(location,
columns)) {
+ version = dataset.getVersion().getId();
+ }
+ List<ColumnEntity> storedColumns = new ArrayList<>();
+ for (int i = 0; i < columns.length; i++) {
+ storedColumns.add(
+ ColumnEntity.builder()
+ .withId(10L + i)
+ .withName(columns[i].name())
+ .withDataType(columns[i].dataType())
+ .withPosition(i)
+ .withNullable(true)
+ .withAuditInfo(AuditInfo.EMPTY)
+ .build());
+ }
+ TableEntity entity =
+ tableEntity(
+ ident,
+ storedColumns,
+ Map.of(
+ Table.PROPERTY_TABLE_FORMAT,
+ "lance",
+ Table.PROPERTY_LOCATION,
+ location,
+ LANCE_TABLE_VERSION,
+ String.valueOf(version)));
+ when(store.get(ident, Entity.EntityType.TABLE,
TableEntity.class)).thenReturn(entity);
+ TableChange rename = TableChange.renameColumn(new String[] {"id"}, "name");
+ TableChange[] changes =
+ switch (path) {
+ case "deleteThenRenameMissing" -> new TableChange[] {
+ TableChange.deleteColumn(new String[] {"id"}, false),
+ TableChange.renameColumn(new String[] {"id"}, "extra")
+ };
+ case "renameThenRename" -> new TableChange[] {
+ TableChange.renameColumn(new String[] {"id"}, "extra"),
+ TableChange.renameColumn(new String[] {"extra"}, "name")
+ };
+ case "renameThenDeleteMissing" -> new TableChange[] {
+ TableChange.renameColumn(new String[] {"id"}, "extra"),
+ TableChange.deleteColumn(new String[] {"missing"}, false)
+ };
+ case "deleteTwice" -> new TableChange[] {
+ TableChange.deleteColumn(new String[] {"id"}, false),
+ TableChange.deleteColumn(new String[] {"id"}, false)
+ };
+ case "deleteMissingDefault" -> new TableChange[] {
+ TableChange.deleteColumn(new String[] {"missing"}, null)
+ };
+ default -> new TableChange[] {rename};
+ };
+ IllegalArgumentException error =
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
lanceTableOps.alterTable(ident, changes));
+ String invalidName =
+ switch (path) {
+ case "renameThenDeleteMissing", "deleteMissingDefault" -> "missing";
+ case "deleteTwice", "deleteThenRenameMissing" -> "id";
+ default -> "name";
+ };
+ Assertions.assertTrue(error.getMessage().contains(invalidName));
+ verify(store, never())
+ .update(eq(ident), eq(TableEntity.class), eq(Entity.EntityType.TABLE),
any());
+ try (Dataset dataset = lanceTableOps.openDataset(location)) {
+ Assertions.assertEquals(version, dataset.getVersion().getId());
+ Assertions.assertEquals(
+ List.of("id", "name"),
+ dataset.getSchema().getFields().stream()
+ .map(Field::getName)
+ .collect(Collectors.toList()));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testColumnNamesCanBeReusedAfterEarlierChanges(boolean drop) {
+ String location = tempDir.resolve("reuse").toString();
+ Column[] columns = {
+ Column.of("id", Types.LongType.get()), Column.of("name",
Types.StringType.get())
+ };
+ try (Dataset ignored = createDatasetForColumnNameValidation(location,
columns)) {
+ // Use real Lance mutations to verify that preflight follows the request
order.
+ }
+ Table table = mock(Table.class);
+ when(table.properties()).thenReturn(Map.of(Table.PROPERTY_LOCATION,
location));
+ TableChange[] changes =
+ drop
+ ? new TableChange[] {
+ TableChange.deleteColumn(new String[] {"name"}, false),
+ TableChange.renameColumn(new String[] {"id"}, "name")
+ }
+ : new TableChange[] {
+ TableChange.renameColumn(new String[] {"id"}, "new"),
+ TableChange.renameColumn(new String[] {"name"}, "id")
+ };
+ lanceTableOps.handleLanceTableChange(table, changes);
+ try (Dataset dataset = lanceTableOps.openDataset(location)) {
+ Assertions.assertEquals(
+ drop ? List.of("name") : List.of("new", "id"),
+ dataset.getSchema().getFields().stream()
+ .map(Field::getName)
+ .collect(Collectors.toList()));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"alone", "twice", "thenRename", "renameThenDrop"})
+ void testOptionalDeletesRespectRequestOrder(String path) {
+ String location = tempDir.resolve("optional-delete").toString();
+ Column[] columns = {
+ Column.of("id", Types.LongType.get()), Column.of("name",
Types.StringType.get())
+ };
+ long version;
+ try (Dataset dataset = createDatasetForColumnNameValidation(location,
columns)) {
+ version = dataset.getVersion().getId();
+ }
+ Table table = mock(Table.class);
+ when(table.properties()).thenReturn(Map.of(Table.PROPERTY_LOCATION,
location));
+ TableChange[] changes =
+ switch (path) {
+ case "twice" -> new TableChange[] {
+ TableChange.deleteColumn(new String[] {"name"}, false),
+ TableChange.deleteColumn(new String[] {"name"}, true)
+ };
+ case "thenRename" -> new TableChange[] {
+ TableChange.deleteColumn(new String[] {"extra"}, true),
+ TableChange.renameColumn(new String[] {"id"}, "extra")
+ };
+ case "renameThenDrop" -> new TableChange[] {
+ TableChange.renameColumn(new String[] {"id"}, "extra"),
+ TableChange.deleteColumn(new String[] {"extra"}, true)
+ };
+ default -> new TableChange[] {TableChange.deleteColumn(new String[]
{"missing"}, true)};
+ };
+ long result = lanceTableOps.handleLanceTableChange(table, changes);
+ if (path.equals("alone")) {
+ Assertions.assertEquals(version, result);
+ } else {
+ Assertions.assertTrue(result > version);
+ }
+ try (Dataset dataset = lanceTableOps.openDataset(location)) {
+ List<String> expected =
+ switch (path) {
+ case "twice" -> List.of("id");
+ case "thenRename" -> List.of("extra", "name");
+ case "renameThenDrop" -> List.of("name");
+ default -> List.of("id", "name");
+ };
+ Assertions.assertEquals(
+ expected,
+ dataset.getSchema().getFields().stream()
+ .map(Field::getName)
+ .collect(Collectors.toList()));
+ }
+ }
+
@Test
public void testHandleLanceTableChangeRespectsOrder() {
Table table = mock(Table.class);
when(table.properties()).thenReturn(Map.of(Table.PROPERTY_LOCATION,
"location"));
Dataset dataset = mock(Dataset.class);
+ when(dataset.getSchema())
+ .thenReturn(
+ new Schema(
+ List.of(
+ Field.nullable("old", new ArrowType.Utf8()),
+ Field.nullable("col1", new ArrowType.Utf8()))));
Version version = mock(Version.class);
when(dataset.getVersion()).thenReturn(version);
when(version.getId()).thenReturn(7L);
@@ -842,6 +1057,12 @@ public class TestLanceTableOperations {
"table-key"));
Dataset dataset = mock(Dataset.class);
+ when(dataset.getSchema())
+ .thenReturn(
+ new Schema(
+ List.of(
+ Field.nullable("old", new ArrowType.Utf8()),
+ Field.nullable("col1", new ArrowType.Utf8()))));
Version version = mock(Version.class);
when(dataset.getVersion()).thenReturn(version);
when(version.getId()).thenReturn(9L);
@@ -1301,4 +1522,20 @@ public class TestLanceTableOperations {
.mode(WriteParams.WriteMode.CREATE)
.execute();
}
+
+ private Dataset createDatasetForColumnNameValidation(String location,
Column[] columns) {
+ Schema schema =
+ new Schema(
+ Arrays.stream(columns)
+ .map(
+ column ->
+ LanceDataTypeConverter.CONVERTER.toArrowField(
+ column.name(), column.dataType(),
column.nullable()))
+ .collect(Collectors.toList()));
+ return Dataset.write()
+ .uri(location)
+ .schema(schema)
+ .mode(WriteParams.WriteMode.CREATE)
+ .execute();
+ }
}
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 d881404ef1..732c7f9bad 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
@@ -488,8 +488,8 @@ public class IcebergCatalogOperations
*/
private Table renameTable(NameIdentifier tableIdent, TableChange.RenameTable
renameTable)
throws NoSuchTableException, IllegalArgumentException {
+ Namespace destNamespace = tableIdent.namespace();
try {
- Namespace destNamespace = tableIdent.namespace();
if (renameTable.getNewSchemaName().isPresent()) {
String[] namespaceLevels = tableIdent.namespace().levels();
String[] destLevels = Arrays.copyOf(namespaceLevels,
namespaceLevels.length);
@@ -511,6 +511,11 @@ public class IcebergCatalogOperations
icebergCatalogWrapper.renameTable(renameTableRequest);
return loadTable(
NameIdentifier.of(ArrayUtils.add(destNamespace.levels(),
renameTable.getNewName())));
+ } catch (AlreadyExistsException e) {
+ throw new TableAlreadyExistsException(
+ e,
+ "Table already exists: %s",
+ NameIdentifier.of(destNamespace, renameTable.getNewName()));
} catch (org.apache.iceberg.exceptions.NoSuchTableException e) {
throw new NoSuchTableException(e, ICEBERG_TABLE_DOES_NOT_EXIST_MSG,
tableIdent.name());
}
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTable.java
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTable.java
index 5b1b0a2874..6b0c9e320a 100644
---
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTable.java
+++
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergTable.java
@@ -56,10 +56,13 @@ import
org.apache.gravitino.rel.expressions.sorts.SortOrders;
import org.apache.gravitino.rel.expressions.transforms.Transform;
import org.apache.gravitino.rel.types.Types;
import org.apache.iceberg.DistributionMode;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.junit.jupiter.api.AfterEach;
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;
public class TestIcebergTable {
@@ -490,6 +493,59 @@ public class TestIcebergTable {
Assertions.assertArrayEquals(createdTable.partitioning(),
alteredTable.partitioning());
}
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testRenameTableToExistingNamePreservesBothTables(boolean acrossSchemas)
{
+ NameIdentifier source =
+ NameIdentifier.of(
+ schemaIdent.namespace().level(0),
+ ICEBERG_CATALOG_NAME,
+ ICEBERG_SCHEMA_NAME,
+ "rename_source");
+ NameIdentifier targetSchema =
+ NameIdentifier.of(
+ META_LAKE_NAME,
+ ICEBERG_CATALOG_NAME,
+ acrossSchemas ? "rename_target_schema" : ICEBERG_SCHEMA_NAME);
+ NameIdentifier target =
+ NameIdentifier.of(
+ META_LAKE_NAME, ICEBERG_CATALOG_NAME, targetSchema.name(),
"rename_target");
+ if (acrossSchemas) {
+ icebergCatalogOperations.createSchema(targetSchema, "target schema",
Map.of());
+ }
+ try {
+ Column[] columns = {Column.of("id", Types.LongType.get())};
+ for (NameIdentifier ident : List.of(source, target)) {
+ icebergCatalogOperations.createTable(
+ ident,
+ columns,
+ ident.name(),
+ Map.of(),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0]);
+ }
+ TableAlreadyExistsException error =
+ Assertions.assertThrows(
+ TableAlreadyExistsException.class,
+ () ->
+ icebergCatalogOperations.alterTable(
+ source,
+ acrossSchemas
+ ? TableChange.rename(target.name(),
targetSchema.name())
+ : TableChange.rename(target.name())));
+ Assertions.assertInstanceOf(AlreadyExistsException.class,
error.getCause());
+ Assertions.assertEquals("Table already exists: " + target,
error.getMessage());
+ Assertions.assertEquals(source.name(),
icebergCatalogOperations.loadTable(source).comment());
+ Assertions.assertEquals(target.name(),
icebergCatalogOperations.loadTable(target).comment());
+ } finally {
+ if (acrossSchemas) {
+ icebergCatalogOperations.dropTable(target);
+ icebergCatalogOperations.dropSchema(targetSchema, false);
+ }
+ }
+ }
+
@Test
public void testRenameTableAcrossSchema() {
NameIdentifier tableIdentifier =
diff --git a/docs/lance-rest-service.md b/docs/lance-rest-service.md
index 933861aba1..75cd2e726d 100644
--- a/docs/lance-rest-service.md
+++ b/docs/lance-rest-service.md
@@ -279,6 +279,23 @@ URL encoded: lance_catalog%24schema%24table01
- Namespace deletion is recursive and irreversible
:::
+## Error responses
+
+Malformed requests and backend validation errors return HTTP `400` without
generated stack traces
+in `detail`. Details explicitly supplied by a backend Lance exception are
preserved. Expected
+backend client errors are logged at INFO; server failures are logged at ERROR
with the original
+exception for diagnosis. Unexpected failures return a generic
internal-server-error response.
+
+Unrecognized or unimplemented routes under the Lance REST servlet return HTTP
`406` with Lance
+error code `0` (`Unsupported`), allowing clients to select a fallback. The
error identifies the HTTP
+method and path. Parameter conversion failures remain HTTP `400`
(`InvalidInput`); missing tables
+and namespaces reported by operation handlers retain HTTP `404` and their
entity-specific codes.
+
+In 1.3, Lance column changes support dropping and renaming top-level columns;
adding columns remains
+unsupported. Column-name validation checks the ordered request against the
current dataset before
+applying changes. Missing optional deletes are skipped. This prevents partial
edits from invalid
+column names, but does not make backend failures or concurrent external
changes atomic.
+
## Examples
The following examples demonstrate how to interact with Lance REST service
using different programming languages and tools.
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
index 3c41ba4f4f..e1ad2fc146 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
@@ -35,6 +35,7 @@ import org.apache.gravitino.lance.common.config.LanceConfig;
import org.apache.gravitino.lance.common.ops.LanceNamespaceBackend;
import org.apache.gravitino.lance.common.ops.NamespaceWrapper;
import org.apache.gravitino.lance.service.LanceExceptionMapper;
+import org.apache.gravitino.lance.service.LanceRequestBodyReaderInterceptor;
import org.apache.gravitino.lance.service.LanceServiceIdentityFilter;
import
org.apache.gravitino.lance.service.authorization.LanceAuthorizationMetadataFilter;
import
org.apache.gravitino.lance.service.authorization.LanceRESTAuthInterceptionService;
@@ -106,7 +107,8 @@ public class LanceRESTService implements
GravitinoAuxiliaryService {
}
ResourceConfig resourceConfig = new ResourceConfig();
- resourceConfig.register(JacksonFeature.class);
+ resourceConfig.register(JacksonFeature.withoutExceptionMappers());
+ resourceConfig.register(LanceRequestBodyReaderInterceptor.class);
resourceConfig.packages(LANCE_REST_SPEC_PACKAGE);
resourceConfig.register(LanceExceptionMapper.class);
resourceConfig.register(new OutOfMemoryErrorListener());
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
index c5b136604b..b39f5f5a76 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java
@@ -20,17 +20,18 @@ package org.apache.gravitino.lance.service;
import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace;
-import javax.ws.rs.NotAcceptableException;
-import javax.ws.rs.NotAllowedException;
-import javax.ws.rs.NotSupportedException;
import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Request;
import javax.ws.rs.core.Response;
+import javax.ws.rs.core.UriInfo;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.server.web.ServerHealth;
+import org.glassfish.jersey.server.ParamException;
import org.lance.namespace.errors.ConcurrentModificationException;
import org.lance.namespace.errors.InternalException;
import org.lance.namespace.errors.InvalidInputException;
@@ -42,6 +43,7 @@ import org.lance.namespace.errors.PermissionDeniedException;
import org.lance.namespace.errors.ServiceUnavailableException;
import org.lance.namespace.errors.TableAlreadyExistsException;
import org.lance.namespace.errors.TableNotFoundException;
+import org.lance.namespace.errors.ThrottlingException;
import org.lance.namespace.errors.UnauthenticatedException;
import org.lance.namespace.model.ErrorResponse;
import org.slf4j.Logger;
@@ -52,42 +54,95 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
private static final Logger LOG =
LoggerFactory.getLogger(LanceExceptionMapper.class);
+ @Context private UriInfo uriInfo;
+ @Context private Request request;
+
public static Response toRESTResponse(String instance, Throwable ex) {
ServerHealth.getInstance().recordFailure(ex);
- if (ex instanceof NotAllowedException
- || ex instanceof NotAcceptableException
- || ex instanceof NotSupportedException) {
- WebApplicationException routingError = (WebApplicationException) ex;
- InvalidInputException invalidInput =
- new InvalidInputException(ex.getMessage(), getStackTrace(ex),
instance);
- try (Response mapped = handleLanceNamespaceException(invalidInput)) {
- Response.ResponseBuilder response =
- Response.status(routingError.getResponse().getStatus())
- .entity(mapped.getEntity())
- .type(MediaType.APPLICATION_JSON_TYPE);
- if (ex instanceof NotAllowedException) {
- response.allow(routingError.getResponse().getAllowedMethods());
- }
- return response.build();
- }
+ if (ex instanceof WebApplicationException webException) {
+ return handleWebApplicationException(instance, webException);
}
-
LanceNamespaceException lanceException =
ex instanceof LanceNamespaceException
? (LanceNamespaceException) ex
: toLanceNamespaceException(instance, ex);
- LOG.error(
- "Operate Lance instance with id '{}' got exception: {}",
- instance,
- lanceException.getMessage());
+ if (toHttpStatus(lanceException) >= 500) {
+ LOG.error("Operate Lance instance with id '{}' failed", instance, ex);
+ } else {
+ LOG.info(
+ "Operate Lance instance with id '{}' got exception: {}",
+ instance,
+ lanceException.getMessage());
+ }
return handleLanceNamespaceException(lanceException);
}
+ /** {@inheritDoc} */
@Override
public Response toResponse(Throwable ex) {
- return toRESTResponse("", ex);
+ String instance = "";
+ if (isRouteMiss(ex) && request != null && uriInfo != null) {
+ instance = request.getMethod() + " " + uriInfo.getPath(false);
+ }
+ return toRESTResponse(instance, ex);
+ }
+
+ private static boolean isRouteMiss(Throwable ex) {
+ return ex instanceof WebApplicationException webException
+ && !(ex instanceof ParamException)
+ && webException.getResponse().getStatus() ==
Response.Status.NOT_FOUND.getStatusCode();
+ }
+
+ private static Response handleWebApplicationException(
+ String instance, WebApplicationException exception) {
+ int status = exception.getResponse().getStatus();
+ String message = exception.getMessage();
+ boolean routeMiss = isRouteMiss(exception);
+ if (exception instanceof ParamException parameterException) {
+ // Jersey can use 404 for parameter conversion failures, but these are
invalid input.
+ status = Response.Status.BAD_REQUEST.getStatusCode();
+ message = "Invalid value for parameter '" +
parameterException.getParameterName() + "'";
+ } else if (routeMiss) {
+ // Unmatched routes include unimplemented Namespace operations. Code 0 /
HTTP 406 lets
+ // Lance clients recognize Unsupported and fall back; domain not-found
errors stay 404.
+ status = Response.Status.NOT_ACCEPTABLE.getStatusCode();
+ message = "Unsupported Lance REST operation" + (instance.isEmpty() ? ""
: ": " + instance);
+ }
+ LanceNamespaceException error;
+ if (routeMiss) {
+ error = new
org.lance.namespace.errors.UnsupportedOperationException(message, "", instance);
+ } else if (status >= 400 && status < 500) {
+ error =
+ switch (status) {
+ case 401 -> new UnauthenticatedException(message, "", instance);
+ case 403 -> new PermissionDeniedException(message, "", instance);
+ case 409 -> new ConcurrentModificationException(message, "",
instance);
+ case 429 -> new ThrottlingException(message, "", instance);
+ default -> new InvalidInputException(message, "", instance);
+ };
+ } else {
+ LOG.error("Lance REST framework failure:", exception);
+ error = new InternalException("Internal server error", "", instance);
+ }
+ // Keep protocol headers such as Allow, WWW-Authenticate and Retry-After,
while replacing any
+ // framework entity with the Lance error envelope. Never expose a
framework stack or 5xx detail.
+ Response.ResponseBuilder response =
Response.fromResponse(exception.getResponse());
+ // The old entity's framing, representation metadata and validators do not
describe our JSON.
+ for (String header :
+ new String[] {
+ "Content-Length", "Content-Encoding", "Content-Language",
"Content-Location",
+ "Content-Range", "Content-Disposition", "Transfer-Encoding", "ETag",
+ "Last-Modified", "Content-MD5", "Digest"
+ }) {
+ response.header(header, null);
+ }
+ return response
+ .status(status)
+ .entity(errorResponse(error))
+ .type(MediaType.APPLICATION_JSON_TYPE)
+ .build();
}
private static LanceNamespaceException toLanceNamespaceException(String
instance, Throwable ex) {
@@ -98,7 +153,8 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
return new NamespaceNotFoundException(ex.getMessage(),
getStackTrace(ex), instance);
} else if (ex instanceof IllegalArgumentException) {
- return new InvalidInputException(ex.getMessage(), getStackTrace(ex),
instance);
+ // Validation failures never expose generated stacks in the client
response.
+ return new InvalidInputException(ex.getMessage(), "", instance);
} else if (ex instanceof
org.apache.gravitino.exceptions.TableAlreadyExistsException) {
return new TableAlreadyExistsException(ex.getMessage(),
getStackTrace(ex), instance);
@@ -108,8 +164,7 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
ex.getMessage(), getStackTrace(ex), instance);
} else {
- LOG.warn("Lance REST server unexpected exception:", ex);
- return new InternalException(ex.getMessage(), getStackTrace(ex),
instance);
+ return new InternalException("Internal server error", "", instance);
}
}
@@ -137,6 +192,10 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
return Response.Status.UNAUTHORIZED.getStatusCode();
}
+ if (exception instanceof ThrottlingException) {
+ return 429;
+ }
+
if (exception instanceof ServiceUnavailableException) {
return Response.Status.SERVICE_UNAVAILABLE.getStatusCode();
}
@@ -150,11 +209,15 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
}
private static Response
handleLanceNamespaceException(LanceNamespaceException ex) {
+ return Response.status(toHttpStatus(ex)).entity(errorResponse(ex)).build();
+ }
+
+ private static ErrorResponse errorResponse(LanceNamespaceException ex) {
ErrorResponse errResp = new ErrorResponse();
errResp.setCode(ex.getCode());
errResp.setError(ex.getMessage());
errResp.setDetail(ex.getDetail());
errResp.setInstance(ex.getInstance());
- return Response.status(toHttpStatus(ex)).entity(errResp).build();
+ return errResp;
}
}
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceRequestBodyReaderInterceptor.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceRequestBodyReaderInterceptor.java
new file mode 100644
index 0000000000..1cdb682dbc
--- /dev/null
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceRequestBodyReaderInterceptor.java
@@ -0,0 +1,51 @@
+/*
+ * 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.lance.service;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import java.io.IOException;
+import javax.ws.rs.BadRequestException;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.ext.Provider;
+import javax.ws.rs.ext.ReaderInterceptor;
+import javax.ws.rs.ext.ReaderInterceptorContext;
+
+/** Keeps JSON request decoding failures in the Lance error-response format. */
+@Provider
+public class LanceRequestBodyReaderInterceptor implements ReaderInterceptor {
+
+ /**
+ * Converts JSON decoding failures before a resource method is invoked.
+ *
+ * @param context the request entity reader context
+ * @return the decoded request entity
+ * @throws IOException if a non-JSON input failure occurs
+ * @throws WebApplicationException if the JSON request cannot be decoded
+ */
+ @Override
+ public Object aroundReadFrom(ReaderInterceptorContext context)
+ throws IOException, WebApplicationException {
+ try {
+ return context.proceed();
+ } catch (JsonProcessingException invalidJson) {
+ // Keep the cause for diagnostics without echoing request contents in
the HTTP error body.
+ throw new BadRequestException("Invalid JSON request", invalidJson);
+ }
+ }
+}
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
index 0b5a769f8e..4fe9edd595 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceNamespaceOperations.java
@@ -27,6 +27,7 @@ import static
org.apache.gravitino.server.authorization.expression.Authorization
import com.codahale.metrics.annotation.ResponseMetered;
import com.codahale.metrics.annotation.Timed;
+import com.google.common.base.Preconditions;
import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.ws.rs.Consumes;
@@ -132,6 +133,7 @@ public class LanceNamespaceOperations {
@DefaultValue(NAMESPACE_DELIMITER_DEFAULT) @QueryParam("delimiter")
String delimiter,
CreateNamespaceRequest request) {
try {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
CreateNamespaceResponse response =
lanceNamespace
.asNamespaceOps()
@@ -156,6 +158,7 @@ public class LanceNamespaceOperations {
@DefaultValue(NAMESPACE_DELIMITER_DEFAULT) @QueryParam("delimiter")
String delimiter,
DropNamespaceRequest request) {
try {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
DropNamespaceResponse response =
lanceNamespace
.asNamespaceOps()
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java
index fb763de1fb..fcc48afdb0 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java
@@ -328,9 +328,8 @@ public class LanceTableOperations {
}
}
- private void validateDeclareTableRequest(
- @SuppressWarnings("unused") DeclareTableRequest request) {
- // No specific fields to validate for now
+ private void validateDeclareTableRequest(DeclareTableRequest request) {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
}
private void validateRegisterTableRequest(RegisterTableRequest request) {
@@ -361,6 +360,8 @@ public class LanceTableOperations {
}
private void validateDropColumnsRequest(AlterTableDropColumnsRequest
request) {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
+ Preconditions.checkArgument(request.getColumns() != null, "columns must
not be null");
Preconditions.checkArgument(
!request.getColumns().isEmpty(), "Columns to drop cannot be empty.");
Preconditions.checkArgument(
@@ -369,10 +370,14 @@ public class LanceTableOperations {
}
private void validateAlterColumnsRequest(AlterTableAlterColumnsRequest
request) {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
+ Preconditions.checkArgument(request.getAlterations() != null, "alterations
must not be null");
Preconditions.checkArgument(
!request.getAlterations().isEmpty(), "Columns to alter cannot be
empty.");
- for (AlterColumnsEntry alteration : request.getAlterations()) {
+ for (int i = 0; i < request.getAlterations().size(); i++) {
+ AlterColumnsEntry alteration = request.getAlterations().get(i);
+ Preconditions.checkArgument(alteration != null, "alterations[%s] must
not be null", i);
Preconditions.checkArgument(
StringUtils.isNotBlank(alteration.getPath()), "Column path to alter
cannot be empty.");
Preconditions.checkArgument(
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
index b4762fedf0..db06aedc23 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
@@ -24,6 +24,10 @@ import com.google.common.collect.Sets;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -52,6 +56,7 @@ import org.apache.gravitino.json.JsonUtils;
import org.apache.gravitino.lance.common.utils.ArrowUtils;
import org.apache.gravitino.lance.common.utils.LanceConstants;
import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.TableChange;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -64,6 +69,7 @@ import org.lance.namespace.client.apache.ApiException;
import org.lance.namespace.client.apache.api.TableApi;
import org.lance.namespace.errors.ErrorCode;
import org.lance.namespace.errors.LanceNamespaceException;
+import org.lance.namespace.errors.UnsupportedOperationException;
import org.lance.namespace.model.AlterColumnsEntry;
import org.lance.namespace.model.AlterTableAlterColumnsRequest;
import org.lance.namespace.model.AlterTableAlterColumnsResponse;
@@ -89,6 +95,8 @@ import org.lance.namespace.model.ListNamespacesRequest;
import org.lance.namespace.model.ListNamespacesResponse;
import org.lance.namespace.model.ListTablesRequest;
import org.lance.namespace.model.NamespaceExistsRequest;
+import org.lance.namespace.model.QueryTableRequest;
+import org.lance.namespace.model.QueryTableRequestVector;
import org.lance.namespace.model.RegisterTableRequest;
import org.lance.namespace.model.RegisterTableResponse;
import org.lance.namespace.model.TableExistsRequest;
@@ -671,16 +679,31 @@ public class LanceRESTServiceIT extends BaseIT {
AlterTableDropColumnsRequest dropNonExistingColumnsRequest = new
AlterTableDropColumnsRequest();
dropNonExistingColumnsRequest.setId(ids);
dropNonExistingColumnsRequest.setColumns(List.of("non_existing_column"));
- Exception dropColumnException =
+ long versionBeforeRejectedDrop;
+ try (Dataset dataset = Dataset.open().uri(newLocation).build()) {
+ versionBeforeRejectedDrop = dataset.version();
+ }
+ ApiException dropColumnException =
Assertions.assertThrows(
- Exception.class,
+ ApiException.class,
() ->
tableApi.alterTableDropColumns(
String.join(delimiter, ids),
dropNonExistingColumnsRequest, delimiter));
- Assertions.assertTrue(
- dropColumnException
- .getMessage()
- .contains("Column non_existing_column does not exist in the
dataset"));
+ Assertions.assertEquals(400, dropColumnException.getCode());
+ ErrorResponse error =
+ JsonUtils.anyFieldMapper()
+ .readValue(dropColumnException.getResponseBody(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertTrue(error.getError().contains("non_existing_column"));
+ Assertions.assertEquals("", error.getDetail());
+ Assertions.assertEquals(
+ List.of("id"),
+ ns.describeTable(describeTableRequest).getSchema().getFields().stream()
+ .map(JsonArrowField::getName)
+ .toList());
+ try (Dataset dataset = Dataset.open().uri(newLocation).build()) {
+ Assertions.assertEquals(versionBeforeRejectedDrop, dataset.version());
+ }
}
@Test
@@ -723,6 +746,59 @@ public class LanceRESTServiceIT extends BaseIT {
Assertions.assertFalse(loadTable.getMetadata().containsKey("lance.storage.secret_access_key"));
}
+ @Test
+ void testUnimplementedOperationUsesUnsupportedError() throws Exception {
+ QueryTableRequest request = new QueryTableRequest();
+ request.setId(List.of("missing", "schema", "table"));
+ request.setK(1);
+ request.setVector(new QueryTableRequestVector().singleVector(List.of(1.0f,
2.0f)));
+ request.setVectorColumn("vector");
+ UnsupportedOperationException error =
+ Assertions.assertThrows(UnsupportedOperationException.class, () ->
ns.queryTable(request));
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(), error.getCode());
+ // The native client classifies by code but does not retain instance;
inspect diagnostics
+ // on the wire independently rather than requiring it to preserve every
response field.
+ HttpRequest httpRequest =
+ HttpRequest.newBuilder(
+ URI.create(getLanceRestServiceUrl() +
"/v1/table/missing.schema.table/query"))
+ .header("Content-Type", "application/json")
+ .POST(
+ HttpRequest.BodyPublishers.ofString(
+ JsonUtils.anyFieldMapper().writeValueAsString(request)))
+ .build();
+ HttpResponse<String> response =
+ HttpClient.newHttpClient().send(httpRequest,
HttpResponse.BodyHandlers.ofString());
+ Assertions.assertEquals(406, response.statusCode());
+ ErrorResponse body = JsonUtils.anyFieldMapper().readValue(response.body(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(), body.getCode());
+ Assertions.assertTrue(body.getInstance().startsWith("POST "));
+ Assertions.assertTrue(body.getInstance().endsWith("/query"));
+ Assertions.assertTrue(body.getError().contains(body.getInstance()));
+ Assertions.assertEquals("", body.getDetail());
+ }
+
+ @Test
+ void testInvalidJsonRequestUsesLanceEnvelope() throws Exception {
+ HttpClient http = HttpClient.newHttpClient();
+ for (String body : List.of("{", "[]")) {
+ HttpRequest request =
+ HttpRequest.newBuilder(
+ URI.create(getLanceRestServiceUrl() +
"/v1/namespace/missing/create"))
+ .header("Content-Type", "application/json")
+ .POST(HttpRequest.BodyPublishers.ofString(body))
+ .build();
+ HttpResponse<String> response = http.send(request,
HttpResponse.BodyHandlers.ofString());
+ Assertions.assertEquals(400, response.statusCode());
+ Assertions.assertTrue(
+
response.headers().firstValue("Content-Type").orElse("").contains("application/json"));
+ ErrorResponse error =
+ JsonUtils.anyFieldMapper().readValue(response.body(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertEquals("Invalid JSON request", error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
@Test
void testAlterColumns() throws Exception {
catalog = createCatalog(CATALOG_NAME);
@@ -742,6 +818,33 @@ public class LanceRESTServiceIT extends BaseIT {
TableApi tableApi = createTableApi();
String delimiter = DELIMITER;
+ AlterTableAlterColumnsRequest conflictRequest = new
AlterTableAlterColumnsRequest();
+ conflictRequest.setId(ids);
+ AlterColumnsEntry conflict = new AlterColumnsEntry();
+ conflict.setPath("id");
+ conflict.setRename("value");
+ conflictRequest.setAlterations(List.of(conflict));
+ ApiException conflictError =
+ Assertions.assertThrows(
+ ApiException.class,
+ () ->
+ tableApi.alterTableAlterColumns(
+ String.join(delimiter, ids), conflictRequest, delimiter));
+ Assertions.assertEquals(400, conflictError.getCode());
+ ErrorResponse conflictBody =
+ JsonUtils.anyFieldMapper().readValue(conflictError.getResponseBody(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
conflictBody.getCode());
+ Assertions.assertTrue(conflictBody.getError().contains("value"));
+ Assertions.assertEquals("", conflictBody.getDetail());
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalog
+ .asTableCatalog()
+ .alterTable(
+ NameIdentifier.of(SCHEMA_NAME, ids.get(2)),
+ TableChange.renameColumn(new String[] {"id"}, "value")));
+
AlterTableAlterColumnsRequest alterRequest = new
AlterTableAlterColumnsRequest();
alterRequest.setId(ids);
AlterColumnsEntry columnAlteration = new AlterColumnsEntry();
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
index edaa1fbed9..16d985dcb8 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceExceptionMapper.java
@@ -18,24 +18,51 @@
*/
package org.apache.gravitino.lance.service;
+import com.fasterxml.jackson.core.JsonParseException;
import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ReaderInterceptorContext;
+import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.rest.RESTUtils;
import org.apache.gravitino.server.web.ServerHealth;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.AbstractConfiguration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.config.Property;
import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
import org.glassfish.jersey.test.TestProperties;
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;
+import org.lance.namespace.errors.ErrorCode;
+import org.lance.namespace.errors.ErrorFactory;
+import org.lance.namespace.errors.InternalException;
+import org.lance.namespace.errors.InvalidInputException;
+import org.lance.namespace.errors.NamespaceNotFoundException;
+import org.lance.namespace.errors.TableNotFoundException;
+import org.lance.namespace.errors.ThrottlingException;
+import org.lance.namespace.errors.UnsupportedOperationException;
+import org.lance.namespace.model.CreateNamespaceRequest;
import org.lance.namespace.model.ErrorResponse;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
@@ -106,6 +133,50 @@ public class TestLanceExceptionMapper extends JerseyTest {
public String consume(String body) {
return body;
}
+
+ /**
+ * Converts a framework response whose headers describe its original
entity.
+ *
+ * @return the mapped Lance error response
+ */
+ @GET
+ @Path("framework")
+ public Response frameworkFailure() {
+ try (Response original =
+ Response.status(415)
+ .header("content-encoding", "gzip")
+ .header("content-length", "3")
+ .header("Retry-After", "3")
+ .entity("old")
+ .build()) {
+ return LanceExceptionMapper.toRESTResponse(
+ "table", new WebApplicationException("request rejected",
original));
+ }
+ }
+ /**
+ * Requires a numeric path value so Jersey exercises path conversion
failures.
+ *
+ * @param version the version parsed by Jersey
+ * @return the parsed version
+ */
+ @GET
+ @Path("path/{version}")
+ public String version(@PathParam("version") int version) {
+ return String.valueOf(version);
+ }
+
+ /**
+ * Reads a JSON request using the same Jackson reader configuration as the
service.
+ *
+ * @param request the decoded namespace request
+ * @return its mode
+ */
+ @POST
+ @Path("json")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public String json(CreateNamespaceRequest request) {
+ return request.getMode();
+ }
}
/**
@@ -124,7 +195,8 @@ public class TestLanceExceptionMapper extends JerseyTest {
return new ResourceConfig()
.register(ErrorResource.class)
.register(LanceExceptionMapper.class)
- .register(JacksonFeature.class);
+ .register(JacksonFeature.withoutExceptionMappers())
+ .register(LanceRequestBodyReaderInterceptor.class);
}
@Test
@@ -132,7 +204,8 @@ public class TestLanceExceptionMapper extends JerseyTest {
try (Response response = target("error").request().put(Entity.json("{}")))
{
Assertions.assertEquals(405, response.getStatus());
Assertions.assertTrue(response.getAllowedMethods().contains("GET"));
- Assertions.assertEquals(13,
response.readEntity(ErrorResponse.class).getCode());
+ Assertions.assertEquals(
+ ErrorCode.INVALID_INPUT.getCode(),
response.readEntity(ErrorResponse.class).getCode());
}
}
@@ -140,7 +213,8 @@ public class TestLanceExceptionMapper extends JerseyTest {
void testUnsupportedMediaType() {
try (Response response =
target("error/consume").request().post(Entity.text("body"))) {
Assertions.assertEquals(415, response.getStatus());
- Assertions.assertEquals(13,
response.readEntity(ErrorResponse.class).getCode());
+ Assertions.assertEquals(
+ ErrorCode.INVALID_INPUT.getCode(),
response.readEntity(ErrorResponse.class).getCode());
}
}
@@ -148,6 +222,8 @@ public class TestLanceExceptionMapper extends JerseyTest {
void testNotAcceptable() {
try (Response response =
target("error").request(MediaType.TEXT_PLAIN).get()) {
Assertions.assertEquals(406, response.getStatus());
+ Assertions.assertEquals(
+ ErrorCode.INVALID_INPUT.getCode(),
response.readEntity(ErrorResponse.class).getCode());
}
}
@@ -158,12 +234,317 @@ public class TestLanceExceptionMapper extends JerseyTest
{
Assertions.assertEquals(
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
ErrorResponse entity = response.readEntity(ErrorResponse.class);
- Assertions.assertEquals("assertion failure", entity.getError());
+ Assertions.assertEquals("Internal server error", entity.getError());
Assertions.assertEquals("", entity.getInstance());
- Assertions.assertTrue(
- entity.getDetail().contains("java.lang.AssertionError: assertion
failure"));
- Assertions.assertTrue(
- entity.getDetail().contains("Caused by:
java.lang.IllegalStateException: root cause"));
+ Assertions.assertEquals("", entity.getDetail());
+ }
+ }
+
+ /** Verifies unexpected exceptions do not expose internal details in the
response. */
+ @Test
+ public void testInternalFailureDoesNotExposeException() {
+ try (Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "catalog.schema.table", new
RuntimeException("private-backend-detail"))) {
+ Assertions.assertEquals(500, response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals("Internal server error", error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
+ /** Verifies intentional protocol validation details remain available to
callers. */
+ @Test
+ public void testProtocolValidationDetailsArePreserved() {
+ try (Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "table",
+ new InvalidInputException("Invalid field", "field must be
positive", "table"))) {
+ Assertions.assertEquals(400, response.getStatus());
+ Assertions.assertEquals(
+ "field must be positive", ((ErrorResponse)
response.getEntity()).getDetail());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {400, 401, 403, 405, 406, 409, 415, 429})
+ void testFrameworkClientStatusAndHeadersArePreserved(int status) {
+ try (Response original =
+ Response.status(status)
+ .header("Allow", "GET")
+ .header("WWW-Authenticate", "Basic realm=\"lance\"")
+ .header("Retry-After", "3")
+ .entity("framework entity")
+ .build();
+ Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "table", new WebApplicationException("request rejected",
original))) {
+ Assertions.assertEquals(status, response.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
response.getMediaType());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals("request rejected", error.getError());
+ Assertions.assertEquals("table", error.getInstance());
+ Assertions.assertEquals("", error.getDetail());
+ Assertions.assertNotEquals(new InternalException("", "", "").getCode(),
error.getCode());
+ Assertions.assertEquals("GET", response.getHeaderString("Allow"));
+ Assertions.assertEquals(
+ "Basic realm=\"lance\"",
response.getHeaderString("WWW-Authenticate"));
+ Assertions.assertEquals("3", response.getHeaderString("Retry-After"));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "Content-Length", "Content-Encoding", "Content-Language",
"Content-Location",
+ "Content-Range", "Content-Disposition", "Transfer-Encoding", "ETag",
+ "Last-Modified", "Content-MD5", "Digest"
+ })
+ void testFrameworkEntityHeadersAreRemoved(String header) {
+ try (Response original =
+ Response.status(415).header(header,
"original-entity-metadata").entity("old").build();
+ Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "table", new WebApplicationException("request rejected",
original))) {
+ Assertions.assertNull(response.getHeaderString(header));
+ Assertions.assertEquals(415, response.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
response.getMediaType());
+ Assertions.assertEquals(
+ "request rejected", ((ErrorResponse)
response.getEntity()).getError());
+ }
+ }
+
+ @Test
+ void testMappedFrameworkEntityCanBeReadOverHttp() {
+ try (Response response = target("error/framework").request().get()) {
+ Assertions.assertEquals(415, response.getStatus());
+ Assertions.assertNull(response.getHeaderString("Content-Encoding"));
+ Assertions.assertEquals("3", response.getHeaderString("Retry-After"));
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
response.getMediaType());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals("request rejected", error.getError());
+ Assertions.assertEquals("table", error.getInstance());
+ }
+ }
+
+ @Test
+ void testFrameworkServerErrorDoesNotExposeDetails() {
+ try (Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "table", new WebApplicationException("private-backend-detail",
503))) {
+ Assertions.assertEquals(503, response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals("Internal server error", error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "missing",
+ "v1/table/catalog$schema$table/missing",
+ "v1/namespace/catalog$schema/missing"
+ })
+ void testUnknownRouteUsesUnsupportedEnvelope(String path) {
+ try (Response response =
target(path).request(MediaType.APPLICATION_JSON_TYPE).get()) {
+ Assertions.assertEquals(406, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(),
error.getCode());
+ Assertions.assertEquals("GET " + path, error.getInstance());
+ Assertions.assertTrue(error.getError().contains(error.getInstance()));
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"query", "create_index"})
+ void testUnimplementedOperationsAreClassifiedAsUnsupported(String operation)
{
+ String path = "v1/table/catalog$schema$table/" + operation;
+ try (Response response = target(path).request().post(Entity.json("{}"))) {
+ Assertions.assertEquals(406, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(),
error.getCode());
+ Assertions.assertEquals("POST " + path, error.getInstance());
+ Assertions.assertTrue(error.getError().contains(error.getInstance()));
+ Assertions.assertInstanceOf(
+ UnsupportedOperationException.class,
+ ErrorFactory.fromErrorCode(
+ error.getCode(), error.getError(), error.getDetail(),
error.getInstance()));
+ }
+ }
+
+ @Test
+ void testFramework404IsUnsupportedAndPreservesHeaders() {
+ try (Response original = Response.status(404).header("Retry-After",
"3").build();
+ Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "POST v1/table/table/query", new
WebApplicationException(original))) {
+ Assertions.assertEquals(406, response.getStatus());
+ Assertions.assertEquals("3", response.getHeaderString("Retry-After"));
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(),
error.getCode());
+ Assertions.assertTrue(error.getError().contains("POST
v1/table/table/query"));
+ }
+ }
+
+ @Test
+ void testValidationErrorsNeverIncludeStackTraces() {
+ IllegalArgumentException failure =
+ new IllegalArgumentException(
+ "Invalid column name", new
IllegalStateException("private-backend-detail"));
+ try (Response response = LanceExceptionMapper.toRESTResponse("table",
failure)) {
+ Assertions.assertEquals(400, response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertEquals("Invalid column name", error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
+ @Test
+ void testLogLevelsSeparateClientErrorsFromServerFailures() {
+ IllegalStateException internal = new
IllegalStateException("private-backend-detail");
+ WebApplicationException framework = new WebApplicationException("framework
failure", 503);
+ List<LogEvent> events =
+ captureMapperLogs(
+ () -> {
+ for (Exception failure :
+ new Exception[] {
+ new IllegalArgumentException("Invalid column name"),
+ new NoSuchTableException("Table missing"),
+ internal,
+ framework
+ }) {
+ try (Response ignored =
LanceExceptionMapper.toRESTResponse("table", failure)) {
+ // Collect logs from the same mapper entry point used by
operation handlers.
+ }
+ }
+ });
+ Assertions.assertEquals(
+ List.of(Level.INFO, Level.INFO, Level.ERROR, Level.ERROR),
+ events.stream().map(LogEvent::getLevel).toList());
+ Assertions.assertNull(events.get(0).getThrown());
+ Assertions.assertNull(events.get(1).getThrown());
+ Assertions.assertSame(internal, events.get(2).getThrown());
+ Assertions.assertSame(framework, events.get(3).getThrown());
+ }
+
+ @Test
+ void testInvalidPathParameterIsInvalidInput() {
+ try (Response response =
target("error/path/not-a-number").request().get()) {
+ Assertions.assertEquals(400, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertTrue(error.getError().contains("version"));
+ }
+ }
+
+ @Test
+ void testTypedEntityNotFoundErrorsKeepTheirCodes() {
+ for (Exception exception :
+ new Exception[] {
+ new TableNotFoundException("table absent", "", "table"),
+ new NamespaceNotFoundException("namespace absent", "", "namespace")
+ }) {
+ try (Response response = LanceExceptionMapper.toRESTResponse("instance",
exception)) {
+ Assertions.assertEquals(404, response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(
+ exception instanceof TableNotFoundException
+ ? ErrorCode.TABLE_NOT_FOUND.getCode()
+ : ErrorCode.NAMESPACE_NOT_FOUND.getCode(),
+ error.getCode());
+ }
+ }
+ }
+
+ @Test
+ void testBackendThrottlingReturns429() {
+ try (Response response =
+ LanceExceptionMapper.toRESTResponse(
+ "table", new ThrottlingException("Slow down", "retry later",
"table"))) {
+ Assertions.assertEquals(429, response.getStatus());
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorCode.THROTTLING.getCode(), error.getCode());
+ Assertions.assertEquals("retry later", error.getDetail());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"{", "[]",
"{\"mode\":{\"secret\":\"private-value\"}}"})
+ void testInvalidJsonUsesLanceEnvelope(String json) {
+ try (Response response =
target("error/json").request().post(Entity.json(json))) {
+ Assertions.assertEquals(400, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertEquals("Invalid JSON request", error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ }
+
+ @Test
+ void testValidJsonStillReachesResource() {
+ try (Response response =
+
target("error/json").request().post(Entity.json("{\"mode\":\"create\"}"))) {
+ Assertions.assertEquals(200, response.getStatus());
+ Assertions.assertEquals("create", response.readEntity(String.class));
+ }
+ }
+
+ @Test
+ void testReaderPreservesCauseAndDoesNotTranslateOtherFailures() throws
IOException {
+ ReaderInterceptorContext context =
Mockito.mock(ReaderInterceptorContext.class);
+ JsonParseException invalid = new JsonParseException(null, "private-value");
+ Mockito.when(context.proceed()).thenThrow(invalid);
+ LanceRequestBodyReaderInterceptor reader = new
LanceRequestBodyReaderInterceptor();
+ BadRequestException response =
+ Assertions.assertThrows(BadRequestException.class, () ->
reader.aroundReadFrom(context));
+ Assertions.assertSame(invalid, response.getCause());
+ Assertions.assertEquals("Invalid JSON request", response.getMessage());
+ IOException readFailure = new IOException("stream failed");
+ Mockito.doThrow(readFailure).when(context).proceed();
+ Assertions.assertSame(
+ readFailure,
+ Assertions.assertThrows(IOException.class, () ->
reader.aroundReadFrom(context)));
+ try (Response backend = LanceExceptionMapper.toRESTResponse("table",
invalid)) {
+ Assertions.assertEquals(500, backend.getStatus());
+ Assertions.assertEquals(
+ "Internal server error", ((ErrorResponse)
backend.getEntity()).getError());
+ }
+ }
+
+ private static List<LogEvent> captureMapperLogs(Runnable action) {
+ String name = LanceExceptionMapper.class.getName();
+ LoggerContext context =
+ (LoggerContext)
LogManager.getContext(LanceExceptionMapper.class.getClassLoader(), false);
+ AbstractConfiguration configuration = (AbstractConfiguration)
context.getConfiguration();
+ LoggerConfig previous = configuration.getLoggers().get(name);
+ List<LogEvent> events = new CopyOnWriteArrayList<>();
+ AbstractAppender appender =
+ new AbstractAppender("lanceMapperCapture", null, null, true,
Property.EMPTY_ARRAY) {
+ @Override
+ public void append(LogEvent event) {
+ events.add(event.toImmutable());
+ }
+ };
+ try {
+ appender.start();
+ configuration.addAppender(appender);
+ LoggerConfig logger = new LoggerConfig(name, Level.ALL, false);
+ logger.addAppender(appender, Level.ALL, null);
+ configuration.addLogger(name, logger);
+ context.updateLoggers();
+ action.run();
+ return events;
+ } finally {
+ configuration.removeLogger(name);
+ if (previous != null) {
+ configuration.addLogger(name, previous);
+ }
+ configuration.removeAppender(appender.getName());
+ appender.stop();
+ context.updateLoggers();
}
}
}
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java
index a80c70a06b..2947bbb3ee 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestLanceNamespaceOperations.java
@@ -38,6 +38,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.regex.Pattern;
+import java.util.stream.Stream;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Application;
@@ -47,6 +48,8 @@ import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.lance.common.ops.LanceTableOperations;
import org.apache.gravitino.lance.common.ops.NamespaceWrapper;
import org.apache.gravitino.lance.common.utils.LanceConstants;
+import org.apache.gravitino.lance.service.LanceExceptionMapper;
+import org.apache.gravitino.lance.service.LanceRequestBodyReaderInterceptor;
import org.apache.gravitino.rest.RESTUtils;
import org.glassfish.jersey.CommonProperties;
import org.glassfish.jersey.internal.inject.AbstractBinder;
@@ -56,6 +59,10 @@ 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.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.lance.namespace.errors.ErrorCode;
import org.lance.namespace.errors.TableNotFoundException;
import org.lance.namespace.model.AlterColumnsEntry;
import org.lance.namespace.model.AlterTableAlterColumnsRequest;
@@ -120,6 +127,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
JacksonJaxbJsonProvider provider = new JacksonJaxbJsonProvider();
provider.setMapper(mapper);
resourceConfig.register(provider);
+ resourceConfig.register(LanceExceptionMapper.class);
+ resourceConfig.register(LanceRequestBodyReaderInterceptor.class);
resourceConfig.register(LanceNamespaceOperations.class);
resourceConfig.register(org.apache.gravitino.lance.service.rest.LanceTableOperations.class);
resourceConfig.register(
@@ -140,6 +149,61 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
when(namespaceWrapper.asTableOps()).thenReturn(tableOps);
}
+ @Test
+ void testUnsupportedCreateContentTypeReturns415() {
+ try (Response response =
+
target("/v1/table/catalog$schema$table/create").request().post(Entity.json("{}")))
{
+ Assertions.assertEquals(415, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals("", error.getDetail());
+ Assertions.assertNotEquals(18, error.getCode());
+ }
+ }
+
+ @Test
+ void testInvalidListLimitReturns400() {
+ try (Response response =
+ target("/v1/namespace/catalog$schema/table/list")
+ .queryParam("limit", "x")
+ .request()
+ .get()) {
+ Assertions.assertEquals(400, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertTrue(error.getError().contains("limit"));
+ Assertions.assertEquals("", error.getDetail());
+ Assertions.assertNotEquals(18, error.getCode());
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("nullRequestInputs")
+ void testNullRequestInputsReturn400(String path, String json, String
expectedMessage) {
+ Mockito.clearInvocations(namespaceWrapper, namespaceOps, tableOps);
+ try (Response response = target(path).request().post(Entity.json(json))) {
+ Assertions.assertEquals(400, response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.INVALID_INPUT.getCode(),
error.getCode());
+ Assertions.assertTrue(error.getError().contains(expectedMessage),
error.getError());
+ Assertions.assertEquals("", error.getDetail());
+ }
+ Mockito.verifyNoInteractions(namespaceWrapper, namespaceOps, tableOps);
+ }
+
+ private static Stream<Arguments> nullRequestInputs() {
+ String namespace = "/v1/namespace/catalog$schema/";
+ String table = "/v1/table/catalog$schema$table/";
+ return Stream.of(
+ Arguments.of(namespace + "create", "null", "Request body"),
+ Arguments.of(namespace + "drop", "null", "Request body"),
+ Arguments.of(table + "declare", "null", "Request body"),
+ Arguments.of(table + "drop_columns", "null", "Request body"),
+ Arguments.of(table + "alter_columns", "null", "Request body"),
+ Arguments.of(table + "drop_columns", "{\"columns\":null}", "columns"),
+ Arguments.of(table + "alter_columns", "{\"alterations\":null}",
"alterations"),
+ Arguments.of(table + "alter_columns", "{\"alterations\":[null]}",
"alterations[0]"),
+ Arguments.of(table + "drop_columns", "{\"columns\":[null]}", "cannot
be blank"));
+ }
+
@Test
public void testListNamespaces() {
String namespaceId = "ns1.ns2";
@@ -208,10 +272,9 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
Assertions.assertEquals(18, errorResp.getCode());
- Assertions.assertEquals("Test exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
Assertions.assertEquals("ns1.ns2", errorResp.getInstance());
- Assertions.assertNotNull(errorResp.getDetail());
- Assertions.assertTrue(errorResp.getDetail().contains("Test exception"));
// root endpoint should use explicit root identifier instead of delimiter
in error instance
resp =
@@ -262,18 +325,22 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
Assertions.assertEquals(18, errorResp.getCode());
- Assertions.assertEquals("Test exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
public void testDescribeNamespaceOnRootUnsupported() {
Mockito.clearInvocations(namespaceOps);
- Response resp =
-
target("/v1/namespace/describe").request(MediaType.APPLICATION_JSON_TYPE).post(null);
-
- Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
resp.getStatus());
- Mockito.verifyNoInteractions(namespaceOps);
+ try (Response resp =
+
target("/v1/namespace/describe").request(MediaType.APPLICATION_JSON_TYPE).post(null))
{
+ Assertions.assertEquals(Response.Status.NOT_ACCEPTABLE.getStatusCode(),
resp.getStatus());
+ ErrorResponse error = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorCode.UNSUPPORTED.getCode(),
error.getCode());
+ Assertions.assertEquals("POST v1/namespace/describe",
error.getInstance());
+ Mockito.verifyNoInteractions(namespaceOps);
+ }
}
@Test
@@ -321,7 +388,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
Assertions.assertEquals(18, errorResp.getCode());
- Assertions.assertEquals("Test exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -393,7 +461,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
Assertions.assertEquals(18, errorResp.getCode());
- Assertions.assertEquals("Test exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -457,7 +526,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals("Runtime exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -513,7 +583,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals("Runtime exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -620,7 +691,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals("Runtime exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -677,7 +749,8 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals("Runtime exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
@Test
@@ -999,6 +1072,7 @@ public class TestLanceNamespaceOperations extends
JerseyTest {
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
- Assertions.assertEquals("Runtime exception", errorResp.getError());
+ Assertions.assertEquals("Internal server error", errorResp.getError());
+ Assertions.assertEquals("", errorResp.getDetail());
}
}
