LuciferYang opened a new pull request, #12658:
URL: https://github.com/apache/gravitino/pull/12658
### What changes were proposed in this pull request?
`PartitionStatisticsUpdateDTO.validate()` now walks the `statistics` map and
applies the same two per-entry checks, with the same messages, as
`StatisticsUpdateRequest.validate()`: the statistic name must not be blank, and
the statistic value must not be null.
### Why are the changes needed?
The two statistics update endpoints disagreed on the same request body. `PUT
.../statistics/partitions` with `{"custom-k": null}` returned 200 and the null
reached the dispatcher, while `PUT .../statistics` with the equivalent body
returned 400.
The deserializer cannot catch this: Jackson's `MapDeserializer` does not
invoke a `contentUsing` deserializer for a `VALUE_NULL` content token, so the
null guard in `JsonUtils.getStatisticValue` never sees a top-level null and
`validate()` is the only place that can reject one.
The blank-name check matters on a second path too. The server rejects a
non-`custom-` prefixed name in `StatisticOperations`, but
`PartitionStatisticsUpdateDTO.of()` is what `clients/client-java` calls when
building a request, and there the check runs before anything is sent.
Fix: #12657
### Does this PR introduce _any_ user-facing change?
Yes. `PUT
/metalakes/{metalake}/objects/{type}/{fullName}/statistics/partitions` now
returns 400 for a null statistic value or a blank statistic name, matching the
object-level endpoint. It previously accepted both. Java callers going through
`PartitionStatisticsUpdateDTO.of()` get an `IllegalArgumentException` for the
same inputs.
### How was this patch tested?
`TestStatisticOperations.testUpdatePartitionStatisticsWithNullStatisticValue`
drives the endpoint with a raw JSON body and asserts 400,
`ILLEGAL_ARGUMENTS_CODE`, and an error message naming the offending statistic.
Against the pre-fix code it fails with `expected: <400> but was: <200>`. The
body is sent as a raw string rather than a serialized DTO on purpose: `of()`
now rejects it, and serializing a map would risk the client mapper dropping the
null entry, which would let the test pass on a different validation error.
New `TestPartitionStatisticsUpdateDTO` covers the DTO, which had no tests: a
null value is rejected with the statistic named, a blank name is rejected, a
valid map is accepted, and the real request body deserializes into a map
holding a null value before `validate()` rejects it. That last assertion pins
the Jackson behaviour the fix depends on; its first half passes with or without
the fix. All three rejection cases were confirmed to fail against the pre-fix
code.
```
./gradlew :common:test :server:test :core:test :clients:client-java:test
:common:javadoc :common:spotlessCheck :server:spotlessCheck -PskipITs
```
passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
