This is an automated email from the ASF dual-hosted git repository. vladimirsitnikov pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/jmeter.git
commit 41981f0aa46957e89f5b69f354f1c4cf1484ed7c Author: Vladimir Sitnikov <[email protected]> AuthorDate: Fri May 5 22:42:41 2023 +0300 fix: avoid calling getScript and getFilename two times in BeanShellSampler, and JSR223TestSampler Previously, BeanShellSampler called getScript several times which might cause wrong results when the script included a function that changes on each call (e.g. couter). Now getScript is called only once per sample. --- .../apache/jmeter/util/BeanShellTestElement.java | 33 +++++++++++-- .../org/apache/jmeter/util/JSR223TestElement.java | 21 ++++---- src/protocol/java/build.gradle.kts | 6 +++ .../protocol/java/sampler/BeanShellSampler.java | 10 +--- .../protocol/java/sampler/BeanShellSamplerTest.kt | 57 ++++++++++++++++++++++ 5 files changed, 106 insertions(+), 21 deletions(-) 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 a58a992f09..816b3299e1 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 @@ package org.apache.jmeter.util; 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 abstract class BeanShellTestElement extends AbstractTestElement * <li>Parameters</li> * <li>bsh.args</li> * </ul> - * @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. + * <p> + * Sets the following script variables: + * <ul> + * <li>FileName</li> + * <li>Parameters</li> + * <li>bsh.args</li> + * </ul> * + * @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 @@ public abstract class BeanShellTestElement extends AbstractTestElement 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 dded66ed6e..b46b555d15 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 @@ public abstract class JSR223TestElement extends ScriptingTestElement 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 @@ public abstract class JSR223TestElement extends ScriptingTestElement } }); 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 abstract class JSR223TestElement extends ScriptingTestElement /** * 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 abstract class JSR223TestElement extends ScriptingTestElement @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 4f4211d074..4a3cb322ec 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 e95bf87141..45f215eb2e 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 class BeanShellSampler extends BeanShellTestElement implements Sampler, I 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 class BeanShellSampler extends BeanShellTestElement implements Sampler, I 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 0000000000..143b91a75f --- /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" + ) + } +}
