From 8eaeef19133f680fd00aa5479b52028f740f2749 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Wed, 11 Dec 2024 22:30:39 +0100 Subject: [PATCH 01/10] feat: add ResourceKeyed, LocalizedValue interfaces for combobox localization --- .../kotlin/build-logic.autostyle.gradle.kts | 7 ++- .../apache/jorphan/locale/ComboBoxValue.kt | 40 ++++++++++++ .../apache/jorphan/locale/LocalizedString.kt | 36 +++++++++++ .../apache/jorphan/locale/LocalizedValue.kt | 51 +++++++++++++++ .../org/apache/jorphan/locale/PlainValue.kt | 26 ++++++++ .../apache/jorphan/locale/ResourceKeyed.kt | 63 +++++++++++++++++++ .../jorphan/locale/ResourceLocalizer.kt | 40 ++++++++++++ .../org/apache/jorphan/util/EnumUtils.kt | 31 +++++---- 8 files changed, 276 insertions(+), 18 deletions(-) create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ComboBoxValue.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedString.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedValue.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/PlainValue.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceKeyed.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceLocalizer.kt diff --git a/build-logic/verification/src/main/kotlin/build-logic.autostyle.gradle.kts b/build-logic/verification/src/main/kotlin/build-logic.autostyle.gradle.kts index ffcf7b8888e..a5c9dce78e3 100644 --- a/build-logic/verification/src/main/kotlin/build-logic.autostyle.gradle.kts +++ b/build-logic/verification/src/main/kotlin/build-logic.autostyle.gradle.kts @@ -101,7 +101,12 @@ plugins.withId("org.jetbrains.kotlin.jvm") { kotlin { license() trimTrailingWhitespace() - ktlint("0.40.0") + ktlint("0.40.0") { + filter { + // TODO: remove exclusion when update ktlint + exclude("**/org/apache/jorphan/locale/PlainValue.kt") + } + } endWithNewline() } } diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ComboBoxValue.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ComboBoxValue.kt new file mode 100644 index 00000000000..02ac6fbc174 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ComboBoxValue.kt @@ -0,0 +1,40 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API + +/** + * Marker interface for values that can be used in a combo box. + * + * This interface is intended to standardize handling of values for GUI components + * like combo boxes. Classes implementing this interface can define how their + * values are represented and localized, providing flexibility for different types + * of combo box content. + * + * Implementers may include: + * - Simple values (e.g., strings or plain objects). + * - Values with additional localization support for display in different locales. + * - Wrapper types that encapsulate richer behaviors for combo box items. + * + * This interface is marked as experimental and may change in future releases. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public interface ComboBoxValue diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedString.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedString.kt new file mode 100644 index 00000000000..97edda99918 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedString.kt @@ -0,0 +1,36 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls + +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public class LocalizedString( + override val resourceKey: @NonNls String, + private val resourceLocalizer: ResourceLocalizer, +) : ResourceKeyed, ComboBoxValue { + override fun toString(): String = + resourceLocalizer.localize(resourceKey) + + override fun equals(other: Any?): Boolean = + other is LocalizedString && other.resourceKey == resourceKey + + override fun hashCode(): Int = + resourceKey.hashCode() +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedValue.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedValue.kt new file mode 100644 index 00000000000..46ac72cacb2 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/LocalizedValue.kt @@ -0,0 +1,51 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API + +/** + * Represents a localized value that combines an object implementing the [ResourceKeyed] interface + * with a localization function. This allows the object's resource key to be localized dynamically + * into a string representation. + * + * @param T The type of the value, which must implement the [ResourceKeyed] interface. + * @property value The object implementing [ResourceKeyed], which holds a resource key for localization. + * @property localizer A function that takes a resource key string and returns its localized string representation. + * + * The localized representation of this object is produced by applying the `localizer` function + * to the resource key of the `value`. + * + * The class provides equality and hash code implementations based on the underlying `value`. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public class LocalizedValue( + public val value: T, + private val localizer: ResourceLocalizer, +) : ResourceKeyed by value, ComboBoxValue { + override fun toString(): String = + localizer.localize(value.resourceKey) + + override fun equals(other: Any?): Boolean = + other is LocalizedValue<*> && other.value == value + + override fun hashCode(): Int = + value.hashCode() +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/PlainValue.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/PlainValue.kt new file mode 100644 index 00000000000..8523be8769a --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/PlainValue.kt @@ -0,0 +1,26 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API + +@JvmInline +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public value class PlainValue(public val value: String): ComboBoxValue { + override fun toString(): String = value +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceKeyed.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceKeyed.kt new file mode 100644 index 00000000000..eaaca37e81e --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceKeyed.kt @@ -0,0 +1,63 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls + +/** + * Interface for enum types that provide their own localization resource key for GUI display. + * + * When an enum implements this interface, the [resourceKey] will be used for: + * - Storing the value in JMeter properties (instead of the enum name) + * - Looking up localized display text via `JMeterUtils.getResString(resourceKey)` + * - Binding to GUI components + * + * This allows enums to have stable, localization-friendly identifiers that are independent + * of the enum constant names. + * + * Example: + * ```java + * public enum ResponseMode implements ResourceKeyed { + * STORE_COMPRESSED("response_mode_store"), + * FETCH_DISCARD("response_mode_discard"); + * + * private final String key; + * ResponseMode(String key) { this.key = key; } + * + * @Override + * public String getResourceKey() { return key; } + * } + * ``` + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public interface ResourceKeyed { + /** + * Returns the resource key used for localization and property storage. + * + * This key should: + * - Be stable across JMeter versions (don't change it) + * - Have a corresponding entry in messages.properties + * - Use lowercase with underscores by convention + * + * @return the resource key for this enum value + */ + public val resourceKey: @NonNls String +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceLocalizer.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceLocalizer.kt new file mode 100644 index 00000000000..05bea83838a --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/locale/ResourceLocalizer.kt @@ -0,0 +1,40 @@ +/* + * 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.jorphan.locale + +import org.apiguardian.api.API +import org.jetbrains.annotations.Nls +import org.jetbrains.annotations.NonNls + +/** + * Functional interface for localizing resource strings. + * + * This interface provides a single method, [localize], which takes a non-localized input string + * and returns its localized representation. Implementations of this interface define how the + * localization is performed, such as looking up translations in a resource bundle or applying + * other localization mechanisms. + * + * The input string is expected to be a resource identifier or key, and the output is a string + * appropriate for display in a user interface or other localized context. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public fun interface ResourceLocalizer { + public fun localize(input: @NonNls String): @Nls String +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/util/EnumUtils.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/util/EnumUtils.kt index ecee2105e5d..27213ee68f7 100644 --- a/src/jorphan/src/main/kotlin/org/apache/jorphan/util/EnumUtils.kt +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/util/EnumUtils.kt @@ -18,6 +18,7 @@ @file:JvmName("EnumUtils") package org.apache.jorphan.util +import org.apache.jorphan.locale.ResourceKeyed import org.apiguardian.api.API import java.util.Collections.unmodifiableList import java.util.Collections.unmodifiableMap @@ -39,34 +40,30 @@ private val VALUE_MAP = object : ClassValue>>() { require(type.isEnum) { "Class $type is not an enum" } - @Suppress("UNCHECKED_CAST") - type as Class> return unmodifiableMap( - VALUES.get(type).associateBy { it.stringValue } + VALUES.get(type).associateBy { + (it as ResourceKeyed).resourceKey + } ) } } @Suppress("UNCHECKED_CAST") -@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") -public fun > values(klass: Class): List = - VALUES.get(klass) as List +@get:API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public val > Class.enumValues: List + get() = VALUES.get(this) as List @Suppress("UNCHECKED_CAST") -@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") -public fun > valueMap(klass: Class): List = - VALUES.get(klass) as List +@get:API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public val Class.enumValueMap: Map where T : Enum<*>, T : ResourceKeyed + get() = VALUE_MAP.get(this) as Map @Suppress("UNCHECKED_CAST") @API(status = API.Status.EXPERIMENTAL, since = "6.0.0") -public inline fun > valueOf(value: String): T? = - valueOf(T::class.java, value) +public inline fun valueOf(value: String): T? where T : Enum<*>, T : ResourceKeyed = + T::class.java.valueOf(value) @Suppress("UNCHECKED_CAST") @API(status = API.Status.EXPERIMENTAL, since = "6.0.0") -public fun > valueOf(klass: Class, value: String): T? = - VALUE_MAP.get(klass)[value] as T? - -@get:API(status = API.Status.EXPERIMENTAL, since = "6.0.0") -public val Enum<*>.stringValue: String - get() = toString() +public fun Class.valueOf(value: String): T? where T : Enum<*>, T : ResourceKeyed = + enumValueMap[value] From cde92acd847829c27daf83688aaaebd3cdcc78b9 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Fri, 29 May 2026 22:36:44 +0300 Subject: [PATCH 02/10] feat: add ModifiedGutter and editor mode primitives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the shared building blocks the editable property editors depend on, ahead of the editors themselves: * ModifiedGutter — a left-side accent strip that lights up when a control holds an explicitly stored value, following the gutter pattern used by IntelliJ IDEA and VS Code. The slot is reserved even when the strip is transparent, so toggling the state never shifts the layout, and the colour is muted when the wrapped control is disabled. * ResetMode — a sealed type that wires a "Reset to default" entry into an editor's popup menu, enabled only while the gutter is lit. * ExpressionMode — a sealed type that controls whether an editor may switch to a free-form ${expression} field. ModifiedGutterTest covers the layout reservation, the property-change semantics, and the accessibility description. Co-Authored-By: Claude Opus 4.8 --- .../org/apache/jorphan/gui/ExpressionMode.kt | 57 ++++ .../org/apache/jorphan/gui/ModifiedGutter.kt | 263 ++++++++++++++++++ .../org/apache/jorphan/gui/ResetMode.kt | 49 ++++ .../apache/jorphan/gui/ModifiedGutterTest.kt | 102 +++++++ xdocs/usermanual/build-test-plan.xml | 39 +++ 5 files changed, 510 insertions(+) create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ExpressionMode.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ModifiedGutter.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ResetMode.kt create mode 100644 src/jorphan/src/test/kotlin/org/apache/jorphan/gui/ModifiedGutterTest.kt diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ExpressionMode.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ExpressionMode.kt new file mode 100644 index 00000000000..7768cf4b5ea --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ExpressionMode.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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apiguardian.api.API + +/** + * Controls whether a property editor allows entering custom expressions. + * + * JMeter supports runtime expressions using syntax like `${variableName}` or + * `${__functionName(args)}`. When expression mode is allowed, the property editor + * provides a mechanism to switch from selecting predefined values to entering + * arbitrary text that may contain these expressions. + * + * This mode is applicable to property editors where it makes sense to provide + * both structured value selection and free-form text entry. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public sealed class ExpressionMode { + /** + * Only allow selecting from predefined values. The property editor does not + * provide any way to enter custom text or expressions, forcing the user to + * choose from the available options only. + */ + public object Forbid : ExpressionMode() + + /** + * Allow entering custom expressions. The property editor offers a + * "Use Expression" popup-menu item that switches to a text field where the + * user can type expressions like `${__P(property)}` or `${variableName}`. + * + * @property useExpression Text for the menu item to switch to expression mode + * @property useExpressionTooltip Tooltip shown on the "Use Expression" menu item + */ + public data class Allow( + val useExpression: LocalizedString, + val useExpressionTooltip: LocalizedString, + ) : ExpressionMode() +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ModifiedGutter.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ModifiedGutter.kt new file mode 100644 index 00000000000..0c4176ad35c --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ModifiedGutter.kt @@ -0,0 +1,263 @@ +/* + * 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.jorphan.gui + +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls +import java.awt.BorderLayout +import java.awt.Color +import java.awt.Dimension +import java.awt.Graphics +import javax.swing.BorderFactory +import javax.swing.JComponent +import javax.swing.JPanel +import javax.swing.UIManager +import javax.swing.event.AncestorEvent +import javax.swing.event.AncestorListener + +/** + * Wraps a target component and renders a thin coloured "gutter" on the left + * to signal that the value differs from the default ("modified" indicator). + * + * The gutter pattern follows the convention used by IDEs (IntelliJ IDEA, + * Visual Studio Code) where a vertical accent strip next to a setting denotes + * that the user has overridden the default value. Reset/expression actions are + * expected to live in the component's context menu, not as adjacent buttons, + * to keep configuration forms visually quiet. + * + * The decorator is a plain [JPanel] (not a [javax.swing.border.Border]) because + * applying a custom border to controls such as [javax.swing.JComboBox] or + * [javax.swing.JCheckBox] interferes with the look-and-feel UI delegate. + * + * Typical usage: + * ```kotlin + * val editor: JEditableCheckBox = ... + * val gutter = ModifiedGutter(editor) + * editor.addPropertyChangeListener(JEditableCheckBox.VALUE_PROPERTY) { + * gutter.isModified = editor.value != defaultValue + * } + * formPanel.add(gutter) + * ``` + * + * Or, fluently: + * ```kotlin + * val gutter = editor.withModifiedGutter() + * ``` + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public class ModifiedGutter( + public val target: JComponent, +) : JPanel(BorderLayout(GUTTER_GAP, 0)) { + public companion object { + /** Width of the accent strip in pixels. */ + public const val GUTTER_WIDTH: Int = 3 + + /** + * Horizontal gap between the gutter strip and the target component. + * Kept very small (1 px) so that the strip visually belongs to the + * target rather than reading as a separator between siblings. + */ + public const val GUTTER_GAP: Int = 1 + + /** + * Empty space reserved to the left of the gutter strip, between the + * preceding sibling and the start of the strip. Without it, a 3 px + * strip with a 1 px gap to the target ends up roughly equidistant + * between two adjacent controls, which makes it look like a divider. + */ + public const val LEFT_INSET: Int = 4 + + /** Property name fired by [setModified] when the modified flag changes. */ + @NonNls + public const val MODIFIED_PROPERTY: String = "modified" + + /** + * UIManager keys consulted (in order) when picking the gutter colour + * for an enabled control. The first non-null match wins. Designed to + * play well with FlatLaf (`Component.accentColor`, `Component.linkColor`) + * and to fall back to standard Swing keys for other look-and-feels. + */ + @NonNls + private val GUTTER_COLOR_KEYS: List = listOf( + "ModifiedGutter.color", + "Component.accentColor", + "Component.linkColor", + "Component.focusColor", + "controlHighlight", + ) + + /** + * UIManager keys consulted (in order) for the disabled gutter colour. + * If none match, the enabled colour is dimmed via [DISABLED_ALPHA]. + */ + @NonNls + private val DISABLED_GUTTER_COLOR_KEYS: List = listOf( + "ModifiedGutter.disabledColor", + "Component.disabledBorderColor", + "Component.disabledForeground", + "controlShadow", + ) + + /** Fallback colour when the look-and-feel does not provide one. */ + private val FALLBACK_COLOR: Color = Color(0x4C84FF) + + /** + * Alpha applied to the enabled colour when no LaF-provided disabled + * colour is available. Roughly 38 % opacity — enough to read as + * "muted" without disappearing. + */ + private const val DISABLED_ALPHA: Int = 96 + } + + private var modifiedFlag: Boolean = false + + private val gutter: JComponent = object : JComponent() { + init { + preferredSize = Dimension(GUTTER_WIDTH, 0) + minimumSize = Dimension(GUTTER_WIDTH, 0) + } + + override fun paintComponent(g: Graphics) { + super.paintComponent(g) + if (!modifiedFlag) { + return + } + g.color = lookupGutterColor(this@ModifiedGutter.isEnabled) + // Paint the strip only as tall as the target's preferred height + // so it does not overshoot the wrapped control vertically when + // the parent layout stretches the panel (e.g. inside a row of + // taller siblings). The strip is centred on the target. + val targetH = target.preferredSize.height.coerceAtLeast(0) + val drawH = if (targetH in 1 until height) targetH else height + val y = ((height - drawH) / 2).coerceAtLeast(0) + g.fillRect(0, y, width, drawH) + } + + override fun isOpaque(): Boolean = false + } + + /** + * Whether the wrapped component currently holds a value that differs + * from the default. When `true`, the gutter is painted; when `false`, + * the gutter slot is reserved but transparent (so toggling the flag + * never causes a layout shift). + * + * Setting the same value twice is a no-op and does not fire any event. + * A [java.beans.PropertyChangeEvent] with name [MODIFIED_PROPERTY] is + * fired when the value actually changes. + */ + public var isModified: Boolean + get() = modifiedFlag + set(value) { + if (modifiedFlag == value) { + return + } + val old = modifiedFlag + modifiedFlag = value + gutter.repaint() + updateAccessibility() + firePropertyChange(MODIFIED_PROPERTY, old, value) + } + + init { + isOpaque = false + // The empty border on the left separates the strip from the + // preceding sibling control, so the strip clearly belongs to the + // target on the right rather than reading as a divider. + border = BorderFactory.createEmptyBorder(0, LEFT_INSET, 0, 0) + add(gutter, BorderLayout.WEST) + add(target, BorderLayout.CENTER) + updateAccessibility() + // Repaint after look-and-feel changes so that the gutter colour + // tracks the new theme. We can't rely on DynamicStyle.onLaFChange + // here because that registers a global listener whose lifetime + // outlives the panel; instead, repaint when this panel is shown + // and trust the regular updateUI cascade for theme switches. + addAncestorListener(object : AncestorListener { + override fun ancestorAdded(event: AncestorEvent?) { + gutter.repaint() + } + override fun ancestorRemoved(event: AncestorEvent?) = Unit + override fun ancestorMoved(event: AncestorEvent?) = Unit + }) + } + + override fun updateUI() { + super.updateUI() + // updateUI() is called from the JPanel super-constructor before our + // own fields have been assigned, hence the null check. + @Suppress("SENSELESS_COMPARISON") + if (gutter != null) { + // Triggered by the look-and-feel cascade; re-pick the gutter colour. + gutter.repaint() + } + } + + override fun setEnabled(enabled: Boolean) { + if (enabled == isEnabled) return + super.setEnabled(enabled) + // Repaint to pick up the muted disabled colour. JComponent.setEnabled + // already schedules a repaint on the panel itself, but the strip is + // a child component whose paintComponent reads our enabled flag, so + // we forward the repaint explicitly. + gutter.repaint() + } + + private fun lookupGutterColor(enabled: Boolean): Color { + val base = lookupBaseGutterColor() + if (enabled) { + return base + } + for (key in DISABLED_GUTTER_COLOR_KEYS) { + UIManager.getColor(key)?.let { return it } + } + // Fallback: same hue as the enabled colour, but semi-transparent so + // the strip still reads as "modified" while clearly muted. + return Color(base.red, base.green, base.blue, DISABLED_ALPHA) + } + + private fun lookupBaseGutterColor(): Color { + for (key in GUTTER_COLOR_KEYS) { + UIManager.getColor(key)?.let { return it } + } + return FALLBACK_COLOR + } + + private fun updateAccessibility() { + // Screen readers can pick this up even when the colour is invisible + // to colour-blind users, satisfying the "no information by colour + // alone" accessibility requirement. + accessibleContext?.accessibleDescription = if (modifiedFlag) { + "modified" + } else { + null + } + } +} + +/** + * Wraps the receiver in a [ModifiedGutter]. The gutter is initially + * not modified; callers must drive [ModifiedGutter.isModified] explicitly + * (typically from a property-change listener on the wrapped component). + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public fun T.withModifiedGutter(): ModifiedGutter = ModifiedGutter(this) diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ResetMode.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ResetMode.kt new file mode 100644 index 00000000000..98ae9cc2396 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/ResetMode.kt @@ -0,0 +1,49 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apiguardian.api.API + +/** + * Controls whether a property editor exposes a "Reset to default" action + * in its component popup menu. + * + * The action is the keyboard / context-menu counterpart of the + * [ModifiedGutter] indicator: when the gutter is lit (the value differs + * from the default), users can right-click the control and pick this + * item to revert to the default value. The action is automatically + * enabled only while the gutter is lit. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public sealed class ResetMode { + /** + * Do not add a "Reset to default" action to the popup menu. + */ + public object Forbid : ResetMode() + + /** + * Add a "Reset to default" action to the popup menu. The action + * delegates to the editor's `resetToDefault()` method. + * + * @property label Menu item title (typically the localized "Reset" string) + */ + public data class Allow(val label: LocalizedString) : ResetMode() +} diff --git a/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/ModifiedGutterTest.kt b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/ModifiedGutterTest.kt new file mode 100644 index 00000000000..db23681f023 --- /dev/null +++ b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/ModifiedGutterTest.kt @@ -0,0 +1,102 @@ +/* + * 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.jorphan.gui + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.awt.BorderLayout +import java.beans.PropertyChangeEvent +import javax.swing.JCheckBox +import javax.swing.JLabel + +class ModifiedGutterTest { + @Test + fun `target is placed in CENTER`() { + val target = JLabel("hi") + val gutter = ModifiedGutter(target) + val layout = gutter.layout as BorderLayout + assertSame(target, layout.getLayoutComponent(BorderLayout.CENTER)) + assertNotNull(layout.getLayoutComponent(BorderLayout.WEST), "gutter strip must occupy WEST") + assertSame(target, gutter.target) + } + + @Test + fun `gutter slot is reserved even when not modified`() { + // The whole point of the pattern is that toggling modified must not + // shift other form fields, so the WEST slot must stay non-empty. + val gutter = ModifiedGutter(JLabel("x")) + assertFalse(gutter.isModified) + val strip = (gutter.layout as BorderLayout).getLayoutComponent(BorderLayout.WEST) + assertNotNull(strip) + assertEquals(ModifiedGutter.GUTTER_WIDTH, strip!!.preferredSize.width) + } + + @Test + fun `setting modified fires a property change`() { + val gutter = ModifiedGutter(JCheckBox()) + val events = mutableListOf() + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { events += it } + + gutter.isModified = true + + assertTrue(gutter.isModified) + assertEquals(1, events.size, "exactly one event expected") + assertEquals(false, events[0].oldValue) + assertEquals(true, events[0].newValue) + } + + @Test + fun `setting modified to the same value is a no-op`() { + val gutter = ModifiedGutter(JCheckBox()) + val events = mutableListOf() + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { events += it } + + gutter.isModified = false // already false + gutter.isModified = true + gutter.isModified = true // already true + + assertEquals(1, events.size, "redundant assignments must not fire events") + } + + @Test + fun `accessibility description reflects modified state`() { + // Screen readers / colour-blind users rely on something other than colour + // to learn the modified state; we expose it via accessibleDescription. + val gutter = ModifiedGutter(JCheckBox()) + assertNull(gutter.accessibleContext?.accessibleDescription) + + gutter.isModified = true + assertEquals("modified", gutter.accessibleContext?.accessibleDescription) + + gutter.isModified = false + assertNull(gutter.accessibleContext?.accessibleDescription) + } + + @Test + fun `withModifiedGutter wraps the receiver`() { + val checkbox = JCheckBox() + val gutter = checkbox.withModifiedGutter() + assertSame(checkbox, gutter.target) + assertFalse(gutter.isModified) + } +} diff --git a/xdocs/usermanual/build-test-plan.xml b/xdocs/usermanual/build-test-plan.xml index f619502107b..4e50466adc9 100644 --- a/xdocs/usermanual/build-test-plan.xml +++ b/xdocs/usermanual/build-test-plan.xml @@ -61,6 +61,45 @@ controls allow you to configure the behavior of that particular test element. W configured for an element depends on what type of element it is.

The Test Tree itself can be manipulated by dragging and dropping components around the test tree. + +

Modified value indicator

+

Some editable controls show a thin coloured strip along their left edge — the +modified gutter. The strip lights up when the value shown in the control is +stored explicitly on the test element, and stays dark while the control is using +its default (the property is absent on the element, so JMeter falls back to the +descriptor's default or to the value inherited from a parent +HTTP Request Defaults / similar).

+

The indicator lets you tell at a glance which fields you have overridden and +which are still using their defaults:

+
    +
  • Loading a saved .jmx file lights the gutter only on fields whose +value is actually written into the file. Untouched fields stay dark, so you can +spot at a glance which settings the test plan really overrides.
    • +
  • Editing a value lights the gutter immediately, even if you happen to type back +the original default — a value you typed is considered explicit until you reset +it. Saving and re-opening the file preserves this distinction: an explicit value +that happens to equal the default is still written and still lights the gutter +on reload.
    • +
  • The gutter colour follows the current Look-and-Feel accent colour. When the +control is disabled, the strip is shown in a muted tone.
    • +
+ +

Resetting a value to default

+

Right-click on a control whose gutter is lit — the popup menu contains a +Reset entry. Picking it clears the explicit value, restores the +default and turns the gutter off. The menu entry is greyed out when there is +nothing to reset.

+

Single-line text fields support a quick keyboard shortcut: when the field is +empty and the gutter is still lit (because you just cleared a custom +value), pressing Backspace (or +Delete) once more performs the reset. +In practice, holding Backspace until the +field is empty and then pressing it one more time is enough to roll the field +back to its default.

+

While editing an expression (a value typed in ${variable} or +${__function(...)} form), the same Reset entry is +available from the popup menu of the expression text editor, so you do not have +to switch back to the regular control to clear an unwanted override.

From 464286f3973bd57c783887708a519d90d5ec1258 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Fri, 29 May 2026 22:37:07 +0300 Subject: [PATCH 03/10] feat: make JEditableCheckBox gutter-aware and localizable Wire JEditableCheckBox and its JBooleanPropertyEditor binding into the ModifiedGutter pattern and route their captions through a ResourceLocalizer. * The check-box label, the true/false captions, and the "Use Expression" action are localized from JMeter resource bundles. * "Use Expression" is offered as a popup-menu entry (right-click the control); there is no separate on-screen button. * The gutter lights up when the value is explicitly stored on the test element, and a "Reset to default" entry appears in the popup while it is lit. A suppress flag gives explicit-set semantics: any user-driven change marks the editor modified and it stays modified until reset, even when the explicit value equals the default. updateElement is gated on the modified flag, so the gutter, the element, and a save/reload round-trip agree. Tests cover the value getter/setter after the layout refactor, the full gutter state machine, and the updateElement -> updateUi cycle through a real AbstractTestElement, including round-trip preservation of an explicit value that equals the default. Co-Authored-By: Claude Opus 4.8 --- .../jmeter/control/gui/TestPlanGui.java | 9 +- .../control/gui/TransactionControllerGui.java | 6 +- .../jmeter/threads/gui/ThreadGroupGui.java | 9 +- .../jmeter/gui/JBooleanPropertyEditor.kt | 98 ++++-- .../jmeter/resources/messages.properties | 5 +- .../jmeter/resources/messages_fr.properties | 3 +- .../jmeter/gui/JBooleanPropertyEditorTest.kt | 153 +++++++++ .../apache/jorphan/gui/JEditableCheckBox.kt | 163 +++++++--- .../JEditableCheckBoxGutterSemanticsTest.kt | 290 ++++++++++++++++++ .../gui/JEditableCheckBoxModifiedTest.kt | 137 +++++++++ .../http/config/gui/HttpDefaultsGui.java | 9 +- .../http/config/gui/UrlConfigGui.java | 9 +- .../http/control/gui/HttpTestSampleGui.java | 9 +- .../java/control/gui/BeanShellSamplerGui.java | 3 +- 14 files changed, 814 insertions(+), 89 deletions(-) create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditorTest.kt create mode 100644 src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxGutterSemanticsTest.kt create mode 100644 src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxModifiedTest.kt diff --git a/src/core/src/main/java/org/apache/jmeter/control/gui/TestPlanGui.java b/src/core/src/main/java/org/apache/jmeter/control/gui/TestPlanGui.java index 74b9b0f3fae..8528b531cde 100644 --- a/src/core/src/main/java/org/apache/jmeter/control/gui/TestPlanGui.java +++ b/src/core/src/main/java/org/apache/jmeter/control/gui/TestPlanGui.java @@ -57,17 +57,20 @@ public class TestPlanGui extends AbstractJMeterGuiComponent { private final JBooleanPropertyEditor functionalMode = new JBooleanPropertyEditor( TestPlanSchema.INSTANCE.getFunctionalMode(), - JMeterUtils.getResString("functional_mode")); + "functional_mode", + JMeterUtils::getResString); private final JBooleanPropertyEditor serializedMode = new JBooleanPropertyEditor( TestPlanSchema.INSTANCE.getSerializeThreadgroups(), - JMeterUtils.getResString("testplan.serialized")); + "testplan.serialized", + JMeterUtils::getResString); private final JBooleanPropertyEditor tearDownOnShutdown = new JBooleanPropertyEditor( TestPlanSchema.INSTANCE.getTearDownOnShutdown(), - JMeterUtils.getResString("teardown_on_shutdown")); + "teardown_on_shutdown", + JMeterUtils::getResString); /** A panel allowing the user to define variables. */ private final ArgumentsPanel argsPanel; diff --git a/src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java b/src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java index ee513facade..c54984b47c1 100644 --- a/src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java +++ b/src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java @@ -39,13 +39,15 @@ public class TransactionControllerGui extends AbstractControllerGui { private final JBooleanPropertyEditor generateParentSample = new JBooleanPropertyEditor( TransactionControllerSchema.INSTANCE.getGenearteParentSample(), - JMeterUtils.getResString("transaction_controller_parent")); + "transaction_controller_parent", + JMeterUtils::getResString); /** if selected, add duration of timers to total runtime */ private final JBooleanPropertyEditor includeTimers = new JBooleanPropertyEditor( TransactionControllerSchema.INSTANCE.getIncludeTimers(), - JMeterUtils.getResString("transaction_controller_include_timers")); + "transaction_controller_parent", + JMeterUtils::getResString); /** * Create a new TransactionControllerGui instance. diff --git a/src/core/src/main/java/org/apache/jmeter/threads/gui/ThreadGroupGui.java b/src/core/src/main/java/org/apache/jmeter/threads/gui/ThreadGroupGui.java index 3ddcd1f96ee..3c096d0b0eb 100644 --- a/src/core/src/main/java/org/apache/jmeter/threads/gui/ThreadGroupGui.java +++ b/src/core/src/main/java/org/apache/jmeter/threads/gui/ThreadGroupGui.java @@ -65,7 +65,8 @@ public class ThreadGroupGui extends AbstractThreadGroupGui implements ItemListen private final JBooleanPropertyEditor scheduler = new JBooleanPropertyEditor( ThreadGroupSchema.INSTANCE.getUseScheduler(), - JMeterUtils.getResString("scheduler")); + "scheduler", + JMeterUtils::getResString); private final JTextField duration = new JTextField(); private final JLabel durationLabel = labelFor(duration, "duration"); @@ -76,7 +77,8 @@ public class ThreadGroupGui extends AbstractThreadGroupGui implements ItemListen private final JBooleanPropertyEditor sameUserBox = new JBooleanPropertyEditor( AbstractThreadGroupSchema.INSTANCE.getSameUserOnNextIteration(), - JMeterUtils.getResString("threadgroup_same_user")); + "threadgroup_same_user", + JMeterUtils::getResString); public ThreadGroupGui() { this(true); @@ -199,7 +201,8 @@ private void init() { // WARNING: called from ctor so must not be overridden (i. if (showDelayedStart) { delayedStart = new JBooleanPropertyEditor( ThreadGroupSchema.INSTANCE.getDelayedStart(), - JMeterUtils.getResString("delayed_start")); // $NON-NLS-1$ + "delayed_start", + JMeterUtils::getResString); // $NON-NLS-1$ threadPropsPanel.add(delayedStart, "span 2"); } scheduler.addPropertyChangeListener( diff --git a/src/core/src/main/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditor.kt b/src/core/src/main/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditor.kt index 1b4e70af789..b622d751d0d 100644 --- a/src/core/src/main/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditor.kt +++ b/src/core/src/main/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditor.kt @@ -20,52 +20,104 @@ package org.apache.jmeter.gui import org.apache.jmeter.testelement.TestElement import org.apache.jmeter.testelement.property.BooleanProperty import org.apache.jmeter.testelement.schema.BooleanPropertyDescriptor -import org.apache.jmeter.util.JMeterUtils +import org.apache.jorphan.gui.ExpressionMode import org.apache.jorphan.gui.JEditableCheckBox +import org.apache.jorphan.gui.ResetMode +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.PlainValue +import org.apache.jorphan.locale.ResourceLocalizer import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls /** - * Provides editor component for boolean properties that accommodate both true/false and expression string. + * Provides an editor component for boolean properties that accommodate both true/false and expression string. * @since 5.6 */ @API(status = API.Status.EXPERIMENTAL, since = "5.6") public class JBooleanPropertyEditor( private val propertyDescriptor: BooleanPropertyDescriptor<*>, - label: String, -) : JEditableCheckBox(label, DEFAULT_CONFIGURATION), Binding { + label: @NonNls String, + resourceLocalizer: ResourceLocalizer, +) : JEditableCheckBox(label, createConfiguration(resourceLocalizer), resourceLocalizer), Binding { private companion object { - @JvmField - val DEFAULT_CONFIGURATION: Configuration = Configuration( - startEditing = JMeterUtils.getResString("editable_checkbox.use_expression"), - trueValue = "true", - falseValue = "false", - extraValues = listOf( - "\${__P(property_name)}", - "\${variable_name}", + private fun createConfiguration(resourceLocalizer: ResourceLocalizer) = + Configuration( + expressionMode = ExpressionMode.Allow( + useExpression = LocalizedString("edit_as_expression_action", resourceLocalizer), + useExpressionTooltip = LocalizedString("edit_as_expression_tooltip", resourceLocalizer) + ), + trueValue = LocalizedString("editable_checkbox.true", resourceLocalizer), + falseValue = LocalizedString("editable_checkbox.false", resourceLocalizer), + extraValues = listOf( + PlainValue("\${__P(property_name)}"), + PlainValue("\${variable_name}"), + ), + resetMode = ResetMode.Allow(LocalizedString("reset", resourceLocalizer)), ) - ) + } + + /** + * Suppresses automatic [isModified] updates while the editor is being + * driven programmatically (loading from a [TestElement] or being reset). + * Without this flag the value-change listener below would lift the + * "modified" indicator on every internal mutation. + */ + private var suppressModifiedUpdate: Boolean = false + + init { + // Any user-driven value change marks the editor as modified — once + // the user touches the control, the value is considered explicit + // and stays explicit until reset. + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } } public fun reset() { - value = Value.of(propertyDescriptor.defaultValue ?: false) + suppressModifiedUpdate = true + try { + value = Value.of(propertyDescriptor.defaultValue ?: false) + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + override fun resetToDefault() { + reset() } public override fun updateElement(testElement: TestElement) { + if (!isModified) { + // Not explicitly set — drop the property so the element falls + // back to the descriptor's default. Symmetric with updateUi(), + // which marks the editor modified iff the property is present. + testElement.removeProperty(propertyDescriptor) + return + } when (val value = value) { - // For now, UI does not distinguish between "false" and "absent" values, - // so we treat "false" as "absent". - is Value.Boolean -> - testElement[propertyDescriptor] = - value.value.takeIf { it || propertyDescriptor.defaultValue == true } + is Value.Boolean -> testElement[propertyDescriptor] = value.value is Value.Text -> testElement[propertyDescriptor] = value.value } } public override fun updateUi(testElement: TestElement) { - value = when (val value = testElement.getPropertyOrNull(propertyDescriptor)) { - is BooleanProperty, null -> Value.of(value?.booleanValue ?: propertyDescriptor.defaultValue ?: false) - // TODO: should we rather fail in case we detect an unknown property? - else -> Value.Text(value.stringValue) + suppressModifiedUpdate = true + try { + val prop = testElement.getPropertyOrNull(propertyDescriptor) + value = when (prop) { + is BooleanProperty, null -> Value.of(prop?.booleanValue ?: propertyDescriptor.defaultValue ?: false) + // TODO: should we rather fail in case we detect an unknown property? + else -> Value.Text(prop.stringValue) + } + // Modified means "the property is stored explicitly on the element"; + // an absent property means "use the default" and should leave the + // gutter dark. + isModified = prop != null + } finally { + suppressModifiedUpdate = false } } } diff --git a/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties b/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties index 23721c4b4c9..dc34c8f41cb 100644 --- a/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties +++ b/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties @@ -315,7 +315,10 @@ duration_assertion_label=Duration in milliseconds\: duration_assertion_title=Duration Assertion duration_tooltip=Elapsed time of current running Test edit=Edit -editable_checkbox.use_expression=Use Expression +edit_as_expression_action=Use Expression +edit_as_expression_tooltip=Switch to expression mode to use variables like ${__P(property)} +editable_checkbox.true=True +editable_checkbox.false=False email_results_title=Email Results en=English enable=Enable diff --git a/src/core/src/main/resources/org/apache/jmeter/resources/messages_fr.properties b/src/core/src/main/resources/org/apache/jmeter/resources/messages_fr.properties index ed0f7be54d1..1fb8d18ea7b 100644 --- a/src/core/src/main/resources/org/apache/jmeter/resources/messages_fr.properties +++ b/src/core/src/main/resources/org/apache/jmeter/resources/messages_fr.properties @@ -310,7 +310,8 @@ duration_assertion_label=Durée en millisecondes \: duration_assertion_title=Assertion Durée duration_tooltip=Temps passé depuis le début du test en cours edit=Editer -editable_checkbox.use_expression=Utiliser l'expression +edit_as_expression_action=Utiliser l'expression +edit_as_expression_tooltip=Passer en mode expression pour utiliser des variables comme ${__P(property)} email_results_title=Résultat d'email en=Anglais enable=Activer diff --git a/src/core/src/test/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditorTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditorTest.kt new file mode 100644 index 00000000000..3fb0764b458 --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/gui/JBooleanPropertyEditorTest.kt @@ -0,0 +1,153 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.AbstractTestElement +import org.apache.jmeter.testelement.TestElementSchema +import org.apache.jmeter.testelement.schema.BooleanPropertyDescriptor +import org.apache.jorphan.gui.JEditableCheckBox +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * Round-trip tests for [JBooleanPropertyEditor]: the gutter (modified + * indicator) and the underlying test element must stay in sync, in + * particular the editor must: + * + * - persist `false` as an explicit value when default is `false` + * (the previous "treat false as absent" workaround was a bug + * visible to users as a lost gutter on save/reload); + * - keep "absent" absent when the user did not touch the field; + * - restore the modified flag from the property's presence on load. + */ +class JBooleanPropertyEditorTest { + private val identityLocalizer = ResourceLocalizer { it } + + private class TestElementImpl : AbstractTestElement() + + private fun newElement(): AbstractTestElement = TestElementImpl() + + private fun newEditor(default: Boolean? = false): JBooleanPropertyEditor { + val descriptor = BooleanPropertyDescriptor( + shortName = "test", + name = "test.bool", + defaultValue = default, + ) + return JBooleanPropertyEditor(descriptor, "test_label", identityLocalizer) + } + + @Test + fun `untouched editor leaves the element clean`() { + val element = newElement() + val editor = newEditor(default = false) + editor.updateUi(element) + // No user interaction → not modified → property stays absent. + editor.updateElement(element) + + assertFalse(editor.isModified, "freshly loaded editor must not be modified") + assertNull(element.getPropertyOrNull("test.bool"), "untouched value must not be persisted") + } + + @Test + fun `explicit true round-trips`() { + val element = newElement() + val editor = newEditor(default = false) + editor.updateUi(element) + + editor.value = JEditableCheckBox.Value.Boolean.TRUE + editor.updateElement(element) + + val reloaded = newEditor(default = false) + reloaded.updateUi(element) + + assertTrue(reloaded.isModified) + assertEquals(JEditableCheckBox.Value.Boolean.TRUE, reloaded.value) + } + + @Test + fun `explicit false round-trips even when default is false`() { + // Regression guard: the old `takeIf { it || defaultValue == true }` + // would drop an explicit `false` value when the descriptor's + // default was `false`, so the gutter went dark after save/reload + // even though the user had explicitly toggled the checkbox. + val element = newElement() + val editor = newEditor(default = false) + editor.updateUi(element) + + // Switch to TRUE, then back to FALSE so the property-change listener + // fires on a real transition and isModified flips to true. + editor.value = JEditableCheckBox.Value.Boolean.TRUE + editor.value = JEditableCheckBox.Value.Boolean.FALSE + assertTrue(editor.isModified, "explicit FALSE must be marked modified before save") + + editor.updateElement(element) + + val stored = element.getPropertyOrNull("test.bool") + assertNotNull(stored, "explicit FALSE must be persisted, not dropped") + assertEquals(false, stored!!.booleanValue) + + val reloaded = newEditor(default = false) + reloaded.updateUi(element) + assertTrue(reloaded.isModified, "reload must preserve the modified state") + assertEquals(JEditableCheckBox.Value.Boolean.FALSE, reloaded.value) + } + + @Test + fun `explicit false round-trips when default is true`() { + // The complementary case: default=true, the user unchecks the box. + val element = newElement() + val editor = newEditor(default = true) + editor.updateUi(element) // editor shows TRUE, not modified + + editor.value = JEditableCheckBox.Value.Boolean.FALSE + assertTrue(editor.isModified) + editor.updateElement(element) + + val reloaded = newEditor(default = true) + reloaded.updateUi(element) + assertTrue(reloaded.isModified) + assertEquals(JEditableCheckBox.Value.Boolean.FALSE, reloaded.value) + } + + @Test + fun `reset removes the property on save`() { + // After resetToDefault() the editor must persist as "absent" so + // subsequent reloads keep the gutter dark. + val element = newElement() + val editor = newEditor(default = false) + editor.updateUi(element) + + editor.value = JEditableCheckBox.Value.Boolean.TRUE + editor.updateElement(element) + assertNotNull(element.getPropertyOrNull("test.bool")) + + editor.reset() + assertFalse(editor.isModified) + editor.updateElement(element) + assertNull(element.getPropertyOrNull("test.bool"), "reset must drop the stored property") + + val reloaded = newEditor(default = false) + reloaded.updateUi(element) + assertFalse(reloaded.isModified, "reloaded editor must not light the gutter for an absent property") + } +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableCheckBox.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableCheckBox.kt index ea825cf9206..38d281fa0af 100644 --- a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableCheckBox.kt +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableCheckBox.kt @@ -17,18 +17,25 @@ package org.apache.jorphan.gui +import org.apache.jorphan.locale.ComboBoxValue +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.PlainValue +import org.apache.jorphan.locale.ResourceKeyed +import org.apache.jorphan.locale.ResourceLocalizer import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls +import java.awt.BorderLayout import java.awt.Container import java.awt.FlowLayout import java.awt.event.ActionEvent import javax.swing.AbstractAction +import javax.swing.Action import javax.swing.Box import javax.swing.JCheckBox import javax.swing.JComboBox import javax.swing.JLabel import javax.swing.JPanel import javax.swing.JPopupMenu -import javax.swing.SwingUtilities import javax.swing.event.ChangeEvent /** @@ -37,8 +44,9 @@ import javax.swing.event.ChangeEvent */ @API(status = API.Status.EXPERIMENTAL, since = "5.6") public open class JEditableCheckBox( - label: String, - private val configuration: Configuration + label: @NonNls String, + private val configuration: Configuration, + resourceLocalizer: ResourceLocalizer, ) : JPanel() { public companion object { public const val CHECKBOX_CARD: String = "checkbox" @@ -82,65 +90,82 @@ public open class JEditableCheckBox( /** * Supplies the parameters to [JEditableCheckBox]. */ + @API(status = API.Status.EXPERIMENTAL, since = "5.6.0") public data class Configuration( - /** Menu item title to "start editing" the checkbox value. */ - val startEditing: String = "Use Expression", + /** Controls whether custom expressions can be entered (contains useExpression and useExpressionTooltip if Allow). */ + val expressionMode: ExpressionMode, /** The title to be used for "true" value in the checkbox. */ - val trueValue: String = "true", + val trueValue: LocalizedString, /** The title to be used for "false" value in the checkbox. */ - val falseValue: String = "false", + val falseValue: LocalizedString, /** Extra values to be added for the combobox. */ - val extraValues: List = listOf(), + val extraValues: List = listOf(), + /** Controls whether a "Reset to default" item is shown in the component popup menu. */ + val resetMode: ResetMode = ResetMode.Forbid, ) private val cards = CardLayoutWithSizeOfCurrentVisibleElement() + private val cardPanel: JPanel = JPanel(cards).apply { isOpaque = false } + private val gutter: ModifiedGutter = ModifiedGutter(cardPanel) - private val useExpressionAction = object : AbstractAction(configuration.startEditing) { - override fun actionPerformed(e: ActionEvent?) { - cards.next(this@JEditableCheckBox) - comboBox.requestFocusInWindow() - fireValueChanged() + private val useExpressionAction = when (val mode = configuration.expressionMode) { + is ExpressionMode.Allow -> object : AbstractAction(mode.useExpression.toString()) { + init { + putValue(Action.SHORT_DESCRIPTION, mode.useExpressionTooltip.toString()) + } + override fun actionPerformed(e: ActionEvent?) { + cards.next(cardPanel) + comboBox.selectedItem = if (checkbox.isSelected) configuration.trueValue else configuration.falseValue + comboBox.requestFocusInWindow() + } } + ExpressionMode.Forbid -> null } - private val checkbox: JCheckBox = JCheckBox(label).apply { - val cb = this - componentPopupMenu = JPopupMenu().apply { - add(useExpressionAction) + private val resetAction = when (val mode = configuration.resetMode) { + is ResetMode.Allow -> object : AbstractAction(mode.label.toString()) { + init { + isEnabled = false + } + override fun actionPerformed(e: ActionEvent?) { + resetToDefault() + } + } + ResetMode.Forbid -> null + } + + private val checkbox: JCheckBox = JCheckBox(resourceLocalizer.localize(label)).apply { + if (useExpressionAction != null || resetAction != null) { + componentPopupMenu = JPopupMenu().apply { + if (resetAction != null) { + add(resetAction) + } + if (useExpressionAction != null) { + add(useExpressionAction) + } + } } addItemListener { fireValueChanged() } } - private val comboBox: JComboBox = JComboBox().apply { + private val comboBox: JComboBox = JComboBox().apply { isEditable = true configuration.extraValues.forEach { addItem(it) } addItem(configuration.trueValue) addItem(configuration.falseValue) - addActionListener { - val jComboBox = it.source as JComboBox<*> - SwingUtilities.invokeLater { - if (jComboBox.isPopupVisible) { - fireValueChanged() - return@invokeLater - } - when (val value = jComboBox.selectedItem as String) { - configuration.trueValue, configuration.falseValue -> { - checkbox.isSelected = value == configuration.trueValue - cards.show(this@JEditableCheckBox, CHECKBOX_CARD) - checkbox.requestFocusInWindow() - fireValueChanged() - } - } + // Reset must remain reachable while in expression mode. + if (resetAction != null) { + componentPopupMenu = JPopupMenu().apply { + add(resetAction) } } - // TODO: trigger value changed when the text is changed } - private val textFieldLabel = JLabel(label).apply { + private val textFieldLabel = JLabel(resourceLocalizer.localize(label)).apply { labelFor = comboBox } @@ -148,8 +173,17 @@ public open class JEditableCheckBox( private var changeEvent: ChangeEvent? = null init { - layout = cards - add( + layout = BorderLayout() + isOpaque = false + if (resetAction != null) { + // Keep the "Reset to default" menu item enabled only while the + // editor is in the modified state. We listen to the gutter + // because it is the canonical source of the modified flag. + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { + resetAction.isEnabled = it.newValue == true + } + } + cardPanel.add( // A dummy container ensures popup menu appears on top of the checkbox Container().apply { layout = FlowLayout(FlowLayout.LEADING, 0, 0) @@ -157,7 +191,7 @@ public open class JEditableCheckBox( }, CHECKBOX_CARD ) - add( + cardPanel.add( Container().apply { // FlowLayout adds horizontal gap before the first element, so we set zero gaps // and add the gap between the components manually. @@ -168,6 +202,7 @@ public open class JEditableCheckBox( }, EDITABLE_CARD ) + add(gutter, BorderLayout.CENTER) } private var oldValue = value @@ -176,7 +211,10 @@ public open class JEditableCheckBox( super.setEnabled(enabled) checkbox.isEnabled = enabled comboBox.isEnabled = enabled - useExpressionAction.isEnabled = enabled + useExpressionAction?.isEnabled = enabled + // Forward to the gutter so its strip is painted in the muted + // disabled colour rather than the accent colour. + gutter.isEnabled = enabled } private fun fireValueChanged() { @@ -188,27 +226,60 @@ public open class JEditableCheckBox( } public var value: Value - get() = when (components.indexOfFirst { it.isVisible }) { + get() = when (cardPanel.components.indexOfFirst { it.isVisible }) { 0 -> if (checkbox.isSelected) Value.Boolean.TRUE else Value.Boolean.FALSE - else -> Value.Text(comboBox.selectedItem as String) + else -> + when (val value = comboBox.selectedItem) { + is ResourceKeyed -> + when (value.resourceKey) { + configuration.trueValue.resourceKey -> Value.Boolean.TRUE + configuration.falseValue.resourceKey -> Value.Boolean.FALSE + else -> Value.Text(value.resourceKey) + } + else -> Value.Text(value?.toString() ?: "") + } } set(value) { when (value) { is Value.Boolean -> { - comboBox.selectedItem = "" checkbox.isSelected = value.value - cards.show(this, CHECKBOX_CARD) + cards.show(cardPanel, CHECKBOX_CARD) } is Value.Text -> { - checkbox.isSelected = false - comboBox.selectedItem = value.value - cards.show(this, EDITABLE_CARD) + comboBox.selectedItem = PlainValue(value.value) + cards.show(cardPanel, EDITABLE_CARD) } } fireValueChanged() } + /** + * Whether the editor's current value differs from the default ("modified" state). + * Drives the [ModifiedGutter] strip rendered to the left of the control. + * Subclasses are responsible for updating this flag whenever the value or + * the default value changes. + * + * @since 6.0.0 + */ + public var isModified: Boolean + get() = gutter.isModified + set(value) { + gutter.isModified = value + } + + /** + * Reset the editor to its default value. The default implementation is + * a no-op; subclasses that know about a default value should override + * this method. Invoked by the "Reset to default" popup menu item when + * [Configuration.resetMode] is [ResetMode.Allow]. + * + * @since 6.0.0 + */ + protected open fun resetToDefault() { + // Default no-op: the base class does not know what "default" means. + } + @get:JvmSynthetic public var booleanValue: Boolean @Deprecated(message = "write-only property", level = DeprecationLevel.HIDDEN) diff --git a/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxGutterSemanticsTest.kt b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxGutterSemanticsTest.kt new file mode 100644 index 00000000000..2e1dd21e7e9 --- /dev/null +++ b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxGutterSemanticsTest.kt @@ -0,0 +1,290 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import javax.swing.JCheckBox +import javax.swing.JComboBox +import javax.swing.JMenuItem + +/** + * Behaviour test for the gutter-modified semantics layered on top of + * [JEditableCheckBox] by subclasses such as + * [org.apache.jmeter.gui.JBooleanPropertyEditor]. + * + * The user-facing contract is: + * 1. Initially the gutter is dark — the value is implicit / default. + * 2. Any user-driven value change lights the gutter and the editor stays + * "modified" thereafter, even if the user happens to land back on the + * default value (because the property is now explicitly stored). + * 3. The reset action (popup menu / `resetToDefault`) clears the + * modified flag and restores the default value. + * 4. Loading from a [TestElement]-like source lights the gutter only + * when the source actually holds an explicit value; loading "absent" + * reverts the editor to a clean state. + * + * The test reuses the suppress-flag pattern that lives in + * `JBooleanPropertyEditor` so that the production wiring is exercised + * (listener + reset path + simulated `updateUi`) rather than just + * poking [JEditableCheckBox.isModified] directly. + */ +class JEditableCheckBoxGutterSemanticsTest { + private val identityLocalizer = ResourceLocalizer { it } + + /** + * Test double that mirrors the production "explicit-set" pattern used + * by `JBooleanPropertyEditor` and friends, without dragging in the + * core/testbeans dependency tree. + */ + private class ExplicitCheckBox( + private val defaultValue: Boolean, + localizer: ResourceLocalizer, + ) : JEditableCheckBox( + label = "label", + configuration = Configuration( + expressionMode = ExpressionMode.Allow( + useExpression = LocalizedString("use_expression", localizer), + useExpressionTooltip = LocalizedString("use_expression_tooltip", localizer), + ), + trueValue = LocalizedString("true_label", localizer), + falseValue = LocalizedString("false_label", localizer), + resetMode = ResetMode.Allow(LocalizedString("reset", localizer)), + ), + resourceLocalizer = localizer, + ) { + private var suppressModifiedUpdate = false + + init { + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } + } + + public override fun resetToDefault() { + suppressModifiedUpdate = true + try { + value = Value.of(defaultValue) + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + /** Mimics what `JBooleanPropertyEditor.updateUi(testElement)` does. */ + fun loadFromElement(explicitValue: Boolean?) { + suppressModifiedUpdate = true + try { + value = Value.of(explicitValue ?: defaultValue) + isModified = explicitValue != null + } finally { + suppressModifiedUpdate = false + } + } + } + + private fun newCheckbox(default: Boolean = false): ExplicitCheckBox = + ExplicitCheckBox(default, identityLocalizer) + + // --- Initial state --- + + @Test + fun `gutter is dark before any interaction`() { + assertFalse(newCheckbox().isModified) + } + + // --- User-driven changes --- + + @Test + fun `user-driven value change lights the gutter`() { + val box = newCheckbox(default = false) + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertTrue(box.isModified, "first user change must light the gutter") + } + + @Test + fun `user-driven change to the default value still counts as modified`() { + // The whole point of the new semantics: returning to the default + // value via the UI is still an explicit user assignment and must + // keep the gutter lit. The earlier "value != default" rule was wrong. + val box = newCheckbox(default = false) + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertTrue(box.isModified) + + box.value = JEditableCheckBox.Value.Boolean.FALSE // == default + assertTrue(box.isModified, "an explicit assignment equal to default is still explicit") + } + + @Test + fun `gutter stays lit through several user-driven changes`() { + val box = newCheckbox(default = true) + box.value = JEditableCheckBox.Value.Boolean.FALSE + box.value = JEditableCheckBox.Value.Boolean.TRUE + box.value = JEditableCheckBox.Value.Boolean.FALSE + assertTrue(box.isModified) + } + + // --- Reset --- + + @Test + fun `resetToDefault restores default value and clears the gutter`() { + val box = newCheckbox(default = true) + // Establish a clean baseline: editor reflects the default (true), + // gutter is dark. Without this, the visible-state of a freshly + // constructed JCheckBox is "unchecked", which would make a + // subsequent `value = FALSE` a no-op (no PropertyChangeEvent fires). + box.loadFromElement(explicitValue = null) + assertFalse(box.isModified) + + box.value = JEditableCheckBox.Value.Boolean.FALSE + assertTrue(box.isModified) + + box.resetToDefault() + + assertFalse(box.isModified, "reset must clear the modified flag") + assertEquals(JEditableCheckBox.Value.Boolean.TRUE, box.value, "reset must restore the default value") + } + + @Test + fun `user can re-modify after reset`() { + val box = newCheckbox(default = false) + box.value = JEditableCheckBox.Value.Boolean.TRUE + box.resetToDefault() + assertFalse(box.isModified) + + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertTrue(box.isModified, "post-reset user change must light the gutter again") + } + + // --- Loading from a TestElement (updateUi parallel) --- + + @Test + fun `loadFromElement with absent property leaves the gutter dark`() { + val box = newCheckbox(default = true) + box.loadFromElement(explicitValue = null) + assertFalse(box.isModified) + assertEquals(JEditableCheckBox.Value.Boolean.TRUE, box.value) + } + + @Test + fun `loadFromElement with explicit property lights the gutter`() { + val box = newCheckbox(default = true) + box.loadFromElement(explicitValue = false) + assertTrue(box.isModified) + assertEquals(JEditableCheckBox.Value.Boolean.FALSE, box.value) + } + + @Test + fun `loadFromElement with explicit value equal to default still lights the gutter`() { + // Mirrors a real .jmx that explicitly stores the default value. + // The user's intent ("I want this stored explicitly") must survive + // a save/reload round-trip. + val box = newCheckbox(default = true) + box.loadFromElement(explicitValue = true) + assertTrue(box.isModified) + } + + @Test + fun `loadFromElement does not fire spurious modifications from the listener`() { + // Regression guard: without the suppress flag, value-change events + // fired during loadFromElement would set isModified=true even when + // the property is absent. + val box = newCheckbox(default = true) + box.loadFromElement(explicitValue = null) // start with default visible + box.value = JEditableCheckBox.Value.Boolean.FALSE // user toggle → modified + assertTrue(box.isModified) + + box.loadFromElement(explicitValue = null) // another absent-property load + assertFalse(box.isModified, "absent property must clear the gutter even if it was lit") + } + + // --- Reset menu item --- + + @Test + fun `reset menu item is disabled when not modified and enabled when modified`() { + val box = newCheckbox(default = false) + val item = findResetMenuItemOnCheckbox(box) + assertFalse(item.isEnabled, "reset must start disabled — nothing to reset") + + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertTrue(item.isEnabled, "reset must enable when the editor becomes modified") + + box.resetToDefault() + assertFalse(item.isEnabled, "reset must disable again after resetToDefault") + } + + @Test + fun `clicking the reset menu item resets value and gutter`() { + val box = newCheckbox(default = false) + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertTrue(box.isModified) + + val item = findResetMenuItemOnCheckbox(box) + item.doClick() + + assertFalse(box.isModified, "click on reset must clear the gutter") + assertEquals(JEditableCheckBox.Value.Boolean.FALSE, box.value, "click on reset must restore the default") + } + + // --- Reset must be reachable from the expression card too --- + + @Test + fun `reset menu item exists on the expression card editor`() { + // Regression guard for the issue where switching to expression mode + // hid the popup menu entirely, leaving the user no way to reset. + val box = newCheckbox(default = false) + val expressionCombo = findExpressionCombo(box) + val popup = expressionCombo.componentPopupMenu + assertNotNull(popup, "expression combo must carry a popup so reset stays reachable") + val resetItem = popup.subElements + .map { it.component as JMenuItem } + .firstOrNull { it.text == "reset" } + assertNotNull(resetItem, "popup on the expression combo must include the reset action") + } + + // --- Helpers --- + + private fun findResetMenuItemOnCheckbox(box: JEditableCheckBox): JMenuItem { + val checkbox = findChild(box) { it is JCheckBox } as JCheckBox + val popup = checkbox.componentPopupMenu + assertNotNull(popup, "checkbox must have a popup menu when resetMode=Allow") + return popup.subElements.map { it.component as JMenuItem } + .first { it.text == "reset" } + } + + private fun findExpressionCombo(box: JEditableCheckBox): JComboBox<*> = + findChild(box) { it is JComboBox<*> } as JComboBox<*> + + private fun findChild(root: java.awt.Container, match: (java.awt.Component) -> Boolean): java.awt.Component { + val queue = ArrayDeque() + queue += root + while (queue.isNotEmpty()) { + val c = queue.removeFirst() + if (c !== root && match(c)) return c + if (c is java.awt.Container) queue += c.components + } + error("no matching child component found") + } +} diff --git a/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxModifiedTest.kt b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxModifiedTest.kt new file mode 100644 index 00000000000..0e140238a70 --- /dev/null +++ b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableCheckBoxModifiedTest.kt @@ -0,0 +1,137 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import javax.swing.JCheckBox +import javax.swing.JMenuItem + +/** + * Smoke test for the integration between [JEditableCheckBox] and the + * embedded [ModifiedGutter]. Subclasses ([JBooleanPropertyEditor], etc.) + * drive the gutter through the inherited `isModified` property; this + * verifies the wiring exists and does not regress with the layout + * refactor that moved the CardLayout into a child panel. + */ +class JEditableCheckBoxModifiedTest { + private val identityLocalizer = ResourceLocalizer { it } + + private fun newCheckbox() = JEditableCheckBox( + label = "label", + configuration = JEditableCheckBox.Configuration( + expressionMode = ExpressionMode.Forbid, + trueValue = LocalizedString("true_label", identityLocalizer), + falseValue = LocalizedString("false_label", identityLocalizer), + ), + resourceLocalizer = identityLocalizer, + ) + + @Test + fun `isModified default is false`() { + assertFalse(newCheckbox().isModified) + } + + @Test + fun `isModified setter is reflected by getter`() { + val box = newCheckbox() + box.isModified = true + assertTrue(box.isModified) + box.isModified = false + assertFalse(box.isModified) + } + + @Test + fun `value setter still works after layout refactor`() { + // Regression guard: when CardLayout moved off `this` onto an inner + // child panel, value getter / setter must still locate the active + // card via cardPanel.components rather than this.components. + val box = newCheckbox() + box.value = JEditableCheckBox.Value.Boolean.TRUE + assertEquals(JEditableCheckBox.Value.Boolean.TRUE, box.value) + box.value = JEditableCheckBox.Value.Boolean.FALSE + assertEquals(JEditableCheckBox.Value.Boolean.FALSE, box.value) + } + + private fun newCheckboxWithReset(onReset: () -> Unit = {}) = + object : JEditableCheckBox( + label = "label", + configuration = Configuration( + expressionMode = ExpressionMode.Forbid, + trueValue = LocalizedString("true_label", identityLocalizer), + falseValue = LocalizedString("false_label", identityLocalizer), + resetMode = ResetMode.Allow(LocalizedString("reset", identityLocalizer)), + ), + resourceLocalizer = identityLocalizer, + ) { + override fun resetToDefault() = onReset() + } + + private fun findResetMenuItem(box: JEditableCheckBox): JMenuItem { + // The popup menu lives on the inner JCheckBox. + val checkbox = findCheckbox(box) + val menu = checkbox.componentPopupMenu + assertNotNull(menu, "componentPopupMenu must exist when resetMode=Allow") + return menu.subElements.map { it.component as JMenuItem } + .first { it.text == "reset" } + } + + private fun findCheckbox(box: JEditableCheckBox): JCheckBox { + // Walk the component tree until we find the JCheckBox. + val queue = ArrayDeque() + queue += box + while (queue.isNotEmpty()) { + val c = queue.removeFirst() + if (c is JCheckBox) return c + if (c is java.awt.Container) { + queue += c.components + } + } + error("inner JCheckBox not found") + } + + @Test + fun `reset menu item is disabled until modified`() { + val box = newCheckboxWithReset() + val item = findResetMenuItem(box) + assertFalse(item.isEnabled, "must start disabled — nothing to reset") + + box.isModified = true + assertTrue(item.isEnabled, "must become enabled when value diverges from default") + + box.isModified = false + assertFalse(item.isEnabled, "must go back to disabled when reset to default") + } + + @Test + fun `reset menu item triggers resetToDefault`() { + var called = 0 + val box = newCheckboxWithReset(onReset = { called++ }) + val item = findResetMenuItem(box) + // Force-enable: doClick respects enabled state and we are testing the + // action wiring, not the enable logic (covered above). + item.isEnabled = true + item.doClick() + assertEquals(1, called) + } +} 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 8b49bcbb4ec..c056b8a8f02 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 @@ -61,14 +61,17 @@ public class HttpDefaultsGui extends AbstractConfigGui { private UrlConfigGui urlConfigGui; private final JBooleanPropertyEditor retrieveEmbeddedResources = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getRetrieveEmbeddedResources(), - JMeterUtils.getResString("web_testing_retrieve_images")); + "web_testing_retrieve_images", + JMeterUtils::getResString); private final JBooleanPropertyEditor concurrentDwn = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getConcurrentDownload(), - JMeterUtils.getResString("web_testing_concurrent_download")); + "web_testing_concurrent_download", + JMeterUtils::getResString); private JTextField concurrentPool; private final JBooleanPropertyEditor useMD5 = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(), - JMeterUtils.getResString("response_save_as_md5")); // $NON-NLS-1$ + "response_save_as_md5", + JMeterUtils::getResString); // $NON-NLS-1$ private JTextField embeddedAllowRE; // regular expression used to match against embedded resource URLs to allow private JTextField embeddedExcludeRE; // regular expression used to match against embedded resource URLs to discard private JTextField sourceIpAddr; // does not apply to Java implementation 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 2cb82f654c8..055a81551b6 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 @@ -380,19 +380,22 @@ protected Component getPathPanel() { useKeepAlive = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getUseKeepalive(), - JMeterUtils.getResString("use_keepalive")); + "use_keepalive", + JMeterUtils::getResString); JFactory.small(useKeepAlive); useKeepAlive.setVisible(getUrlConfigDefaults().isUseKeepAliveVisible()); useMultipart = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getUseMultipartPost(), - JMeterUtils.getResString("use_multipart_for_http_post")); // $NON-NLS-1$ + "use_multipart_for_http_post", + JMeterUtils::getResString); JFactory.small(useMultipart); useMultipart.setVisible(getUrlConfigDefaults().isUseMultipartVisible()); useBrowserCompatibleMultipartMode = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getUseBrowserCompatibleMultipart(), - JMeterUtils.getResString("use_multipart_mode_browser")); // $NON-NLS-1$ + "use_multipart_mode_browser", + JMeterUtils::getResString); JFactory.small(useBrowserCompatibleMultipartMode); useBrowserCompatibleMultipartMode.setVisible(getUrlConfigDefaults().isUseBrowserCompatibleMultipartModeVisible()); } diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java index 77863a286bd..88da93fb9d6 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java @@ -63,14 +63,17 @@ public class HttpTestSampleGui extends AbstractSamplerGui { private UrlConfigGui urlConfigGui; private final JBooleanPropertyEditor retrieveEmbeddedResources = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getRetrieveEmbeddedResources(), - JMeterUtils.getResString("web_testing_retrieve_images")); + "web_testing_retrieve_images", + JMeterUtils::getResString); private final JBooleanPropertyEditor concurrentDwn = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getConcurrentDownload(), - JMeterUtils.getResString("web_testing_concurrent_download")); + "web_testing_concurrent_download", + JMeterUtils::getResString); private JTextField concurrentPool; private final JBooleanPropertyEditor useMD5 = new JBooleanPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(), - JMeterUtils.getResString("response_save_as_md5")); // $NON-NLS-1$ + "response_save_as_md5", + JMeterUtils::getResString); private JTextField embeddedAllowRE; // regular expression used to match against embedded resource URLs to allow private JTextField embeddedExcludeRE; // regular expression used to match against embedded resource URLs to exclude private JTextField sourceIpAddr; // does not apply to Java implementation diff --git a/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/control/gui/BeanShellSamplerGui.java b/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/control/gui/BeanShellSamplerGui.java index 019347e7952..739c92f1230 100644 --- a/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/control/gui/BeanShellSamplerGui.java +++ b/src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/control/gui/BeanShellSamplerGui.java @@ -49,7 +49,8 @@ public class BeanShellSamplerGui extends AbstractSamplerGui { private final JBooleanPropertyEditor resetInterpreter = new JBooleanPropertyEditor( BeanShellSamplerSchema.INSTANCE.getResetInterpreter(), - JMeterUtils.getResString("bsh_script_reset_interpreter")); + "bsh_script_reset_interpreter", + JMeterUtils::getResString); private final FilePanelEntry filename = new FilePanelEntry(JMeterUtils.getResString("bsh_script_file"),".bsh"); // script file name (if present) From 51428bf0bdd8e02a9d2494bdbdd67444bc857a1e Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Fri, 29 May 2026 22:37:21 +0300 Subject: [PATCH 04/10] feat: add gutter-aware JEnumPropertyEditor and JEditableComboBox Add a pair of enum-typed editors built on the same gutter and explicit-set foundation as the check-box editor. * JEditableComboBox is the underlying control; JEnumPropertyEditor binds it to an enum-typed test element property and exposes a create(...) factory with sensible defaults plus a Configuration data class for full control. * UnsetMode (Forbid or Allow(unsetValue)) controls whether the user can clear the selection; the unset entry renders in italics and the editor reports null when it is selected. * The combo carries the ModifiedGutter, the "Reset to default" and "Use Expression" popup entries, and the same explicit-set semantics as the check-box editor. EnumEditor, ConstantThroughputTimer, and CSVDataSet move to the schema-based descriptors the new editor expects, and GenericTestBeanCustomizer is tightened to match. Tests drive the updateElement -> updateUi cycle through a real AbstractTestElement, including round-trip preservation of explicit values that equal the default. Co-Authored-By: Claude Opus 4.8 --- .../org/apache/jmeter/config/CSVDataSet.java | 14 +- .../jmeter/config/CSVDataSetBeanInfo.java | 2 +- .../timers/ConstantThroughputTimer.java | 15 +- .../ConstantThroughputTimerBeanInfo.java | 1 - .../jmeter/testbeans/gui/EnumEditor.java | 40 +- .../gui/GenericTestBeanCustomizer.java | 23 +- .../apache/jmeter/gui/JEnumPropertyEditor.kt | 230 +++++++++++ .../jmeter/gui/JEnumPropertyEditorTest.kt | 136 +++++++ .../apache/jorphan/gui/JEditableComboBox.kt | 357 ++++++++++++++++++ .../org/apache/jorphan/gui/UnsetMode.kt | 47 +++ 10 files changed, 819 insertions(+), 46 deletions(-) create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/gui/JEnumPropertyEditor.kt create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/gui/JEnumPropertyEditorTest.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableComboBox.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/UnsetMode.kt diff --git a/src/components/src/main/java/org/apache/jmeter/config/CSVDataSet.java b/src/components/src/main/java/org/apache/jmeter/config/CSVDataSet.java index 05f5f8f6f05..285810c5bcc 100644 --- a/src/components/src/main/java/org/apache/jmeter/config/CSVDataSet.java +++ b/src/components/src/main/java/org/apache/jmeter/config/CSVDataSet.java @@ -32,7 +32,7 @@ import org.apache.jmeter.threads.JMeterContext; import org.apache.jmeter.threads.JMeterVariables; import org.apache.jmeter.util.JMeterUtils; -import org.apache.jorphan.util.EnumUtils; +import org.apache.jorphan.locale.ResourceKeyed; import org.apache.jorphan.util.JMeterStopThreadException; import org.apache.jorphan.util.JOrphanUtils; import org.apache.jorphan.util.StringUtilities; @@ -70,20 +70,20 @@ public class CSVDataSet extends ConfigTestElement implements TestBean, LoopIterationListener, NoConfigMerge { - public enum ShareMode { + public enum ShareMode implements ResourceKeyed { ALL("shareMode.all"), GROUP("shareMode.group"), THREAD("shareMode.thread"); - private final String value; + private final String propertyName; - ShareMode(String value) { - this.value = value; + ShareMode(String propertyName) { + this.propertyName = propertyName; } @Override - public String toString() { - return value; + public String getResourceKey() { + return propertyName; } } diff --git a/src/components/src/main/java/org/apache/jmeter/config/CSVDataSetBeanInfo.java b/src/components/src/main/java/org/apache/jmeter/config/CSVDataSetBeanInfo.java index 15c27b60b43..5492eeadcc0 100644 --- a/src/components/src/main/java/org/apache/jmeter/config/CSVDataSetBeanInfo.java +++ b/src/components/src/main/java/org/apache/jmeter/config/CSVDataSetBeanInfo.java @@ -47,7 +47,7 @@ public class CSVDataSetBeanInfo extends BeanInfoSupport { for (CSVDataSet.ShareMode value : CSVDataSet.ShareMode.values()) { @SuppressWarnings("EnumOrdinal") int index = value.ordinal(); - SHARE_TAGS[index] = value.toString(); + SHARE_TAGS[index] = value.getResourceKey(); } } diff --git a/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimer.java b/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimer.java index 21a6edb7aea..7fa9aaf9794 100644 --- a/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimer.java +++ b/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimer.java @@ -32,6 +32,7 @@ import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.util.JMeterUtils; import org.apache.jorphan.collections.IdentityKey; +import org.apache.jorphan.locale.ResourceKeyed; import org.apache.jorphan.util.EnumUtils; import org.apiguardian.api.API; @@ -71,7 +72,7 @@ private static class ThroughputInfo{ /** * This enum defines the calculation modes used by the ConstantThroughputTimer. */ - public enum Mode { + public enum Mode implements ResourceKeyed { ThisThreadOnly("calcMode.1"), // NOSONAR Keep naming for compatibility AllActiveThreads("calcMode.2"), // NOSONAR Keep naming for compatibility AllActiveThreadsInCurrentThreadGroup("calcMode.3"), // NOSONAR Keep naming for compatibility @@ -89,6 +90,11 @@ public enum Mode { public String toString() { return propertyName; } + + @Override + public String getResourceKey() { + return propertyName; + } } /** @@ -161,15 +167,13 @@ public Mode getMode() { } @Deprecated - @SuppressWarnings("EnumOrdinal") public void setCalcMode(int mode) { - setMode(EnumUtils.values(Mode.class).get(mode)); + setMode(EnumUtils.getEnumValues(Mode.class).get(mode)); } - @SuppressWarnings("EnumOrdinal") @API(status = API.Status.MAINTAINED, since = "6.0.0") public void setMode(Mode newMode) { - getSchema().getCalcMode().set(this, newMode.toString()); + getSchema().getCalcMode().set(this, newMode.getResourceKey()); } /** @@ -296,7 +300,6 @@ public String toString() { * so the conversion only needs to happen once. */ @Override - @SuppressWarnings("EnumOrdinal") public void setProperty(JMeterProperty property) { String propertyName = property.getName(); if (propertyName.equals("calcMode")) { diff --git a/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimerBeanInfo.java b/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimerBeanInfo.java index 4f75ceaa707..4d6378dd469 100644 --- a/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimerBeanInfo.java +++ b/src/components/src/main/java/org/apache/jmeter/timers/ConstantThroughputTimerBeanInfo.java @@ -26,7 +26,6 @@ */ public class ConstantThroughputTimerBeanInfo extends BeanInfoSupport { - @SuppressWarnings("EnumOrdinal") public ConstantThroughputTimerBeanInfo() { super(ConstantThroughputTimer.class); diff --git a/src/core/src/main/java/org/apache/jmeter/testbeans/gui/EnumEditor.java b/src/core/src/main/java/org/apache/jmeter/testbeans/gui/EnumEditor.java index 7c241aa4255..a77ac353f1d 100644 --- a/src/core/src/main/java/org/apache/jmeter/testbeans/gui/EnumEditor.java +++ b/src/core/src/main/java/org/apache/jmeter/testbeans/gui/EnumEditor.java @@ -31,6 +31,7 @@ import javax.swing.JList; import org.apache.jmeter.gui.ClearGui; +import org.apache.jorphan.locale.ResourceKeyed; import org.apache.jorphan.util.EnumUtils; /** @@ -41,14 +42,13 @@ * The provided GUI is a combo box with an option for each value in the enum. *

*/ -class EnumEditor extends PropertyEditorSupport implements ClearGui { +class EnumEditor & ResourceKeyed> extends PropertyEditorSupport implements ClearGui { + private final JComboBox combo; - private final JComboBox> combo; + private final T defaultValue; - private final Enum defaultValue; - - public EnumEditor(final PropertyDescriptor descriptor, final Class> enumClazz, final ResourceBundle rb) { - DefaultComboBoxModel> model = new DefaultComboBoxModel<>(); + public EnumEditor(final PropertyDescriptor descriptor, final Class enumClass, final ResourceBundle rb) { + DefaultComboBoxModel model = new DefaultComboBoxModel<>(); combo = new JComboBox<>(model); combo.setEditable(false); combo.setRenderer( @@ -56,19 +56,18 @@ public EnumEditor(final PropertyDescriptor descriptor, final Class list, Object value, int index, boolean isSelected, boolean cellHasFocus) { JLabel label = (JLabel) super.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus); - Enum enumValue = (Enum) value; - label.setText(rb.getString(EnumUtils.getStringValue(enumValue))); + label.setText(rb.getString(enumClass.cast(value).getResourceKey())); return label; } } ); - List> values = EnumUtils.values(enumClazz); - for(Enum e : values) { + List values = EnumUtils.getEnumValues(enumClass); + for (T e : values) { model.addElement(e); } Object def = descriptor.getValue(GenericTestBeanCustomizer.DEFAULT); if (def instanceof Enum enumValue) { - defaultValue = enumValue; + defaultValue = enumClass.cast(enumValue); } else if (def instanceof Integer index) { defaultValue = values.get(index); } else { @@ -99,25 +98,24 @@ public void setValue(Object value) { } else if (value instanceof Integer integer) { combo.setSelectedIndex(integer); } else if (value instanceof String string) { - ComboBoxModel> model = combo.getModel(); - for (int i = 0; i < model.getSize(); i++) { - Enum element = model.getElementAt(i); - if (EnumUtils.getStringValue(element).equals(string)) { - combo.setSelectedItem(element); - return; - } - } + setAsText(string); } } @Override public void setAsText(String value) { - throw new UnsupportedOperationException("Not supported yet. Use enum value rather than text, got " + value); + ComboBoxModel model = combo.getModel(); + for (int i = 0; i < model.getSize(); i++) { + T element = model.getElementAt(i); + if (value.equals(element.getResourceKey())) { + combo.setSelectedItem(element); + return; + } + } } @Override public void clearGui() { combo.setSelectedItem(defaultValue); } - } diff --git a/src/core/src/main/java/org/apache/jmeter/testbeans/gui/GenericTestBeanCustomizer.java b/src/core/src/main/java/org/apache/jmeter/testbeans/gui/GenericTestBeanCustomizer.java index 9f25190d335..e15bd731f13 100644 --- a/src/core/src/main/java/org/apache/jmeter/testbeans/gui/GenericTestBeanCustomizer.java +++ b/src/core/src/main/java/org/apache/jmeter/testbeans/gui/GenericTestBeanCustomizer.java @@ -43,7 +43,6 @@ import javax.swing.JScrollPane; import javax.swing.SwingConstants; -import org.apache.jmeter.JMeter; import org.apache.jmeter.gui.ClearGui; import org.apache.jmeter.testbeans.TestBeanHelper; import org.apache.jmeter.testelement.property.IntegerProperty; @@ -51,6 +50,7 @@ import org.apache.jmeter.testelement.property.LongProperty; import org.apache.jmeter.testelement.property.StringProperty; import org.apache.jmeter.util.JMeterUtils; +import org.apache.jorphan.locale.ResourceKeyed; import org.apache.jorphan.util.EnumUtils; import org.apiguardian.api.API; import org.jspecify.annotations.Nullable; @@ -338,9 +338,9 @@ public GenericTestBeanCustomizer(){ * @return a StringProperty containing the normalized enum value, or null if the property is invalid or unrecognized */ @API(status = API.Status.INTERNAL, since = "6.0.0") - public static > @Nullable JMeterProperty normalizeEnumProperty( + public static & ResourceKeyed> @Nullable JMeterProperty normalizeEnumProperty( Class klass, Class enumKlass, JMeterProperty property) { - List values = EnumUtils.values(enumKlass); + List values = EnumUtils.getEnumValues(enumKlass); T value; if (property instanceof IntegerProperty intProperty) { int index = intProperty.getIntValue(); @@ -362,26 +362,29 @@ public GenericTestBeanCustomizer(){ return null; } value = normalizeEnumStringValue(stringValue, klass, enumKlass); - if (stringValue.equals(EnumUtils.getStringValue(value))) { + if (value == null) { + return null; + } + if (stringValue.equals(value.getResourceKey())) { // If the input property was good enough, keep it return property; } } else { return null; } - return new StringProperty(property.getName(), EnumUtils.getStringValue(value)); + return new StringProperty(property.getName(), value.getResourceKey()); } @API(status = API.Status.INTERNAL, since = "6.0.0") - public static > T normalizeEnumStringValue(String value, Class klass, Class enumKlass) { + public static & ResourceKeyed> @Nullable T normalizeEnumStringValue(String value, Class klass, Class enumKlass) { T enumValue = EnumUtils.valueOf(enumKlass, value); if (enumValue != null) { return enumValue; } - return normalizeEnumStringValue(value, klass, EnumUtils.values(enumKlass)); + return normalizeEnumStringValue(value, klass, EnumUtils.getEnumValues(enumKlass)); } - private static > T normalizeEnumStringValue(String value, Class klass, List values) { + private static & ResourceKeyed> @Nullable T normalizeEnumStringValue(String value, Class klass, List values) { // Fallback: the value might be localized, thus check the current and root locales String bundleName = null; try { @@ -408,9 +411,9 @@ private static > T normalizeEnumStringValue(String value, Clas return findEnumValue(value, rootBundle, values); } - private static > @Nullable T findEnumValue(String stringValue, ResourceBundle rb, List values) { + private static & ResourceKeyed> @Nullable T findEnumValue(String stringValue, ResourceBundle rb, List values) { for (T enumValue : values) { - if (stringValue.equals(rb.getObject(enumValue.toString()))) { + if (stringValue.equals(rb.getObject(enumValue.getResourceKey()))) { log.debug("Converted {} to {} using Locale: {}", stringValue, enumValue, rb.getLocale()); return enumValue; } diff --git a/src/core/src/main/kotlin/org/apache/jmeter/gui/JEnumPropertyEditor.kt b/src/core/src/main/kotlin/org/apache/jmeter/gui/JEnumPropertyEditor.kt new file mode 100644 index 00000000000..cc30d6a8567 --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/gui/JEnumPropertyEditor.kt @@ -0,0 +1,230 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.TestElement +import org.apache.jmeter.testelement.schema.StringPropertyDescriptor +import org.apache.jorphan.gui.ExpressionMode +import org.apache.jorphan.gui.JEditableComboBox +import org.apache.jorphan.gui.ResetMode +import org.apache.jorphan.gui.UnsetMode +import org.apache.jorphan.locale.ComboBoxValue +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.LocalizedValue +import org.apache.jorphan.locale.PlainValue +import org.apache.jorphan.locale.ResourceKeyed +import org.apache.jorphan.locale.ResourceLocalizer +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls + +/** + * Property editor for enum values that implements [ResourceKeyed]. + * + * This editor provides a combo box that displays localized enum values and can also + * accept custom expressions like `${__P(property)}` for dynamic configuration. + * + * The editor: + * - Displays predefined enum values with localized text + * - Stores enum resource keys (not enum names) in the test element property + * - Allows switching to an editable text field for expressions + * - Automatically detects and handles both enum values and custom expressions + * + * Example usage with factory method: + * ```java + * private final JEnumPropertyEditor modeEditor; + * + * modeEditor = JEnumPropertyEditor.create( + * schema.getResponseProcessingMode(), + * "response_mode_label", + * ResponseProcessingMode.class, + * JMeterUtils::getResString, + * UnsetMode.ALLOW, + * ExpressionMode.ALLOW + * ); + * bindingGroup.add(modeEditor); + * ``` + * + * @param E the enum type that implements [ResourceKeyed] + * @property propertyDescriptor the property descriptor for the enum property + * @property configuration the configuration for the enum editor + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public class JEnumPropertyEditor( + private val propertyDescriptor: StringPropertyDescriptor<*>, + private val configuration: Configuration, + label: @NonNls String, + resourceLocalizer: ResourceLocalizer, +) : JEditableComboBox(label, createConfiguration(configuration, resourceLocalizer), resourceLocalizer), Binding + where E : Enum, E : ResourceKeyed { + + /** + * Configuration for [JEnumPropertyEditor]. + * + * @property values the list of enum values to display + * @property unsetMode whether to allow clearing/unsetting the value + * @property expressionMode whether to allow editing expressions instead of selecting from the list + * @property extraValues additional example values to show (e.g., expression templates) + */ + public data class Configuration( + val values: List, + val unsetMode: UnsetMode, + val expressionMode: ExpressionMode, + val extraValues: List = emptyList(), + ) where E : Enum, E : ResourceKeyed + + public companion object { + /** + * Creates a [JEnumPropertyEditor] with the specified configuration options. + * + * This is a convenience factory method for common use cases where you want to + * create an editor with enum mode flags rather than building a full Configuration object. + * + * @param propertyDescriptor the property descriptor for the enum property + * @param label the label text to display next to the combo box + * @param enumClass the class of the enum type + * @param resourceLocalizer the resource localizer for text translation + * @param unsetMode whether to allow clearing/unsetting the value (default: Allow with localized "Undefined") + * @param expressionMode whether to allow editing expressions (default: Allow with standard action text) + * @return a configured [JEnumPropertyEditor] + */ + @JvmStatic + @JvmOverloads + public fun create( + propertyDescriptor: StringPropertyDescriptor<*>, + label: @NonNls String, + enumClass: Class, + resourceLocalizer: ResourceLocalizer, + unsetMode: UnsetMode? = null, + expressionMode: ExpressionMode? = null, + ): JEnumPropertyEditor + where E : Enum, E : ResourceKeyed { + val config = Configuration( + values = enumClass.enumConstants.toList(), + unsetMode = unsetMode ?: UnsetMode.Allow( + unsetValue = LocalizedString("property_undefined", resourceLocalizer) + ), + expressionMode = expressionMode ?: ExpressionMode.Allow( + useExpression = LocalizedString("edit_as_expression_action", resourceLocalizer), + useExpressionTooltip = LocalizedString("edit_as_expression_tooltip", resourceLocalizer) + ), + extraValues = listOf( + PlainValue("\${__P(property_name)}"), + PlainValue("\${variable_name}"), + ) + ) + return JEnumPropertyEditor(propertyDescriptor, config, label, resourceLocalizer) + } + + private fun createConfiguration( + config: Configuration, + resourceLocalizer: ResourceLocalizer + ): JEditableComboBox.Configuration + where E : Enum, E : ResourceKeyed = Configuration( + unsetMode = config.unsetMode, + expressionMode = config.expressionMode, + values = config.values.map { + LocalizedValue(it, resourceLocalizer) + }, + extraValues = config.extraValues, + resourceLocalizer = resourceLocalizer, + resetMode = ResetMode.Allow(LocalizedString("reset", resourceLocalizer)), + ) + } + + /** + * Suppresses automatic [isModified] updates while the editor is being + * driven programmatically (loading from a [TestElement] or being reset). + */ + private var suppressModifiedUpdate: Boolean = false + + init { + // Any user-driven value change marks the editor as modified — once + // the user picks something, the value is considered explicit and + // stays explicit until reset. + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } + } + + /** + * Resets the editor to the default value specified in the property descriptor. + */ + public fun reset() { + suppressModifiedUpdate = true + try { + value = when (configuration.unsetMode) { + is UnsetMode.Allow -> null + UnsetMode.Forbid -> PlainValue(propertyDescriptor.defaultValue ?: "") + } + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + override fun resetToDefault() { + reset() + } + + /** + * Updates the test element with the current value from the editor. + * + * The value is stored as-is (either a resource key or a custom expression). + * If the editor is not modified (the user has not explicitly set a value), + * the property is removed so the element falls back to the descriptor's + * default. Symmetric with [updateUi], which marks the editor modified + * iff the property is present on the element. + */ + override fun updateElement(testElement: TestElement) { + if (!isModified) { + testElement.removeProperty(propertyDescriptor) + return + } + val currentValue = value + testElement[propertyDescriptor] = when (currentValue) { + null -> null + is ResourceKeyed -> currentValue.resourceKey + else -> currentValue.toString() + } + } + + /** + * Updates the editor UI from the test element's property value. + * + * Handles both enum resource keys and custom expression strings. + */ + override fun updateUi(testElement: TestElement) { + suppressModifiedUpdate = true + try { + val property = testElement.getPropertyOrNull(propertyDescriptor) + value = when (property) { + null -> null + else -> PlainValue(property.stringValue) + } + // Modified means "the property is stored explicitly on the element"; + // an absent property means "use the default" and should leave the + // gutter dark. + isModified = property != null + } finally { + suppressModifiedUpdate = false + } + } +} diff --git a/src/core/src/test/kotlin/org/apache/jmeter/gui/JEnumPropertyEditorTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/gui/JEnumPropertyEditorTest.kt new file mode 100644 index 00000000000..fbee6b34884 --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/gui/JEnumPropertyEditorTest.kt @@ -0,0 +1,136 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.AbstractTestElement +import org.apache.jmeter.testelement.TestElementSchema +import org.apache.jmeter.testelement.schema.StringPropertyDescriptor +import org.apache.jorphan.locale.PlainValue +import org.apache.jorphan.locale.ResourceKeyed +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * Round-trip tests for [JEnumPropertyEditor]: the modified-gutter and + * the underlying property must stay in sync across `updateElement` → + * `updateUi`, mirroring what happens on save / open of a `.jmx`. + */ +class JEnumPropertyEditorTest { + private val identityLocalizer = ResourceLocalizer { it } + + private enum class Mode(override val resourceKey: String) : ResourceKeyed { + FAST("mode_fast"), + SLOW("mode_slow"), + } + + private class TestElementImpl : AbstractTestElement() + + private fun newElement(): AbstractTestElement = TestElementImpl() + + private fun newEditor(default: String? = null): JEnumPropertyEditor { + val descriptor = StringPropertyDescriptor( + shortName = "test", + name = "test.mode", + defaultValue = default, + ) + return JEnumPropertyEditor.create( + descriptor, + "test_label", + Mode::class.java, + identityLocalizer, + ) + } + + @Test + fun `untouched editor leaves the element clean`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + editor.updateElement(element) + + assertFalse(editor.isModified) + assertNull(element.getPropertyOrNull("test.mode")) + } + + @Test + fun `selected enum value round-trips`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + + editor.value = PlainValue(Mode.SLOW.resourceKey) + assertTrue(editor.isModified) + editor.updateElement(element) + + val stored = element.getPropertyOrNull("test.mode") + assertNotNull(stored) + assertEquals(Mode.SLOW.resourceKey, stored!!.stringValue) + + val reloaded = newEditor() + reloaded.updateUi(element) + assertTrue(reloaded.isModified) + assertEquals(Mode.SLOW.resourceKey, (reloaded.value as ResourceKeyed).resourceKey) + } + + @Test + fun `selected enum value round-trips even when it equals the default`() { + // Round-trip preservation: a value the user has explicitly chosen + // must survive save/reload as "modified" even when that value + // happens to coincide with the descriptor's default. + val element = newElement() + val editor = newEditor(default = Mode.FAST.resourceKey) + editor.updateUi(element) + + editor.value = PlainValue(Mode.FAST.resourceKey) + assertTrue(editor.isModified) + editor.updateElement(element) + + val stored = element.getPropertyOrNull("test.mode") + assertNotNull(stored, "explicit selection equal to the default must be persisted") + assertEquals(Mode.FAST.resourceKey, stored!!.stringValue) + + val reloaded = newEditor(default = Mode.FAST.resourceKey) + reloaded.updateUi(element) + assertTrue(reloaded.isModified, "reload must light the gutter for an explicit value, even == default") + } + + @Test + fun `reset removes the property on save`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + + editor.value = PlainValue(Mode.SLOW.resourceKey) + editor.updateElement(element) + assertNotNull(element.getPropertyOrNull("test.mode")) + + editor.reset() + assertFalse(editor.isModified) + editor.updateElement(element) + assertNull(element.getPropertyOrNull("test.mode"), "reset must drop the stored property") + + val reloaded = newEditor() + reloaded.updateUi(element) + assertFalse(reloaded.isModified) + } +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableComboBox.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableComboBox.kt new file mode 100644 index 00000000000..1a03ecaa502 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableComboBox.kt @@ -0,0 +1,357 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.ComboBoxValue +import org.apache.jorphan.locale.LocalizedValue +import org.apache.jorphan.locale.PlainValue +import org.apache.jorphan.locale.ResourceKeyed +import org.apache.jorphan.locale.ResourceLocalizer +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls +import java.awt.BorderLayout +import java.awt.Container +import java.awt.FlowLayout +import java.awt.event.ActionEvent +import javax.swing.AbstractAction +import javax.swing.Action +import javax.swing.Box +import javax.swing.DefaultListCellRenderer +import javax.swing.JComboBox +import javax.swing.JLabel +import javax.swing.JPanel +import javax.swing.JPopupMenu +import javax.swing.event.ChangeEvent + +/** + * A combo box that can display predefined enum values (with localized text) or switch to + * an editable text field for custom expressions like `${__P(property)}`. + * + * This component uses a CardLayout to switch between: + * - A non-editable combo box showing predefined values with localized display text + * - An editable combo box allowing custom text input (expressions) + * + * The component stores resource keys (non-localized) as values, but displays localized + * text to the user via a cell renderer. + * + * Example usage: + * ```kotlin + * val config = JEditableComboBox.Configuration( + * startEditing = JMeterUtils.getResString("editable_combobox_use_expression"), + * values = listOf("option_key_1", "option_key_2"), + * extraValues = listOf("\${__P(my_property)}", "\${variable_name}") + * ) + * val comboBox = JEditableComboBox("Label:", config) + * ``` + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public open class JEditableComboBox( + label: @NonNls String, + private val configuration: Configuration, + localizer: ResourceLocalizer, +) : JPanel() { + public companion object { + public const val COMBO_CARD: String = "combo" + public const val EDITABLE_CARD: String = "editable" + public const val VALUE_PROPERTY: String = "value" + } + + /** + * Configuration for the editable combo box. + * + * @property unsetMode whether to allow clearing/unsetting the value (contains unsetValue if Allow) + * @property expressionMode whether to allow editing expressions (contains strings if Allow) + * @property values List of predefined resource keys (stored values) + * @property extraValues Additional template values to show in editable mode (like expressions) + * @property resourceLocalizer Resource localizer for translating strings + * + * @since 6.0.0 + */ + @API(status = API.Status.EXPERIMENTAL, since = "6.0.0") + public data class Configuration( + val unsetMode: UnsetMode, + val expressionMode: ExpressionMode, + val values: List>, + val extraValues: List = listOf(), + val resourceLocalizer: ResourceLocalizer, + /** Controls whether a "Reset to default" item is shown in the component popup menu. */ + val resetMode: ResetMode = ResetMode.Forbid, + ) + + private val cards = CardLayoutWithSizeOfCurrentVisibleElement() + private val cardPanel: JPanel = JPanel(cards).apply { isOpaque = false } + private val gutter: ModifiedGutter = ModifiedGutter(cardPanel) + + // Extract values from sealed classes for easier access + private val unsetValue: ComboBoxValue? = when (val mode = configuration.unsetMode) { + is UnsetMode.Forbid -> null + is UnsetMode.Allow -> mode.unsetValue + } + + private val useExpressionAction = when (val expressionMode = configuration.expressionMode) { + is ExpressionMode.Allow -> object : AbstractAction(expressionMode.useExpression.toString()) { + init { + putValue(Action.SHORT_DESCRIPTION, expressionMode.useExpressionTooltip.toString()) + } + override fun actionPerformed(e: ActionEvent?) { + editableCombo.selectedItem = nonEditableCombo.selectedItem + cards.show(cardPanel, EDITABLE_CARD) + editableCombo.requestFocusInWindow() + } + } + else -> null + } + + private val resetAction = when (val mode = configuration.resetMode) { + is ResetMode.Allow -> object : AbstractAction(mode.label.toString()) { + init { + isEnabled = false + } + override fun actionPerformed(e: ActionEvent?) { + resetToDefault() + } + } + ResetMode.Forbid -> null + } + + private val nonEditableCombo: JComboBox = JComboBox().apply { + isEditable = false + if (useExpressionAction != null || resetAction != null) { + componentPopupMenu = JPopupMenu().apply { + if (resetAction != null) { + add(resetAction) + } + if (useExpressionAction != null) { + add(useExpressionAction) + } + } + } + + if (unsetValue != null) { + addItem(unsetValue) + } + + // Add predefined values + configuration.values.forEach { + addItem(it) + } + + // Custom renderer to show unset value in italics + renderer = object : DefaultListCellRenderer() { + override fun getListCellRendererComponent( + list: javax.swing.JList<*>, + value: Any?, + index: Int, + isSelected: Boolean, + cellHasFocus: Boolean + ): java.awt.Component { + val component = super.getListCellRendererComponent( + list, value, index, isSelected, cellHasFocus + ) as JLabel + if (value === unsetValue) { + component.font = component.font.deriveFont(java.awt.Font.ITALIC) + } + return component + } + } + + addActionListener { + fireValueChanged() + } + } + + private val editableCombo: JComboBox = JComboBox().apply { + isEditable = true + + if (unsetValue != null) { + addItem(unsetValue) + } + // Add template expressions first, then predefined values + configuration.extraValues.forEach { + addItem(it) + } + configuration.values.forEach { + addItem(it) + } + + // Custom renderer to show unset value in italics + renderer = object : DefaultListCellRenderer() { + override fun getListCellRendererComponent( + list: javax.swing.JList<*>, + value: Any?, + index: Int, + isSelected: Boolean, + cellHasFocus: Boolean + ): java.awt.Component { + val component = super.getListCellRendererComponent( + list, value, index, isSelected, cellHasFocus + ) as JLabel + if (value === unsetValue) { + component.font = component.font.deriveFont(java.awt.Font.ITALIC) + } + return component + } + } + + // Reset must remain reachable while in expression mode. + if (resetAction != null) { + componentPopupMenu = JPopupMenu().apply { + add(resetAction) + } + } + } + + private val comboLabel = JLabel(localizer.localize(label)).apply { + labelFor = nonEditableCombo + } + + private val editableLabel = JLabel(localizer.localize(label)).apply { + labelFor = editableCombo + } + + @Transient + private var changeEvent: ChangeEvent? = null + + init { + layout = BorderLayout() + isOpaque = false + if (resetAction != null) { + // Keep the "Reset to default" menu item enabled only while the + // editor is in the modified state. + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { + resetAction.isEnabled = it.newValue == true + } + } + cardPanel.add( + Container().apply { + layout = FlowLayout(FlowLayout.LEADING, 0, 0) + add(comboLabel) + add(Box.createHorizontalStrut(5)) + add(nonEditableCombo) + }, + COMBO_CARD + ) + cardPanel.add( + Container().apply { + layout = FlowLayout(FlowLayout.LEADING, 0, 0) + add(editableLabel) + add(Box.createHorizontalStrut(5)) + add(editableCombo) + }, + EDITABLE_CARD + ) + add(gutter, BorderLayout.CENTER) + } + + private var oldValue = value + + override fun setEnabled(enabled: Boolean) { + super.setEnabled(enabled) + nonEditableCombo.isEnabled = enabled + editableCombo.isEnabled = enabled + useExpressionAction?.isEnabled = enabled + // Forward to the gutter so its strip is painted in the muted + // disabled colour rather than the accent colour. + gutter.isEnabled = enabled + } + + private fun fireValueChanged() { + val newValue = value + if (value != oldValue) { + firePropertyChange(VALUE_PROPERTY, oldValue, newValue) + oldValue = newValue + } + } + + /** + * Gets or sets the current value (resource key or custom expression). + */ + public var value: ComboBoxValue? + get() = when (cardPanel.components.indexOfFirst { it.isVisible }) { + 0 -> nonEditableCombo.selectedItem as? ComboBoxValue + else -> when (val value = editableCombo.selectedItem) { + is ComboBoxValue -> value + is String -> PlainValue(value) + else -> null + } + }.takeIf { it != unsetValue } + set(value) { + // The user might provide a free-text value which coincides with a resourceKey + // For instance, it might be the case when loading a value from jmx test plan + val knownValue = when (value) { + null -> unsetValue + else -> findKnownValue(value) + } + if (knownValue != null) { + // Predefined value - use non-editable combo + nonEditableCombo.selectedItem = knownValue + cards.show(cardPanel, COMBO_CARD) + } else { + // Custom expression - use editable combo + editableCombo.selectedItem = value + cards.show(cardPanel, EDITABLE_CARD) + } + fireValueChanged() + } + + /** + * Whether the editor's current value differs from the default ("modified" state). + * Drives the [ModifiedGutter] strip rendered to the left of the control. + * Subclasses are responsible for updating this flag whenever the value or + * the default value changes. + * + * @since 6.0.0 + */ + public var isModified: Boolean + get() = gutter.isModified + set(value) { + gutter.isModified = value + } + + /** + * Reset the editor to its default value. The default implementation is + * a no-op; subclasses that know about a default value should override + * this method. Invoked by the "Reset to default" popup menu item when + * [Configuration.resetMode] is [ResetMode.Allow]. + * + * @since 6.0.0 + */ + protected open fun resetToDefault() { + // Default no-op: the base class does not know what "default" means. + } + + private fun findKnownValue(value: ComboBoxValue): ComboBoxValue? { + return when (value) { + is PlainValue -> { + // Plain value might match with one of the known resource keys + configuration.values.find { it.value.resourceKey == value.value } + } + else -> { + configuration.values.find { it == value } + } + } + } + + public fun makeSmall() { + JFactory.small(comboLabel) + JFactory.small(editableLabel) + // Note: JFactory.small() doesn't support JComboBox + } +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/UnsetMode.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/UnsetMode.kt new file mode 100644 index 00000000000..f25ab025d16 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/UnsetMode.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.jorphan.gui + +import org.apache.jorphan.locale.ComboBoxValue +import org.apiguardian.api.API + +/** + * Controls whether a property editor allows clearing/unsetting the selected value. + * + * This mode is applicable to property editors that display a selection of values + * (such as combo boxes or enum selectors) where it may be useful to distinguish + * between "no value selected" and "a specific value is selected". + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public sealed class UnsetMode { + /** + * Do not allow clearing the value. The user must always have a value selected, + * and there is no way to return to an unset/null state. + */ + public object Forbid : UnsetMode() + + /** + * Allow clearing the value. For combo boxes, this adds an empty option + * to the dropdown that represents "no selection". + * + * @property unsetValue The value to display when nothing is selected (typically localized "Undefined") + */ + public data class Allow(val unsetValue: ComboBoxValue) : UnsetMode() +} From 8623926be6cf4192fb8d8db2dbb784572ef0775a Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Thu, 7 May 2026 12:41:58 +0300 Subject: [PATCH 05/10] feat: add JEditableTextField with JStringPropertyEditor and backspace-as-reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a third gutter-aware property editor for plain string properties, together with a backspace gesture that resets the field to its default when invoked on an already-empty modified field. Key changes: * New JEditableTextField in jorphan.gui — a gutter-wrapped JTextField with an optional Reset action in the popup menu. There is no separate expression card: free-form text already accepts ${expressions}, so a card switch would just add chrome. * "Quick reset" via Backspace / Delete: pressing the delete key on an empty modified field is interpreted as "undo my custom value", saving a popup trip. The first backspace clears the last character; the next backspace on the now-empty field resets to default and clears the gutter. * New JStringPropertyEditor in core.gui — binds JEditableTextField to a StringPropertyDescriptor with the same explicit-set semantics as JBooleanPropertyEditor / JEnumPropertyEditor (suppress flag, value-change listener, updateUi marks the editor modified iff the property is stored on the element). Persistence is symmetric: updateElement removes the property when not modified and stores the value verbatim when modified, so explicit empty strings now survive a save/reload round-trip. * HttpTestSampleGui: embeddedAllowRE / embeddedExcludeRE are now JStringPropertyEditor instances and provide the first smoke-test of the gutter pattern on text inputs. The MigLayout column constraints on the surrounding panel are tightened so that the two URL labels share column 0 and their fields line up under the wider checkbox row above (the original switch dragged the labels far from their fields). * JEditableTextFieldGutterSemanticsTest covers the full state machine, the popup menu wiring, and the backspace / delete gestures. * JStringPropertyEditorTest exercises updateElement → updateUi through a real AbstractTestElement, including the round-trip case for an explicit empty string. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../jmeter/gui/JStringPropertyEditor.kt | 116 +++++++ .../jmeter/gui/JStringPropertyEditorTest.kt | 129 ++++++++ .../apache/jorphan/gui/JEditableTextField.kt | 178 ++++++++++ .../JEditableTextFieldGutterSemanticsTest.kt | 313 ++++++++++++++++++ .../http/control/gui/HttpTestSampleGui.java | 43 ++- 5 files changed, 765 insertions(+), 14 deletions(-) create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/gui/JStringPropertyEditor.kt create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/gui/JStringPropertyEditorTest.kt create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextField.kt create mode 100644 src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextFieldGutterSemanticsTest.kt diff --git a/src/core/src/main/kotlin/org/apache/jmeter/gui/JStringPropertyEditor.kt b/src/core/src/main/kotlin/org/apache/jmeter/gui/JStringPropertyEditor.kt new file mode 100644 index 00000000000..1a63917c784 --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/gui/JStringPropertyEditor.kt @@ -0,0 +1,116 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.TestElement +import org.apache.jmeter.testelement.schema.StringPropertyDescriptor +import org.apache.jorphan.gui.JEditableTextField +import org.apache.jorphan.gui.ResetMode +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.ResourceLocalizer +import org.apiguardian.api.API + +/** + * Property editor for string properties with [JEditableTextField] as the + * underlying control. + * + * The editor binds to a [StringPropertyDescriptor] and follows the same + * "explicit-set" semantics as [JBooleanPropertyEditor] / + * [JEnumPropertyEditor]: + * * the gutter is dark while the property is absent on the test element, + * * any user-driven change lights the gutter and keeps it lit until reset, + * * `resetToDefault` clears both the value and the modified flag. + * + * Empty input is treated as "use default" — i.e. when the user clears the + * field, the property is removed from the test element. Storing an + * explicit empty string is intentionally not supported in this iteration; + * the rare case can be handled later via a dedicated popup action. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public open class JStringPropertyEditor( + private val propertyDescriptor: StringPropertyDescriptor<*>, + resourceLocalizer: ResourceLocalizer, +) : JEditableTextField(createConfiguration(resourceLocalizer)), Binding { + private companion object { + private fun createConfiguration(resourceLocalizer: ResourceLocalizer): Configuration = + Configuration( + resetMode = ResetMode.Allow(LocalizedString("reset", resourceLocalizer)), + ) + } + + /** + * Suppresses automatic [isModified] updates while the editor is being + * driven programmatically (loading from a [TestElement] or being reset). + */ + private var suppressModifiedUpdate: Boolean = false + + init { + // Any user-driven value change marks the editor as modified — once + // the user touches the field, the value is considered explicit and + // stays explicit until reset. + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } + } + + public fun reset() { + suppressModifiedUpdate = true + try { + value = propertyDescriptor.defaultValue ?: "" + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + override fun resetToDefault() { + reset() + } + + override fun updateElement(testElement: TestElement) { + if (!isModified) { + // Not explicitly set — drop the property so the element falls + // back to the descriptor's default. Symmetric with updateUi(), + // which marks the editor modified iff the property is present. + // This also makes "explicit empty string" survive a save/reload + // round-trip: when the user has typed and then cleared the + // field (gutter still lit), we now persist "" rather than null. + testElement.removeProperty(propertyDescriptor) + return + } + testElement[propertyDescriptor] = value + } + + override fun updateUi(testElement: TestElement) { + suppressModifiedUpdate = true + try { + val prop = testElement.getPropertyOrNull(propertyDescriptor) + value = prop?.stringValue ?: (propertyDescriptor.defaultValue ?: "") + // Modified means "the property is stored explicitly on the + // element"; an absent property means "use the default" and + // should leave the gutter dark. + isModified = prop != null + } finally { + suppressModifiedUpdate = false + } + } +} diff --git a/src/core/src/test/kotlin/org/apache/jmeter/gui/JStringPropertyEditorTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/gui/JStringPropertyEditorTest.kt new file mode 100644 index 00000000000..e8ed1931daf --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/gui/JStringPropertyEditorTest.kt @@ -0,0 +1,129 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.AbstractTestElement +import org.apache.jmeter.testelement.TestElementSchema +import org.apache.jmeter.testelement.schema.StringPropertyDescriptor +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * Round-trip tests for [JStringPropertyEditor]: the modified-gutter + * indicator and the underlying test element must stay in sync across a + * `updateElement` → `updateUi` cycle, which is what happens on Save and + * Open of a `.jmx` test plan. + */ +class JStringPropertyEditorTest { + private val identityLocalizer = ResourceLocalizer { it } + + private class TestElementImpl : AbstractTestElement() + + private fun newElement(): AbstractTestElement = TestElementImpl() + + private fun newEditor(default: String? = null): JStringPropertyEditor { + val descriptor = StringPropertyDescriptor( + shortName = "test", + name = "test.string", + defaultValue = default, + ) + return JStringPropertyEditor(descriptor, identityLocalizer) + } + + @Test + fun `untouched editor leaves the element clean`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + editor.updateElement(element) + + assertFalse(editor.isModified) + assertNull(element.getPropertyOrNull("test.string")) + } + + @Test + fun `non-empty value round-trips`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + + editor.value = "custom" + assertTrue(editor.isModified) + editor.updateElement(element) + + val stored = element.getPropertyOrNull("test.string") + assertNotNull(stored) + assertEquals("custom", stored!!.stringValue) + + val reloaded = newEditor() + reloaded.updateUi(element) + assertTrue(reloaded.isModified) + assertEquals("custom", reloaded.value) + } + + @Test + fun `explicit empty string round-trips`() { + // Regression guard: with the new "isModified-aware" updateElement, + // an empty string the user has typed (gutter lit) must survive a + // save/reload cycle as an explicit empty value rather than being + // silently converted to "absent". + val element = newElement() + val editor = newEditor(default = "default-value") + editor.updateUi(element) + + // Make the editor look modified, then clear it back to "". + editor.value = "anything" + editor.value = "" + assertTrue(editor.isModified, "explicit empty string must keep the editor modified") + editor.updateElement(element) + + val stored = element.getPropertyOrNull("test.string") + assertNotNull(stored, "explicit empty string must be persisted, not dropped") + assertEquals("", stored!!.stringValue) + + val reloaded = newEditor(default = "default-value") + reloaded.updateUi(element) + assertTrue(reloaded.isModified, "reloaded editor must keep the gutter lit for an explicit empty value") + assertEquals("", reloaded.value) + } + + @Test + fun `reset removes the property on save`() { + val element = newElement() + val editor = newEditor() + editor.updateUi(element) + + editor.value = "custom" + editor.updateElement(element) + assertNotNull(element.getPropertyOrNull("test.string")) + + editor.reset() + assertFalse(editor.isModified) + editor.updateElement(element) + assertNull(element.getPropertyOrNull("test.string"), "reset must drop the stored property") + + val reloaded = newEditor() + reloaded.updateUi(element) + assertFalse(reloaded.isModified) + } +} diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextField.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextField.kt new file mode 100644 index 00000000000..df7db0934e8 --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextField.kt @@ -0,0 +1,178 @@ +/* + * 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.jorphan.gui + +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls +import java.awt.BorderLayout +import java.awt.event.ActionEvent +import java.awt.event.KeyAdapter +import java.awt.event.KeyEvent +import javax.swing.AbstractAction +import javax.swing.JPanel +import javax.swing.JPopupMenu +import javax.swing.JTextField +import javax.swing.event.DocumentEvent +import javax.swing.event.DocumentListener + +/** + * A text field with an attached [ModifiedGutter] and an optional + * "Reset to default" action in its popup menu. + * + * Unlike [JEditableCheckBox] / [JEditableComboBox], this class has no + * dedicated expression mode: free-form text already accepts JMeter + * expressions like `${variable}` or `${__P(name)}`, so a separate card + * for expression editing is unnecessary. + * + * Subclasses are expected to drive [isModified] from a property-change + * listener on [VALUE_PROPERTY] and to override [resetToDefault] so that + * the popup item works. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public open class JEditableTextField( + private val configuration: Configuration = Configuration(), +) : JPanel(BorderLayout()) { + public companion object { + /** Property name fired when the text value changes. */ + @NonNls + public const val VALUE_PROPERTY: String = "value" + } + + /** + * Configuration for [JEditableTextField]. + * + * @property resetMode Controls whether a "Reset to default" item is + * shown in the component popup menu. + */ + public data class Configuration( + val resetMode: ResetMode = ResetMode.Forbid, + ) + + private val textField: JTextField = JTextField() + private val gutter: ModifiedGutter = ModifiedGutter(textField) + + private val resetAction = when (val mode = configuration.resetMode) { + is ResetMode.Allow -> object : AbstractAction(mode.label.toString()) { + init { + isEnabled = false + } + override fun actionPerformed(e: ActionEvent?) { + resetToDefault() + } + } + ResetMode.Forbid -> null + } + + private var oldValue: String = "" + + init { + isOpaque = false + if (resetAction != null) { + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { + resetAction.isEnabled = it.newValue == true + } + textField.componentPopupMenu = JPopupMenu().apply { + add(resetAction) + } + } + textField.document.addDocumentListener(object : DocumentListener { + override fun insertUpdate(e: DocumentEvent?) = fireValueChanged() + override fun removeUpdate(e: DocumentEvent?) = fireValueChanged() + override fun changedUpdate(e: DocumentEvent?) = fireValueChanged() + }) + textField.addKeyListener(object : KeyAdapter() { + override fun keyPressed(e: KeyEvent) { + // Backspace / Delete on an already-empty modified field is + // treated as a quick "reset to default" gesture: the user + // cleared the input once (which lit the gutter) and pressing + // the delete key again is the natural "undo my custom value" + // action, saving a trip to the popup menu. + if (e.keyCode == KeyEvent.VK_BACK_SPACE || e.keyCode == KeyEvent.VK_DELETE) { + if (textField.text.isNullOrEmpty() && isModified) { + e.consume() + resetToDefault() + } + } + } + }) + add(gutter, BorderLayout.CENTER) + } + + private fun fireValueChanged() { + val newValue = value + if (newValue != oldValue) { + val old = oldValue + oldValue = newValue + firePropertyChange(VALUE_PROPERTY, old, newValue) + } + } + + /** + * The current text value. Setting the same value as the current one is + * a no-op (no [VALUE_PROPERTY] event is fired). + */ + public var value: String + get() = textField.text ?: "" + set(v) { + if (textField.text != v) { + // The DocumentListener installed in init{} will pick up the + // change and call fireValueChanged() on its own — no need + // to fire it explicitly here. + textField.text = v + } + } + + /** + * Whether the editor's current value differs from the default + * ("modified" state). Drives the [ModifiedGutter] strip rendered to + * the left of the text field. Subclasses are responsible for updating + * this flag whenever the value or the default value changes. + */ + public var isModified: Boolean + get() = gutter.isModified + set(value) { + gutter.isModified = value + } + + /** + * Reset the editor to its default value. The default implementation + * is a no-op; subclasses that know about a default value should + * override this method. Invoked by the "Reset to default" popup menu + * item when [Configuration.resetMode] is [ResetMode.Allow]. + */ + protected open fun resetToDefault() { + // Default no-op: the base class does not know what "default" means. + } + + /** + * Returns the inner [JTextField] so that callers can attach + * `labelFor` from a sibling [javax.swing.JLabel] for accessibility. + */ + public fun getInnerTextField(): JTextField = textField + + override fun setEnabled(enabled: Boolean) { + super.setEnabled(enabled) + textField.isEnabled = enabled + // Forward to the gutter so its strip is painted in the muted + // disabled colour rather than the accent colour. + gutter.isEnabled = enabled + resetAction?.isEnabled = enabled && isModified + } +} diff --git a/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextFieldGutterSemanticsTest.kt b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextFieldGutterSemanticsTest.kt new file mode 100644 index 00000000000..6c2806ca617 --- /dev/null +++ b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextFieldGutterSemanticsTest.kt @@ -0,0 +1,313 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.awt.event.KeyEvent +import javax.swing.JMenuItem +import javax.swing.JTextField + +/** + * Behaviour test for the gutter-modified semantics layered on top of + * [JEditableTextField] by subclasses (`JStringPropertyEditor`). + * + * The contract mirrors [JEditableCheckBoxGutterSemanticsTest]: + * 1. Initially the gutter is dark. + * 2. Any user-driven value change lights the gutter and keeps it lit + * until reset, even when the user types back the default value. + * 3. `resetToDefault` clears the modified flag and restores the default. + * 4. `loadFromElement` lights the gutter only when an explicit value is + * stored on the test element. + */ +class JEditableTextFieldGutterSemanticsTest { + private val identityLocalizer = ResourceLocalizer { it } + + private class ExplicitTextField( + private val defaultValue: String, + localizer: ResourceLocalizer, + ) : JEditableTextField( + Configuration( + resetMode = ResetMode.Allow(LocalizedString("reset", localizer)), + ), + ) { + private var suppressModifiedUpdate = false + + init { + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } + } + + public override fun resetToDefault() { + suppressModifiedUpdate = true + try { + value = defaultValue + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + /** Mimics what `JStringPropertyEditor.updateUi(testElement)` does. */ + fun loadFromElement(explicitValue: String?) { + suppressModifiedUpdate = true + try { + value = explicitValue ?: defaultValue + isModified = explicitValue != null + } finally { + suppressModifiedUpdate = false + } + } + } + + private fun newField(default: String = ""): ExplicitTextField = + ExplicitTextField(default, identityLocalizer) + + // --- Initial state --- + + @Test + fun `gutter is dark before any interaction`() { + assertFalse(newField().isModified) + } + + // --- User-driven changes --- + + @Test + fun `user-driven value change lights the gutter`() { + val field = newField(default = "") + field.value = "abc" + assertTrue(field.isModified) + } + + @Test + fun `user-driven change back to the default still counts as modified`() { + // Same key invariant as for checkbox: returning to the default via + // typing is still an explicit user action. + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) // start clean at default + field.value = "custom" + assertTrue(field.isModified) + + field.value = "default-value" + assertTrue(field.isModified, "typing the default value is still an explicit assignment") + } + + @Test + fun `clearing a non-empty default to empty string lights the gutter`() { + // The user's intent is "I want this stored as empty"; the explicit + // empty case is intentionally captured as modified. + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + field.value = "" + assertTrue(field.isModified) + } + + // --- Reset --- + + @Test + fun `resetToDefault restores default value and clears the gutter`() { + val field = newField(default = "default-value") + field.value = "custom" + assertTrue(field.isModified) + + field.resetToDefault() + + assertFalse(field.isModified) + assertEquals("default-value", field.value) + } + + @Test + fun `user can re-modify after reset`() { + val field = newField(default = "") + field.value = "first" + field.resetToDefault() + assertFalse(field.isModified) + + field.value = "second" + assertTrue(field.isModified) + } + + // --- Loading from a TestElement-like source --- + + @Test + fun `loadFromElement with absent property leaves the gutter dark`() { + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + assertFalse(field.isModified) + assertEquals("default-value", field.value) + } + + @Test + fun `loadFromElement with explicit value lights the gutter`() { + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = "custom") + assertTrue(field.isModified) + assertEquals("custom", field.value) + } + + @Test + fun `loadFromElement with explicit value equal to default still lights the gutter`() { + // Round-trip preservation: a .jmx that explicitly stores the + // default value must light the gutter on reload. + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = "default-value") + assertTrue(field.isModified) + } + + @Test + fun `loadFromElement does not fire spurious modifications from the listener`() { + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + field.value = "user typed" + assertTrue(field.isModified) + + field.loadFromElement(explicitValue = null) + assertFalse(field.isModified, "absent property must clear the gutter even if it was lit") + } + + // --- Reset menu item --- + + @Test + fun `reset menu item is disabled when not modified and enabled when modified`() { + val field = newField(default = "default-value") + val item = findResetMenuItem(field) + assertFalse(item.isEnabled, "must start disabled") + + field.loadFromElement(explicitValue = null) + field.value = "custom" + assertTrue(item.isEnabled) + + field.resetToDefault() + assertFalse(item.isEnabled) + } + + @Test + fun `clicking the reset menu item resets value and gutter`() { + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + field.value = "custom" + assertTrue(field.isModified) + + val item = findResetMenuItem(field) + item.doClick() + + assertFalse(field.isModified) + assertEquals("default-value", field.value) + } + + // --- Backspace / Delete on an empty modified field acts as reset --- + + @Test + fun `backspace on empty modified field resets to default`() { + // Scenario: user typed "abc" (gutter lit), then deleted everything + // by holding backspace (gutter still lit, text empty). Pressing + // backspace once more on the empty modified field is interpreted + // as "I want to roll this back to the default" — saves a popup trip. + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + field.value = "" + assertTrue(field.isModified) + + sendBackspace(field) + + assertFalse(field.isModified, "second backspace on empty must reset") + assertEquals("default-value", field.value, "second backspace must restore the default") + } + + @Test + fun `delete on empty modified field also resets to default`() { + // Forward-delete (Fn+Backspace on macOS, Delete on Windows/Linux) + // is treated identically to backspace. + val field = newField(default = "default-value") + field.loadFromElement(explicitValue = null) + field.value = "" + assertTrue(field.isModified) + + sendKey(field, KeyEvent.VK_DELETE) + + assertFalse(field.isModified) + assertEquals("default-value", field.value) + } + + @Test + fun `backspace on empty unmodified field is a no-op`() { + // Initial / post-reset state. Backspace must not consume the event, + // so the standard JTextField behaviour (typically a UI beep) + // remains the platform default. + val field = newField(default = "") + field.loadFromElement(explicitValue = null) + assertFalse(field.isModified) + assertEquals("", field.value) + + val event = sendBackspace(field) + + assertFalse(event.isConsumed, "no reset to perform — event must pass through to the JTextField") + assertFalse(field.isModified) + } + + @Test + fun `backspace on non-empty field is not consumed`() { + // The custom handling kicks in only when text is empty; + // otherwise the standard JTextField backspace behaviour stays. + val field = newField(default = "") + field.loadFromElement(explicitValue = null) + field.value = "abc" + assertTrue(field.isModified) + + val event = sendBackspace(field) + + assertFalse(event.isConsumed, "non-empty backspace must reach the standard text-field handler") + assertTrue(field.isModified, "non-empty backspace must not lower the modified flag") + } + + // --- Helpers --- + + private fun findResetMenuItem(field: JEditableTextField): JMenuItem { + val popup = field.getInnerTextField().componentPopupMenu + assertNotNull(popup, "text field must have a popup menu when resetMode=Allow") + return popup.subElements.map { it.component as JMenuItem } + .first { it.text == "reset" } + } + + private fun sendBackspace(field: JEditableTextField): KeyEvent = + sendKey(field, KeyEvent.VK_BACK_SPACE) + + /** + * Delivers a key-press to the inner text field's KeyListeners directly. + * `Component.dispatchEvent` is unreliable for synthetic KeyEvents in + * headless test environments (the toolkit may swallow them before the + * listeners are reached), so we invoke the listeners ourselves. + */ + private fun sendKey(field: JEditableTextField, keyCode: Int): KeyEvent { + val tf = field.getInnerTextField() + val event = keyPressEvent(tf, keyCode) + tf.keyListeners.forEach { it.keyPressed(event) } + return event + } + + private fun keyPressEvent(tf: JTextField, keyCode: Int): KeyEvent = + KeyEvent(tf, KeyEvent.KEY_PRESSED, System.currentTimeMillis(), 0, keyCode, KeyEvent.CHAR_UNDEFINED) +} diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java index 88da93fb9d6..44592cd14a5 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java @@ -32,6 +32,7 @@ import org.apache.jmeter.gui.GUIMenuSortOrder; import org.apache.jmeter.gui.JBooleanPropertyEditor; +import org.apache.jmeter.gui.JStringPropertyEditor; import org.apache.jmeter.gui.JTextComponentBinding; import org.apache.jmeter.gui.TestElementMetadata; import org.apache.jmeter.gui.util.HorizontalPanel; @@ -74,8 +75,12 @@ public class HttpTestSampleGui extends AbstractSamplerGui { HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(), "response_save_as_md5", JMeterUtils::getResString); - private JTextField embeddedAllowRE; // regular expression used to match against embedded resource URLs to allow - private JTextField embeddedExcludeRE; // regular expression used to match against embedded resource URLs to exclude + private final JStringPropertyEditor embeddedAllowRE = new JStringPropertyEditor( + HTTPSamplerBaseSchema.INSTANCE.getEmbeddedUrlAllowRegex(), + JMeterUtils::getResString); + private final JStringPropertyEditor embeddedExcludeRE = new JStringPropertyEditor( + HTTPSamplerBaseSchema.INSTANCE.getEmbeddedUrlExcludeRegex(), + JMeterUtils::getResString); private JTextField sourceIpAddr; // does not apply to Java implementation private final JComboBox sourceIpType = new JComboBox<>(HTTPSamplerBase.getSourceTypeList()); private JTextField proxyScheme; @@ -104,8 +109,8 @@ protected HttpTestSampleGui(boolean ajp) { concurrentDwn, new JTextComponentBinding(concurrentPool, schema.getConcurrentDownloadPoolSize()), useMD5, - new JTextComponentBinding(embeddedAllowRE, schema.getEmbeddedUrlAllowRegex()), - new JTextComponentBinding(embeddedExcludeRE, schema.getEmbeddedUrlExcludeRegex()) + embeddedAllowRE, + embeddedExcludeRE ) ); if (!isAJP) { @@ -315,29 +320,39 @@ protected JPanel createEmbeddedRsrcPanel() { concurrentPool.setMinimumSize(new Dimension(10, (int) concurrentPool.getPreferredSize().getHeight())); concurrentPool.setMaximumSize(new Dimension(60, (int) concurrentPool.getPreferredSize().getHeight())); - final JPanel embeddedRsrcPanel = new JPanel(new MigLayout()); + // Two-column grid: [label][editor grows]. The first row uses + // `split 3, span` so its three controls share a single cell and + // do not influence the column widths used by the URL rows below + // — that way the two URL labels end up in the same column and + // their text fields align with each other. + final JPanel embeddedRsrcPanel = new JPanel(new MigLayout("", "[][grow,fill]")); embeddedRsrcPanel.setBorder(BorderFactory.createTitledBorder( JMeterUtils.getResString("web_testing_retrieve_title"))); // $NON-NLS-1$ - embeddedRsrcPanel.add(retrieveEmbeddedResources); + embeddedRsrcPanel.add(retrieveEmbeddedResources, "split 3, span"); embeddedRsrcPanel.add(concurrentDwn); embeddedRsrcPanel.add(concurrentPool, "wrap"); // Embedded URL match regex - embeddedAllowRE = addTextFieldWithLabel(embeddedRsrcPanel, JMeterUtils.getResString("web_testing_embedded_url_pattern")); // $NON-NLS-1$ + addEditableTextFieldWithLabel(embeddedRsrcPanel, + JMeterUtils.getResString("web_testing_embedded_url_pattern"), // $NON-NLS-1$ + embeddedAllowRE); // Embedded URL to not match regex - embeddedExcludeRE = addTextFieldWithLabel(embeddedRsrcPanel, JMeterUtils.getResString("web_testing_embedded_url_exclude_pattern")); // $NON-NLS-1$ + addEditableTextFieldWithLabel(embeddedRsrcPanel, + JMeterUtils.getResString("web_testing_embedded_url_exclude_pattern"), // $NON-NLS-1$ + embeddedExcludeRE); return embeddedRsrcPanel; } - private static JTextField addTextFieldWithLabel(JPanel panel, String labelText) { - JLabel label = new JLabel(labelText); // $NON-NLS-1$ - JTextField field = new JTextField(100); - label.setLabelFor(field); + private static void addEditableTextFieldWithLabel(JPanel panel, String labelText, JStringPropertyEditor editor) { + JLabel label = new JLabel(labelText); + // Wire labelFor to the inner JTextField so screen readers announce + // the label when focus enters the editable area. + label.setLabelFor(editor.getInnerTextField()); + editor.getInnerTextField().setColumns(100); panel.add(label); - panel.add(field, "span"); - return field; + panel.add(editor, "growx, wrap"); } /** From 7a2728d8469a8995db7275b2c2c997dd80355105 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Thu, 7 May 2026 12:57:46 +0300 Subject: [PATCH 06/10] feat: add JEditableTextArea and wire it into the comment field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a multi-line counterpart to JEditableTextField — same gutter, same Reset action, same backspace-as-reset gesture (triggered only when the entire text area is empty). * New JEditableTextArea in jorphan.gui. The text area is added directly to the gutter without a JScrollPane so that short comment-style fields render naturally; callers that need scrolling can wrap the editor or its inner JTextArea externally. * AbstractJMeterGuiComponent.commentField is now backed by a JEditableTextArea. The legacy `commentField` JTextArea reference is retained and points at the editor's inner text area so existing setText / getText callers keep working unchanged. The editor uses a simple "non-empty == modified" rule rather than the explicit-set semantics from JStringPropertyEditor — comments have no notion of "default vs absent", just "set vs cleared", so the listener recomputes the modified flag from the live text on every value change. * New JEditableTextAreaGutterSemanticsTest covers the same scenarios as the text-field test plus a check that multi-line content is read back verbatim and that backspace inside multi-line text falls through to the standard JTextArea behaviour (16 scenarios). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../gui/AbstractJMeterGuiComponent.java | 23 +- .../apache/jorphan/gui/JEditableTextArea.kt | 177 +++++++++++ .../JEditableTextAreaGutterSemanticsTest.kt | 294 ++++++++++++++++++ 3 files changed, 492 insertions(+), 2 deletions(-) create mode 100644 src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextArea.kt create mode 100644 src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextAreaGutterSemanticsTest.kt diff --git a/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java b/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java index d3f5615bc5f..f74f70ac603 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java @@ -40,7 +40,10 @@ import org.apache.jmeter.testelement.TestElementSchema; import org.apache.jmeter.util.JMeterUtils; import org.apache.jmeter.visualizers.Printable; +import org.apache.jorphan.gui.JEditableTextArea; import org.apache.jorphan.gui.JFactory; +import org.apache.jorphan.gui.ResetMode; +import org.apache.jorphan.locale.LocalizedString; import org.apiguardian.api.API; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,7 +84,23 @@ public abstract class AbstractJMeterGuiComponent extends JPanel implements JMete @SuppressWarnings("DeprecatedIsStillUsed") protected NamePanel namePanel; - private final JTextArea commentField = JFactory.tabMovesFocus(new JTextArea()); + private final JEditableTextArea commentEditor = createCommentEditor(); + // The legacy commentField reference still points at the inner JTextArea + // so existing setText / getText callers keep working unchanged. + private final JTextArea commentField = JFactory.tabMovesFocus(commentEditor.getInnerTextArea()); + + private static JEditableTextArea createCommentEditor() { + JEditableTextArea editor = new JEditableTextArea( + new JEditableTextArea.Configuration( + new ResetMode.Allow(new LocalizedString("reset", JMeterUtils::getResString)))); + // Comment-field semantics: the gutter lights up while the comment + // is non-empty. There is no concept of "explicit empty" for a + // comment, so we recompute the modified flag from the live text on + // every value change (programmatic loads as well as typing). + editor.addPropertyChangeListener(JEditableTextArea.VALUE_PROPERTY, + evt -> editor.setModified(!editor.getValue().isEmpty())); + return editor; + } /** * Stores a collection of property editors, so GuiCompoenent can have default implementations that @@ -305,7 +324,7 @@ protected Container makeTitlePanel() { titlePanel.add(labelFor(nameField, "testplan_comments")); commentField.setWrapStyleWord(true); commentField.setLineWrap(true); - titlePanel.add(commentField); + titlePanel.add(commentEditor); // Note: VerticalPanel has a workaround for Box layout which aligns elements, so we can't // use trivial JPanel. diff --git a/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextArea.kt b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextArea.kt new file mode 100644 index 00000000000..f0f984b141b --- /dev/null +++ b/src/jorphan/src/main/kotlin/org/apache/jorphan/gui/JEditableTextArea.kt @@ -0,0 +1,177 @@ +/* + * 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.jorphan.gui + +import org.apiguardian.api.API +import org.jetbrains.annotations.NonNls +import java.awt.BorderLayout +import java.awt.event.ActionEvent +import java.awt.event.KeyAdapter +import java.awt.event.KeyEvent +import javax.swing.AbstractAction +import javax.swing.JPanel +import javax.swing.JPopupMenu +import javax.swing.JTextArea +import javax.swing.event.DocumentEvent +import javax.swing.event.DocumentListener + +/** + * A multi-line text area with an attached [ModifiedGutter] and an + * optional "Reset to default" action in its popup menu. + * + * Multi-line counterpart of [JEditableTextField]: same gutter wiring, + * same popup-menu wiring, same backspace-as-reset gesture (triggered + * only when the entire text area is empty). Like the text field, this + * class does not have a separate expression mode — free-form text + * already accepts JMeter expressions. + * + * The text area is added directly to the gutter, without a + * [javax.swing.JScrollPane], which suits short comment-style fields. + * Callers that need scrolling should wrap the editor's inner + * [JTextArea] (see [getInnerTextArea]) or the editor itself in a + * scroll pane externally. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public open class JEditableTextArea( + private val configuration: Configuration = Configuration(), +) : JPanel(BorderLayout()) { + public companion object { + /** Property name fired when the text value changes. */ + @NonNls + public const val VALUE_PROPERTY: String = "value" + } + + /** + * Configuration for [JEditableTextArea]. + * + * @property resetMode Controls whether a "Reset to default" item is + * shown in the component popup menu. + */ + public data class Configuration( + val resetMode: ResetMode = ResetMode.Forbid, + ) + + private val textArea: JTextArea = JTextArea() + private val gutter: ModifiedGutter = ModifiedGutter(textArea) + + private val resetAction = when (val mode = configuration.resetMode) { + is ResetMode.Allow -> object : AbstractAction(mode.label.toString()) { + init { + isEnabled = false + } + override fun actionPerformed(e: ActionEvent?) { + resetToDefault() + } + } + ResetMode.Forbid -> null + } + + private var oldValue: String = "" + + init { + isOpaque = false + if (resetAction != null) { + gutter.addPropertyChangeListener(ModifiedGutter.MODIFIED_PROPERTY) { + resetAction.isEnabled = it.newValue == true + } + textArea.componentPopupMenu = JPopupMenu().apply { + add(resetAction) + } + } + textArea.document.addDocumentListener(object : DocumentListener { + override fun insertUpdate(e: DocumentEvent?) = fireValueChanged() + override fun removeUpdate(e: DocumentEvent?) = fireValueChanged() + override fun changedUpdate(e: DocumentEvent?) = fireValueChanged() + }) + textArea.addKeyListener(object : KeyAdapter() { + override fun keyPressed(e: KeyEvent) { + // Backspace / Delete on an already-empty modified text area + // is treated as a quick "reset to default" gesture, mirroring + // the JEditableTextField behaviour. The trigger condition + // ("entire text empty") means the user has manually cleared + // every character before this final keystroke fires reset. + if (e.keyCode == KeyEvent.VK_BACK_SPACE || e.keyCode == KeyEvent.VK_DELETE) { + if (textArea.text.isNullOrEmpty() && isModified) { + e.consume() + resetToDefault() + } + } + } + }) + add(gutter, BorderLayout.CENTER) + } + + private fun fireValueChanged() { + val newValue = value + if (newValue != oldValue) { + val old = oldValue + oldValue = newValue + firePropertyChange(VALUE_PROPERTY, old, newValue) + } + } + + /** + * The current text value. Setting the same value as the current one is + * a no-op (no [VALUE_PROPERTY] event is fired). + */ + public var value: String + get() = textArea.text ?: "" + set(v) { + if (textArea.text != v) { + textArea.text = v + } + } + + /** + * Whether the editor's current value differs from the default + * ("modified" state). Drives the [ModifiedGutter] strip rendered to + * the left of the text area. + */ + public var isModified: Boolean + get() = gutter.isModified + set(value) { + gutter.isModified = value + } + + /** + * Reset the editor to its default value. The default implementation + * is a no-op; subclasses that know about a default value should + * override this method. + */ + protected open fun resetToDefault() { + // Default no-op: the base class does not know what "default" means. + } + + /** + * Returns the inner [JTextArea] so that callers can configure + * presentation knobs (rows, columns, line wrap, font) and attach + * `labelFor` from a sibling [javax.swing.JLabel] for accessibility. + */ + public fun getInnerTextArea(): JTextArea = textArea + + override fun setEnabled(enabled: Boolean) { + super.setEnabled(enabled) + textArea.isEnabled = enabled + // Forward to the gutter so its strip is painted in the muted + // disabled colour rather than the accent colour. + gutter.isEnabled = enabled + resetAction?.isEnabled = enabled && isModified + } +} diff --git a/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextAreaGutterSemanticsTest.kt b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextAreaGutterSemanticsTest.kt new file mode 100644 index 00000000000..2f18121948f --- /dev/null +++ b/src/jorphan/src/test/kotlin/org/apache/jorphan/gui/JEditableTextAreaGutterSemanticsTest.kt @@ -0,0 +1,294 @@ +/* + * 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.jorphan.gui + +import org.apache.jorphan.locale.LocalizedString +import org.apache.jorphan.locale.ResourceLocalizer +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.awt.event.KeyEvent +import javax.swing.JMenuItem +import javax.swing.JTextArea + +/** + * Behaviour test for the gutter-modified semantics on + * [JEditableTextArea]. Mirrors [JEditableTextFieldGutterSemanticsTest]: + * the multi-line variant must follow exactly the same rules + * (explicit-set, reset, backspace-as-reset, popup wiring) so the two + * editors stay interchangeable from the user's point of view. + */ +class JEditableTextAreaGutterSemanticsTest { + private val identityLocalizer = ResourceLocalizer { it } + + private class ExplicitTextArea( + private val defaultValue: String, + localizer: ResourceLocalizer, + ) : JEditableTextArea( + Configuration( + resetMode = ResetMode.Allow(LocalizedString("reset", localizer)), + ), + ) { + private var suppressModifiedUpdate = false + + init { + addPropertyChangeListener(VALUE_PROPERTY) { + if (!suppressModifiedUpdate) { + isModified = true + } + } + } + + public override fun resetToDefault() { + suppressModifiedUpdate = true + try { + value = defaultValue + isModified = false + } finally { + suppressModifiedUpdate = false + } + } + + fun loadFromElement(explicitValue: String?) { + suppressModifiedUpdate = true + try { + value = explicitValue ?: defaultValue + isModified = explicitValue != null + } finally { + suppressModifiedUpdate = false + } + } + } + + private fun newArea(default: String = ""): ExplicitTextArea = + ExplicitTextArea(default, identityLocalizer) + + // --- Initial state --- + + @Test + fun `gutter is dark before any interaction`() { + assertFalse(newArea().isModified) + } + + // --- User-driven changes --- + + @Test + fun `user-driven value change lights the gutter`() { + val area = newArea(default = "") + area.value = "line one\nline two" + assertTrue(area.isModified) + } + + @Test + fun `user-driven change back to default still counts as modified`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "custom" + assertTrue(area.isModified) + + area.value = "default text" + assertTrue(area.isModified, "typing the default value is still an explicit assignment") + } + + @Test + fun `clearing a non-empty default to empty string lights the gutter`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "" + assertTrue(area.isModified) + } + + @Test + fun `multi-line value is read back verbatim`() { + // Sanity-check that the wrapper does not strip newlines or + // otherwise tamper with multi-line content as it crosses the + // gutter / value-listener boundary. + val area = newArea(default = "") + area.value = "first\nsecond\nthird" + assertEquals("first\nsecond\nthird", area.value) + assertTrue(area.isModified) + } + + // --- Reset --- + + @Test + fun `resetToDefault restores default value and clears the gutter`() { + val area = newArea(default = "default text") + area.value = "custom" + assertTrue(area.isModified) + + area.resetToDefault() + + assertFalse(area.isModified) + assertEquals("default text", area.value) + } + + @Test + fun `user can re-modify after reset`() { + val area = newArea(default = "") + area.value = "first" + area.resetToDefault() + assertFalse(area.isModified) + + area.value = "second" + assertTrue(area.isModified) + } + + // --- Loading from a TestElement-like source --- + + @Test + fun `loadFromElement with absent property leaves the gutter dark`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + assertFalse(area.isModified) + assertEquals("default text", area.value) + } + + @Test + fun `loadFromElement with explicit value lights the gutter`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = "custom") + assertTrue(area.isModified) + assertEquals("custom", area.value) + } + + @Test + fun `loadFromElement with explicit value equal to default still lights the gutter`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = "default text") + assertTrue(area.isModified) + } + + @Test + fun `loadFromElement does not fire spurious modifications from the listener`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "user typed" + assertTrue(area.isModified) + + area.loadFromElement(explicitValue = null) + assertFalse(area.isModified, "absent property must clear the gutter even if it was lit") + } + + // --- Reset menu item --- + + @Test + fun `reset menu item is disabled when not modified and enabled when modified`() { + val area = newArea(default = "default text") + val item = findResetMenuItem(area) + assertFalse(item.isEnabled) + + area.loadFromElement(explicitValue = null) + area.value = "custom" + assertTrue(item.isEnabled) + + area.resetToDefault() + assertFalse(item.isEnabled) + } + + @Test + fun `clicking the reset menu item resets value and gutter`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "custom" + val item = findResetMenuItem(area) + item.doClick() + + assertFalse(area.isModified) + assertEquals("default text", area.value) + } + + // --- Backspace / Delete on an empty modified field acts as reset --- + + @Test + fun `backspace on empty modified text area resets to default`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "" + assertTrue(area.isModified) + + sendBackspace(area) + + assertFalse(area.isModified) + assertEquals("default text", area.value) + } + + @Test + fun `delete on empty modified text area also resets to default`() { + val area = newArea(default = "default text") + area.loadFromElement(explicitValue = null) + area.value = "" + assertTrue(area.isModified) + + sendKey(area, KeyEvent.VK_DELETE) + + assertFalse(area.isModified) + assertEquals("default text", area.value) + } + + @Test + fun `backspace on empty unmodified text area is a no-op`() { + val area = newArea(default = "") + area.loadFromElement(explicitValue = null) + assertFalse(area.isModified) + + val event = sendBackspace(area) + assertFalse(event.isConsumed) + assertFalse(area.isModified) + } + + @Test + fun `backspace on multi-line text is not consumed`() { + // Backspace inside a non-empty multi-line text area must follow + // the standard JTextArea behaviour (delete the previous character + // or merge two lines), not trigger a reset. + val area = newArea(default = "") + area.loadFromElement(explicitValue = null) + area.value = "line one\nline two" + assertTrue(area.isModified) + + val event = sendBackspace(area) + + assertFalse(event.isConsumed) + assertTrue(area.isModified) + } + + // --- Helpers --- + + private fun findResetMenuItem(area: JEditableTextArea): JMenuItem { + val popup = area.getInnerTextArea().componentPopupMenu + assertNotNull(popup, "text area must have a popup menu when resetMode=Allow") + return popup.subElements.map { it.component as JMenuItem } + .first { it.text == "reset" } + } + + private fun sendBackspace(area: JEditableTextArea): KeyEvent = + sendKey(area, KeyEvent.VK_BACK_SPACE) + + private fun sendKey(area: JEditableTextArea, keyCode: Int): KeyEvent { + val ta = area.getInnerTextArea() + val event = keyPressEvent(ta, keyCode) + ta.keyListeners.forEach { it.keyPressed(event) } + return event + } + + private fun keyPressEvent(ta: JTextArea, keyCode: Int): KeyEvent = + KeyEvent(ta, KeyEvent.KEY_PRESSED, System.currentTimeMillis(), 0, keyCode, KeyEvent.CHAR_UNDEFINED) +} From ffd82f7622004904ef1da1fde61e843028b390dd Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Thu, 28 May 2026 15:43:54 +0300 Subject: [PATCH 07/10] feat: show the modified gutter on the element name field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the name field into the modified-gutter pattern and, while in the same shared base class, fixes the comment field's Reset action. * NamePanel wraps its text field in a gutter-aware JEditableTextField. The gutter lights up when the name differs from the owning component's static label (its default name) and goes dark when it matches. AbstractJMeterGuiComponent feeds the static label through NamePanel.setDefaultName() on configure() / initGui(), and makeTitlePanel() now adds the gutter-aware editor (getNameComponent()) instead of the raw inner JTextField — adding the raw field bypassed the gutter entirely, so it never showed. * NamePanel.resetToDefault qualifies the call as NamePanel.this.setName; an unqualified setName resolved to Component.setName (the Swing component name), so Reset and the backspace gesture did nothing for the name field. * The comment editor now overrides resetToDefault to clear the text. Previously it inherited the no-op base implementation, so Reset and the backspace gesture did nothing for comments either. * NamePanelTest covers the gutter semantics (lit when name differs from default, dark when equal, recomputed on setDefaultName), the Reset menu item, the backspace gesture, and that makeTitlePanel adds the gutter wrapper rather than the raw field. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../gui/AbstractJMeterGuiComponent.java | 19 +- .../java/org/apache/jmeter/gui/NamePanel.java | 64 ++++++- .../org/apache/jmeter/gui/NamePanelTest.kt | 175 ++++++++++++++++++ 3 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/gui/NamePanelTest.kt diff --git a/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java b/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java index f74f70ac603..e148960323f 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java @@ -90,9 +90,18 @@ public abstract class AbstractJMeterGuiComponent extends JPanel implements JMete private final JTextArea commentField = JFactory.tabMovesFocus(commentEditor.getInnerTextArea()); private static JEditableTextArea createCommentEditor() { + // Reset for a comment means "clear it" — the default comment is empty. + // Overriding resetToDefault is required: the base implementation is a + // no-op, so without this the popup "Reset" item and the backspace + // gesture would do nothing. JEditableTextArea editor = new JEditableTextArea( new JEditableTextArea.Configuration( - new ResetMode.Allow(new LocalizedString("reset", JMeterUtils::getResString)))); + new ResetMode.Allow(new LocalizedString("reset", JMeterUtils::getResString)))) { + @Override + protected void resetToDefault() { + setValue(""); // $NON-NLS-1$ + } + }; // Comment-field semantics: the gutter lights up while the comment // is non-empty. There is no concept of "explicit empty" for a // comment, so we recompute the modified flag from the live text on @@ -228,6 +237,9 @@ protected Component createTitleLabel() { */ @Override public void configure(TestElement element) { + // Drive the name field's modified gutter against the static label, + // so a custom name lights the gutter while the default name does not. + namePanel.setDefaultName(getStaticLabel()); setName(element.getName()); enabled = element.isEnabled(); commentField.setText(element.getComment()); @@ -247,6 +259,7 @@ public void clearGui() { } private void initGui() { + namePanel.setDefaultName(getStaticLabel()); setName(getStaticLabel()); commentField.setText(""); } @@ -319,7 +332,9 @@ protected Container makeTitlePanel() { JTextField nameField = namePanel.getNameField(); titlePanel.add(labelFor(nameField, "name")); - titlePanel.add(nameField); + // Add the gutter-aware editor (not the raw nameField) so the + // modified indicator next to the name is shown. + titlePanel.add(namePanel.getNameComponent()); titlePanel.add(labelFor(nameField, "testplan_comments")); commentField.setWrapStyleWord(true); diff --git a/src/core/src/main/java/org/apache/jmeter/gui/NamePanel.java b/src/core/src/main/java/org/apache/jmeter/gui/NamePanel.java index 2651765d738..14c2d876286 100644 --- a/src/core/src/main/java/org/apache/jmeter/gui/NamePanel.java +++ b/src/core/src/main/java/org/apache/jmeter/gui/NamePanel.java @@ -22,6 +22,7 @@ import java.awt.BorderLayout; import java.util.Collection; +import javax.swing.JComponent; import javax.swing.JLabel; import javax.swing.JPanel; import javax.swing.JPopupMenu; @@ -30,6 +31,9 @@ import org.apache.jmeter.testelement.TestElement; import org.apache.jmeter.testelement.property.StringProperty; import org.apache.jmeter.util.JMeterUtils; +import org.apache.jorphan.gui.JEditableTextField; +import org.apache.jorphan.gui.ResetMode; +import org.apache.jorphan.locale.LocalizedString; import org.apiguardian.api.API; public class NamePanel extends JPanel implements JMeterGUIComponent { @@ -37,14 +41,40 @@ public class NamePanel extends JPanel implements JMeterGUIComponent { private static final String LABEL_RESOURCE = "root"; // $NON-NLS-1$ + /** + * The default name to compare against for the "modified" indicator — + * typically the owning component's {@code getStaticLabel()}. When the + * name equals this default, the modified gutter stays dark. + */ + private String defaultName = ""; // $NON-NLS-1$ + + /** Gutter-aware editor wrapping the name text field. */ + private final JEditableTextField nameEditor = new JEditableTextField( + new JEditableTextField.Configuration( + new ResetMode.Allow(new LocalizedString("reset", JMeterUtils::getResString)))) { + @Override + protected void resetToDefault() { + // Qualify with NamePanel.this: an unqualified setName(...) would + // resolve to Component.setName (the Swing component name), not the + // name-field setter we want. + NamePanel.this.setName(defaultName); + } + }; + /** A text field containing the name. */ - private final JTextField nameField = new JTextField(15); + private final JTextField nameField = nameEditor.getInnerTextField(); /** * Create a new NamePanel with the default name. */ public NamePanel() { + nameField.setColumns(15); + // The gutter lights up whenever the name differs from the default + // (the owning component's static label). This is recomputed on every + // edit and whenever the default changes. + nameEditor.addPropertyChangeListener( + JEditableTextField.VALUE_PROPERTY, evt -> updateModified()); _setName(JMeterUtils.getResString(LABEL_RESOURCE)); init(); } @@ -60,7 +90,7 @@ private void init() { // WARNING: called from ctor so must not be overridden (i. nameLabel.setLabelFor(nameField); add(nameLabel, BorderLayout.WEST); - add(nameField, BorderLayout.CENTER); + add(nameEditor, BorderLayout.CENTER); } @API(status = INTERNAL, since = "5.2.0") @@ -68,6 +98,36 @@ public JTextField getNameField() { return nameField; } + /** + * Returns the gutter-aware editor that wraps the name field. Callers that + * lay out the name control themselves (e.g. + * {@code AbstractJMeterGuiComponent.makeTitlePanel}) must add this + * component — not {@link #getNameField()} — so that the modified gutter + * is shown. + * + * @return the editor component to place in a layout + */ + @API(status = INTERNAL, since = "6.0.0") + public JComponent getNameComponent() { + return nameEditor; + } + + /** + * Sets the default name used to drive the modified-gutter indicator. + * When the displayed name equals this value the gutter stays dark. + * + * @param defaultName the default name, usually the owning component's static label + */ + @API(status = INTERNAL, since = "6.0.0") + public void setDefaultName(String defaultName) { + this.defaultName = defaultName != null ? defaultName : ""; // $NON-NLS-1$ + updateModified(); + } + + private void updateModified() { + nameEditor.setModified(!getName().equals(defaultName)); + } + @Override public void clearGui() { setName(getStaticLabel()); diff --git a/src/core/src/test/kotlin/org/apache/jmeter/gui/NamePanelTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/gui/NamePanelTest.kt new file mode 100644 index 00000000000..0c80afeb625 --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/gui/NamePanelTest.kt @@ -0,0 +1,175 @@ +/* + * 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.gui + +import org.apache.jmeter.testelement.TestElement +import org.apache.jorphan.gui.JEditableTextField +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.awt.Component +import java.awt.Container +import java.awt.event.KeyEvent +import javax.swing.JMenuItem +import javax.swing.JPopupMenu +import javax.swing.JTextField + +/** + * Headless tests for the name-field modified gutter. + * + * These run without a display (Swing components can be instantiated and + * their model inspected as long as nothing is painted), so they let us + * verify the gutter logic and the layout wiring without launching the GUI. + * + * The "name editor is wired into the title panel" test in particular + * guards against the regression where `makeTitlePanel` added the raw + * inner `JTextField` instead of the gutter-aware editor, so the gutter + * was never shown. + */ +class NamePanelTest { + private fun editorOf(panel: NamePanel): JEditableTextField = + panel.nameComponent as JEditableTextField + + @Test + fun `gutter is dark when name equals the default`() { + val panel = NamePanel() + panel.setDefaultName("HTTP Request") + panel.name = "HTTP Request" + assertFalse(editorOf(panel).isModified, "name == default must keep the gutter dark") + } + + @Test + fun `gutter lights up when name differs from the default`() { + val panel = NamePanel() + panel.setDefaultName("HTTP Request") + panel.name = "Login request" + assertTrue(editorOf(panel).isModified, "a custom name must light the gutter") + } + + @Test + fun `gutter goes dark again when the name is typed back to the default`() { + val panel = NamePanel() + panel.setDefaultName("HTTP Request") + panel.name = "Login request" + assertTrue(editorOf(panel).isModified) + + panel.name = "HTTP Request" + assertFalse(editorOf(panel).isModified, "restoring the default name must clear the gutter") + } + + @Test + fun `setDefaultName recomputes the gutter for the current name`() { + // When the owning component reports its static label after the name + // is already set, the gutter must reflect the comparison immediately. + val panel = NamePanel() + panel.name = "Login request" + panel.setDefaultName("Login request") + assertFalse(editorOf(panel).isModified) + + panel.setDefaultName("HTTP Request") + assertTrue(editorOf(panel).isModified) + } + + @Test + fun `reset menu item restores the default name and clears the gutter`() { + // Regression guard: the popup Reset must actually restore the default + // name. (A previous bug had resetToDefault calling the inherited + // Component.setName instead of NamePanel.setName, so Reset did nothing.) + val panel = NamePanel() + panel.setDefaultName("HTTP Request") + panel.name = "Login request" + assertTrue(editorOf(panel).isModified) + + resetMenuItem(panel).doClick() + + assertEquals("HTTP Request", panel.name, "Reset must restore the default name") + assertFalse(editorOf(panel).isModified, "Reset must clear the gutter") + } + + @Test + fun `backspace on an empty modified name resets to default`() { + val panel = NamePanel() + panel.setDefaultName("HTTP Request") + panel.name = "" // user cleared the field -> empty but modified + assertTrue(editorOf(panel).isModified) + + val tf = editorOf(panel).getInnerTextField() + val event = KeyEvent(tf, KeyEvent.KEY_PRESSED, System.currentTimeMillis(), 0, KeyEvent.VK_BACK_SPACE, KeyEvent.CHAR_UNDEFINED) + tf.keyListeners.forEach { it.keyPressed(event) } + + assertEquals("HTTP Request", panel.name, "backspace on empty modified name must reset to default") + assertFalse(editorOf(panel).isModified) + } + + private fun resetMenuItem(panel: NamePanel): JMenuItem { + val tf: JTextField = panel.nameField + val popup = tf.componentPopupMenu + val resetText = org.apache.jmeter.util.JMeterUtils.getResString("reset") + return popup.subElements.map { it.component as JMenuItem }.first { it.text == resetText } + } + + @Test + fun `name editor with gutter is placed in the title panel`() { + // Regression guard: makeTitlePanel must add the gutter-aware editor, + // not the raw inner JTextField — otherwise the gutter never renders. + val gui = object : AbstractJMeterGuiComponent() { + override fun getLabelResource(): String = "dummy_element_for_tests" + override fun createTestElement(): TestElement = TODO() + override fun modifyTestElement(element: TestElement?) = TODO() + override fun createPopupMenu(): JPopupMenu = TODO() + override fun getMenuCategories(): MutableCollection = TODO() + fun titlePanelForTest(): Container = makeTitlePanel() as Container + } + + val titlePanel = gui.titlePanelForTest() + val nameField = gui.namePanel.nameField + + // Find the JEditableTextField that actually contains the name field. + val editor = findDescendant(titlePanel) { it is JEditableTextField } + assertTrue(editor != null) { + "the title panel must contain a JEditableTextField wrapping the name field" + } + assertTrue(isAncestorOf(editor as Container, nameField)) { + "the name field must live inside the gutter-aware editor that is in the title panel" + } + // And the raw name field must NOT be added to the title panel directly. + assertSame(editor, findDescendant(titlePanel) { it is JEditableTextField }) + } + + private fun findDescendant(root: Container, match: (Component) -> Boolean): Component? { + val queue = ArrayDeque() + queue += root + while (queue.isNotEmpty()) { + val c = queue.removeFirst() + if (c !== root && match(c)) return c + if (c is Container) queue += c.components + } + return null + } + + private fun isAncestorOf(ancestor: Container, descendant: Component): Boolean { + var c: Component? = descendant + while (c != null) { + if (c === ancestor) return true + c = c.parent + } + return false + } +} From 7b050aba40a513782bc98cfda2b2ad6f52195cd6 Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Thu, 28 May 2026 15:44:04 +0300 Subject: [PATCH 08/10] test: add Screenshots helper for off-screen Swing rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds org.apache.jmeter.testkit.Screenshots, which renders a Swing component into a BufferedImage / PNG without showing a window, by briefly attaching it to a throwaway undecorated frame and painting it on the EDT. This lets tests (and documentation tooling) capture a component's appearance without a human driving the GUI — useful for eyeballing the modified gutter and for auto-generating screenshots for the manual. It needs a display and throws HeadlessException otherwise, so callers should skip when head-less (run under Xvfb on CI if needed). GutterScreenshotDemoTest is a worked example: it renders an unmodified and a modified NamePanel stacked on a white background and writes build/screenshots/name-gutter.png, skipping when head-less. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../jmeter/gui/GutterScreenshotDemoTest.kt | 70 +++++++++++ .../org/apache/jmeter/testkit/Screenshots.kt | 113 ++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/gui/GutterScreenshotDemoTest.kt create mode 100644 src/testkit/src/main/kotlin/org/apache/jmeter/testkit/Screenshots.kt diff --git a/src/core/src/test/kotlin/org/apache/jmeter/gui/GutterScreenshotDemoTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/gui/GutterScreenshotDemoTest.kt new file mode 100644 index 00000000000..dbdc7063b46 --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/gui/GutterScreenshotDemoTest.kt @@ -0,0 +1,70 @@ +/* + * 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.gui + +import org.apache.jmeter.testkit.Screenshots +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Assumptions.assumeFalse +import org.junit.jupiter.api.Test +import java.awt.Color +import java.awt.GraphicsEnvironment +import java.awt.GridLayout +import java.nio.file.Files +import java.nio.file.Path +import javax.swing.BorderFactory +import javax.swing.JPanel + +/** + * Renders the name field with a lit modified gutter and writes a PNG to + * `build/screenshots/`. Acts both as a smoke test for [Screenshots] and as + * a way to eyeball the gutter without launching the full GUI. Skipped when + * running head-less (no display). + */ +class GutterScreenshotDemoTest { + @Test + fun `render name panel with modified gutter`() { + assumeFalse(GraphicsEnvironment.isHeadless(), "needs a display to render") + + // Two name panels side by side on an opaque white background: + // the top one is unmodified (gutter dark), the bottom one is + // modified (gutter lit), so the strip is easy to compare. + val unmodified = NamePanel().apply { + setDefaultName("HTTP Request") + name = "HTTP Request" + } + val modified = NamePanel().apply { + setDefaultName("HTTP Request") + name = "Login request" + } + + val canvas = JPanel(GridLayout(2, 1, 0, 8)).apply { + isOpaque = true + background = Color.WHITE + border = BorderFactory.createEmptyBorder(8, 8, 8, 8) + add(unmodified) + add(modified) + } + canvas.size = canvas.preferredSize + + val out = Path.of("build", "screenshots", "name-gutter.png") + Screenshots.save(canvas, out) + + assertTrue(Files.exists(out), "screenshot should be written to $out") + assertTrue(Files.size(out) > 0, "screenshot should not be empty") + } +} diff --git a/src/testkit/src/main/kotlin/org/apache/jmeter/testkit/Screenshots.kt b/src/testkit/src/main/kotlin/org/apache/jmeter/testkit/Screenshots.kt new file mode 100644 index 00000000000..ef7b8bf82f9 --- /dev/null +++ b/src/testkit/src/main/kotlin/org/apache/jmeter/testkit/Screenshots.kt @@ -0,0 +1,113 @@ +/* + * 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.testkit + +import java.awt.image.BufferedImage +import java.io.IOException +import java.io.UncheckedIOException +import java.lang.reflect.InvocationTargetException +import java.nio.file.Path +import javax.imageio.ImageIO +import javax.swing.JComponent +import javax.swing.JFrame +import javax.swing.SwingUtilities +import kotlin.io.path.createParentDirectories + +/** + * Renders a Swing component into an off-screen image so tests (and + * documentation tooling) can capture a screenshot without a human driving + * the GUI. + * + * The component is briefly attached to an undecorated, never-shown + * [JFrame] so that it is realized (gets a peer) and laid out at its + * preferred size, then painted with [JComponent.printAll]. Rendering always + * runs on the Event Dispatch Thread. + * + * Requires a graphics environment: it throws [java.awt.HeadlessException] + * when run head-less, so on CI run it under a virtual framebuffer such as + * `Xvfb`. Set the desired Look and Feel before calling if a specific theme + * is needed — otherwise the current default LaF is used. + * + * @since 6.0.0 + */ +public object Screenshots { + /** + * Renders the given component into a new ARGB image at its preferred + * size. The component is re-parented into a throwaway frame. + */ + @JvmStatic + public fun render(component: JComponent): BufferedImage { + lateinit var image: BufferedImage + runOnEventDispatchThread { image = renderOnEdt(component) } + return image + } + + /** + * Renders the component and writes it to [target] as a PNG, creating + * parent directories as needed. + * + * @return [target], for chaining + */ + @JvmStatic + public fun save(component: JComponent, target: Path): Path { + val image = render(component) + try { + target.toAbsolutePath().createParentDirectories() + ImageIO.write(image, "png", target.toFile()) + } catch (e: IOException) { + throw UncheckedIOException("Unable to write screenshot to $target", e) + } + return target + } + + private fun renderOnEdt(component: JComponent): BufferedImage { + val frame = JFrame() + frame.isUndecorated = true + try { + frame.contentPane.add(component) + frame.pack() + val width = component.width.coerceAtLeast(1) + val height = component.height.coerceAtLeast(1) + val image = BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB) + val g = image.createGraphics() + try { + component.printAll(g) + } finally { + g.dispose() + } + return image + } finally { + frame.dispose() + } + } + + private fun runOnEventDispatchThread(block: Runnable) { + if (SwingUtilities.isEventDispatchThread()) { + block.run() + return + } + try { + SwingUtilities.invokeAndWait(block) + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + throw IllegalStateException("Interrupted while rendering screenshot", e) + } catch (e: InvocationTargetException) { + throw IllegalStateException("Failed to render screenshot", e.cause) + } + } +} From 8bbbe21f1e776f9481e18043c6d96301ecf36af3 Mon Sep 17 00:00:00 2001 From: jvangaalen Date: Mon, 1 Dec 2025 19:30:49 +0300 Subject: [PATCH 09/10] Store raw body in responseData and only decompress when responseBody is accessed --- .../apache/jmeter/samplers/SampleResult.java | 28 +- .../apache/jmeter/samplers/ResponseDecoder.kt | 95 +++++++ .../samplers/ResponseDecoderRegistry.kt | 188 ++++++++++++++ .../samplers/decoders/DeflateDecoder.kt | 72 ++++++ .../jmeter/samplers/decoders/GzipDecoder.kt | 39 +++ .../jmeter/resources/messages.properties | 7 + .../samplers/ResponseDecoderRegistryTest.kt | 184 ++++++++++++++ .../samplers/decoders/DeflateDecoderTest.kt | 88 +++++++ .../samplers/decoders/GzipDecoderTest.kt | 82 ++++++ src/protocol/http/build.gradle.kts | 4 +- .../http/config/gui/HttpDefaultsGui.java | 29 ++- .../http/control/gui/HttpTestSampleGui.java | 31 +-- .../http/sampler/HTTPAbstractImpl.java | 97 +------ .../protocol/http/sampler/HTTPHC4Impl.java | 89 +------ .../protocol/http/sampler/HTTPJavaImpl.java | 70 ++--- .../http/sampler/HTTPSamplerBase.java | 239 +++++++++++++----- .../http/sampler/HTTPSamplerBaseSchema.kt | 7 + .../http/sampler/decoders/BrotliDecoder.kt | 41 +++ .../sampler/decoders/BrotliDecoderTest.kt | 62 +++++ 19 files changed, 1138 insertions(+), 314 deletions(-) create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoder.kt create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistry.kt create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoder.kt create mode 100644 src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoder.kt create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistryTest.kt create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoderTest.kt create mode 100644 src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoderTest.kt create mode 100644 src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoder.kt create mode 100644 src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoderTest.kt diff --git a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java index 4888d250fcb..4eed1e9e9ce 100644 --- a/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java +++ b/src/core/src/main/java/org/apache/jmeter/samplers/SampleResult.java @@ -17,6 +17,7 @@ package org.apache.jmeter.samplers; +import java.io.IOException; import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.net.HttpURLConnection; @@ -161,6 +162,8 @@ public class SampleResult implements Serializable, Cloneable, Searchable { private byte[] responseData = EMPTY_BA; + private String contentEncoding; // Stores gzip/deflate encoding if response is compressed + private String responseCode = "";// Never return null private String label = "";// Never return null @@ -792,6 +795,16 @@ public void setResponseData(final String response, final String encoding) { * @return the responseData value (cannot be null) */ public byte[] getResponseData() { + if (responseData == null) { + return EMPTY_BA; + } + if (contentEncoding != null && responseData.length > 0) { + try { + return ResponseDecoderRegistry.decode(contentEncoding, responseData); + } catch (IOException e) { + log.warn("Failed to decompress response data", e); + } + } return responseData; } @@ -803,12 +816,12 @@ public byte[] getResponseData() { public String getResponseDataAsString() { try { if(responseDataAsString == null) { - responseDataAsString= new String(responseData,getDataEncodingWithDefault()); + responseDataAsString= new String(getResponseData(),getDataEncodingWithDefault()); } return responseDataAsString; } catch (UnsupportedEncodingException e) { log.warn("Using platform default as {} caused {}", getDataEncodingWithDefault(), e.getLocalizedMessage()); - return new String(responseData,Charset.defaultCharset()); // N.B. default charset is used deliberately here + return new String(getResponseData(),Charset.defaultCharset()); // N.B. default charset is used deliberately here } } @@ -1666,4 +1679,15 @@ public TestLogicalAction getTestLogicalAction() { public void setTestLogicalAction(TestLogicalAction testLogicalAction) { this.testLogicalAction = testLogicalAction; } + + /** + * Sets the response data and its contentEncoding. + * @param data The response data + * @param contentEncoding The content contentEncoding (e.g. gzip, deflate) + */ + public void setResponseData(byte[] data, String contentEncoding) { + responseData = data == null ? EMPTY_BA : data; + this.contentEncoding = contentEncoding; + responseDataAsString = null; + } } diff --git a/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoder.kt b/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoder.kt new file mode 100644 index 00000000000..98954392729 --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoder.kt @@ -0,0 +1,95 @@ +/* + * 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.samplers + +import org.apache.jorphan.io.DirectAccessByteArrayOutputStream +import org.apache.jorphan.reflect.JMeterService +import org.apiguardian.api.API +import java.io.ByteArrayInputStream +import java.io.InputStream + +/** + * Interface for response data decoders that handle different content encodings. + * Implementations can be automatically discovered via [java.util.ServiceLoader]. + * + * To add a custom decoder: + * 1. Implement this interface + * 2. Create `META-INF/services/org.apache.jmeter.samplers.ResponseDecoder` file + * 4. Add your implementation's fully qualified class name to the file + * + * Example decoders: gzip, deflate, brotli + * + * @since 6.0.0 + */ +@JMeterService +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public interface ResponseDecoder { + + /** + * Returns the content encodings handled by this decoder. + * These should match Content-Encoding header values (case-insensitive). + * + * A decoder can handle multiple encoding names (e.g., "gzip" and "x-gzip"). + * + * Examples: ["gzip", "x-gzip"], ["deflate"], ["br"] + * + * @return list of encoding names this decoder handles (must not be null or empty) + */ + public val encodings: List + + /** + * Decodes (decompresses) the given compressed data. + * + * @param compressed the compressed data to decode + * @return the decompressed data + * @throws java.io.IOException if decompression fails + */ + public fun decode(compressed: ByteArray): ByteArray { + val out = DirectAccessByteArrayOutputStream() + decodeStream(ByteArrayInputStream(compressed)).use { + it.transferTo(out) + } + return out.toByteArray() + } + + /** + * Creates a decompressing InputStream that wraps the given compressed input stream. + * This allows streaming decompression without buffering the entire response in memory. + * + * Used for scenarios like MD5 computation on decompressed data, where we want to + * compute the hash on-the-fly without storing the entire decompressed response. + * + * @param input the compressed input stream to wrap + * @return an InputStream that decompresses data as it's read + * @throws java.io.IOException if the decompressing stream cannot be created + */ + public fun decodeStream(input: InputStream): InputStream + + /** + * Returns the priority of this decoder. + * When multiple decoders are registered for the same encoding, + * the one with the highest priority is used. + * + * Default priority is 0. Built-in decoders use priority 0. + * Plugins can override built-in decoders by returning a higher priority. + * + * @return priority value (higher = preferred), default is 0 + */ + public val priority: Int + get() = 0 +} diff --git a/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistry.kt b/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistry.kt new file mode 100644 index 00000000000..f7fb3d22cb9 --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistry.kt @@ -0,0 +1,188 @@ +/* + * 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.samplers + +import org.apache.jmeter.samplers.decoders.DeflateDecoder +import org.apache.jmeter.samplers.decoders.GzipDecoder +import org.apache.jmeter.util.JMeterUtils +import org.apache.jorphan.reflect.LogAndIgnoreServiceLoadExceptionHandler +import org.apiguardian.api.API +import org.slf4j.LoggerFactory +import java.io.IOException +import java.io.InputStream +import java.util.Locale +import java.util.ServiceLoader +import java.util.concurrent.ConcurrentHashMap + +/** + * Registry for [ResponseDecoder] implementations. + * Provides centralized management of response decoders for different content encodings. + * + * Decoders are discovered via: + * - Built-in decoders (gzip, deflate) + * - ServiceLoader mechanism (META-INF/services) + * + * Thread-safe singleton registry. + * + * @since 6.0.0 + */ +@API(status = API.Status.EXPERIMENTAL, since = "6.0.0") +public object ResponseDecoderRegistry { + + private val log = LoggerFactory.getLogger(ResponseDecoderRegistry::class.java) + + /** + * Map of encoding name (lowercase) to decoder implementation. + * Uses ConcurrentHashMap for thread-safe access. + */ + private val decoders = ConcurrentHashMap() + + init { + // Register built-in decoders, this ensures the decoders are there even if service registration fails + registerDecoder(GzipDecoder()) + registerDecoder(DeflateDecoder()) + + // Load decoders via ServiceLoader + loadServiceLoaderDecoders() + } + + /** + * Loads decoders using ServiceLoader mechanism. + */ + private fun loadServiceLoaderDecoders() { + try { + JMeterUtils.loadServicesAndScanJars( + ResponseDecoder::class.java, + ServiceLoader.load(ResponseDecoder::class.java), + Thread.currentThread().contextClassLoader, + LogAndIgnoreServiceLoadExceptionHandler(log) + ).forEach { registerDecoder(it) } + } catch (e: Exception) { + log.error("Error loading ResponseDecoder services", e) + } + } + + /** + * Registers a decoder for all its encoding types. + * If a decoder already exists for an encoding, the one with higher priority is kept. + * + * @param decoder the decoder to register + */ + @JvmStatic + public fun registerDecoder(decoder: ResponseDecoder) { + val encodings = decoder.encodings + if (encodings.isEmpty()) { + log.warn("Decoder {} has null or empty encodings list, skipping registration", decoder.javaClass.name) + return + } + + for (encoding in encodings) { + val key = encoding.lowercase(Locale.ROOT) + + decoders.merge(key, decoder) { existing, newDecoder -> + // Keep the decoder with higher priority + if (newDecoder.priority > existing.priority) { + log.info( + "Replacing decoder for '{}': {} (priority {}) -> {} (priority {})", + encoding, + existing.javaClass.simpleName, existing.priority, + newDecoder.javaClass.simpleName, newDecoder.priority + ) + newDecoder + } else { + log.debug( + "Keeping existing decoder for '{}': {} (priority {}) over {} (priority {})", + encoding, + existing.javaClass.simpleName, existing.priority, + newDecoder.javaClass.simpleName, newDecoder.priority + ) + existing + } + } + } + } + + /** + * Decodes the given data using the decoder registered for the specified encoding. + * If no decoder is found for the encoding, returns the data unchanged. + * + * @param encoding the content encoding (e.g., "gzip", "deflate", "br") + * @param data the data to decode + * @return decoded data, or original data if no decoder found or encoding is null + * @throws IOException if decoding fails + */ + @JvmStatic + @Throws(IOException::class) + public fun decode(encoding: String?, data: ByteArray?): ByteArray { + if (encoding.isNullOrEmpty() || data == null || data.isEmpty()) { + return data ?: ByteArray(0) + } + + val decoder = decoders[encoding] ?: decoders[encoding.lowercase(Locale.ROOT)] + + if (decoder == null) { + log.debug("No decoder found for encoding '{}', returning data unchanged", encoding) + return data + } + + return decoder.decode(data) + } + + /** + * Creates a decompressing InputStream that wraps the given input stream using the decoder + * registered for the specified encoding. + * + * This enables streaming decompression without buffering the entire response in memory, + * which is useful for computing checksums on decompressed data or processing large responses. + * + * If no decoder is found for the encoding, returns the original input stream unchanged. + * + * @param encoding the content encoding (e.g., "gzip", "deflate", "br") + * @param input the input stream to wrap with decompression + * @return a decompressing InputStream, or the original stream if no decoder found or encoding is null + * @throws IOException if the decompressing stream cannot be created + * @since 6.0.0 + */ + @JvmStatic + @Throws(IOException::class) + public fun decodeStream(encoding: String?, input: InputStream): InputStream { + if (encoding.isNullOrEmpty()) { + return input + } + + val decoder = decoders[encoding] ?: decoders[encoding.lowercase(Locale.ROOT)] + + if (decoder == null) { + log.debug("No decoder found for encoding '{}', returning input stream unchanged", encoding) + return input + } + + return decoder.decodeStream(input) + } + + /** + * Checks if a decoder is registered for the given encoding. + * Primarily for testing purposes. + * + * @param encoding the encoding to check + * @return true if a decoder is registered for this encoding + */ + @JvmStatic + public fun hasDecoder(encoding: String): Boolean = + decoders.containsKey(encoding.lowercase(Locale.ROOT)) +} diff --git a/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoder.kt b/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoder.kt new file mode 100644 index 00000000000..db2653bb2ae --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoder.kt @@ -0,0 +1,72 @@ +/* + * 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.samplers.decoders + +import org.apache.jmeter.samplers.ResponseDecoder +import org.apache.jorphan.io.DirectAccessByteArrayOutputStream +import org.apiguardian.api.API +import java.io.ByteArrayInputStream +import java.io.IOException +import java.io.InputStream +import java.util.zip.Inflater +import java.util.zip.InflaterInputStream + +/** + * Decoder for deflate compressed response data. + * Attempts decompression with ZLIB wrapper first, falls back to raw DEFLATE if that fails. + * + * @since 6.0.0 + */ +@API(status = API.Status.INTERNAL, since = "6.0.0") +public class DeflateDecoder : ResponseDecoder { + override val encodings: List + get() = listOf("deflate") + + override fun decode(compressed: ByteArray): ByteArray { + // Try with ZLIB wrapper first + return try { + decompressWithInflater(compressed, nowrap = false) + } catch (e: IOException) { + // If that fails, try with NO_WRAP for raw DEFLATE + decompressWithInflater(compressed, nowrap = true) + } + } + + override fun decodeStream(input: InputStream): InputStream { + // For streaming, use ZLIB wrapper (nowrap=false) which is the most common case. + // The fallback to raw DEFLATE is only available in the byte array version + // since we cannot retry with a stream without buffering it first. + return InflaterInputStream(input, Inflater(false)) + } + + /** + * Decompresses data using Inflater with specified nowrap setting. + * + * @param compressed the compressed data + * @param nowrap if true, uses raw DEFLATE (no ZLIB wrapper) + * @return decompressed data + * @throws IOException if decompression fails + */ + private fun decompressWithInflater(compressed: ByteArray, nowrap: Boolean): ByteArray { + val out = DirectAccessByteArrayOutputStream() + InflaterInputStream(ByteArrayInputStream(compressed), Inflater(nowrap)).use { + it.transferTo(out) + } + return out.toByteArray() + } +} diff --git a/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoder.kt b/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoder.kt new file mode 100644 index 00000000000..b636f48fd80 --- /dev/null +++ b/src/core/src/main/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoder.kt @@ -0,0 +1,39 @@ +/* + * 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.samplers.decoders + +import org.apache.jmeter.samplers.ResponseDecoder +import org.apiguardian.api.API +import java.io.InputStream +import java.util.zip.GZIPInputStream + +/** + * Decoder for gzip compressed response data. + * Handles both "gzip" and "x-gzip" content encodings. + * + * @since 6.0.0 + */ +@API(status = API.Status.INTERNAL, since = "6.0.0") +public class GzipDecoder : ResponseDecoder { + override val encodings: List + get() = listOf("gzip", "x-gzip") + + override fun decodeStream(input: InputStream): InputStream { + return GZIPInputStream(input) + } +} diff --git a/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties b/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties index dc34c8f41cb..5791663bd1e 100644 --- a/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties +++ b/src/core/src/main/resources/org/apache/jmeter/resources/messages.properties @@ -1002,6 +1002,13 @@ reportgenerator_summary_total=Total request_data=Request Data reset=Reset response_save_as_md5=Save response as MD5 hash? +expression_mode_button_tooltip=Switch to expression mode to use variables like ${__P(property)} +response_processing_title=Response Processing +response_processing_mode=Processing mode\: +response_processing_store_compressed=Store response (decompress on access) +response_processing_fetch_discard=Fetch and discard (headers only) +response_processing_checksum_encoded_md5=Checksum (MD5 of compressed) +response_processing_checksum_decoded_md5=Checksum (MD5 of decompressed) response_time_distribution_satisfied_label=Requests having \nresponse time <= {0}ms response_time_distribution_tolerated_label= Requests having \nresponse time > {0}ms and <= {1}ms response_time_distribution_untolerated_label=Requests having \nresponse time > {0}ms diff --git a/src/core/src/test/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistryTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistryTest.kt new file mode 100644 index 00000000000..441f61b818e --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/samplers/ResponseDecoderRegistryTest.kt @@ -0,0 +1,184 @@ +/* + * 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.samplers + +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.io.ByteArrayOutputStream +import java.io.InputStream +import java.util.zip.GZIPOutputStream + +class ResponseDecoderRegistryTest { + @Test + fun testBuiltInDecodersAreRegistered() { + assertTrue(ResponseDecoderRegistry.hasDecoder("gzip"), "gzip decoder should be registered") + assertTrue(ResponseDecoderRegistry.hasDecoder("x-gzip"), "x-gzip decoder should be registered") + assertTrue(ResponseDecoderRegistry.hasDecoder("deflate"), "deflate decoder should be registered") + } + + @Test + fun testDecodeWithGzip() { + val originalText = "Hello, World! This is a test of gzip compression." + val originalData = originalText.toByteArray(Charsets.UTF_8) + + // Compress with gzip + val compressed = compressGzip(originalData) + + // Decode using registry + val decoded = ResponseDecoderRegistry.decode("gzip", compressed) + + assertArrayEquals(originalData, decoded, "Decoded data should match original") + } + + @Test + fun testDecodeWithXGzip() { + val originalText = "Testing x-gzip encoding" + val originalData = originalText.toByteArray(Charsets.UTF_8) + + // Compress with gzip (x-gzip uses same compression) + val compressed = compressGzip(originalData) + + // Decode using registry with x-gzip encoding + val decoded = ResponseDecoderRegistry.decode("x-gzip", compressed) + + assertArrayEquals(originalData, decoded, "Decoded data should match original for x-gzip") + } + + @Test + fun testDecodeWithUnknownEncoding() { + val originalData = "Test data".toByteArray(Charsets.UTF_8) + + // Decode with unknown encoding should return original data + val result = ResponseDecoderRegistry.decode("unknown-encoding", originalData) + + assertArrayEquals(originalData, result, "Unknown encoding should return data unchanged") + } + + @Test + fun testDecodeWithNullEncoding() { + val originalData = "Test data".toByteArray(Charsets.UTF_8) + + // Decode with null encoding should return original data + val result = ResponseDecoderRegistry.decode(null, originalData) + + assertArrayEquals(originalData, result, "Null encoding should return data unchanged") + } + + @Test + fun testDecodeWithEmptyData() { + val emptyData = ByteArray(0) + + val result = ResponseDecoderRegistry.decode("gzip", emptyData) + + assertArrayEquals(emptyData, result, "Empty data should return empty data") + } + + @Test + fun testCaseInsensitiveEncoding() { + val originalText = "Case insensitive test" + val originalData = originalText.toByteArray(Charsets.UTF_8) + val compressed = compressGzip(originalData) + + // Test various case combinations + val decoded1 = ResponseDecoderRegistry.decode("GZIP", compressed) + val decoded2 = ResponseDecoderRegistry.decode("GZip", compressed) + val decoded3 = ResponseDecoderRegistry.decode("gzip", compressed) + + assertArrayEquals(originalData, decoded1, "GZIP should decode correctly") + assertArrayEquals(originalData, decoded2, "GZip should decode correctly") + assertArrayEquals(originalData, decoded3, "gzip should decode correctly") + } + + @Test + fun testRegisterCustomDecoder() { + // Create a custom decoder that reverses bytes (for testing) + val reverseDecoder = object : ResponseDecoder { + override val encodings: List + get() = listOf("test-reverse") + + override fun decode(compressed: ByteArray): ByteArray = + compressed.reversedArray() + + override fun decodeStream(input: InputStream): InputStream { + TODO("Not yet implemented") + } + } + + ResponseDecoderRegistry.registerDecoder(reverseDecoder) + + val data = "ABC".toByteArray(Charsets.UTF_8) + val decoded = ResponseDecoderRegistry.decode("test-reverse", data) + + assertEquals("CBA", decoded.toString(Charsets.UTF_8), "Custom decoder should reverse bytes") + } + + @Test + fun testDecoderPriority() { + // Register a low priority decoder + val lowPriorityDecoder = object : ResponseDecoder { + override val encodings: List + get() = listOf("priority-test") + + override fun decode(compressed: ByteArray): ByteArray = + "low".toByteArray(Charsets.UTF_8) + + override fun decodeStream(input: InputStream): InputStream { + TODO("Not yet implemented") + } + + override val priority: Int + get() = 1 + } + + // Register a high priority decoder for same encoding + val highPriorityDecoder = object : ResponseDecoder { + override val encodings: List + get() = listOf("priority-test") + + override fun decode(compressed: ByteArray): ByteArray = + "high".toByteArray(Charsets.UTF_8) + + override fun decodeStream(input: InputStream): InputStream { + TODO("Not yet implemented") + } + + override val priority: Int + get() = 10 + } + + ResponseDecoderRegistry.registerDecoder(lowPriorityDecoder) + ResponseDecoderRegistry.registerDecoder(highPriorityDecoder) + + val result = ResponseDecoderRegistry.decode("priority-test", "test".toByteArray(Charsets.UTF_8)) + + assertEquals("high", result.toString(Charsets.UTF_8), "Higher priority decoder should be used") + } + + /** + * Helper method to compress data with gzip + */ + private fun compressGzip(data: ByteArray): ByteArray { + val baos = ByteArrayOutputStream() + GZIPOutputStream(baos).use { gzipOut -> + gzipOut.write(data) + } + return baos.toByteArray() + } +} diff --git a/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoderTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoderTest.kt new file mode 100644 index 00000000000..add44e26304 --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/DeflateDecoderTest.kt @@ -0,0 +1,88 @@ +/* + * 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.samplers.decoders + +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import java.io.ByteArrayOutputStream +import java.util.zip.Deflater +import java.util.zip.DeflaterOutputStream + +class DeflateDecoderTest { + private val decoder = DeflateDecoder() + + @Test + fun testGetEncodings() { + assertEquals(listOf("deflate"), decoder.encodings, "encodings") + } + + @Test + fun testGetPriority() { + assertEquals(0, decoder.priority, "Default priority should be 0") + } + + @Test + fun testDecodeDeflateWithZlibWrapper() { + val originalText = "Hello, World! This is a test message for deflate compression with ZLIB wrapper." + val originalData = originalText.toByteArray(Charsets.UTF_8) + + // Compress with ZLIB wrapper (default) + val compressed = compressDeflate(originalData, nowrap = false) + + // Decode + val decoded = decoder.decode(compressed) + + assertArrayEquals(originalData, decoded, "Decoded data should match original (ZLIB wrapper)") + } + + @Test + fun testDecodeDeflateRaw() { + val originalText = "Testing raw deflate without ZLIB wrapper." + val originalData = originalText.toByteArray(Charsets.UTF_8) + + // Compress with NO_WRAP (raw deflate) + val compressed = compressDeflate(originalData, nowrap = true) + + // Decode - should fallback to raw deflate + val decoded = decoder.decode(compressed) + + assertArrayEquals(originalData, decoded, "Decoded data should match original (raw deflate)") + } + + @Test + fun testDecodeEmptyData() { + val emptyCompressed = compressDeflate(ByteArray(0), nowrap = false) + val decoded = decoder.decode(emptyCompressed) + + assertEquals(0, decoded.size, "Empty data should decode to empty array") + } + + /** + * Helper method to compress data with deflate + * @param data the data to compress + * @param nowrap if true, uses raw deflate (no ZLIB wrapper) + */ + private fun compressDeflate(data: ByteArray, nowrap: Boolean): ByteArray { + val baos = ByteArrayOutputStream() + DeflaterOutputStream(baos, Deflater(Deflater.DEFAULT_COMPRESSION, nowrap)).use { deflaterOut -> + deflaterOut.write(data) + } + return baos.toByteArray() + } +} diff --git a/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoderTest.kt b/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoderTest.kt new file mode 100644 index 00000000000..9be1f83cbae --- /dev/null +++ b/src/core/src/test/kotlin/org/apache/jmeter/samplers/decoders/GzipDecoderTest.kt @@ -0,0 +1,82 @@ +/* + * 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.samplers.decoders + +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Test +import java.io.ByteArrayOutputStream +import java.util.zip.GZIPOutputStream + +class GzipDecoderTest { + private val decoder = GzipDecoder() + + @Test + fun testGetEncodings() { + assertEquals(listOf("gzip", "x-gzip"), decoder.encodings, "encodings") + } + + @Test + fun testGetPriority() { + assertEquals(0, decoder.priority, "Default priority should be 0") + } + + @Test + fun testDecodeGzipData() { + val originalText = "Hello, World! This is a test message for gzip compression." + val originalData = originalText.toByteArray(Charsets.UTF_8) + + // Compress data with gzip + val compressed = compressGzip(originalData) + + // Decode + val decoded = decoder.decode(compressed) + + assertArrayEquals(originalData, decoded, "Decoded data should match original") + assertEquals(originalText, decoded.toString(Charsets.UTF_8), "Decoded text should match original") + } + + @Test + fun testDecodeEmptyData() { + val emptyCompressed = compressGzip(ByteArray(0)) + val decoded = decoder.decode(emptyCompressed) + + assertEquals(0, decoded.size, "Empty data should decode to empty array") + } + + @Test + fun testDecodeInvalidData() { + val invalidData = "This is not gzip compressed data".toByteArray(Charsets.UTF_8) + + assertThrows(Exception::class.java) { + decoder.decode(invalidData) + } + } + + /** + * Helper method to compress data with gzip + */ + private fun compressGzip(data: ByteArray): ByteArray { + val baos = ByteArrayOutputStream() + GZIPOutputStream(baos).use { gzipOut -> + gzipOut.write(data) + } + return baos.toByteArray() + } +} diff --git a/src/protocol/http/build.gradle.kts b/src/protocol/http/build.gradle.kts index af6b3482f5d..7f2d78fa474 100644 --- a/src/protocol/http/build.gradle.kts +++ b/src/protocol/http/build.gradle.kts @@ -63,10 +63,12 @@ dependencies { implementation("dnsjava:dnsjava") implementation("org.apache.httpcomponents:httpmime") implementation("org.apache.httpcomponents:httpcore") - implementation("org.brotli:dec") implementation("com.miglayout:miglayout-swing") implementation("com.fasterxml.jackson.core:jackson-core") implementation("com.fasterxml.jackson.core:jackson-databind") + implementation("org.brotli:dec") { + because("BrotliDecoder for HTTP response decompression") + } testImplementation(testFixtures(projects.src.core)) testImplementation(testFixtures(projects.src.testkitWiremock)) testImplementation("org.wiremock:wiremock") 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 c056b8a8f02..2bb2ff5c0c6 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 @@ -33,6 +33,7 @@ import org.apache.jmeter.config.gui.AbstractConfigGui; import org.apache.jmeter.gui.GUIMenuSortOrder; import org.apache.jmeter.gui.JBooleanPropertyEditor; +import org.apache.jmeter.gui.JEnumPropertyEditor; import org.apache.jmeter.gui.JTextComponentBinding; import org.apache.jmeter.gui.TestElementMetadata; import org.apache.jmeter.gui.util.HorizontalPanel; @@ -68,10 +69,13 @@ public class HttpDefaultsGui extends AbstractConfigGui { "web_testing_concurrent_download", JMeterUtils::getResString); private JTextField concurrentPool; - private final JBooleanPropertyEditor useMD5 = new JBooleanPropertyEditor( - HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(), - "response_save_as_md5", - JMeterUtils::getResString); // $NON-NLS-1$ + private final JEnumPropertyEditor responseProcessingMode = + JEnumPropertyEditor.create( + HTTPSamplerBaseSchema.INSTANCE.getResponseProcessingMode(), + "response_processing_mode", + HTTPSamplerBase.ResponseProcessingMode.class, + JMeterUtils::getResString + ); private JTextField embeddedAllowRE; // regular expression used to match against embedded resource URLs to allow private JTextField embeddedExcludeRE; // regular expression used to match against embedded resource URLs to discard private JTextField sourceIpAddr; // does not apply to Java implementation @@ -94,7 +98,7 @@ public HttpDefaultsGui() { retrieveEmbeddedResources, concurrentDwn, new JTextComponentBinding(concurrentPool, schema.getConcurrentDownloadPoolSize()), - useMD5, + responseProcessingMode, new JTextComponentBinding(embeddedAllowRE, schema.getEmbeddedUrlAllowRegex()), new JTextComponentBinding(embeddedExcludeRE, schema.getEmbeddedUrlExcludeRegex()), new JTextComponentBinding(sourceIpAddr, schema.getIpSource()), @@ -191,7 +195,7 @@ private void init() { // WARNING: called from ctor so must not be overridden (i. advancedPanel.add(createEmbeddedRsrcPanel()); advancedPanel.add(createSourceAddrPanel()); advancedPanel.add(getProxyServerPanel()); - advancedPanel.add(createOptionalTasksPanel()); + advancedPanel.add(createResponseProcessingPanel()); JTabbedPane tabbedPane = new JTabbedPane(); tabbedPane.add(JMeterUtils @@ -297,13 +301,12 @@ protected JPanel createSourceAddrPanel() { return sourceAddrPanel; } - protected JPanel createOptionalTasksPanel() { - // OPTIONAL TASKS - final JPanel checkBoxPanel = new VerticalPanel(); - checkBoxPanel.setBorder(BorderFactory.createTitledBorder( - JMeterUtils.getResString("optional_tasks"))); // $NON-NLS-1$ - checkBoxPanel.add(useMD5); - return checkBoxPanel; + protected JPanel createResponseProcessingPanel() { + JPanel panel = new JPanel(new MigLayout()); + panel.setBorder(BorderFactory.createTitledBorder( + JMeterUtils.getResString("response_processing_title"))); // $NON-NLS-1$ + panel.add(responseProcessingMode); + return panel; } @Override diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java index 44592cd14a5..dce3a66ef76 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java @@ -32,6 +32,7 @@ import org.apache.jmeter.gui.GUIMenuSortOrder; import org.apache.jmeter.gui.JBooleanPropertyEditor; +import org.apache.jmeter.gui.JEnumPropertyEditor; import org.apache.jmeter.gui.JStringPropertyEditor; import org.apache.jmeter.gui.JTextComponentBinding; import org.apache.jmeter.gui.TestElementMetadata; @@ -71,10 +72,13 @@ public class HttpTestSampleGui extends AbstractSamplerGui { "web_testing_concurrent_download", JMeterUtils::getResString); private JTextField concurrentPool; - private final JBooleanPropertyEditor useMD5 = new JBooleanPropertyEditor( - HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(), - "response_save_as_md5", - JMeterUtils::getResString); + private final JEnumPropertyEditor responseProcessingMode = + JEnumPropertyEditor.create( + HTTPSamplerBaseSchema.INSTANCE.getResponseProcessingMode(), + "response_processing_mode", + HTTPSamplerBase.ResponseProcessingMode.class, + JMeterUtils::getResString + ); private final JStringPropertyEditor embeddedAllowRE = new JStringPropertyEditor( HTTPSamplerBaseSchema.INSTANCE.getEmbeddedUrlAllowRegex(), JMeterUtils::getResString); @@ -108,7 +112,7 @@ protected HttpTestSampleGui(boolean ajp) { retrieveEmbeddedResources, concurrentDwn, new JTextComponentBinding(concurrentPool, schema.getConcurrentDownloadPoolSize()), - useMD5, + responseProcessingMode, embeddedAllowRE, embeddedExcludeRE ) @@ -265,7 +269,7 @@ private JPanel createAdvancedConfigPanel() { advancedPanel.add(getProxyServerPanel()); } - advancedPanel.add(createOptionalTasksPanel()); + advancedPanel.add(createResponseProcessingPanel()); return advancedPanel; } @@ -370,15 +374,12 @@ protected final JPanel getImplementationPanel(){ return implPanel; } - protected JPanel createOptionalTasksPanel() { - // OPTIONAL TASKS - final JPanel checkBoxPanel = new VerticalPanel(); - checkBoxPanel.setBorder(BorderFactory.createTitledBorder( - JMeterUtils.getResString("optional_tasks"))); // $NON-NLS-1$ - - checkBoxPanel.add(useMD5); - - return checkBoxPanel; + protected JPanel createResponseProcessingPanel() { + JPanel panel = new JPanel(new MigLayout()); + panel.setBorder(BorderFactory.createTitledBorder( + JMeterUtils.getResString("response_processing_title"))); // $NON-NLS-1$ + panel.add(responseProcessingMode, "span"); + return panel; } @SuppressWarnings("EnumOrdinal") diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java index 9c724a01727..28546e6186c 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java @@ -17,7 +17,6 @@ package org.apache.jmeter.protocol.http.sampler; -import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.net.Inet4Address; @@ -28,6 +27,7 @@ import java.net.SocketException; import java.net.URL; import java.net.UnknownHostException; +import java.util.List; import java.util.function.Predicate; import org.apache.jmeter.config.Arguments; @@ -42,6 +42,8 @@ import org.apache.jmeter.samplers.Interruptible; import org.apache.jmeter.samplers.SampleResult; import org.apache.jmeter.util.JMeterUtils; +import org.apache.jorphan.util.EnumUtils; +import org.jspecify.annotations.Nullable; /** * Base class for HTTP implementations used by the HTTPSamplerProxy sampler. @@ -230,12 +232,12 @@ protected String getIpSource() { * @throws UnknownHostException if the hostname/ip for {@link #getIpSource()} could not be resolved or not interface was found for it * @throws SocketException if an I/O error occurs */ - @SuppressWarnings("EnumOrdinal") protected InetAddress getIpSourceAddress() throws UnknownHostException, SocketException { final String ipSource = getIpSource(); if (!ipSource.isBlank()) { Class ipClass = null; - final SourceType sourceType = HTTPSamplerBase.SourceType.values()[testElement.getIpSourceType()]; + List sourceTypes = EnumUtils.getEnumValues(SourceType.class); + final SourceType sourceType = sourceTypes.get(testElement.getIpSourceType()); switch (sourceType) { case DEVICE -> ipClass = InetAddress.class; case DEVICE_IPV4 -> ipClass = Inet4Address.class; @@ -428,7 +430,7 @@ protected boolean isSuccessCode(int errorLevel) { * Closes the inputStream *

* Invokes - * {@link HTTPSamplerBase#readResponse(SampleResult, InputStream, long)} + * {@link HTTPSamplerBase#readResponse(SampleResult, InputStream, long, String)} * * @param res * sample to store information about the response into @@ -436,93 +438,12 @@ protected boolean isSuccessCode(int errorLevel) { * input stream from which to read the response * @param responseContentLength * expected input length or zero - * @return the response or the MD5 of the response * @throws IOException * if reading the result fails */ - protected byte[] readResponse(SampleResult res, InputStream instream, - int responseContentLength) throws IOException { - return readResponse(res, instream, (long)responseContentLength); - } - /** - * Read response from the input stream, converting to MD5 digest if the - * useMD5 property is set. - *

- * For the MD5 case, the result byte count is set to the size of the - * original response. - *

- * Closes the inputStream - *

- * Invokes - * {@link HTTPSamplerBase#readResponse(SampleResult, InputStream, long)} - * - * @param res - * sample to store information about the response into - * @param instream - * input stream from which to read the response - * @param responseContentLength - * expected input length or zero - * @return the response or the MD5 of the response - * @throws IOException - * if reading the result fails - */ - protected byte[] readResponse(SampleResult res, InputStream instream, - long responseContentLength) throws IOException { - return testElement.readResponse(res, instream, responseContentLength); - } - - /** - * Read response from the input stream, converting to MD5 digest if the - * useMD5 property is set. - *

- * For the MD5 case, the result byte count is set to the size of the - * original response. - *

- * Closes the inputStream - *

- * Invokes {@link HTTPSamplerBase#readResponse(SampleResult, InputStream, long)} - * - * @param res - * sample to store information about the response into - * @param in - * input stream from which to read the response - * @param contentLength - * expected input length or zero - * @return the response or the MD5 of the response - * @throws IOException - * when reading the result fails - * @deprecated use {@link HTTPAbstractImpl#readResponse(SampleResult, BufferedInputStream, long)} - */ - @Deprecated - protected byte[] readResponse(SampleResult res, BufferedInputStream in, - int contentLength) throws IOException { - return testElement.readResponse(res, in, contentLength); - } - - /** - * Read response from the input stream, converting to MD5 digest if the - * useMD5 property is set. - *

- * For the MD5 case, the result byte count is set to the size of the - * original response. - *

- * Closes the inputStream - *

- * Invokes {@link HTTPSamplerBase#readResponse(SampleResult, InputStream, long)} - * - * @param res - * sample to store information about the response into - * @param in - * input stream from which to read the response - * @param contentLength - * expected input length or zero - * @return the response or the MD5 of the response - * @throws IOException - * when reading the result fails - */ - protected byte[] readResponse(SampleResult res, BufferedInputStream in, - long contentLength) throws IOException { - return testElement.readResponse(res, in, contentLength); + protected void readResponse(SampleResult res, InputStream instream, + long responseContentLength, @Nullable String contentEncoding) throws IOException { + testElement.readResponse(res, instream, responseContentLength, contentEncoding); } /** diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java index 569bb164b6a..11db6bcb0e9 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java @@ -20,6 +20,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.UnsupportedEncodingException; import java.net.InetAddress; @@ -55,7 +56,6 @@ import org.apache.http.HttpRequest; import org.apache.http.HttpRequestInterceptor; import org.apache.http.HttpResponse; -import org.apache.http.HttpResponseInterceptor; import org.apache.http.NameValuePair; import org.apache.http.StatusLine; import org.apache.http.auth.AuthSchemeProvider; @@ -70,7 +70,6 @@ import org.apache.http.client.config.AuthSchemes; import org.apache.http.client.config.CookieSpecs; import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.entity.InputStreamFactory; import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; @@ -84,7 +83,6 @@ import org.apache.http.client.methods.HttpTrace; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.HttpClientContext; -import org.apache.http.client.protocol.ResponseContentEncoding; import org.apache.http.config.Lookup; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; @@ -146,8 +144,6 @@ import org.apache.jmeter.protocol.http.control.DynamicKerberosSchemeFactory; import org.apache.jmeter.protocol.http.control.DynamicSPNegoSchemeFactory; import org.apache.jmeter.protocol.http.control.HeaderManager; -import org.apache.jmeter.protocol.http.sampler.hc.LaxDeflateInputStream; -import org.apache.jmeter.protocol.http.sampler.hc.LaxGZIPInputStream; import org.apache.jmeter.protocol.http.sampler.hc.LazyLayeredConnectionSocketFactory; import org.apache.jmeter.protocol.http.util.ConversionUtils; import org.apache.jmeter.protocol.http.util.HTTPArgument; @@ -165,7 +161,6 @@ import org.apache.jmeter.util.SSLManager; import org.apache.jorphan.util.JOrphanUtils; import org.apache.jorphan.util.StringUtilities; -import org.brotli.dec.BrotliInputStream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -194,20 +189,8 @@ public class HTTPHC4Impl extends HTTPHCAbstractImpl { private static final boolean DISABLE_DEFAULT_UA = JMeterUtils.getPropDefault("httpclient4.default_user_agent_disabled", false); - private static final boolean GZIP_RELAX_MODE = JMeterUtils.getPropDefault("httpclient4.gzip_relax_mode", false); - - private static final boolean DEFLATE_RELAX_MODE = JMeterUtils.getPropDefault("httpclient4.deflate_relax_mode", false); - private static final Logger log = LoggerFactory.getLogger(HTTPHC4Impl.class); - private static final InputStreamFactory GZIP = - instream -> new LaxGZIPInputStream(instream, GZIP_RELAX_MODE); - - private static final InputStreamFactory DEFLATE = - instream -> new LaxDeflateInputStream(instream, DEFLATE_RELAX_MODE); - - private static final InputStreamFactory BROTLI = BrotliInputStream::new; - private static final class ManagedCredentialsProvider implements CredentialsProvider { private final AuthManager authManager; private final Credentials proxyCredentials; @@ -464,55 +447,6 @@ protected HttpResponse doSendRequest( } }; - private static final String[] HEADERS_TO_SAVE = new String[]{ - "content-length", - "content-encoding", - "content-md5" - }; - - /** - * Custom implementation that backups headers related to Compressed responses - * that HC core {@link ResponseContentEncoding} removes after uncompressing - * See Bug 59401 - */ - @SuppressWarnings("UnnecessaryAnonymousClass") - private static final HttpResponseInterceptor RESPONSE_CONTENT_ENCODING = new ResponseContentEncoding(createLookupRegistry()) { - @Override - public void process(HttpResponse response, HttpContext context) - throws HttpException, IOException { - ArrayList headersToSave = null; - - final HttpEntity entity = response.getEntity(); - final HttpClientContext clientContext = HttpClientContext.adapt(context); - final RequestConfig requestConfig = clientContext.getRequestConfig(); - // store the headers if necessary - if (requestConfig.isContentCompressionEnabled() && entity != null && entity.getContentLength() != 0) { - final Header ceheader = entity.getContentEncoding(); - if (ceheader != null) { - headersToSave = new ArrayList<>(3); - for(String name : HEADERS_TO_SAVE) { - Header[] hdr = response.getHeaders(name); // empty if none - headersToSave.add(hdr); - } - } - } - - // Now invoke original parent code - super.process(response, clientContext); - // Should this be in a finally ? - if(headersToSave != null) { - for (Header[] headers : headersToSave) { - for (Header headerToRestore : headers) { - if (response.containsHeader(headerToRestore.getName())) { - break; - } - response.addHeader(headerToRestore); - } - } - } - } - }; - /** * 1 HttpClient instance per combination of (HttpClient,HttpClientKey) */ @@ -549,19 +483,6 @@ protected HTTPHC4Impl(HTTPSamplerBase testElement) { super(testElement); } - /** - * Customize to plug Brotli - * @return {@link Lookup} - */ - private static Lookup createLookupRegistry() { - return - RegistryBuilder.create() - .register("br", BROTLI) - .register("gzip", GZIP) - .register("x-gzip", GZIP) - .register("deflate", DEFLATE).build(); - } - /** * Implementation that allows GET method to have a body */ @@ -665,8 +586,10 @@ protected HTTPSampleResult sample(URL url, String method, res.setEncodingAndType(ct); } HttpEntity entity = httpResponse.getEntity(); - if (entity != null) { - res.setResponseData(readResponse(res, entity.getContent(), entity.getContentLength())); + try (InputStream instream = entity.getContent()) { + Header contentEncodingHeader = entity.getContentEncoding(); + String contentEncoding = contentEncodingHeader != null ? contentEncodingHeader.getValue() : null; + readResponse(res, instream, entity.getContentLength(), contentEncoding); } res.sampleEnd(); // Done with the sampling proper. @@ -1147,7 +1070,7 @@ private HttpClientState setupClient(HttpClientKey key, JMeterVariables jMeterVar } builder.setDefaultCredentialsProvider(credsProvider); } - builder.disableContentCompression().addInterceptorLast(RESPONSE_CONTENT_ENCODING); + builder.disableContentCompression(); // Disable automatic decompression if(BASIC_AUTH_PREEMPTIVE) { builder.addInterceptorFirst(PREEMPTIVE_AUTH_INTERCEPTOR); } else { diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java index 527ed485aad..f2505416101 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; -import java.util.zip.GZIPInputStream; import org.apache.jmeter.protocol.http.control.AuthManager; import org.apache.jmeter.protocol.http.control.Authorization; @@ -219,15 +218,11 @@ protected HttpURLConnection setupConnection(URL u, String method, HTTPSampleResu /** * Reads the response from the URL connection. * - * @param conn - * URL from which to read response - * @param res - * {@link SampleResult} to read response into - * @return response content - * @exception IOException - * if an I/O exception occurs + * @param res {@link SampleResult} to read response into + * @param conn URL from which to read response + * @throws IOException if an I/O exception occurs */ - protected byte[] readResponse(HttpURLConnection conn, SampleResult res) throws IOException { + protected void readResponse(SampleResult res, HttpURLConnection conn) throws IOException { InputStream in; final long contentLength = conn.getContentLength(); @@ -236,26 +231,19 @@ protected byte[] readResponse(HttpURLConnection conn, SampleResult res) throws I log.info("Content-Length: 0, not reading http-body"); res.setResponseHeaders(getResponseHeaders(conn)); res.latencyEnd(); - return NULL_BA; + res.setResponseData(NULL_BA); + return; } - // works OK even if ContentEncoding is null - boolean gzipped = HTTPConstants.ENCODING_GZIP.equals(conn.getContentEncoding()); - - CountingInputStream instream = null; + CountingInputStream counterStream = null; try { - instream = new CountingInputStream(conn.getInputStream()); - if (gzipped) { - in = new GZIPInputStream(instream); - } else { - in = instream; - } + counterStream = new CountingInputStream(conn.getInputStream()); + in = counterStream; } catch (IOException e) { - if (! (e.getCause() instanceof FileNotFoundException)) - { + if (!(e.getCause() instanceof FileNotFoundException)) { log.error("readResponse: {}", e.toString()); Throwable cause = e.getCause(); - if (cause != null){ + if (cause != null) { log.error("Cause: {}", cause.toString()); if(cause instanceof Error error) { throw error; @@ -270,36 +258,21 @@ protected byte[] readResponse(HttpURLConnection conn, SampleResult res) throws I } res.setResponseHeaders(getResponseHeaders(conn)); res.latencyEnd(); - return NULL_BA; + res.setResponseData(NULL_BA); + return; } if(log.isInfoEnabled()) { log.info("Error Response Code: {}", conn.getResponseCode()); } - if (gzipped) { - in = new GZIPInputStream(errorStream); - } else { - in = errorStream; - } - } catch (Exception e) { - log.error("readResponse: {}", e.toString()); - Throwable cause = e.getCause(); - if (cause != null){ - log.error("Cause: {}", cause.toString()); - if(cause instanceof Error error) { - throw error; - } - } - in = conn.getErrorStream(); + in = errorStream; } - // N.B. this closes 'in' - byte[] responseData = readResponse(res, in, contentLength); - if (instream != null) { - res.setBodySize(instream.getBytesRead()); - instream.close(); + + readResponse(res, in, contentLength, conn.getContentEncoding()); + if (counterStream != null) { + res.setBodySize(counterStream.getBytesRead()); } - return responseData; } /** @@ -565,15 +538,10 @@ protected HTTPSampleResult sample(URL url, String method, boolean areFollowingRe res.setQueryString(putBody); } // Request sent. Now get the response: - byte[] responseData = readResponse(conn, res); + readResponse(res, conn); res.sampleEnd(); // Done with the sampling proper. - - // Now collect the results into the HTTPSampleResult: - - res.setResponseData(responseData); - int errorLevel = conn.getResponseCode(); String respMsg = conn.getResponseMessage(); String hdr=conn.getHeaderField(0); diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java index 3105d0cf1ec..89416f89e76 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java @@ -17,7 +17,6 @@ package org.apache.jmeter.protocol.http.sampler; -import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.Serializable; @@ -69,11 +68,13 @@ import org.apache.jmeter.report.utils.MetricUtils; import org.apache.jmeter.samplers.AbstractSampler; import org.apache.jmeter.samplers.Entry; +import org.apache.jmeter.samplers.ResponseDecoderRegistry; import org.apache.jmeter.samplers.SampleResult; import org.apache.jmeter.testelement.TestElement; import org.apache.jmeter.testelement.TestIterationListener; import org.apache.jmeter.testelement.TestStateListener; import org.apache.jmeter.testelement.ThreadListener; +import org.apache.jmeter.testelement.property.BooleanProperty; import org.apache.jmeter.testelement.property.CollectionProperty; import org.apache.jmeter.testelement.property.JMeterProperty; import org.apache.jmeter.testelement.schema.PropertiesAccessor; @@ -82,13 +83,15 @@ import org.apache.jmeter.threads.JMeterContextService; import org.apache.jmeter.util.JMeterUtils; import org.apache.jorphan.io.DirectAccessByteArrayOutputStream; +import org.apache.jorphan.locale.ResourceKeyed; +import org.apache.jorphan.util.EnumUtils; import org.apache.jorphan.util.ExceptionUtils; import org.apache.jorphan.util.JOrphanUtils; import org.apache.jorphan.util.StringUtilities; import org.apache.oro.text.MalformedCachePatternException; import org.apache.oro.text.regex.Pattern; import org.apache.oro.text.regex.Perl5Matcher; -import org.jetbrains.annotations.Nullable; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -206,6 +209,7 @@ public abstract class HTTPSamplerBase extends AbstractSampler private static final boolean IGNORE_FAILED_EMBEDDED_RESOURCES = JMeterUtils.getPropDefault("httpsampler.ignore_failed_embedded_resources", false); // $NON-NLS-1$ // default value: false + // TODO: replace with responseProcessingMode enum? private static final boolean IGNORE_EMBEDDED_RESOURCES_DATA = JMeterUtils.getPropDefault("httpsampler.embedded_resources_use_md5", false); // $NON-NLS-1$ // default value: false @@ -377,6 +381,54 @@ public static String[] getSourceTypeList() { } return displayStrings; } + + /** + * Enum for response processing modes that control how HTTP response data is handled. + * Supports different strategies for storing, discarding, or checksumming responses. + * + * @since 6.0.0 + */ + public enum ResponseProcessingMode implements ResourceKeyed { + /** + * Store compressed response data, decompress on-demand when accessed. + * Default mode for normal operation. Saves memory and supports lazy decompression. + */ + STORE_COMPRESSED("response_processing_store_compressed"), //$NON-NLS-1$ + + /** + * Fetch response data but discard it immediately. + * Useful when you only care about response code/headers, not the body. + * Avoids storing large responses in memory. + */ + FETCH_AND_DISCARD("response_processing_fetch_discard"), //$NON-NLS-1$ + + /** + * Compute MD5 checksum on the compressed response stream. + * Stores MD5 hash instead of full response. Useful for validating + * that compressed data hasn't been modified in transit. + */ + CHECKSUM_ENCODED_MD5("response_processing_checksum_encoded_md5"), //$NON-NLS-1$ + + /** + * Compute MD5 checksum on the decompressed response stream. + * Stores MD5 hash instead of full response. Uses streaming decompression + * to avoid buffering entire response in memory. This is the traditional + * "Store as MD5" mode from earlier versions. + */ + CHECKSUM_DECODED_MD5("response_processing_checksum_decoded_md5"); //$NON-NLS-1$ + + public final String propertyName; + + ResponseProcessingMode(String propertyName) { + this.propertyName = propertyName; + } + + @Override + public String getResourceKey() { + return propertyName; + } + } + /** * Determine if the file should be sent as the entire Content body, * i.e. without any additional wrapping. @@ -640,11 +692,56 @@ public String getImplementation() { return get(getSchema().getImplementation()); } + /** + * Gets the response processing mode for this sampler. + * Controls how response data is handled (stored, discarded, or checksummed). + * + * @return the current response processing mode + * @since 6.0.0 + */ + public @Nullable ResponseProcessingMode getResponseProcessingMode() { + String value = get(getSchema().getResponseProcessingMode()); + return EnumUtils.valueOf(ResponseProcessingMode.class, value); + } + + /** + * Sets the response processing mode for this sampler. + * Controls how response data is handled (stored, discarded, or checksummed). + * + * @param mode the response processing mode to set + * @since 6.0.0 + */ + public void setResponseProcessingMode(ResponseProcessingMode mode) { + set(getSchema().getResponseProcessingMode(), mode.getResourceKey()); + } + + /** + * Returns whether this sampler should store response data as MD5 hash. + * + * @return true if MD5 mode is enabled (CHECKSUM_DECODED_MD5 or CHECKSUM_ENCODED_MD5) + * @deprecated Use {@link #getResponseProcessingMode()} instead. + * This method returns true if mode is any checksum mode. + */ + @Deprecated public boolean useMD5() { - return get(getSchema().getStoreAsMD5()); + ResponseProcessingMode mode = getResponseProcessingMode(); + return mode == ResponseProcessingMode.CHECKSUM_DECODED_MD5; } + /** + * Sets whether this sampler should store response data as MD5 hash. + * + * @param value true to enable MD5 mode (CHECKSUM_DECODED_MD5), + * false to use default mode (STORE_COMPRESSED) + * @deprecated Use {@link #setResponseProcessingMode(ResponseProcessingMode)} instead. + * This method sets mode to CHECKSUM_DECODED_MD5 if true, STORE_COMPRESSED if false. + */ + @Deprecated public void setMD5(boolean value) { + setResponseProcessingMode( + value ? ResponseProcessingMode.CHECKSUM_DECODED_MD5 : ResponseProcessingMode.STORE_COMPRESSED + ); + // Also set old property for backward compatibility with older code set(getSchema().getStoreAsMD5(), value); } @@ -1926,80 +2023,80 @@ public void testIterationStart(LoopIterationEvent event) { * @param sampleResult sample to store information about the response into * @param in input stream from which to read the response * @param length expected input length or zero - * @return the response or the MD5 of the response * @throws IOException if reading the result fails */ - public byte[] readResponse(SampleResult sampleResult, InputStream in, long length) throws IOException { + public void readResponse(SampleResult sampleResult, InputStream in, long length, @Nullable String contentEncoding) throws IOException { + ResponseProcessingMode responseProcessingMode = getResponseProcessingMode(); + if (responseProcessingMode == ResponseProcessingMode.CHECKSUM_DECODED_MD5) { + in = ResponseDecoderRegistry.decodeStream(contentEncoding, in); + contentEncoding = null; // already decoded + } - DirectAccessByteArrayOutputStream w = null; - try (Closeable ignore = in) { // NOSONAR No try with resource as performance is critical here - byte[] readBuffer = new byte[8192]; // 8kB is the (max) size to have the latency ('the first packet') - int bufferSize = 32;// Enough for MD5 + // 8kB is the (max) size to have the latency ('the first packet') + byte[] readBuffer = new byte[Math.toIntExact(length > 0 ? Math.min(length, 8192) : 8192)]; - MessageDigest md = null; - boolean knownResponseLength = length > 0;// may also happen if long value > int.max - if (useMD5()) { + MessageDigest md = null; + DirectAccessByteArrayOutputStream w = null; + switch (responseProcessingMode) { + case FETCH_AND_DISCARD -> { + } + case STORE_COMPRESSED -> { + w = new DirectAccessByteArrayOutputStream(Math.toIntExact(length > 0 ? Math.min(length, MAX_BUFFER_SIZE) : MAX_BUFFER_SIZE)); + } + case CHECKSUM_DECODED_MD5, CHECKSUM_ENCODED_MD5 -> { try { md = MessageDigest.getInstance("MD5"); //$NON-NLS-1$ } catch (NoSuchAlgorithmException e) { - log.error("Should not happen - could not find MD5 digest", e); - } - } else { - if (!knownResponseLength) { - bufferSize = 4 * 1024; - } else { - bufferSize = (int) Math.min(MAX_BUFFER_SIZE, length); + throw new IllegalStateException("MD5 digest algorithm not supported", e); } } + } - - int bytesReadInBuffer = 0; - long totalBytes = 0; - boolean first = true; - boolean storeInBOS = true; - while ((bytesReadInBuffer = in.read(readBuffer)) > -1) { - if (first) { - sampleResult.latencyEnd(); - first = false; - if (md == null) { - w = new DirectAccessByteArrayOutputStream(knownResponseLength ? bufferSize : 8192); - } - } - - if (md == null) { - if(storeInBOS) { - if(MAX_BYTES_TO_STORE_PER_REQUEST <= 0 || - (totalBytes+bytesReadInBuffer<=MAX_BYTES_TO_STORE_PER_REQUEST) || - JMeterContextService.getContext().isRecording()) { - w.write(readBuffer, 0, bytesReadInBuffer); - } else { - log.debug("Big response, truncating it to {} bytes", MAX_BYTES_TO_STORE_PER_REQUEST); - w.write(readBuffer, 0, (int)(MAX_BYTES_TO_STORE_PER_REQUEST-totalBytes)); - storeInBOS = false; - } - } - } else { - md.update(readBuffer, 0, bytesReadInBuffer); - } - totalBytes += bytesReadInBuffer; + int bytesReadInBuffer; + long totalBytes = 0; + boolean first = true; + boolean storeInBOS = true; + while ((bytesReadInBuffer = in.read(readBuffer)) != -1) { + if (bytesReadInBuffer == 0) { + continue; } - - if (first) { // Bug 46838 - if there was no data, still need to set latency + if (first) { sampleResult.latencyEnd(); - return new byte[0]; + first = false; } - if (md == null) { - return w.toByteArray(); - } else { - byte[] md5Result = md.digest(); - sampleResult.setBytes(totalBytes); - return JOrphanUtils.baToHexBytes(md5Result); + if (md != null) { + md.update(readBuffer, 0, bytesReadInBuffer); + } else if (storeInBOS && w != null) { + if (MAX_BYTES_TO_STORE_PER_REQUEST <= 0 || + (totalBytes + bytesReadInBuffer <= MAX_BYTES_TO_STORE_PER_REQUEST) || + JMeterContextService.getContext().isRecording()) { + w.write(readBuffer, 0, bytesReadInBuffer); + } else { + log.debug("Big response, truncating it to {} bytes", MAX_BYTES_TO_STORE_PER_REQUEST); + w.write(readBuffer, 0, (int) (MAX_BYTES_TO_STORE_PER_REQUEST - totalBytes)); + storeInBOS = false; + } } + totalBytes += bytesReadInBuffer; + } + + if (first) { // Bug 46838 - if there was no data, still need to set latency + sampleResult.latencyEnd(); + sampleResult.setResponseData(new byte[0]); + return; + } - } finally { - JOrphanUtils.closeQuietly(w); + byte[] resultBody; + if (w != null) { + resultBody = w.toByteArray(); + } else if (md != null) { + byte[] md5Result = md.digest(); + resultBody = JOrphanUtils.baToHexBytes(md5Result); + } else { + resultBody = new byte[0]; } + sampleResult.setResponseData(resultBody, contentEncoding); } /** @@ -2053,6 +2150,23 @@ void mergeFileProperties() { removeProperty(MIMETYPE); } + @Override + public void setProperty(JMeterProperty property) { + @SuppressWarnings("deprecation") + PropertyDescriptor storeAsMD5 = HTTPSamplerBaseSchema.INSTANCE.getStoreAsMD5(); + if (property.getName().equals(storeAsMD5.getName())) { + if (property instanceof BooleanProperty booleanProperty) { + setResponseProcessingMode( + booleanProperty.getBooleanValue() ? ResponseProcessingMode.CHECKSUM_DECODED_MD5 : ResponseProcessingMode.STORE_COMPRESSED + ); + } else { + setResponseProcessingMode(ResponseProcessingMode.STORE_COMPRESSED); + } + return; + } + super.setProperty(property); + } + /** * set IP source to use - does not apply to Java HTTP implementation currently * @@ -2145,7 +2259,10 @@ private static class ASyncSample implements Callable { CookieManager clonedCookieManager = (CookieManager) cookieManager.clone(); this.sampler.setCookieManagerProperty(clonedCookieManager); } - this.sampler.setMD5(this.sampler.useMD5() || IGNORE_EMBEDDED_RESOURCES_DATA); + ResponseProcessingMode responseProcessingMode = base.getResponseProcessingMode(); + this.sampler.setResponseProcessingMode( + IGNORE_EMBEDDED_RESOURCES_DATA ? ResponseProcessingMode.CHECKSUM_DECODED_MD5 : responseProcessingMode + ); this.jmeterContextOfParentThread = JMeterContextService.getContext(); } diff --git a/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBaseSchema.kt b/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBaseSchema.kt index 89bb2f58e28..d5e1b6cdf16 100644 --- a/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBaseSchema.kt +++ b/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBaseSchema.kt @@ -123,9 +123,16 @@ public abstract class HTTPSamplerBaseSchema : TestElementSchema() { public val embeddedUrlExcludeRegex: StringPropertyDescriptor by string("HTTPSampler.embedded_url_exclude_re") + @Deprecated(message = "Use responseProcessingMode instead") public val storeAsMD5: BooleanPropertyDescriptor by boolean("HTTPSampler.md5", default = false) + public val responseProcessingMode: StringPropertyDescriptor + by string( + "HTTPSampler.responseProcessingMode", + default = HTTPSamplerBase.ResponseProcessingMode.STORE_COMPRESSED.resourceKey + ) + public val postBodyRaw: BooleanPropertyDescriptor by boolean("HTTPSampler.postBodyRaw", default = false) diff --git a/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoder.kt b/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoder.kt new file mode 100644 index 00000000000..3e34d13972d --- /dev/null +++ b/src/protocol/http/src/main/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoder.kt @@ -0,0 +1,41 @@ +/* + * 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.http.sampler.decoders + +import com.google.auto.service.AutoService +import org.apache.jmeter.samplers.ResponseDecoder +import org.apiguardian.api.API +import org.brotli.dec.BrotliInputStream +import java.io.InputStream + +/** + * Decoder for Brotli compressed response data. + * Handles "br" content encoding. + * + * @since 6.0.0 + */ +@AutoService(ResponseDecoder::class) +@API(status = API.Status.INTERNAL, since = "6.0.0") +public class BrotliDecoder : ResponseDecoder { + override val encodings: List + get() = listOf("br") + + override fun decodeStream(input: InputStream): InputStream { + return BrotliInputStream(input) + } +} diff --git a/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoderTest.kt b/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoderTest.kt new file mode 100644 index 00000000000..9eb2bcc11b9 --- /dev/null +++ b/src/protocol/http/src/test/kotlin/org/apache/jmeter/protocol/http/sampler/decoders/BrotliDecoderTest.kt @@ -0,0 +1,62 @@ +/* + * 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.http.sampler.decoders + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Test +import java.io.IOException +import java.util.Base64 + +/** + * Basic tests for BrotliDecoder. + * Full integration tests for brotli decompression are covered by HTTP sampler tests. + */ +class BrotliDecoderTest { + private val decoder = BrotliDecoder() + + @Test + fun testGetEncodings() { + assertEquals(listOf("br"), decoder.encodings, "encodings") + } + + @Test + fun testGetPriority() { + assertEquals(0, decoder.priority, "Default priority should be 0") + } + + @Test + fun testDecodeBrotliData() { + // Pre-compressed "Hello World" with Brotli + // Generated using: printf 'Hello World' | brotli | base64 + val compressed = Base64.getDecoder().decode("DwWASGVsbG8gV29ybGQD") + + val decoded = decoder.decode(compressed) + + assertEquals("Hello World", decoded.toString(Charsets.UTF_8), "Decoded text should match original") + } + + @Test + fun testDecodeInvalidData() { + val invalidData = "This is not brotli compressed data".toByteArray(Charsets.UTF_8) + + assertThrows(IOException::class.java) { + decoder.decode(invalidData) + } + } +} From f9701d80d3a347b5c01d6c7f090ad874ee76b6fc Mon Sep 17 00:00:00 2001 From: Vladimir Sitnikov Date: Sat, 30 May 2026 00:34:20 +0300 Subject: [PATCH 10/10] fix(http): guard against null HC4 entity for empty responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lazy-decompression refactor dropped the `entity != null` check before calling `entity.getContent()`. HttpClient4 returns a null entity when the response has no body — HEAD requests and 304 Not Modified — so those samples failed with a NullPointerException ("Non HTTP response code: java.lang.NullPointerException"). Restore the guard and set empty response data when there is no body. This matches HTTPJavaImpl, whose stream is never null. Covered by TestRedirects. Co-Authored-By: Claude Opus 4.8 --- .../jmeter/protocol/http/sampler/HTTPHC4Impl.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java index 11db6bcb0e9..bec3565831d 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java @@ -586,10 +586,15 @@ protected HTTPSampleResult sample(URL url, String method, res.setEncodingAndType(ct); } HttpEntity entity = httpResponse.getEntity(); - try (InputStream instream = entity.getContent()) { - Header contentEncodingHeader = entity.getContentEncoding(); - String contentEncoding = contentEncodingHeader != null ? contentEncodingHeader.getValue() : null; - readResponse(res, instream, entity.getContentLength(), contentEncoding); + if (entity == null) { + // No response body (e.g. HEAD request or 304 Not Modified) + res.setResponseData(new byte[0]); + } else { + try (InputStream instream = entity.getContent()) { + Header contentEncodingHeader = entity.getContentEncoding(); + String contentEncoding = contentEncodingHeader != null ? contentEncodingHeader.getValue() : null; + readResponse(res, instream, entity.getContentLength(), contentEncoding); + } } res.sampleEnd(); // Done with the sampling proper.