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.

Author: vlsi

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() {
      * <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 @@ 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);
     }

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() {

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")
+    }
 }

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

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"
+        )
+    }
+}