nfsantos commented on code in PR #2145:
URL: https://github.com/apache/jackrabbit-oak/pull/2145#discussion_r1984878237


##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java:
##########
@@ -31,6 +31,110 @@ public class ElasticIndexUtils {
 
     private static final Logger LOG = 
LoggerFactory.getLogger(ElasticIndexUtils.class);
 
+    /**
+     * Convert a JCR property name to a Elasticsearch field name.
+     * Notice that "|" is not allowed in JCR names.
+     *
+     * @param propertyName the property name
+     * @return the field name
+     */
+    public static String fieldName(String propertyName) {
+        if(propertyName.startsWith(":")) {
+            // there are some hardcoded field names
+            return propertyName;
+        }
+        String fieldName = propertyName;
+        boolean escape = false;
+        if (fieldName.isBlank()) {
+            // empty field name or field names that only consist of spaces
+            escape = true;
+        } else {
+            // 99.99% property names are OK,
+            // so we loop over the characters first
+            for (int i = 0; i < fieldName.length(); i++) {
+                switch (fieldName.charAt(i)) {
+                case '|':
+                case '.':
+                case '^':
+                case '_':
+                    escape = true;
+                }
+            }
+        }
+        if (escape) {
+            StringBuilder buff = new StringBuilder(fieldName.length());
+            for (int i = 0; i < fieldName.length(); i++) {
+                char c = fieldName.charAt(i);
+                switch (c) {
+                case '|':
+                    buff.append("||");
+                    break;
+                case '.':
+                case '^':
+                    
buff.append('|').append(Integer.toHexString(c)).append('|');

Review Comment:
   The characters here can only be `.` and `^`, so we could avoid the call to 
`toHexString(c)` and just append a constant string, like `|XXX|` and `|YYY|`. 
For instance:
   ```
                   case '.':
                       buff.append("|XXX|"); break;
                   case '^':
                        buff.append("|YYY|"); break;
   ```                   
   



##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java:
##########
@@ -31,6 +31,110 @@ public class ElasticIndexUtils {
 
     private static final Logger LOG = 
LoggerFactory.getLogger(ElasticIndexUtils.class);
 
+    /**
+     * Convert a JCR property name to a Elasticsearch field name.
+     * Notice that "|" is not allowed in JCR names.
+     *
+     * @param propertyName the property name
+     * @return the field name
+     */
+    public static String fieldName(String propertyName) {
+        if(propertyName.startsWith(":")) {
+            // there are some hardcoded field names
+            return propertyName;
+        }
+        String fieldName = propertyName;
+        boolean escape = false;
+        if (fieldName.isBlank()) {
+            // empty field name or field names that only consist of spaces
+            escape = true;
+        } else {
+            // 99.99% property names are OK,
+            // so we loop over the characters first
+            for (int i = 0; i < fieldName.length(); i++) {
+                switch (fieldName.charAt(i)) {
+                case '|':
+                case '.':
+                case '^':
+                case '_':
+                    escape = true;
+                }
+            }
+        }
+        if (escape) {
+            StringBuilder buff = new StringBuilder(fieldName.length());
+            for (int i = 0; i < fieldName.length(); i++) {
+                char c = fieldName.charAt(i);
+                switch (c) {
+                case '|':
+                    buff.append("||");
+                    break;
+                case '.':
+                case '^':
+                    
buff.append('|').append(Integer.toHexString(c)).append('|');
+                    break;
+                default:
+                    buff.append(c);
+                }
+            }
+            fieldName = buff.toString();
+            // internal field start with a _
+            // we also support empty or just spaces
+            if (fieldName.startsWith("_") || fieldName.isBlank()) {
+                fieldName = "|" + fieldName;
+            }

Review Comment:
   Would be nice to avoid creating an additional String here if we have to 
prefix it with `|`. Can we do this check at the start, when the StringBuilder 
is empty, and add the pipe as the first character  of the StringBuilder if 
needed?



##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java:
##########
@@ -31,6 +31,110 @@ public class ElasticIndexUtils {
 
     private static final Logger LOG = 
LoggerFactory.getLogger(ElasticIndexUtils.class);
 
+    /**
+     * Convert a JCR property name to a Elasticsearch field name.
+     * Notice that "|" is not allowed in JCR names.
+     *
+     * @param propertyName the property name
+     * @return the field name
+     */
+    public static String fieldName(String propertyName) {
+        if(propertyName.startsWith(":")) {
+            // there are some hardcoded field names
+            return propertyName;
+        }
+        String fieldName = propertyName;
+        boolean escape = false;
+        if (fieldName.isBlank()) {
+            // empty field name or field names that only consist of spaces
+            escape = true;
+        } else {
+            // 99.99% property names are OK,
+            // so we loop over the characters first
+            for (int i = 0; i < fieldName.length(); i++) {

Review Comment:
   ```suggestion
               for (int i = 0; i < fieldName.length() && !escape ; i++) {
   ```



-- 
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]

Reply via email to