-
Notifications
You must be signed in to change notification settings - Fork 32
Add string extensions quote and reverse #998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
75fafc9
6207811
0e1a30f
57e795a
943ae33
03d0a36
79ff206
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,8 @@ | |
| import dev.cel.runtime.CelEvaluationException; | ||
| import dev.cel.runtime.CelRuntime; | ||
| import dev.cel.runtime.CelRuntimeFactory; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.List; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
|
|
@@ -70,7 +72,9 @@ public void library() { | |
| "lastIndexOf", | ||
| "lowerAscii", | ||
| "replace", | ||
| "reverse", | ||
| "split", | ||
| "strings.quote", | ||
| "substring", | ||
| "trim", | ||
| "upperAscii"); | ||
|
|
@@ -1467,6 +1471,99 @@ public void stringExtension_functionSubset_success() throws Exception { | |
| assertThat(evaluatedResult).isEqualTo(true); | ||
| } | ||
|
|
||
| @Test | ||
| @TestParameters("{string: 'abcd', expectedResult: 'dcba'}") | ||
| @TestParameters("{string: '', expectedResult: ''}") | ||
| @TestParameters("{string: 'a', expectedResult: 'a'}") | ||
| @TestParameters("{string: 'hello world', expectedResult: 'dlrow olleh'}") | ||
| @TestParameters("{string: 'ab가cd', expectedResult: 'dc가ba'}") | ||
| public void reverse_success(String string, String expectedResult) throws Exception { | ||
| CelAbstractSyntaxTree ast = COMPILER.compile("s.reverse()").getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = program.eval(ImmutableMap.of("s", string)); | ||
|
|
||
| assertThat(evaluatedResult).isEqualTo(expectedResult); | ||
| } | ||
|
|
||
| @Test | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we also port the test over for invisible formatting characters?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 943ae33. |
||
| @TestParameters("{string: '😁😑😦', expectedResult: '😦😑😁'}") | ||
| @TestParameters("{string: '\u180e\u200b\u200c\u200d\u2060\ufeff', expectedResult: '\ufeff\u2060\u200d\u200c\u200b\u180e'}") | ||
| public void reverse_unicode(String string, String expectedResult) throws Exception { | ||
| CelAbstractSyntaxTree ast = COMPILER.compile("s.reverse()").getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = program.eval(ImmutableMap.of("s", string)); | ||
|
|
||
| assertThat(evaluatedResult).isEqualTo(expectedResult); | ||
| } | ||
|
|
||
| @Test | ||
| @TestParameters("{string: 'hello', expectedResult: '\"hello\"'}") | ||
| @TestParameters("{string: '', expectedResult: '\"\"'}") | ||
| @TestParameters("{string: 'contains \\\"quotes\\\"', expectedResult: '\"contains \\\\\\\"quotes\\\\\\\"\"'}") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test for single quote case as well:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 943ae33. |
||
| @TestParameters("{string: 'ends with \\\\', expectedResult: '\"ends with \\\\\\\\\"'}") | ||
| @TestParameters("{string: '\\\\ starts with', expectedResult: '\"\\\\\\\\ starts with\"'}") | ||
| public void quote_success(String string, String expectedResult) throws Exception { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Boundary cases for backslashes appearing at the very beginning or the end are also worth adding:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 943ae33. |
||
| CelAbstractSyntaxTree ast = COMPILER.compile("strings.quote(s)").getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = program.eval(ImmutableMap.of("s", string)); | ||
|
|
||
| assertThat(evaluatedResult).isEqualTo(expectedResult); | ||
| } | ||
|
|
||
| @Test | ||
| public void quote_singleWithDoubleQuotes() throws Exception { | ||
| CelAbstractSyntaxTree ast = COMPILER.compile( | ||
| "strings.quote('single-quote with \"double quote\"') == \"\\\"single-quote with \\\\\\\"double quote\\\\\\\"\\\"\"" | ||
| ).getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = program.eval(); | ||
|
|
||
| assertThat(evaluatedResult).isEqualTo(true); | ||
| } | ||
|
|
||
| @Test | ||
| public void quote_escapesSpecialCharacters() throws Exception { | ||
| CelAbstractSyntaxTree ast = COMPILER.compile("strings.quote(s)").getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = | ||
| program.eval( | ||
| ImmutableMap.of( | ||
| "s", "\u0007bell\u000Bvtab\bback\ffeed\rret\nline\ttab\\slash 가 😁")); | ||
|
|
||
| assertThat(evaluatedResult) | ||
| .isEqualTo("\"\\abell\\vvtab\\bback\\ffeed\\rret\\nline\\ttab\\\\slash 가 😁\""); | ||
| } | ||
|
|
||
| @Test | ||
| @TestParameters({"{rawString: !!binary 'ZmlsbGVyIJ8=', expectedResult: '\"filler \uFFFD\"'}"}) // "filler \x9f" | ||
| public void quote_escapesMalformed(byte[] rawString, String expectedResult) throws Exception { | ||
| CelAbstractSyntaxTree ast = COMPILER.compile("strings.quote(s)").getAst(); | ||
| CelRuntime.Program program = RUNTIME.createProgram(ast); | ||
|
|
||
| Object evaluatedResult = program.eval(ImmutableMap.of("s", new String(rawString, StandardCharsets.UTF_8))); | ||
|
|
||
| assertThat(evaluatedResult).isEqualTo(expectedResult); | ||
| } | ||
|
|
||
| @Test | ||
| public void quote_escapesMalformed_endWithHighSurrogate() throws Exception { | ||
| CelRuntime.Program program = RUNTIME.createProgram(COMPILER.compile("strings.quote(s)").getAst()); | ||
| assertThat(program.eval(ImmutableMap.of("s", "end with high surrogate \uD83D"))) | ||
| .isEqualTo("\"end with high surrogate \uFFFD\""); | ||
| } | ||
|
|
||
| @Test | ||
| public void quote_escapesMalformed_unpairedHighSurrogate() throws Exception { | ||
| CelRuntime.Program program = RUNTIME.createProgram(COMPILER.compile("strings.quote(s)").getAst()); | ||
| assertThat(program.eval(ImmutableMap.of("s", "bad pair \uD83DA"))) | ||
| .isEqualTo("\"bad pair \uFFFDA\""); | ||
| } | ||
|
|
||
| @Test | ||
| public void stringExtension_compileUnallowedFunction_throws() { | ||
| CelCompiler celCompiler = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test for the replacement behavior here? Something similar to cel-go's test here should work: https://github.com/google/cel-go/blob/7114ed27a63255f33c689fbff0ee8a08298f70ab/ext/strings_test.go#L623
We should also add tests for low/high surrogate boundaries:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests for low/high boundaries. The test for cel-go isn't 100% matching what is possible in Go as converting from bytes to a String already replaces invalid bytes w/ replacement characters. I can remove the
quote_escapesMalformedif it isn't of high value to keep it.