[NAE-2470] - PFQL double quote string support - #468
Conversation
- update PFQL grammar to support double quote strings - add new queries to QueryLangTest.java - update SearchUtils to support double quote strings
WalkthroughPFQL now supports single- and double-quoted string literals with escapes. String normalization removes only matching outer quotes. Tests cover double-quoted and mixed-quote process and case queries. ChangesPFQL quoted string support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.java`:
- Line 183: Update SearchUtils.getStringValue() to decode the escape sequences
accepted by QueryLang.g4 after removing the surrounding quote delimiters, so
escaped quotes and backslashes reach predicate evaluation as their literal
characters. Preserve unescaped string handling and add an end-to-end regression
test covering escaped quotes and backslash escapes.
- Line 183: Update the quote-stripping logic in SearchUtils to remove exactly
one outer pair only when the first and last characters are matching single or
double quotes. Preserve all remaining quote characters, including repeated or
mismatched boundary quotes, instead of using a regex that removes consecutive
delimiters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: feda0337-32bc-48e0-9c5e-3e5d9a2a7306
📒 Files selected for processing (3)
src/main/java/com/netgrif/application/engine/pfql/domain/antlr4/QueryLang.g4src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.javasrc/test/java/com/netgrif/application/engine/pfql/QueryLangTest.java
|
|
||
| public static String getStringValue(String queryLangString) { | ||
| return queryLangString.replace("'", ""); | ||
| return queryLangString.replaceAll("^[\"']+|[\"']+$", ""); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'SearchUtils.java|QueryLang.g4|QueryLangEvaluator.java' . | sed 's#^\./##'
echo
echo "== SearchUtils relevant section =="
file=$(fd 'SearchUtils.java' . | head -n 1)
if [ -n "$file" ]; then
wc -l "$file"
sed -n '130,210p' "$file" | cat -n
fi
echo
echo "== QueryLang parser/evaluator relevant sections =="
for f in $(fd 'QueryLang.g4|QueryLangEvaluator.java' .); do
echo "--- $f"
wc -l "$f"
sed -n '320,360p' "$f" | cat +n
sed -n '120,175p' "$f" | cat +n
done
echo
echo "== search for STRING literal handling / eval methods =="
rg -n "getString|QueryLangString|QueryLang|STRING|Escape|replaceAll|O\\\\'|\\\\\\'" src/main || true
echo
echo "== read-only deterministic probe: Java source literal token value extraction would be needed; inspect whether any unescape helper exists =="
rg -n "parse.*String|unescape|\\\\\\\\|replaceFirst|QueryLangString" src/main/java/com/netgrif/application/engine/pfql || trueRepository: netgrif/application-engine
Length of output: 4771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== QueryLang.g4 string tokens =="
sed -n '320,360p' src/main/java/com/netgrif/application/engine/pfql/domain/antlr4/QueryLang.g4
echo
echo "== QueryLang.g4 parse/string rule annotations =="
rg -n "(@|STRING|queryLangString|EscapedCharacter)" src/main/java/com/netgrif/application/engine/pfql/domain/antlr4/QueryLang.g4
echo
echo "== QueryLangEvaluator string handling =="
sed -n '120,175p' src/main/java/com/netgrif/application/engine/pfql/service/QueryLangEvaluator.java
echo
echo "== SearchUtils predicate builders using getStringValue =="
rg -n "getStringValue\\(" src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.java -A 8 -B 8
echo
echo "== unescape/substring literals and helper usages =="
rg -n "queryString|queryLangString|getStringValue|getString\\(|unescape|replaceFirst\\(\"^[\"']|\"\"\\)|\"\\\\\\\\\"" src/main/java/com/netgrif/application/engine/pfql/service src/main/java/com/netgrif/application/engine/pfql/domain/antlr4 -C 3
echo
echo "== read-only behavioral probe: Java lexer token text model for escaped string =="
python3 - <<'PY'
import re
text = r"['O\\'Reilly'\""
# Simulate Java-style source-text tokenization where escaped quotes are part of the token text.
# This does not execute Java code; it checks whether stripping delimiters alone leaves backslash.
s = "O\\'Reilly"
stripped = re.sub(r"^[\"']+|[\"']+$", "", "'" + s + "'")
print("lexer_token_text:", "'" + s + "'")
print("getStringValue_result:", stripped)
print("contains_backslash:", "\\" in stripped)
print("expected_after_decode_Java:", r"O'Reilly")
PYRepository: netgrif/application-engine
Length of output: 44866
Decode STRING escape sequences before predicate construction.
QueryLang.g4 accepts escaped characters in STRING literals, but SearchUtils.getStringValue() only strips delimiters at src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.java:182-183. This makes valid queries like 'O\'Reilly' reach the evaluator as O\'Reilly instead of O'Reilly. Decode escaped characters when extracting literal values, or narrow the lexer rule and call sites so that backslashes are not valid in string payloads. Add an end-to-end regression test for escaped quotes and backslash escapes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.java`
at line 183, Update SearchUtils.getStringValue() to decode the escape sequences
accepted by QueryLang.g4 after removing the surrounding quote delimiters, so
escaped quotes and backslashes reach predicate evaluation as their literal
characters. Preserve unescaped string handling and add an end-to-end regression
test covering escaped quotes and backslash escapes.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove only the matching outer quote pair.
Line 183 removes every consecutive ' or " character at each boundary. It can remove quote characters that belong to the value. The valid PFQL literal '"foo"' becomes foo, not "foo".
Check the first and last characters and remove exactly one matching pair.
Proposed delimiter fix
- return queryLangString.replaceAll("^[\"']+|[\"']+$", "");
+ if (queryLangString.length() < 2) {
+ return queryLangString;
+ }
+ char delimiter = queryLangString.charAt(0);
+ if ((delimiter != '\'' && delimiter != '"')
+ || queryLangString.charAt(queryLangString.length() - 1) != delimiter) {
+ return queryLangString;
+ }
+ return queryLangString.substring(1, queryLangString.length() - 1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return queryLangString.replaceAll("^[\"']+|[\"']+$", ""); | |
| if (queryLangString.length() < 2) { | |
| return queryLangString; | |
| } | |
| char delimiter = queryLangString.charAt(0); | |
| if ((delimiter != '\'' && delimiter != '"') | |
| || queryLangString.charAt(queryLangString.length() - 1) != delimiter) { | |
| return queryLangString; | |
| } | |
| return queryLangString.substring(1, queryLangString.length() - 1); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/netgrif/application/engine/pfql/service/utils/SearchUtils.java`
at line 183, Update the quote-stripping logic in SearchUtils to remove exactly
one outer pair only when the first and last characters are matching single or
double quotes. Preserve all remaining quote characters, including repeated or
mismatched boundary quotes, instead of using a regex that removes consecutive
delimiters.
Description
Implements NAE-2470
How Has Been This Tested?
Manually and also adding new queries to existing PFQL test
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes