Skip to content

Split TestBean into its GUI-generation and field-storage halves #6728

Description

@vlsi

Summary

TestBean is a marker interface that means two unrelated things at once. Elements can't opt into one without the other, which is what turned a simple bug report (#6451) into an open-ended design question in #6726. This issue proposes splitting it, and asks for a decision on how existing elements opt out of the storage half.

Background

Implementing TestBean currently buys an element two separate things:

  1. A generated GUI. Write a BeanInfo and TestBeanGUI builds the editor by introspection, with no Swing code in the element.
  2. Field-backed storage with a sync step. TestBeanHelper.prepare() copies the property map into the element's setters, so the element's plain Java fields hold the values during a run.

The second is the source of a recurring surprise: the fields are only populated after prepare(), so every TestBean getter returns null (or a field initializer) on a tree that was just loaded from JMX. #6451 hit this with CSVDataSet.getFilename(). It's working as designed, but the design is a trap, and it applies to all 32 elements that implement TestBean — JSR223 elements, ConstantThroughputTimer, DataSourceElement, and the rest.

The two jobs have nothing to do with each other. An element that stores its configuration in the property map still wants a generated GUI; it just doesn't want the field sync.

The split is already there in the code

Every place the framework tests for TestBean uses it for exactly one of the two jobs, and no call site needs both:

GUI generation (5)

  • TestBeanGUI:159 — guard that the class is a TestBean
  • GuiPackage:368 — instantiate TestBeanGUI for the element
  • MenuFactory:138, MenuFactory:209 — menu registration
  • JMeterTreeNode:145 — icon and label resolution

Field-backed storage (3)

  • TestBeanHelper:129 — entry guard for prepare()
  • TestBeanHelper:180 — recursion into nested TestElementProperty
  • StandardJMeterEngine:243 — prepare test listeners

Proposal

Split the marker in two and have TestBean extend both. Names are open to bikeshedding:

public interface BeanInfoGui {}           // "generate my GUI from my BeanInfo"
public interface FieldBackedProperties {} // "copy my properties into my setters before use"

public interface TestBean extends BeanInfoGui, FieldBackedProperties {}

Then narrow the eight call sites above to whichever half they actually mean.

This is a no-op for existing code

Adding superinterfaces to an existing interface is binary compatible, and because TestBean extends both, every instanceof result stays identical:

  • el instanceof TestBean at TestBeanHelper:129 becomes el instanceof FieldBackedProperties. Any element implementing TestBean inherits it, so the result is unchanged.
  • TestBean.class.isAssignableFrom(c) at GuiPackage:368 becomes BeanInfoGui.class.isAssignableFrom(c). Same.

All 32 in-tree elements and every third-party plugin keep working with no source change. Both interfaces must stay method-free markers for this to hold.

So the split can land on its own merits, independently of anything else discussed here.

What it enables

New elements implement only BeanInfoGui: they get the generated GUI, store their configuration in the property map, and their getters are valid at any point in the lifecycle. No marker, no opt-out, no prepare() on the sampling path.

That also unblocks the newer property editors (JBooleanPropertyEditor and friends) and the modified-value gutter for these elements, since those bind to schema property descriptors rather than to bean fields.

The unresolved part: existing elements can't un-implement

An element like CSVDataSet has to keep implementing TestBean for compatibility — user code may cast to it or test for it. Since TestBean extends FieldBackedProperties, it inherits the storage half and Java gives us no way to drop it. So migrating an existing element to property-backed storage still needs an explicit opt-out. Options:

A. A negative marker. interface NotFieldBacked {}, checked alongside the positive one in TestBeanHelper.prepare(). Visible in the element's declaration, and its javadoc can scope it honestly as a legacy-compatibility wart that new elements should never need. Cost: a double negative in the type hierarchy.

B. A BeanInfo attribute. beanDescriptor.setValue("propertyBacked", TRUE), read once inside the existing GOOD_PROPS ClassValue in TestBeanHelper, so zero per-call cost. The "cannot un-implement" problem disappears because this is metadata rather than a type, and the decision sits with the element's other bean metadata. Cost: stringly-typed and less visible when reading the element class.

C. Neither — accept that existing elements stay field-backed forever, and property-backed storage is only for new elements. Cheapest, but leaves the #6451 trap in place for all 32.

I lean toward B, precisely because "cannot un-implement an interface" is a permanent Java constraint and metadata sidesteps it entirely. I don't feel strongly.

Decisions requested

These are independent; the first can land without settling the second.

  1. Do we split TestBean as described?
  2. If we do, which opt-out mechanism for existing elements — A, B, or C?

And if the answer to 2 is A or B, two semantics questions need settling once, project-wide, rather than being rediscovered per element:

  • Should a getter return null or the schema default for an unset property? Property-backed getters return the default, which differs from today's field behavior before prepare().
  • Function properties: today prepare() freezes ${...} into a field at compile time; property-backed access re-evaluates per read, using FunctionProperty's per-iteration cache. The latter looks more correct, but it is an observable change.

Origin

Came out of #6451 (reported as a CSVDataSet.getFilename() bug, closed as works-as-designed) and #6726, where @e345ee implemented property-backed storage for CSVDataSet with a PropertyBackedTestBean marker. That PR is a working reference implementation of the migration; it's on hold pending this decision.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions