diff --git a/bin/jmeter.properties b/bin/jmeter.properties index 69b77d46bdf..410ae168199 100644 --- a/bin/jmeter.properties +++ b/bin/jmeter.properties @@ -1055,6 +1055,13 @@ csvdataset.file.encoding_list=UTF-8|UTF-16|ISO-8859-15|US-ASCII # ORO PatternCacheLRU size #oro.patterncache.size=1000 +# Cache function execution during test execution +# By default, JMeter caches function properties, however, it might cause unexpected results +# when the component is shared across threads and the expression depends on the thread variables. +# The caching behaviour would likely change in the upcoming versions +# Deprecation notice: the setting will likely disappear, so if you need it, consider raising an issue with the use-case. +#function.cache.per.iteration=false + #TestBeanGui # #propertyEditorSearchPath=null diff --git a/src/core/src/main/java/org/apache/jmeter/testelement/property/FunctionProperty.java b/src/core/src/main/java/org/apache/jmeter/testelement/property/FunctionProperty.java index a4adf027747..238ec0184f3 100644 --- a/src/core/src/main/java/org/apache/jmeter/testelement/property/FunctionProperty.java +++ b/src/core/src/main/java/org/apache/jmeter/testelement/property/FunctionProperty.java @@ -21,19 +21,28 @@ import org.apache.jmeter.testelement.TestElement; import org.apache.jmeter.threads.JMeterContext; import org.apache.jmeter.threads.JMeterContextService; +import org.apache.jmeter.util.JMeterUtils; /** * Class that implements the Function property */ public class FunctionProperty extends AbstractProperty { private static final long serialVersionUID = 233L; + private static final boolean FUNCTION_CACHE_PER_ITERATION = + JMeterUtils.getPropDefault("function.cache.per.iteration", false); private transient CompoundVariable function; private int testIteration = -1; + /** + * The cache will be removed in the subsequent releases. + * For now, it is kept for backward compatibility. + */ private String cacheValue; + private String overrideValue; + public FunctionProperty(String name, CompoundVariable func) { super(name); function = func; @@ -48,7 +57,7 @@ public void setObjectValue(Object v) { if (v instanceof CompoundVariable && !isRunningVersion()) { function = (CompoundVariable) v; } else { - cacheValue = v.toString(); + overrideValue = v.toString(); } } @@ -87,7 +96,11 @@ public String getStringValue() { log.debug("Not running version, return raw function string"); return function.getRawParameters(); } - if(!ctx.isSamplingStarted()) { + String overrideValue = this.overrideValue; + if (overrideValue != null) { + return overrideValue; + } + if (!FUNCTION_CACHE_PER_ITERATION || !ctx.isSamplingStarted()) { return function.execute(); } log.debug("Running version, executing function"); @@ -115,6 +128,7 @@ public Object getObjectValue() { public FunctionProperty clone() { FunctionProperty prop = (FunctionProperty) super.clone(); prop.cacheValue = cacheValue; + prop.overrideValue = overrideValue; prop.testIteration = testIteration; prop.function = function; return prop; @@ -126,5 +140,6 @@ public FunctionProperty clone() { @Override public void recoverRunningVersion(TestElement owner) { cacheValue = null; + overrideValue = null; } } diff --git a/src/core/src/main/java/org/apache/jmeter/util/BeanShellTestElement.java b/src/core/src/main/java/org/apache/jmeter/util/BeanShellTestElement.java index a58a992f094..816b3299e1d 100644 --- a/src/core/src/main/java/org/apache/jmeter/util/BeanShellTestElement.java +++ b/src/core/src/main/java/org/apache/jmeter/util/BeanShellTestElement.java @@ -19,6 +19,7 @@ import java.io.Serializable; +import org.apache.jmeter.samplers.SampleResult; import org.apache.jmeter.testelement.AbstractTestElement; import org.apache.jmeter.testelement.TestStateListener; import org.apache.jmeter.testelement.ThreadListener; @@ -130,12 +131,31 @@ public Object clone() { *
  • Parameters
  • *
  • bsh.args
  • * - * @param bsh the interpreter, not {@code null} + * + * @param bsh the interpreter, not {@code null} * @return the result of the script, may be {@code null} + * @throws JMeterException when working with the bsh fails + */ + protected Object processFileOrScript(BeanShellInterpreter bsh) throws JMeterException { + return processFileOrScript(bsh, null); + } + + /** + * Process the file or script from the test element. + *

    + * Sets the following script variables: + *

    * + * @param bsh the interpreter, not {@code null} + * @param sampleResult sampler result to set {@code setSamplerData} or {@code null} + * @return the result of the script, may be {@code null} * @throws JMeterException when working with the bsh fails */ - protected Object processFileOrScript(BeanShellInterpreter bsh) throws JMeterException{ + protected Object processFileOrScript(BeanShellInterpreter bsh, SampleResult sampleResult) throws JMeterException { String fileName = getFilename(); String params = getParameters(); @@ -147,7 +167,14 @@ protected Object processFileOrScript(BeanShellInterpreter bsh) throws JMeterExce JOrphanUtils.split(params, " "));//$NON-NLS-1$ if (fileName.length() == 0) { - return bsh.eval(getScript()); + String bshScript = getScript(); + if (sampleResult != null) { + sampleResult.setSamplerData(bshScript); + } + return bsh.eval(bshScript); + } + if (sampleResult != null) { + sampleResult.setSamplerData(fileName); } return bsh.source(fileName); } diff --git a/src/core/src/main/java/org/apache/jmeter/util/JSR223TestElement.java b/src/core/src/main/java/org/apache/jmeter/util/JSR223TestElement.java index dded66ed6e3..b46b555d152 100644 --- a/src/core/src/main/java/org/apache/jmeter/util/JSR223TestElement.java +++ b/src/core/src/main/java/org/apache/jmeter/util/JSR223TestElement.java @@ -182,13 +182,14 @@ protected Object processFileOrScript(ScriptEngine scriptEngine, final Bindings p bindings = scriptEngine.createBindings(); } populateBindings(bindings); - File scriptFile = new File(getFilename()); + String filename = getFilename(); + File scriptFile = new File(filename); // Hack: bsh-2.0b5.jar BshScriptEngine implements Compilable but throws // "java.lang.Error: unimplemented" boolean supportsCompilable = scriptEngine instanceof Compilable && !"bsh.engine.BshScriptEngine".equals(scriptEngine.getClass().getName()); // NOSONAR // $NON-NLS-1$ try { - if (!StringUtils.isEmpty(getFilename())) { + if (!StringUtils.isEmpty(filename)) { if (!scriptFile.isFile()) { throw new ScriptException("Script file '" + scriptFile.getAbsolutePath() + "' is not a file for element: " + getName()); @@ -213,20 +214,22 @@ protected Object processFileOrScript(ScriptEngine scriptEngine, final Bindings p } }); return compiledScript.eval(bindings); - } else if (!StringUtils.isEmpty(getScript())) { + } + String script = getScript(); + if (!StringUtils.isEmpty(script)) { if (supportsCompilable && !ScriptingBeanInfoSupport.FALSE_AS_STRING.equals(cacheKey)) { - computeScriptMD5(); + computeScriptMD5(script); CompiledScript compiledScript = getCompiledScript(scriptMd5, key -> { try { - return ((Compilable) scriptEngine).compile(getScript()); + return ((Compilable) scriptEngine).compile(script); } catch (ScriptException e) { throw new ScriptCompilationInvocationTargetException(e); } }); return compiledScript.eval(bindings); } else { - return scriptEngine.eval(getScript(), bindings); + return scriptEngine.eval(script, bindings); } } else { throw new ScriptException("Both script file and script text are empty for element:" + getName()); @@ -304,10 +307,10 @@ public boolean compile() /** * compute MD5 if it is null */ - private void computeScriptMD5() { + private void computeScriptMD5(String script) { // compute the md5 of the script if needed if(scriptMd5 == null) { - scriptMd5 = ScriptCacheKey.ofString(DigestUtils.md5Hex(getScript())); + scriptMd5 = ScriptCacheKey.ofString(DigestUtils.md5Hex(script)); } } @@ -355,7 +358,7 @@ public void testEnded() { @Override public void testEnded(String host) { COMPILED_SCRIPT_CACHE.invalidateAll(); - this.scriptMd5 = null; + scriptMd5 = null; } public String getScriptLanguage() { diff --git a/src/protocol/java/build.gradle.kts b/src/protocol/java/build.gradle.kts index 4f4211d0745..4a3cb322ec3 100644 --- a/src/protocol/java/build.gradle.kts +++ b/src/protocol/java/build.gradle.kts @@ -30,4 +30,10 @@ dependencies { } testImplementation(project(":src:core", "testClasses")) + testImplementation(projects.src.functions) { + because("We need __counter function for tests") + } + testImplementation("org.apache-extras.beanshell:bsh") { + because("BeanShellTest needs BeanShell, and previously :protocol:java was not including a dependency on BeanShell") + } } diff --git a/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/BeanShellSampler.java b/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/BeanShellSampler.java index e95bf87141b..45f215eb2e3 100644 --- a/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/BeanShellSampler.java +++ b/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/BeanShellSampler.java @@ -102,14 +102,6 @@ public SampleResult sample(Entry e)// Entry tends to be ignored ... return res; } try { - String request = getScript(); - String fileName = getFilename(); - if (fileName.length() == 0) { - res.setSamplerData(request); - } else { - res.setSamplerData(fileName); - } - bshInterpreter.set("SampleResult", res); //$NON-NLS-1$ // Set default values @@ -120,7 +112,7 @@ public SampleResult sample(Entry e)// Entry tends to be ignored ... res.setDataType(SampleResult.TEXT); // assume text output - script can override if necessary savedBsh = bshInterpreter; - Object bshOut = processFileOrScript(bshInterpreter); + Object bshOut = processFileOrScript(bshInterpreter, res); savedBsh = null; if (bshOut != null) {// Set response data diff --git a/src/protocol/java/src/test/kotlin/org/apache/jmeter/protocol/java/sampler/BeanShellSamplerTest.kt b/src/protocol/java/src/test/kotlin/org/apache/jmeter/protocol/java/sampler/BeanShellSamplerTest.kt new file mode 100644 index 00000000000..143b91a75fb --- /dev/null +++ b/src/protocol/java/src/test/kotlin/org/apache/jmeter/protocol/java/sampler/BeanShellSamplerTest.kt @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.jmeter.protocol.java.sampler + +import org.apache.jmeter.engine.util.CompoundVariable +import org.apache.jmeter.testelement.TestElement +import org.apache.jmeter.testelement.property.FunctionProperty +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class BeanShellSamplerTest { + // TODO: move to TestElement itself? + private fun TestElement.setFunctionProperty(name: String, expression: String) { + setProperty( + FunctionProperty( + name, + CompoundVariable(expression).function + ) + ) + } + + @Test + fun `getScript executes only once`() { + val sampler = BeanShellSampler().apply { + name = "BeanShell Sampler" + setFunctionProperty( + BeanShellSampler.SCRIPT, + """ResponseMessage="COUNTER=${"$"}{__counter(FALSE)}"""" + ) + setProperty(BeanShellSampler.FILENAME, "") + setProperty(BeanShellSampler.PARAMETERS, "") + isRunningVersion = true + } + val result = sampler.sample(null) + assertEquals( + "COUNTER=1", + result.responseMessage, + "__counter(false) should return 1 on the first execution. If the value is different, it might mean " + + "the script was evaluated several times" + ) + } +} diff --git a/xdocs/changes.xml b/xdocs/changes.xml index f212ecfbf51..449fa8f4818 100644 --- a/xdocs/changes.xml +++ b/xdocs/changes.xml @@ -104,6 +104,10 @@ Summary
  • 5899Speed up CPU-bound tests by skipping recoverRunningVersion for elements that are shared between threads (the ones that implement NoThreadClone)
  • 5914Use Locale.ROOT instead of default locale for toUpperCase, and toLowerCase to avoid surprises with dotless I in tr_TR locale
  • 5885Use Java's ServiceLoader for loading plugins instead of classpath scanning. It enables faster startup
  • +
  • 5788FunctionProperty no longer caches the value. + Previously it cached the values based on iteration number only which triggered wrong results on concurrent executions. + The previous behavior can be temporary restored with function.cache.per.iteration property. +
  • Non-functional changes diff --git a/xdocs/usermanual/properties_reference.xml b/xdocs/usermanual/properties_reference.xml index 68441b1bb82..cb32afcefa9 100644 --- a/xdocs/usermanual/properties_reference.xml +++ b/xdocs/usermanual/properties_reference.xml @@ -1339,6 +1339,15 @@ JMETER-SERVER ORO PatternCacheLRU size.
    Defaults to: 1000 + +

    Cache function execution during test execution.

    +

    By default, JMeter caches function properties during a test iteration, however, + it might cause unexpected results when a component is shared across threads and the expression depends on + the thread variables.

    + The property will likely be removed in an upcoming version, so if you need it consider raising + an issue with your use-case. + Defaults to: false +
    TestBeanGui
    Defaults to: null