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
The following commit(s) were added to refs/heads/master by this push:
new c1a2a0c8fd fix: treat null or blank values as empty for saved
properties
c1a2a0c8fd is described below
commit c1a2a0c8fd7840165f7c1d5f48bcd224024713dc
Author: Vladimir Sitnikov <[email protected]>
AuthorDate: Fri Jun 30 16:12:23 2023 +0300
fix: treat null or blank values as empty for saved properties
It turned out it was impossible to modify element while iterating over its
properties since ReentrantReadWriteLock is not upgradable in Java.
So this commit adds upgrading locks for writeLock paths
Fixes https://github.com/apache/jmeter/issues/6026
See
https://www.javaspecialists.eu/archive/Issue279-Upgrading-ReadWriteLock.html>Issue279-Upgrading-ReadWriteLock
---
.../jmeter/testelement/AbstractTestElement.java | 148 +++++++++------------
.../org/apache/jmeter/testelement/TestElement.kt | 59 ++++----
.../schema/BooleanPropertyDescriptor.kt | 4 +-
.../java/org/apache/jmeter/junit/JMeterTest.java | 13 +-
.../apache/jmeter/loadsave/IsEnabledNormalizer.kt | 47 +++++++
.../protocol/http/config/gui/HttpDefaultsGui.java | 18 ++-
.../protocol/http/config/gui/UrlConfigGui.java | 11 +-
.../http/sampler/HttpSamplerPrintDslTest.kt | 37 ++++++
8 files changed, 198 insertions(+), 139 deletions(-)
diff --git
a/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
b/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
index 0486721475..95c65cc2ab 100644
---
a/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
+++
b/src/core/src/main/java/org/apache/jmeter/testelement/AbstractTestElement.java
@@ -27,7 +27,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.jmeter.engine.util.NoThreadClone;
@@ -66,7 +65,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
* Note: acquiring locks (read or write) allocates memory, and accesses
{@link ThreadLocal}, so it can be expensive
* in both CPU and allocation terms.
*/
- private transient final ReadWriteLock lock =
+ private transient final ReentrantReadWriteLock lock =
this instanceof NoThreadClone
// Note: thread groups are cloned for every thread,
however, JMeterContext contains a reference
// to a non-cloned ThreadGroup instance.
@@ -76,6 +75,39 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
? new ReentrantReadWriteLock()
: null;
+ private transient final ResourceLock readUnlock = lock == null ? null :
lock.readLock()::unlock;
+
+ /**
+ * Downgrades input lock from write to read on close.
+ * It enables using {@link #writeLock()} in try-with-resources to upgrade
the lock.
+ */
+ interface ResourceLock extends AutoCloseable {
+ @Override
+ void close();
+
+ ResourceLock NO_LOCK = () -> { };
+
+ /**
+ * Upgrades lock to write if needed, and returns a callback for
releasing the write lock and restoring read locks.
+ * @see <a
href="https://www.javaspecialists.eu/archive/Issue279-Upgrading-ReadWriteLock.html>Issue279-Upgrading-ReadWriteLock</a>
+ */
+ static ResourceLock releaseWrite(ReentrantReadWriteLock lock) {
+ int readHoldCount = lock.getWriteHoldCount() == 0 ?
lock.getReadHoldCount() : 0;
+ // We need to unlock read locks since ReentrantReadWriteLock does
not support lock upgrade
+ for (int i = 0; i < readHoldCount; i++) {
+ lock.readLock().unlock();
+ }
+ lock.writeLock().lock();
+ // Restore locks on close
+ return () -> {
+ for (int i = 0; i < readHoldCount; i++) {
+ lock.readLock().lock();
+ }
+ lock.writeLock().unlock();
+ };
+ }
+ }
+
/**
* When the element is shared between threads, then {@link #lock} protects
the access,
* however, when element in not shared, then adds overhead as every lock
and unlock allocates memory.
@@ -146,40 +178,33 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
*/
@Override
public void clear() {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
propMap.clear();
Map<String, JMeterProperty> propMapConcurrent =
this.propMapConcurrent;
if (propMapConcurrent != null) {
propMapConcurrent.clear();
}
- } finally {
- writeUnlock();
- }
- }
-
- private void writeLock() {
- if (lock != null) {
- lock.writeLock().lock();
}
}
- private void writeUnlock() {
- if (lock != null) {
- lock.writeLock().unlock();
- }
- }
-
- private void readLock() {
- if (lock != null) {
- lock.readLock().lock();
+ /**
+ * Acquires write lock, and upgrades the current read lock to write lock
if needed.
+ * Note: this should be used in try-with-resources for automatic cleanup.
+ * @return ResourceLock to close when the write lock is no longer needed
+ */
+ private ResourceLock writeLock() {
+ if (lock == null) {
+ return ResourceLock.NO_LOCK;
}
+ return ResourceLock.releaseWrite(lock);
}
- private void readUnlock() {
- if (lock != null) {
- lock.readLock().unlock();
+ private ResourceLock readLock() {
+ if (lock == null) {
+ return ResourceLock.NO_LOCK;
}
+ lock.readLock().lock();
+ return readUnlock;
}
/**
@@ -197,15 +222,12 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
*/
@Override
public void removeProperty(String key) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
propMap.remove(key);
Map<String, JMeterProperty> propMapConcurrent =
this.propMapConcurrent;
if (propMapConcurrent != null) {
propMapConcurrent.remove(key);
}
- } finally {
- writeUnlock();
}
}
@@ -215,11 +237,8 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
@Override
public boolean equals(Object o) {
if (o instanceof AbstractTestElement) {
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
return ((AbstractTestElement) o).propMap.equals(propMap);
- } finally {
- readUnlock();
}
} else {
return false;
@@ -307,16 +326,13 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
@Override
public void traverse(TestElementTraverser traverser) {
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
PropertyIterator iter = propertyIterator();
traverser.startTestElement(this);
while (iter.hasNext()) {
traverseProperty(traverser, iter.next());
}
traverser.endTestElement(this);
- } finally {
- readUnlock();
}
}
@@ -417,15 +433,12 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
JMeterProperty prop = getProperty(property.getName());
if (prop instanceof NullProperty || (prop instanceof StringProperty &&
prop.getStringValue().isEmpty())) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
propMap.put(property.getName(), propertyToPut);
Map<String, JMeterProperty> propMapConcurrent =
this.propMapConcurrent;
if (propMapConcurrent != null) {
propMapConcurrent.put(property.getName(), propertyToPut);
}
- } finally {
- writeUnlock();
}
} else {
prop.mergeIn(propertyToPut);
@@ -445,13 +458,10 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
* @param property {@link JMeterProperty}
*/
protected void clearTemporary(JMeterProperty property) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
if (temporaryProperties != null) {
temporaryProperties.remove(property);
}
- } finally {
- writeUnlock();
}
}
@@ -480,8 +490,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
getProperty(property.getName()).setObjectValue(property.getObjectValue());
}
} else {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
if (property instanceof StringProperty &&
property.getStringValue() == null) {
// Avoid storing properties with null values since they
will be skipped anyway when saving
// the test plan
@@ -493,8 +502,6 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
propMapConcurrent.put(property.getName(), property);
}
}
- } finally {
- writeUnlock();
}
}
}
@@ -602,15 +609,12 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
@Override
public PropertyIterator propertyIterator() {
// Note: can't use ConcurrentMap here as it would return elements in
unpredictable order
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
if (propMap.isEmpty()) {
return PropertyIteratorImpl.EMPTY_ITERATOR;
}
// TODO: copy the contents of the iterator to avoid
ConcurrentModificationException?
return new PropertyIteratorImpl(this, propMap.values());
- } finally {
- readUnlock();
}
}
@@ -619,15 +623,12 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
* @param element {@link TestElement}
*/
protected void mergeIn(TestElement element) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
PropertyIterator iter = element.propertyIterator();
while (iter.hasNext()) {
JMeterProperty prop = iter.next();
addProperty(prop, false);
}
- } finally {
- writeUnlock();
}
}
@@ -647,8 +648,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
*/
@Override
public void setRunningVersion(boolean runningVersion) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
this.runningVersion = runningVersion;
PropertyIterator iter = propertyIterator();
Map<String, JMeterProperty> propMapConcurrent =
this.propMapConcurrent;
@@ -659,8 +659,6 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
propMapConcurrent.put(property.getName(), property);
}
}
- } finally {
- writeUnlock();
}
}
@@ -674,8 +672,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
// See https://github.com/apache/jmeter/issues/5875
return;
}
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
Iterator<Map.Entry<String, JMeterProperty>> iter =
propMap.entrySet().iterator();
while (iter.hasNext()) {
Map.Entry<String, JMeterProperty> entry = iter.next();
@@ -688,8 +685,6 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
}
}
emptyTemporary();
- } finally {
- writeUnlock();
}
}
@@ -697,13 +692,10 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
* Clears temporaryProperties
*/
protected void emptyTemporary() {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
if (temporaryProperties != null) {
temporaryProperties.clear();
}
- } finally {
- writeUnlock();
}
}
@@ -712,11 +704,8 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
*/
@Override
public boolean isTemporary(JMeterProperty property) {
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
return temporaryProperties != null &&
temporaryProperties.contains(property);
- } finally {
- readUnlock();
}
}
@@ -725,8 +714,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
*/
@Override
public void setTemporary(JMeterProperty property) {
- writeLock();
- try {
+ try (ResourceLock ignored = writeLock()) {
if (temporaryProperties == null) {
LinkedHashSet<JMeterProperty> set = new LinkedHashSet<>();
temporaryProperties = lock != null ? set :
Collections.synchronizedSet(set) ;
@@ -737,8 +725,6 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
setTemporary(jMeterProperty);
}
}
- } finally {
- writeUnlock();
}
}
@@ -833,16 +819,13 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
@Override
public List<String> getSearchableTokens() {
List<String> result = new ArrayList<>(25);
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
PropertyIterator iterator = propertyIterator();
while(iterator.hasNext()) {
JMeterProperty jMeterProperty = iterator.next();
result.add(jMeterProperty.getName());
result.add(jMeterProperty.getStringValue());
}
- } finally {
- readUnlock();
}
return result;
}
@@ -853,8 +836,7 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
* @param propertyNames Set of names of properties to extract
*/
protected final void addPropertiesValues(List<? super String> result,
Set<String> propertyNames) {
- readLock();
- try {
+ try (ResourceLock ignored = readLock()) {
PropertyIterator iterator = propertyIterator();
while (iterator.hasNext()) {
JMeterProperty jMeterProperty = iterator.next();
@@ -862,8 +844,6 @@ public abstract class AbstractTestElement implements
TestElement, Serializable,
result.add(jMeterProperty.getStringValue());
}
}
- } finally {
- readUnlock();
}
}
}
diff --git
a/src/core/src/main/kotlin/org/apache/jmeter/testelement/TestElement.kt
b/src/core/src/main/kotlin/org/apache/jmeter/testelement/TestElement.kt
index 9de8162814..d6cb9ec99f 100644
--- a/src/core/src/main/kotlin/org/apache/jmeter/testelement/TestElement.kt
+++ b/src/core/src/main/kotlin/org/apache/jmeter/testelement/TestElement.kt
@@ -21,9 +21,7 @@ import org.apache.jmeter.testelement.property.BooleanProperty
import org.apache.jmeter.testelement.property.CollectionProperty
import org.apache.jmeter.testelement.property.DoubleProperty
import org.apache.jmeter.testelement.property.FloatProperty
-import org.apache.jmeter.testelement.property.IntegerProperty
import org.apache.jmeter.testelement.property.JMeterProperty
-import org.apache.jmeter.testelement.property.LongProperty
import org.apache.jmeter.testelement.property.NullProperty
import org.apache.jmeter.testelement.property.PropertyIterator
import org.apache.jmeter.testelement.property.StringProperty
@@ -269,7 +267,7 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property.name)
/**
- * Retrieve property name as string.
+ * Retrieve property as string, or default value, or empty string if the
property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -278,7 +276,7 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.stringValue ?: property.defaultValue ?: ""
/**
- * Read property value as string, and return default value if the property
is unset.
+ * Read property value as string, or default value, or empty string if the
property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -287,19 +285,19 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.stringValue ?:
property.defaultValueAsString ?: ""
/**
- * Set property as string, or remove it if the given value matches the
default one for the property.
+ * Set property as string, or remove it if the given value is `null` or
empty.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun set(property: PropertyDescriptor<*, *>, value:
String?) {
- removeOrSet(value.isNullOrEmpty() || property.defaultValueAsString ==
value, property.name) {
+ removeOrSet(value.isNullOrEmpty(), property.name) {
StringProperty(it, value)
}
}
/**
- * Retrieve boolean property value.
+ * Retrieve boolean property value, or default value, or `false` if the
property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -308,14 +306,14 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.booleanValue ?: property.defaultValue ?:
false
/**
- * Set property as boolean, or remove it if the given value matches the
default one for the property.
+ * Set property as boolean, or remove it if the given value is null.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
- public operator fun set(property: BooleanPropertyDescriptor<*>, value:
Boolean) {
- removeOrSet(property.defaultValue == value, property.name) {
- BooleanProperty(it, value)
+ public operator fun set(property: BooleanPropertyDescriptor<*>, value:
Boolean?) {
+ removeOrSet(value == null, property.name) {
+ BooleanProperty(it, value!!)
}
}
@@ -329,19 +327,17 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.intValue ?: property.defaultValue ?: 0
/**
- * Set property as integer, or remove it if the given value matches the
default one for the property.
+ * Set property as integer.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun set(property: IntegerPropertyDescriptor<*>, value:
Int) {
- removeOrSet(property.defaultValue == value, property.name) {
- IntegerProperty(it, value)
- }
+ setProperty(property.name, value)
}
/**
- * Retrieve long property value.
+ * Retrieve long property value, or default value, or `0` if the property
is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -350,19 +346,17 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.longValue ?: property.defaultValue ?: 0
/**
- * Set property as long, or remove it if the given value matches the
default one for the property.
+ * Set property as long.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun set(property: LongPropertyDescriptor<*>, value: Long) {
- removeOrSet(property.defaultValue == value, property.name) {
- LongProperty(it, value)
- }
+ setProperty(property.name, value)
}
/**
- * Retrieve float property value.
+ * Retrieve float property value, or default value, or `0.0f` if the
property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -371,19 +365,17 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.floatValue ?: property.defaultValue ?:
0.0f
/**
- * Set property as float, or remove it if the given value matches the
default one for the property.
+ * Set property as float.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun set(property: FloatPropertyDescriptor<*>, value:
Float) {
- removeOrSet(property.defaultValue == value, property.name) {
- FloatProperty(it, value)
- }
+ setProperty(FloatProperty(property.name, value))
}
/**
- * Retrieve double property value.
+ * Retrieve double property value, or default value, or `0.0` if the
property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -392,19 +384,17 @@ public interface TestElement : Cloneable {
getPropertyOrNull(property)?.doubleValue ?: property.defaultValue ?:
0.0
/**
- * Set property as double, or remove it if the given value matches the
default one for the property.
+ * Set property as double.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun set(property: DoublePropertyDescriptor<*>, value:
Double) {
- removeOrSet(property.defaultValue == value, property.name) {
- DoubleProperty(it, value)
- }
+ setProperty(DoubleProperty(property.name, value))
}
/**
- * Retrieve [Class] property value, or throw [NoSuchElementException] in
case the property is unset.
+ * Retrieve [Class] property value, or default value, or throw
[NoSuchElementException] in case the property is unset.
* @throws NoSuchElementException if the property is unset
* @since 5.6
*/
@@ -414,7 +404,7 @@ public interface TestElement : Cloneable {
getOrNull(property) ?: throw NoSuchElementException("Property
${property.name} is unset for element $this")
/**
- * Retrieve [Class] property value, or return `null` in case the property
is unset.
+ * Retrieve [Class] property value, or default value, or `null` in case
the property is unset.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@@ -426,14 +416,14 @@ public interface TestElement : Cloneable {
} ?: property.defaultValue
/**
- * Set property as [Class], or remove it if the given value matches the
default one for the property.
+ * Set property as [Class], or remove it if the given value is `null`.
* The value is set as [StringProperty] with the name of the given class.
* @since 5.6
*/
@JMeterPropertySchemaUnchecked
@API(status = API.Status.EXPERIMENTAL, since = "5.6")
public operator fun <ValueClass : Any> set(property:
ClassPropertyDescriptor<*, ValueClass>, value: Class<out ValueClass>?) {
- removeOrSet(property.defaultValue == value, property.name) {
+ removeOrSet(value == null, property.name) {
StringProperty(it, value!!.name)
}
}
@@ -583,7 +573,6 @@ public interface TestElement : Cloneable {
imports = ["org.apache.jmeter.threads.JMeterContextService"]
)
)
- @get:API(status = API.Status.DEPRECATED, since = "5.6")
@set:API(status = API.Status.DEPRECATED, since = "5.6")
public var threadName: String
diff --git
a/src/core/src/main/kotlin/org/apache/jmeter/testelement/schema/BooleanPropertyDescriptor.kt
b/src/core/src/main/kotlin/org/apache/jmeter/testelement/schema/BooleanPropertyDescriptor.kt
index d21566d141..0553e20d66 100644
---
a/src/core/src/main/kotlin/org/apache/jmeter/testelement/schema/BooleanPropertyDescriptor.kt
+++
b/src/core/src/main/kotlin/org/apache/jmeter/testelement/schema/BooleanPropertyDescriptor.kt
@@ -40,7 +40,7 @@ public data class BooleanPropertyDescriptor<in Schema :
BaseTestElementSchema>(
public operator fun get(target: TestElement): Boolean =
target[this]
- public operator fun set(target: TestElement, value: Boolean) {
+ public operator fun set(target: TestElement, value: Boolean?) {
target[this] = value
}
@@ -52,7 +52,7 @@ public data class BooleanPropertyDescriptor<in Schema :
BaseTestElementSchema>(
@JvmSynthetic
@Suppress("NOTHING_TO_INLINE")
- public inline operator fun setValue(testElement: TestElement, property:
KProperty<*>, value: Boolean) {
+ public inline operator fun setValue(testElement: TestElement, property:
KProperty<*>, value: Boolean?) {
testElement[this] = value
}
}
diff --git
a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
index f7463cbcd9..3ed11e88c1 100644
--- a/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
+++ b/src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
@@ -51,6 +51,7 @@ import org.apache.jmeter.dsl.DslPrinterTraverser;
import org.apache.jmeter.gui.JMeterGUIComponent;
import org.apache.jmeter.gui.UnsharedComponent;
import org.apache.jmeter.gui.tree.JMeterTreeNode;
+import org.apache.jmeter.loadsave.IsEnabledNormalizer;
import org.apache.jmeter.save.SaveService;
import org.apache.jmeter.testbeans.TestBean;
import org.apache.jmeter.testbeans.gui.TestBeanGUI;
@@ -429,16 +430,8 @@ public class JMeterTest extends JMeterTestCaseJUnit
implements Describable {
}
private static void compareAllProperties(TestElement expected, TestElement
actual, byte[] serializedBytes) {
- // JMeter restores "enabled" as StringProperty, see
- //
org.apache.jmeter.save.converters.ConversionHelp.restoreSpecialProperties
- // So we normalize it back to BooleanProperty
- JMeterProperty expEnabled =
expected.getPropertyOrNull(expected.getSchema().getEnabled());
- if (expEnabled != null && (expEnabled.getStringValue().equals("true")
|| expEnabled.getStringValue().equals("false"))) {
- JMeterProperty actEnabled =
actual.getPropertyOrNull(actual.getSchema().getEnabled());
- if (actEnabled != null &&
actEnabled.getStringValue().equals(expEnabled.getStringValue())) {
- actual.setProperty(expEnabled);
- }
- }
+ expected.traverse(IsEnabledNormalizer.INSTANCE);
+ actual.traverse(IsEnabledNormalizer.INSTANCE);
String expectedStr = new
DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(expected).toString();
if (!Objects.equals(expected, actual)) {
diff --git
a/src/dist-check/src/test/kotlin/org/apache/jmeter/loadsave/IsEnabledNormalizer.kt
b/src/dist-check/src/test/kotlin/org/apache/jmeter/loadsave/IsEnabledNormalizer.kt
new file mode 100644
index 0000000000..3521a6fa81
--- /dev/null
+++
b/src/dist-check/src/test/kotlin/org/apache/jmeter/loadsave/IsEnabledNormalizer.kt
@@ -0,0 +1,47 @@
+/*
+ * 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.loadsave
+
+import org.apache.jmeter.testelement.TestElement
+import org.apache.jmeter.testelement.TestElementTraverser
+import org.apache.jmeter.testelement.property.JMeterProperty
+import org.apache.jmeter.testelement.property.StringProperty
+
+/**
+ * Normalize "enabled" property to boolean if the property is set.
+ * JMeter loads "enabled" as [StringProperty], see
+ * [org.apache.jmeter.save.converters.ConversionHelp.restoreSpecialProperties],
+ * so we need to normalize it back if we want comparing elements
+ */
+object IsEnabledNormalizer : TestElementTraverser {
+ override fun startTestElement(el: TestElement) {
+ el.getPropertyOrNull(el.schema.enabled)
+ ?.stringValue
+ ?.toBooleanStrictOrNull()
+ ?.let { el.isEnabled = it }
+ }
+
+ override fun endTestElement(el: TestElement) {
+ }
+
+ override fun startProperty(key: JMeterProperty) {
+ }
+
+ override fun endProperty(key: JMeterProperty) {
+ }
+}
diff --git
a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/HttpDefaultsGui.java
b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/HttpDefaultsGui.java
index e0672815ce..8d8dd64b32 100644
---
a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/HttpDefaultsGui.java
+++
b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/HttpDefaultsGui.java
@@ -94,6 +94,13 @@ public class HttpDefaultsGui extends AbstractConfigGui {
return config;
}
+ /**
+ * Treat unset checkbox as empty, so "unset" checkboxes do not override
values in HTTP Request Samplers.
+ */
+ private static Boolean nullIfUnset(JCheckBox checkBox) {
+ return checkBox.isSelected() ? true : null;
+ }
+
/**
* Modifies a given TestElement to mirror the data in the gui components.
*
@@ -107,15 +114,16 @@ public class HttpDefaultsGui extends AbstractConfigGui {
cfg.addConfigElement(el);
super.configureTestElement(config);
HTTPSamplerBaseSchema.INSTANCE httpSchema =
HTTPSamplerBaseSchema.INSTANCE;
- config.set(httpSchema.getRetrieveEmbeddedResources(),
retrieveEmbeddedResources.isSelected());
+ config.set(httpSchema.getRetrieveEmbeddedResources(),
nullIfUnset(retrieveEmbeddedResources));
enableConcurrentDwn(retrieveEmbeddedResources.isSelected());
- config.set(httpSchema.getConcurrentDownload(),
retrieveEmbeddedResources.isSelected());
- if (!StringUtils.isEmpty(concurrentPool.getText())) {
+ if (concurrentDwn.isSelected()) {
+ config.set(httpSchema.getConcurrentDownload(), true);
config.set(httpSchema.getConcurrentDownloadPoolSize(),
concurrentPool.getText());
} else {
+ config.removeProperty(httpSchema.getConcurrentDownload());
config.removeProperty(httpSchema.getConcurrentDownloadPoolSize());
}
- config.set(httpSchema.getStoreAsMD5(), useMD5.isSelected());
+ config.set(httpSchema.getStoreAsMD5(), nullIfUnset(useMD5));
config.set(httpSchema.getEmbeddedUrlAllowRegex(),
embeddedAllowRE.getText());
config.set(httpSchema.getEmbeddedUrlExcludeRegex(),
embeddedExcludeRE.getText());
@@ -171,7 +179,7 @@ public class HttpDefaultsGui extends AbstractConfigGui {
HTTPSamplerBaseSchema httpSchema = HTTPSamplerBaseSchema.INSTANCE;
retrieveEmbeddedResources.setSelected(samplerBase.get(httpSchema.getRetrieveEmbeddedResources()));
concurrentDwn.setSelected(samplerBase.get(httpSchema.getConcurrentDownload()));
-
concurrentPool.setText(samplerBase.getString(httpSchema.getConcurrentDownloadPoolSize()));
+
concurrentPool.setText(samplerBase.getPropertyAsString(httpSchema.getConcurrentDownloadPoolSize().getName(),
""));
useMD5.setSelected(samplerBase.get(httpSchema.getStoreAsMD5()));
embeddedAllowRE.setText(samplerBase.get(httpSchema.getEmbeddedUrlAllowRegex()));
embeddedExcludeRE.setText(samplerBase.get(httpSchema.getEmbeddedUrlExcludeRegex()));
diff --git
a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
index ba451fc911..39062621fa 100644
---
a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
+++
b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
@@ -214,7 +214,9 @@ public class UrlConfigGui extends JPanel implements
ChangeListener {
filesPanel.modifyTestElement(element);
}
HTTPSamplerBaseSchema.INSTANCE httpSchema =
HTTPSamplerBaseSchema.INSTANCE;
- element.set(httpSchema.getPostBodyRaw(), useRaw);
+ // Treat "unset" checkbox as "property removal" for HTTP Request
Defaults component
+ // Regular sampler should save both true and false values
+ element.set(httpSchema.getPostBodyRaw(), useRaw ? Boolean.TRUE :
(notConfigOnly ? false : null));
element.set(httpSchema.getArguments(), args);
element.set(httpSchema.getDomain(), domain.getText());
element.set(httpSchema.getPort(), port.getText());
@@ -300,8 +302,11 @@ public class UrlConfigGui extends JPanel implements
ChangeListener {
} else {
port.setText(portString);
}
- protocol.setText(el.getString(httpSchema.getProtocol()));
- contentEncoding.setText(el.getString(httpSchema.getContentEncoding()));
+ // We explicitly
+ String protocol =
el.getPropertyAsString(httpSchema.getProtocol().getName(), "");
+ this.protocol.setText(protocol);
+ String encoding =
el.getPropertyAsString(httpSchema.getContentEncoding().getName(), "");
+ contentEncoding.setText(encoding);
path.setText(el.getString(httpSchema.getPath()));
if (notConfigOnly){
method.setText(el.getString(httpSchema.getMethod()));
diff --git
a/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/HttpSamplerPrintDslTest.kt
b/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/HttpSamplerPrintDslTest.kt
index 2baf053e04..d9348448f9 100644
---
a/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/HttpSamplerPrintDslTest.kt
+++
b/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/HttpSamplerPrintDslTest.kt
@@ -21,6 +21,8 @@ import org.apache.jmeter.dsl.DslPrinterTraverser
import org.apache.jmeter.junit.JMeterTestCase
import org.apache.jmeter.protocol.http.control.gui.HttpTestSampleGui
import org.apache.jmeter.testelement.TestElement
+import org.apache.jmeter.testelement.TestElementTraverser
+import org.apache.jmeter.testelement.property.JMeterProperty
import org.apache.jmeter.treebuilder.dsl.testTree
import org.apache.jmeter.util.JMeterUtils
import org.junit.jupiter.api.AfterAll
@@ -50,6 +52,38 @@ class HttpSamplerPrintDslTest : JMeterTestCase() {
}
}
+ object RemoveDefaultValues : TestElementTraverser {
+ private val elements = mutableListOf<TestElement>()
+ private val defaultProps = mutableListOf<MutableList<String>>()
+
+ override fun startTestElement(el: TestElement) {
+ elements += el
+ defaultProps += mutableListOf<String>()
+ }
+
+ override fun endTestElement(el: TestElement) {
+ val element = elements.removeLast()
+ for (propertyName in defaultProps.removeLast()) {
+ element.removeProperty(propertyName)
+ }
+ }
+
+ override fun startProperty(key: JMeterProperty) {
+ val element = elements.last()
+ val defaultProp = defaultProps.last()
+ element.schema.properties[key.name]?.let {
+ if (key.stringValue == it.defaultValueAsString) {
+ // We can't remove property here as it would break ongoing
iterator
+ // So we remove properties in endTestElement instead
+ defaultProp += key.name
+ }
+ }
+ }
+
+ override fun endProperty(key: JMeterProperty) {
+ }
+ }
+
@Test
fun `http sampler created with UI`() {
val ui = HttpTestSampleGui()
@@ -106,10 +140,13 @@ class HttpSamplerPrintDslTest : JMeterTestCase() {
}
}.keys.first() as TestElement
+ createdWithUi.traverse(RemoveDefaultValues)
+
// We compare elements manually, and call assertEquals(toString,
toString) so
// the test output looks better (diff in IDE) in case of the failure
// If we use just assertEquals(createdWithUi, createdWithDsl), then
there will be no "diff in IDE"
if (createdWithUi != createdWithDsl) {
+ val abc = createdWithDsl.equals(createdWithDsl)
assertEquals(
DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(createdWithUi).toString(),
DslPrinterTraverser(DslPrinterTraverser.DetailLevel.ALL).append(createdWithDsl).toString()
