nfsantos commented on code in PR #1516:
URL: https://github.com/apache/jackrabbit-oak/pull/1516#discussion_r1634538243
##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonDeserializer.java:
##########
@@ -240,9 +239,10 @@ boolean hasOrderableChildren(NodeBuilder builder) {
}
private boolean hasSingleColon(String jsonString) {
- //In case the primaryType was encoded then it would be like
nam:oak:Unstructured
- //So check if there is only one occurrence of ';'
- return CharMatcher.is(':').countIn(jsonString) == 1;
+ // In case the primaryType was encoded then it would be like
+ // "nam:oak:Unstructured". So check if there is only one occurrence
+ // of ':'.
+ return jsonString.replace(":", "").length() == jsonString.length()
- 1;
Review Comment:
I don't think this is test code. This method is called by
`inferPropertyType()` in the same class, which is called by
`JsonDeserializer.readProperty()` and `JsonDeserializer.readArrayProperty()`.
IIUC, the method seems to be on the hot path. We use this class heavily in the
indexing job. Is there a better option than this class?
There are more compact ways of doing the same using streams, but if the
method is indeed hot as I think it is, I would suggest something like this:
```
int colons = 0;
for (int i = 0; i < jsonString.length(); i++) {
if (jsonString.charAt(i) == ':') {
colons++;
}
}
return colons == 1;
```
Could be optimized to abort early if it finds more than a colon, but
probably the size of the argument will be small, so it won't matter.
--
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]
