This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 7cf955cf7f [Cherry-pick to branch-1.3] [#12657] fix(common): Validate
statistic entries in PartitionStatisticsUpdateDTO (#12658) (#12665)
7cf955cf7f is described below
commit 7cf955cf7f83505865431658dadcbf8159bbf5cf
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Aug 28 10:07:14 2026 +0800
[Cherry-pick to branch-1.3] [#12657] fix(common): Validate statistic
entries in PartitionStatisticsUpdateDTO (#12658) (#12665)
**Cherry-pick Information:**
- Original commit: 4456552430b07c394fb9d1582ccb303bce55bda5
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: YangJie <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
---
.../dto/stats/PartitionStatisticsUpdateDTO.java | 7 ++
.../stats/TestPartitionStatisticsUpdateDTO.java | 81 ++++++++++++++++++++++
.../server/web/rest/TestStatisticOperations.java | 41 +++++++++++
3 files changed, 129 insertions(+)
diff --git
a/common/src/main/java/org/apache/gravitino/dto/stats/PartitionStatisticsUpdateDTO.java
b/common/src/main/java/org/apache/gravitino/dto/stats/PartitionStatisticsUpdateDTO.java
index edaf035e28..3d33c5a306 100644
---
a/common/src/main/java/org/apache/gravitino/dto/stats/PartitionStatisticsUpdateDTO.java
+++
b/common/src/main/java/org/apache/gravitino/dto/stats/PartitionStatisticsUpdateDTO.java
@@ -73,6 +73,13 @@ public class PartitionStatisticsUpdateDTO implements
PartitionStatisticsUpdate {
StringUtils.isNotBlank(partitionName), "\"partitionName\" must not be
null or empty");
Preconditions.checkArgument(
statistics != null && !statistics.isEmpty(), "\"statistics\" must not
be null or empty");
+ statistics.forEach(
+ (name, value) -> {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(name), "statistic \"name\" must not be
null or empty");
+ Preconditions.checkArgument(
+ value != null, "statistic \"value\" for '%s' must not be null",
name);
+ });
}
/**
diff --git
a/common/src/test/java/org/apache/gravitino/dto/stats/TestPartitionStatisticsUpdateDTO.java
b/common/src/test/java/org/apache/gravitino/dto/stats/TestPartitionStatisticsUpdateDTO.java
new file mode 100644
index 0000000000..b1486f5d3b
--- /dev/null
+++
b/common/src/test/java/org/apache/gravitino/dto/stats/TestPartitionStatisticsUpdateDTO.java
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.dto.stats;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.gravitino.dto.requests.PartitionStatisticsUpdateRequest;
+import org.apache.gravitino.json.JsonUtils;
+import org.apache.gravitino.stats.StatisticValue;
+import org.apache.gravitino.stats.StatisticValues;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestPartitionStatisticsUpdateDTO {
+
+ @Test
+ public void testValidateAcceptsStatistics() {
+ PartitionStatisticsUpdateDTO dto =
+ PartitionStatisticsUpdateDTO.of(
+ "p1", ImmutableMap.of("custom-k", StatisticValues.longValue(1L)));
+
+ Assertions.assertEquals("p1", dto.partitionName());
+ Assertions.assertEquals(1, dto.statistics().size());
+ }
+
+ @Test
+ public void testValidateRejectsNullStatisticValue() {
+ Map<String, StatisticValue<?>> statistics = new HashMap<>();
+ statistics.put("custom-k", null);
+
+ IllegalArgumentException e =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PartitionStatisticsUpdateDTO.of("p1", statistics));
+
+ Assertions.assertTrue(
+ e.getMessage().contains("custom-k"), () -> "Unexpected message: " +
e.getMessage());
+ }
+
+ @Test
+ public void testValidateRejectsBlankStatisticName() {
+ Map<String, StatisticValue<?>> statistics = new HashMap<>();
+ statistics.put(" ", StatisticValues.longValue(1L));
+
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
PartitionStatisticsUpdateDTO.of("p1", statistics));
+ }
+
+ @Test
+ public void testRequestWithNullStatisticValueIsRejected() throws
JsonProcessingException {
+ // Jackson's MapDeserializer does not invoke the contentUsing deserializer
for a VALUE_NULL
+ // content token, it uses getNullValue(), so a top-level JSON null lands
in the map and only
+ // validate() can catch it. Any mapper reproduces that; no registered
module is involved.
+ String json =
"{\"updates\":[{\"partitionName\":\"p1\",\"statistics\":{\"custom-k\":null}}]}";
+ PartitionStatisticsUpdateRequest request =
+ JsonUtils.objectMapper().readValue(json,
PartitionStatisticsUpdateRequest.class);
+
+ Assertions.assertNull(
+ request.getUpdates().get(0).statistics().get("custom-k"),
+ "the JSON null is expected to survive deserialization as a null map
value");
+ Assertions.assertThrows(IllegalArgumentException.class, request::validate);
+ }
+}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
index 992dd73ebe..a970af8214 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestStatisticOperations.java
@@ -783,6 +783,47 @@ public class TestStatisticOperations extends JerseyTest {
Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
}
+ @Test
+ public void testUpdatePartitionStatisticsWithNullStatisticValue() {
+ when(tableDispatcher.tableExists(any())).thenReturn(true);
+ MetadataObject tableObject =
+ MetadataObjects.parse(
+ String.format("%s.%s.%s", catalog, schema, table),
MetadataObject.Type.TABLE);
+
+ // Sent as raw JSON because PartitionStatisticsUpdateDTO.of rejects this
body. Jackson's
+ // MapDeserializer puts the JSON null straight into the map without
consulting the
+ // StatisticValue deserializer, so only validate() can reject it.
+ String body =
+ "{\"updates\":[{\"partitionName\":\"partition1\",\"statistics\":{\""
+ + Statistic.CUSTOM_PREFIX
+ + "test1\":null}}]}";
+
+ Response resp =
+ target(
+ "/metalakes/"
+ + metalake
+ + "/objects/"
+ + tableObject.type()
+ + "/"
+ + tableObject.fullName()
+ + "/statistics/partitions")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(entity(body, MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
+
+ ErrorResponse errorResp = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResp.getCode());
+ // Pin the reason: a body that lost the entry entirely would fail on
"statistics must not be
+ // null or empty" instead, which would let this test pass for the wrong
reason.
+ Assertions.assertTrue(
+ errorResp.getMessage().contains(Statistic.CUSTOM_PREFIX + "test1")
+ && errorResp.getMessage().contains("must not be null"),
+ () -> "Unexpected rejection reason: " + errorResp.getMessage());
+ }
+
@Test
public void testDropPartitionStatistics() {
List<PartitionStatisticsDropDTO> partitionStatistics =
Lists.newArrayList();
