This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-391493c3-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit b65a5f52dd1ed31c4de06822b2372e70ef8ab619 Author: mchades <[email protected]> AuthorDate: Tue Sep 1 18:32:33 2026 +0800 [#12769] fix(server): handle null request bodies in createView (#12770) ### What changes were proposed in this pull request? - Guard `ViewOperations.createView` against a null request before accessing any request fields or invoking `request.validate()`. - Route the validation failure through the existing view exception handler so the endpoint returns a structured HTTP 400 response. - Cache the view name after the null guard and reuse it for logging, identifier construction, and catch-path error handling. - Add `TestViewOperations` coverage for an empty/null request entity. - Verify that malformed JSON continues to use the existing JSON exception mappers and returns HTTP 400. ### Why are the changes needed? An empty create-view request currently causes `ViewOperations.createView` to dereference `request` before null validation. This produces an unhandled HTTP 500 response with an empty body instead of the structured client-error response used by other REST endpoints. The change follows the existing null-request handling pattern in `MetalakeOperations.createMetalake`. Fix: #12769 ### Does this PR introduce _any_ user-facing change? Yes. A create-view request with an empty or null body now returns a structured HTTP 400 response instead of an empty HTTP 500 response. There are no API schema or configuration changes. Valid requests and malformed JSON handling are unchanged. ### How was this patch tested? Before the fix, the null-request regression test failed with: ```text expected: <400> but was: <500> ``` The malformed-JSON regression test passed independently, confirming that the existing mapper behavior was not the cause. After the fix: ```shell ./gradlew :server:test \ --tests org.apache.gravitino.server.web.rest.TestViewOperations \ -PskipITs --no-daemon ``` Result: 9 tests passed, 0 failures, 0 errors. Additional checks: ```shell ./gradlew :server:spotlessCheck --no-daemon git diff --check ``` Both checks passed. --- .../server/web/rest/CatalogOperations.java | 8 ++++ .../server/web/rest/FilesetOperations.java | 8 ++++ .../server/web/rest/FunctionOperations.java | 8 ++++ .../gravitino/server/web/rest/JobOperations.java | 7 ++++ .../server/web/rest/MetalakeOperations.java | 7 ++++ .../gravitino/server/web/rest/ModelOperations.java | 29 +++++++++++++ .../server/web/rest/PolicyOperations.java | 7 ++++ .../server/web/rest/SchemaOperations.java | 8 ++++ .../gravitino/server/web/rest/TableOperations.java | 8 ++++ .../gravitino/server/web/rest/TagOperations.java | 7 ++++ .../gravitino/server/web/rest/TopicOperations.java | 8 ++++ .../gravitino/server/web/rest/ViewOperations.java | 29 +++++++++---- .../server/web/rest/BaseOperationsTest.java | 13 ++++++ .../server/web/rest/TestCatalogOperations.java | 11 +++++ .../server/web/rest/TestFilesetOperations.java | 11 +++++ .../server/web/rest/TestFunctionOperations.java | 12 ++++++ .../server/web/rest/TestJobOperations.java | 12 ++++++ .../server/web/rest/TestMetalakeOperations.java | 11 +++++ .../server/web/rest/TestModelOperations.java | 40 ++++++++++++++++++ .../server/web/rest/TestPolicyOperations.java | 12 ++++++ .../server/web/rest/TestSchemaOperations.java | 12 ++++++ .../server/web/rest/TestTableOperations.java | 11 +++++ .../server/web/rest/TestTagOperations.java | 12 ++++++ .../server/web/rest/TestTopicOperations.java | 11 +++++ .../server/web/rest/TestViewOperations.java | 49 ++++++++++++++++++++++ 25 files changed, 344 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java index 7c66b4a852..764fd8e0c0 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java @@ -293,6 +293,14 @@ public class CatalogOperations { String catalogName, CatalogUpdatesRequest request) { LOG.info("Received alter catalog request for catalog: {}.{}", metalakeName, catalogName); + if (request == null) { + return ExceptionHandlers.handleCatalogException( + OperationType.ALTER, + catalogName, + metalakeName, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java index eda03954fd..1e534675df 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java @@ -289,6 +289,14 @@ public class FilesetOperations { @PathParam("fileset") @AuthorizationMetadata(type = Entity.EntityType.FILESET) String fileset, FilesetUpdatesRequest request) { LOG.info("Received alter fileset request: {}.{}.{}.{}", metalake, catalog, schema, fileset); + if (request == null) { + return ExceptionHandlers.handleFilesetException( + OperationType.ALTER, + fileset, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java index 70b7b9e2ae..adc62d937c 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java @@ -252,6 +252,14 @@ public class FunctionOperations { String function, FunctionUpdatesRequest request) { LOG.info("Received alter function request: {}.{}.{}.{}", metalake, catalog, schema, function); + if (request == null) { + return ExceptionHandlers.handleFunctionException( + OperationType.ALTER, + function, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java index 77fe02e9d7..a1b456ed98 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java @@ -253,6 +253,13 @@ public class JobOperations { JobTemplateUpdatesRequest request) { LOG.info( "Received request to alter job template: {} in metalake: {}", jobTemplateName, metalake); + if (request == null) { + return ExceptionHandlers.handleJobTemplateException( + OperationType.ALTER, + jobTemplateName, + metalake, + new IllegalArgumentException("Request body cannot be null")); + } try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java index a02eea506b..7cce216e78 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java @@ -218,6 +218,13 @@ public class MetalakeOperations { String metalakeName, MetalakeUpdatesRequest updatesRequest) { LOG.info("Received alter metalake request for metalake: {}", metalakeName); + if (updatesRequest == null) { + return ExceptionHandlers.handleMetalakeException( + OperationType.ALTER, + metalakeName, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java index 3c5dd63946..f4d7741f49 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java @@ -419,6 +419,13 @@ public class ModelOperations { ModelVersionLinkRequest request) { LOG.info("Received link model version request: {}.{}.{}.{}", metalake, catalog, schema, model); NameIdentifier modelId = NameIdentifierUtil.ofModel(metalake, catalog, schema, model); + if (request == null) { + return ExceptionHandlers.handleModelException( + OperationType.LINK, + model, + schema, + new IllegalArgumentException("Request body cannot be null")); + } try { return Utils.doAs( @@ -573,6 +580,13 @@ public class ModelOperations { schema, model, version); + if (request == null) { + return ExceptionHandlers.handleModelException( + OperationType.ALTER, + versionString(model, version), + schema, + new IllegalArgumentException("Request body cannot be null")); + } try { NameIdentifier modelId = NameIdentifierUtil.ofModel(metalake, catalog, schema, model); @@ -627,6 +641,13 @@ public class ModelOperations { schema, model, alias); + if (request == null) { + return ExceptionHandlers.handleModelException( + OperationType.ALTER, + aliasString(model, alias), + schema, + new IllegalArgumentException("Request body cannot be null")); + } try { NameIdentifier modelId = NameIdentifierUtil.ofModel(metalake, catalog, schema, model); @@ -673,6 +694,14 @@ public class ModelOperations { @PathParam("model") @AuthorizationMetadata(type = Entity.EntityType.MODEL) String model, ModelUpdatesRequest request) { LOG.info("Received alter model request: {}.{}.{}.{}", metalake, catalog, schema, model); + if (request == null) { + return ExceptionHandlers.handleModelException( + OperationType.ALTER, + model, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java index 5e73e91236..54be773aa0 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java @@ -205,6 +205,13 @@ public class PolicyOperations { @PathParam("policy") @AuthorizationMetadata(type = Entity.EntityType.POLICY) String name, PolicyUpdatesRequest request) { LOG.info("Received alter policy request for policy: {} under metalake: {}", name, metalake); + if (request == null) { + return ExceptionHandlers.handlePolicyException( + OperationType.ALTER, + name, + metalake, + new IllegalArgumentException("Request body cannot be null")); + } try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java index 1b8167331a..70ff20d448 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java @@ -207,6 +207,14 @@ public class SchemaOperations { @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, SchemaUpdatesRequest request) { LOG.info("Received alter schema request: {}.{}.{}", metalake, catalog, schema); + if (request == null) { + return ExceptionHandlers.handleSchemaException( + OperationType.ALTER, + schema, + catalog, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java index fa89ee0259..9d653e75d2 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java @@ -212,6 +212,14 @@ public class TableOperations { @PathParam("table") @AuthorizationMetadata(type = Entity.EntityType.TABLE) String table, TableUpdatesRequest request) { LOG.info("Received alter table request: {}.{}.{}.{}", metalake, catalog, schema, table); + if (request == null) { + return ExceptionHandlers.handleTableException( + OperationType.ALTER, + table, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java index c69b3a3432..7886856173 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java @@ -207,6 +207,13 @@ public class TagOperations { @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG) String name, TagUpdatesRequest request) { LOG.info("Received alter tag request for tag: {} under metalake: {}", name, metalake); + if (request == null) { + return ExceptionHandlers.handleTagException( + OperationType.ALTER, + name, + metalake, + new IllegalArgumentException("Request body cannot be null")); + } try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java index beedcac793..214e184ceb 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java @@ -208,6 +208,14 @@ public class TopicOperations { @PathParam("topic") @AuthorizationMetadata(type = Entity.EntityType.TOPIC) String topic, TopicUpdatesRequest request) { LOG.info("Received alter topic request: {}.{}.{}.{}", metalake, catalog, schema, topic); + if (request == null) { + return ExceptionHandlers.handleTopicException( + OperationType.ALTER, + topic, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java index 7d2cfd11dd..77e5fbc938 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/ViewOperations.java @@ -99,15 +99,23 @@ public class ViewOperations { @PathParam("catalog") String catalog, @PathParam("schema") String schema, ViewCreateRequest request) { - LOG.info( - "Received create view request: {}.{}.{}.{}", metalake, catalog, schema, request.getName()); + if (request == null) { + LOG.warn("Received create view request with null request body"); + return ExceptionHandlers.handleViewException( + OperationType.CREATE, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String viewName = request.getName(); + LOG.info("Received create view request: {}.{}.{}.{}", metalake, catalog, schema, viewName); try { return Utils.doAs( httpRequest, () -> { request.validate(); - NameIdentifier ident = - NameIdentifierUtil.ofView(metalake, catalog, schema, request.getName()); + NameIdentifier ident = NameIdentifierUtil.ofView(metalake, catalog, schema, viewName); View view = dispatcher.createView( @@ -119,13 +127,12 @@ public class ViewOperations { request.getDefaultSchema(), request.getProperties()); Response response = Utils.ok(new ViewResponse(DTOConverters.toDTO(view))); - LOG.info("View created: {}.{}.{}.{}", metalake, catalog, schema, request.getName()); + LOG.info("View created: {}.{}.{}.{}", metalake, catalog, schema, viewName); return response; }); } catch (Exception e) { - return ExceptionHandlers.handleViewException( - OperationType.CREATE, request.getName(), schema, e); + return ExceptionHandlers.handleViewException(OperationType.CREATE, viewName, schema, e); } } @@ -167,6 +174,14 @@ public class ViewOperations { @PathParam("view") String view, ViewUpdatesRequest request) { LOG.info("Received alter view request: {}.{}.{}.{}", metalake, catalog, schema, view); + if (request == null) { + return ExceptionHandlers.handleViewException( + OperationType.ALTER, + view, + schema, + new IllegalArgumentException("Request body cannot be null")); + } + try { return Utils.doAs( httpRequest, diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/BaseOperationsTest.java b/server/src/test/java/org/apache/gravitino/server/web/rest/BaseOperationsTest.java index 97933126d6..b603ae5c58 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/BaseOperationsTest.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/BaseOperationsTest.java @@ -18,10 +18,14 @@ package org.apache.gravitino.server.web.rest; import java.io.IOException; +import javax.ws.rs.core.Response; +import org.apache.gravitino.dto.responses.ErrorConstants; +import org.apache.gravitino.dto.responses.ErrorResponse; import org.apache.gravitino.server.ServerConfig; import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider; import org.glassfish.jersey.test.JerseyTest; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; public abstract class BaseOperationsTest extends JerseyTest { @@ -35,4 +39,13 @@ public abstract class BaseOperationsTest extends JerseyTest { public static void stop() throws IOException { GravitinoAuthorizerProvider.getInstance().close(); } + + static void assertNullRequestBodyRejected(Response response) { + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + ErrorResponse errorResponse = response.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), errorResponse.getType()); + Assertions.assertTrue(errorResponse.getMessage().contains("Request body cannot be null")); + } } diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java index 4298c5d2c7..c158dacdbf 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java @@ -408,6 +408,17 @@ public class TestCatalogOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); } + @Test + public void testAlterCatalogWithNullRequest() { + Response resp = + target("/metalakes/metalake1/catalogs/catalog1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAlterCatalog() { TestCatalog catalog = buildCatalog("metalake1", "catalog2"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java index 8bfb5bd6a5..4c67ba3132 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java @@ -397,6 +397,17 @@ public class TestFilesetOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp4.getType()); } + @Test + public void testAlterFilesetWithNullRequest() { + Response resp = + target(filesetPath(metalake, catalog, schema) + "fileset1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testRenameFileset() { FilesetUpdateRequest req = new FilesetUpdateRequest.RenameFilesetRequest("new name"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java index 65cf114ab1..5aad775be8 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java @@ -407,6 +407,18 @@ public class TestFunctionOperations extends BaseOperationsTest { Assertions.assertEquals(FunctionType.TABLE, funcResp.getFunction().functionType()); } + @Test + public void testAlterFunctionWithNullRequest() { + Response resp = + target(functionPath()) + .path("func1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAlterFunction() { NameIdentifier funcId = NameIdentifierUtil.ofFunction(metalake, catalog, schema, "func1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java index 651ab955da..52b072b962 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java @@ -511,6 +511,18 @@ public class TestJobOperations extends JerseyTest { Assertions.assertEquals(InUseException.class.getSimpleName(), errorResp4.getType()); } + @Test + public void testAlterJobTemplateWithNullRequest() { + Response resp = + target(jobTemplatePath()) + .path("shell_template_1") + .request(APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], APPLICATION_JSON_TYPE)); + + BaseOperationsTest.assertNullRequestBodyRejected(resp); + } + @Test public void testAlterJobTemplate() { String templateName = "shell_template_1"; diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java index 0cbf2594c8..ae7eb39133 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java @@ -252,6 +252,17 @@ public class TestMetalakeOperations extends BaseOperationsTest { Assertions.assertTrue(errorResponse.getMessage().contains("Request body cannot be null")); } + @Test + public void testAlterMetalakeWithNullRequest() { + Response resp = + target("/metalakes/test") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testLoadMetalake() { String metalakeName = "test"; diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java index 12c1695768..5f26d8e9d1 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java @@ -270,6 +270,46 @@ public class TestModelOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp1.getType()); } + @Test + public void testModelLinkAndAlterOperationsWithNullRequests() { + Response linkVersionResponse = + target(modelPath()) + .path("model1") + .path("versions") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(null, MediaType.APPLICATION_JSON_TYPE)); + assertNullRequestBodyRejected(linkVersionResponse); + + Response alterModelResponse = + target(modelPath()) + .path("model1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + assertNullRequestBodyRejected(alterModelResponse); + + Response alterVersionResponse = + target(modelPath()) + .path("model1") + .path("versions") + .path("0") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + assertNullRequestBodyRejected(alterVersionResponse); + + Response alterVersionByAliasResponse = + target(modelPath()) + .path("model1") + .path("aliases") + .path("alias1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + assertNullRequestBodyRejected(alterVersionByAliasResponse); + } + @Test public void testRegisterModel() { NameIdentifier modelId = NameIdentifierUtil.ofModel(metalake, catalog, schema, "model1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java index af8056b666..382e1f9d8b 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java @@ -504,6 +504,18 @@ public class TestPolicyOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp1.getType()); } + @Test + public void testAlterPolicyWithNullRequest() { + Response resp = + target(policyPath(metalake)) + .path("policy1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAlterPolicy() { ImmutableMap<String, Object> contentFields = ImmutableMap.of("target_file_size_bytes", 1000); 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 8981e5e29d..489b0070f6 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 @@ -38,6 +38,7 @@ import java.io.IOException; import java.time.Instant; import java.util.Map; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.client.Entity; import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -350,6 +351,17 @@ public class TestSchemaOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp2.getType()); } + @Test + public void testAlterSchemaWithNullRequest() { + Response resp = + target("/metalakes/" + metalake + "/catalogs/" + catalog + "/schemas/schema1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAlterSchema() { SchemaUpdateRequest setReq = new SchemaUpdateRequest.SetSchemaPropertyRequest("key2", "value2"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java index e2b12910a8..9c41650d48 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java @@ -567,6 +567,17 @@ public class TestTableOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp2.getType()); } + @Test + public void testAlterTableWithNullRequest() { + Response resp = + target(tablePath(metalake, catalog, schema) + "table1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testRenameTable() { TableUpdateRequest.RenameTableRequest req = new TableUpdateRequest.RenameTableRequest("table2"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java index 14c19791ba..4dac1582a5 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java @@ -398,6 +398,18 @@ public class TestTagOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp1.getType()); } + @Test + public void testAlterTagWithNullRequest() { + Response resp = + target(tagPath(metalake)) + .path("tag1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAlterTag() { TagEntity newTag = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java index f74e49feb4..67c9cdbbb4 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java @@ -303,6 +303,17 @@ public class TestTopicOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp3.getType()); } + @Test + public void testAlterTopicWithNullRequest() { + Response resp = + target(topicPath(metalake, catalog, schema) + "/topic1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testSetTopicProperties() { TopicUpdateRequest req = new TopicUpdateRequest.SetTopicPropertyRequest("key1", "value1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestViewOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestViewOperations.java index 2bd2c0df7e..c82253c497 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestViewOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestViewOperations.java @@ -65,6 +65,9 @@ import org.apache.gravitino.rel.SQLRepresentation; import org.apache.gravitino.rel.View; import org.apache.gravitino.rel.ViewChange; import org.apache.gravitino.rest.RESTUtils; +import org.apache.gravitino.server.web.mapper.JsonMappingExceptionMapper; +import org.apache.gravitino.server.web.mapper.JsonParseExceptionMapper; +import org.apache.gravitino.server.web.mapper.JsonProcessingExceptionMapper; import org.glassfish.jersey.internal.inject.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.test.TestProperties; @@ -123,6 +126,9 @@ public class TestViewOperations extends BaseOperationsTest { .to(HttpServletRequest.class); } }); + resourceConfig.register(JsonProcessingExceptionMapper.class); + resourceConfig.register(JsonParseExceptionMapper.class); + resourceConfig.register(JsonMappingExceptionMapper.class); return resourceConfig; } @@ -296,6 +302,49 @@ public class TestViewOperations extends BaseOperationsTest { Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResp3.getCode()); } + @Test + public void testCreateViewWithNullRequest() { + Response resp = + target(viewPath(metalake, catalog, schema)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept(VND_V1_JSON) + .post(Entity.entity(null, VND_V1_JSON)); + + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); + ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), errorResponse.getType()); + Assertions.assertTrue(errorResponse.getMessage().contains("Request body cannot be null")); + } + + @Test + public void testCreateViewWithMalformedJson() { + Response resp = + target(viewPath(metalake, catalog, schema)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept(VND_V1_JSON) + .post(Entity.entity("{", VND_V1_JSON)); + + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); + ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), errorResponse.getType()); + Assertions.assertTrue(errorResponse.getMessage().contains("Malformed json request")); + } + + @Test + public void testAlterViewWithNullRequest() { + Response resp = + target(viewPath(metalake, catalog, schema) + "/view1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept(VND_V1_JSON) + .put(Entity.entity(new byte[0], VND_V1_JSON)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testRenameView() { ViewUpdateRequest req = new ViewUpdateRequest.RenameViewRequest("view2");
