[#8656] Improvement: add UTs and fix a NPE in TableUpdatesRequest.java

[playasim]

What changes were proposed in this pull request?

Add UTs for TableUpdatesRequest.java validate method.
Also There are 2 validate methods throw NPE when arguments is invalid.

Why are the changes needed?

Add UTs to make sure validate logic.

Fix: #8656

Does this PR introduce any user-facing change?

NO

How was this patch tested?

Passed all UTs

[justinmclean]

Thanks for the unit tests and changes, however, changing checkNotNull to checkArgument wasn't what I had in mind.

To be more explicit:
For RenameTableRequest - If a new schema name is provided, it must not be blank. (Currently allowed to be "".)
For AddTableIndexRequest - Validate index.name() is not blank (currently only type and fieldNames are checked). You might also want to validate that inner fieldNames arrays contain only non-blank field path segments.
For DeleteTableIndexRequest - validate() only checks name != null. Should be not blank to avoid empty string.

[justinmclean]

While making those changes, you may want to update the JavaDoc for UpdateTableColumnNullabilityRequest and ensure the messaging is more consistent, i.e., some messages state “cannot be empty” for null checks, while others use “cannot be null”.

[yuqi1129]

@playasim
Can you please change all checkNotNull in Gravitino to checkArgument in this PR?

[playasim]

@playasim Can you please change all checkNotNull in Gravitino to checkArgument in this PR?

Sure. I will push the new commit in next coming days.

[playasim]

Hi @justinmclean ,added more UTs and enhanced on the request mentioned above.(I've checked RenameTableRequest and the validate method seems fine) Please help review the PR.

@yuqi1129 , in this PR I only changed checkNotNull to checkArgument in TableUpdateRequest.java. Would you like me to do a full project check? If so we may need a new PR to do the replacement.

[yuqi1129]

@yuqi1129 , in this PR I only changed checkNotNull to checkArgument in TableUpdateRequest.java. Would you like me to do a full project check? If so we may need a new PR to do the replacement.

OK. If raising such a large PR is a challenge for you, you can use another PR to resolve it.

[keepConcentration]

Hi all,
I noticed that this PR modifies TableUpdateRequest.java, but the original issue (#8656) requested adding validation specifically for the updates field in TableUpdatesRequest.java. Could anyone clarify whether the updates validation has been implemented in TableUpdatesRequest.java, or if this PR was intended to address a different class? I want to make sure the original issue’s request is fully addressed.