Abyss-lord commented on code in PR #7965:
URL: https://github.com/apache/gravitino/pull/7965#discussion_r2321507663
##########
api/src/main/java/org/apache/gravitino/Namespace.java:
##########
@@ -75,7 +75,10 @@ public static Namespace of(String... levels) {
*/
public static Namespace fromString(String namespace) {
Preconditions.checkArgument(namespace != null, "Cannot create a namespace
with null input");
- Preconditions.checkArgument(!namespace.endsWith("."), "Cannot create a
namespace end with dot");
+ Preconditions.checkArgument(
+ !namespace.contains(" "), "Cannot create a namespace containing
whitespace");
Review Comment:
I think it's better to use `StringUtils.containsOnly()` to check whether
namespace only contains whitespace.
`Preconditions.checkArgument(StringUtils.containsOnly(namespace, " "));`
##########
api/src/test/java/org/apache/gravitino/TestNamespace.java:
##########
@@ -60,6 +60,7 @@ public void testFromStringInvalidArgs() {
Assertions.assertThrows(IllegalArgumentException.class, () ->
Namespace.fromString(".a"));
Assertions.assertThrows(IllegalArgumentException.class, () ->
Namespace.fromString("a."));
Assertions.assertThrows(IllegalArgumentException.class, () ->
Namespace.fromString("a..b"));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
Namespace.fromString(" "));
Review Comment:
I think we can add more cases, e.g. `""`、`"\r\n"`
--
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]
