You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by vl...@apache.org on 2023/05/12 04:23:33 UTC
[jmeter] branch master updated (eedabdd6c0 -> 41981f0aa4)
This is an automated email from the ASF dual-hosted git repository.
vladimirsitnikov pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git
from eedabdd6c0 feat: use ServiceLoader to find implementations instead of searching classes in jars (#5885)
new d0bdbafd7b fix: disable FunctionProperty caching
new 41981f0aa4 fix: avoid calling getScript and getFilename two times in BeanShellSampler, and JSR223TestSampler
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
bin/jmeter.properties | 7 +++
.../testelement/property/FunctionProperty.java | 19 +++++++-
.../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 ++++++++++++++++++++++
xdocs/changes.xml | 4 ++
xdocs/usermanual/properties_reference.xml | 9 ++++
9 files changed, 143 insertions(+), 23 deletions(-)
create mode 100644 src/protocol/java/src/test/kotlin/org/apache/jmeter/protocol/java/sampler/BeanShellSamplerTest.kt
[jmeter] 02/02: fix: avoid calling getScript and getFilename two times in BeanShellSampler, and JSR223TestSampler
Posted by vl...@apache.org.
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 <si...@gmail.com>
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"
+ )
+ }
+}
[jmeter] 01/02: fix: disable FunctionProperty caching
Posted by vl...@apache.org.
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 d0bdbafd7bbedb5e9fc32855740b52cda4282638
Author: Vladimir Sitnikov <si...@gmail.com>
AuthorDate: Thu Mar 2 11:14:47 2023 +0300
fix: disable FunctionProperty caching
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=true property.
---
bin/jmeter.properties | 7 +++++++
.../jmeter/testelement/property/FunctionProperty.java | 19 +++++++++++++++++--
xdocs/changes.xml | 4 ++++
xdocs/usermanual/properties_reference.xml | 9 +++++++++
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/bin/jmeter.properties b/bin/jmeter.properties
index 69b77d46bd..410ae16819 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 a4adf02774..238ec0184f 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.engine.util.CompoundVariable;
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 class FunctionProperty extends AbstractProperty {
if (v instanceof CompoundVariable && !isRunningVersion()) {
function = (CompoundVariable) v;
} else {
- cacheValue = v.toString();
+ overrideValue = v.toString();
}
}
@@ -87,7 +96,11 @@ public class FunctionProperty extends AbstractProperty {
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 class FunctionProperty extends AbstractProperty {
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 class FunctionProperty extends AbstractProperty {
@Override
public void recoverRunningVersion(TestElement owner) {
cacheValue = null;
+ overrideValue = null;
}
}
diff --git a/xdocs/changes.xml b/xdocs/changes.xml
index f212ecfbf5..449fa8f481 100644
--- a/xdocs/changes.xml
+++ b/xdocs/changes.xml
@@ -104,6 +104,10 @@ Summary
<li><pr>5899</pr>Speed up CPU-bound tests by skipping <code>recoverRunningVersion</code> for elements that are shared between threads (the ones that implement <code>NoThreadClone</code>)</li>
<li><pr>5914</pr>Use <code>Locale.ROOT</code> instead of default locale for <code>toUpperCase</code>, and <code>toLowerCase</code> to avoid surprises with dotless I in <code>tr_TR</code> locale</li>
<li><pr>5885</pr>Use Java's <code>ServiceLoader</code> for loading plugins instead of classpath scanning. It enables faster startup</li>
+ <li><pr>5788</pr><code>FunctionProperty</code> 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 <code>function.cache.per.iteration</code> property.
+ </li>
</ul>
<ch_section>Non-functional changes</ch_section>
diff --git a/xdocs/usermanual/properties_reference.xml b/xdocs/usermanual/properties_reference.xml
index 68441b1bb8..cb32afcefa 100644
--- a/xdocs/usermanual/properties_reference.xml
+++ b/xdocs/usermanual/properties_reference.xml
@@ -1339,6 +1339,15 @@ JMETER-SERVER</source>
ORO PatternCacheLRU size.<br/>
Defaults to: <code>1000</code>
</property>
+<property name="function.cache.per.iteration">
+ <p>Cache function execution during test execution.</p>
+ <p>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.</p>
+ <note>The property will likely be removed in an upcoming version, so if you need it consider raising
+ an issue with your use-case.</note>
+ Defaults to: <code>false</code>
+</property>
<property name="propertyEditorSearchPath">
TestBeanGui<br/>
Defaults to: <code>null</code>