rishabhdaim commented on code in PR #560:
URL: https://github.com/apache/jackrabbit-oak/pull/560#discussion_r873842845
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java:
##########
@@ -365,14 +361,17 @@ private static boolean isLongPath(String path) {
}
// check if the parent path is long
byte[] parent = PathUtils.getParentPath(path).getBytes(UTF_8);
- if (parent.length < PATH_LONG) {
- return false;
- }
- String name = PathUtils.getName(path);
- if (name.getBytes(UTF_8).length > NODE_NAME_LIMIT) {
- throw new IllegalArgumentException("Node name is too long: " +
path);
- }
- return true;
+ return parent.length >= PATH_LONG;
+ }
Review Comment:
From @mreutegg earlier comment:
> I suggest we take a slightly different approach. To me it feels wrong that
Utils.isLongPath() is currently responsible for detecting when a node name is
too long. I think this is the reason why so many other code changes are
necessary. E.g. the method is then used in Utils.getIdFromPath(), which in turn
is used in many places. But in many cases the check for the node name length is
not actually needed. I my view it would be better if the node name length check
is moved out of Utils.isLongPath() to a place were it is really needed. That
is, when a new node is added through the DocumentNodeStore or created with an
UpdateOp on the DocumentStore.
--
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]
