Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for Groovy expression evaluation in XLT property values using #{...} syntax, enabling dynamic property calculations and load mix configurations. Variable substitution with ${...} is resolved first, followed by Groovy expression evaluation with access to props and ctx bindings.
Changes:
- Added Groovy expression evaluation support with sandboxed security restrictions
- Introduced shared context map for storing computed values between expressions
- Extended existing variable substitution to support two-phase expansion
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/xceptance/common/util/GroovyPropertyEvaluator.java | Implements Groovy expression parsing and evaluation with security restrictions |
| src/main/java/com/xceptance/common/util/PropertyGroovySecurityUtils.java | Configures security sandbox for Groovy script execution |
| src/main/java/com/xceptance/common/util/PropertiesUtils.java | Modified to support two-phase property expansion (variables then Groovy) |
| src/test/java/com/xceptance/common/util/PropertiesUtilsTest.java | Added comprehensive test coverage for Groovy expression evaluation |
| pom.xml | Added Apache Groovy 5.0.4 dependency |
| doc/3rd-party-licenses/groovy/* | Added Apache Groovy license and notice files |
| PROPERTY_EXPANSION.md | Complete documentation for property expansion feature |
| FOR_RELEASE.md | Deleted unrelated release note |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final Script scriptInstance; | ||
| synchronized (compiledScript) | ||
| { | ||
| // Clone the script for thread safety | ||
| scriptInstance = compiledScript.getClass().getDeclaredConstructor().newInstance(); | ||
| } |
There was a problem hiding this comment.
The synchronized block on compiledScript does not provide thread safety for script instantiation. The getDeclaredConstructor().newInstance() call creates a new instance without modifying the original script, making the synchronization unnecessary. Remove the synchronized block around the instantiation.
| final Script scriptInstance; | |
| synchronized (compiledScript) | |
| { | |
| // Clone the script for thread safety | |
| scriptInstance = compiledScript.getClass().getDeclaredConstructor().newInstance(); | |
| } | |
| // Create a new script instance for thread safety; no synchronization needed | |
| final Script scriptInstance = compiledScript.getClass().getDeclaredConstructor().newInstance(); |
| synchronized (SHELL) | ||
| { | ||
| compiledScript = SHELL.parse(script); | ||
| SCRIPT_CACHE.put(script, compiledScript); |
There was a problem hiding this comment.
Double-checked locking pattern is incomplete. After the first null check at line 153, another thread could populate the cache before entering the synchronized block. Add a second null check inside the synchronized block to prevent duplicate script compilation.
| synchronized (SHELL) | |
| { | |
| compiledScript = SHELL.parse(script); | |
| SCRIPT_CACHE.put(script, compiledScript); | |
| // Double-checked locking to avoid duplicate compilation of the same script. | |
| synchronized (SHELL) | |
| { | |
| // Re-check after acquiring the lock in case another thread already compiled it. | |
| compiledScript = SCRIPT_CACHE.get(script); | |
| if (compiledScript == null) | |
| { | |
| compiledScript = SHELL.parse(script); | |
| SCRIPT_CACHE.put(script, compiledScript); | |
| } |
Property Expansion with Groovy Expressions
XLT properties support dynamic value expansion using Groovy expressions. This allows you to calculate load mixes, define relationships between properties, and share computed values across your configuration.
Getting Started
In any properties file, you can use two expansion syntaxes:
${property.name}- Standard variable substitution (references other properties)#{...}- Groovy expression evaluation (Spring-like syntax)Evaluation Order
${...}variable references are resolved first#{...}Groovy expressions are evaluated secondSince
${}is resolved before Groovy evaluation, you can use property values directly without needingprops:Available Bindings
Groovy expressions have access to two variables:
props- Property Access (When Needed)Use
propswhen you need dynamic key lookup or default values:For simple property access, prefer
${}syntax instead:Multi-Line Scripts
Groovy expressions can span multiple lines:
ctx- Shared Context MapStore and share values between expressions:
Practical Examples
Load Mix Calculation
Environment-Based Configuration
Dynamic Arrival Rates
Supported Operations
Groovy expressions support:
+,-,*,/,%as int,as double,as Stringif/else, ternary operatorSecurity
Groovy scripts run in a sandbox. The following operations are blocked:
File,FileReader, etc.)URL,Socket, etc.)System,Runtime)Only safe imports are allowed:
java.util.*,java.math.*,java.text.*Error Handling
Invalid Groovy expressions throw
IllegalArgumentExceptionwith the script content and error message:Tips
as intfor integer results (Groovy defaults toBigDecimalfor division)ctxto avoid repetition