This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new b0982cba52 [#14581] fix(lance-rest): validate registration and
preserve routing errors (#14583)
b0982cba52 is described below
commit b0982cba52e62c30fc3b72d15288f8e310c26216
Author: Qi Yu <[email protected]>
AuthorDate: Wed Nov 4 09:23:30 2026 +0800
[#14581] fix(lance-rest): validate registration and preserve routing errors
(#14583)
### What changes were proposed in this pull request?
Preserve Jersey 405/406/415 routing statuses in Lance REST with the
invalid-input error envelope and retain Allow for 405. Validate a
registration body and its location before constructing properties or
accessing the namespace backend.
### Why are the changes needed?
These invalid requests currently fall into internal-error handling and
return HTTP 500/code 18.
Fix: #14581
### Does this PR introduce _any_ user-facing change?
Unsupported methods/media types return their routing statuses. Missing
bodies/locations return HTTP 400/code 13. Valid registrations and
existing authentication/error-detail behavior are preserved. No protocol
dependency upgrade or storage change.
### How was this patch tested?
- All 12 TestLanceExceptionMapper and TestRegisterTableValidation tests
passed.
- Real Jersey routing checks cover 405 with Allow, 406, and 415.
- Registration tests verify invalid requests do not call the backend and
valid locations do reach it.
- Existing internal-error redaction and authentication response tests
pass.
- Reverting the two production files reproduces the new regression
failures.
- Module Spotless checks and git diff --check passed.
- No external Lance/Spark backend integration tests were run.
---
.../lance/service/LanceExceptionMapper.java | 23 ++++++
.../lance/service/rest/LanceTableOperations.java | 6 +-
.../lance/service/TestLanceExceptionMapper.java | 42 +++++++++++
.../service/rest/TestRegisterTableValidation.java | 86 ++++++++++++++++++++++
4 files changed, 154 insertions(+), 3 deletions(-)
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 daad886540..648abbe402 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,6 +20,11 @@ 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.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
@@ -77,6 +82,24 @@ public class LanceExceptionMapper implements
ExceptionMapper<Throwable> {
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(), errorDetail(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();
+ }
+ }
+
LanceNamespaceException lanceException =
ex instanceof LanceNamespaceException
? (LanceNamespaceException) ex
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 5eb2f731ed..fb763de1fb 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
@@ -333,9 +333,9 @@ public class LanceTableOperations {
// No specific fields to validate for now
}
- private void validateRegisterTableRequest(
- @SuppressWarnings("unused") RegisterTableRequest request) {
- // No specific fields to validate for now
+ private void validateRegisterTableRequest(RegisterTableRequest request) {
+ Preconditions.checkArgument(request != null, "Request body must not be
null");
+ Preconditions.checkArgument(request.getLocation() != null, "location must
not be null");
}
private void validateDeregisterTableRequest(
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 42c4faac2f..2074225e6e 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
@@ -19,8 +19,12 @@
package org.apache.gravitino.lance.service;
import java.io.IOException;
+import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
+import javax.ws.rs.POST;
import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@@ -49,11 +53,25 @@ public class TestLanceExceptionMapper extends JerseyTest {
* @return never returns normally
*/
@GET
+ @Produces(MediaType.APPLICATION_JSON)
public String fail() {
AssertionError error = new AssertionError("assertion failure");
error.initCause(new IllegalStateException("root cause"));
throw error;
}
+
+ /**
+ * Accepts JSON to exercise unsupported request media types.
+ *
+ * @param body the request body
+ * @return the accepted body
+ */
+ @POST
+ @Path("consume")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public String consume(String body) {
+ return body;
+ }
}
/**
@@ -75,6 +93,30 @@ public class TestLanceExceptionMapper extends JerseyTest {
.register(JacksonFeature.class);
}
+ @Test
+ void testMethodNotAllowed() {
+ 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());
+ }
+ }
+
+ @Test
+ 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());
+ }
+ }
+
+ @Test
+ void testNotAcceptable() {
+ try (Response response =
target("error").request(MediaType.TEXT_PLAIN).get()) {
+ Assertions.assertEquals(406, response.getStatus());
+ }
+ }
+
/** Verifies that an uncaught error is converted to a Lance internal error
response. */
@Test
public void testErrorResponse() {
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestRegisterTableValidation.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestRegisterTableValidation.java
new file mode 100644
index 0000000000..2db9c699f2
--- /dev/null
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestRegisterTableValidation.java
@@ -0,0 +1,86 @@
+/*
+ * 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.rest;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.lance.common.ops.NamespaceWrapper;
+import org.junit.jupiter.api.Test;
+import org.lance.namespace.model.ErrorResponse;
+import org.lance.namespace.model.RegisterTableRequest;
+import org.lance.namespace.model.RegisterTableResponse;
+
+class TestRegisterTableValidation {
+ @Test
+ void testValidLocationReachesBackend() {
+ NamespaceWrapper backend = mock(NamespaceWrapper.class,
RETURNS_DEEP_STUBS);
+ RegisterTableRequest request = new RegisterTableRequest();
+ request.setLocation("s3://bucket/existing.lance");
+ when(backend
+ .asTableOps()
+ .registerTable(
+ eq("cat$schema$table"),
+ eq("create"),
+ eq("$"),
+ argThat(
+ properties ->
"s3://bucket/existing.lance".equals(properties.get("location")))))
+ .thenReturn(new RegisterTableResponse());
+ try (Response response =
+ new LanceTableOperations(backend).registerTable("cat$schema$table",
"$", null, request)) {
+ assertEquals(200, response.getStatus());
+ verify(backend.asTableOps())
+ .registerTable(
+ eq("cat$schema$table"),
+ eq("create"),
+ eq("$"),
+ argThat(
+ properties ->
"s3://bucket/existing.lance".equals(properties.get("location"))));
+ }
+ }
+
+ @Test
+ void testMissingLocation() {
+ NamespaceWrapper backend = mock(NamespaceWrapper.class);
+ LanceTableOperations resource = new LanceTableOperations(backend);
+ try (Response response =
+ resource.registerTable("cat$schema$table", "$", null, new
RegisterTableRequest())) {
+ assertEquals(400, response.getStatus());
+ assertEquals(13, ((ErrorResponse) response.getEntity()).getCode());
+ verifyNoInteractions(backend);
+ }
+ }
+
+ @Test
+ void testMissingBody() {
+ NamespaceWrapper backend = mock(NamespaceWrapper.class);
+ try (Response response =
+ new LanceTableOperations(backend).registerTable("cat$schema$table",
"$", null, null)) {
+ assertEquals(400, response.getStatus());
+ verifyNoInteractions(backend);
+ }
+ }
+}
