-
Notifications
You must be signed in to change notification settings - Fork 148
Generic Filter: programmatic API is inconvenient and unsafe #5083
Description
Summary
When trying to use GenericFilter programmatically — creating configurations and conditions in Java code — I found the API behaves in ways that are very hard to predict. Call order matters but is nowhere documented, errors only surface at runtime without meaningful messages, and some calls silently do nothing. It was practically unusable without reading the source code.
To understand the root causes, I performed a deep analysis of the source code using Claude Code (AI-assisted static analysis). Below is a summary of the findings. The full analysis is included in collapsible sections at the bottom of this issue.
Core Issue: XML and Programmatic API Are Not Equivalent
The most important finding from the analysis is that the declarative XML approach and the programmatic Java API are not equivalent alternatives — they are fundamentally different in terms of safety and correctness.
The XML loader (GenericFilterLoader) performs a series of mandatory initialization steps automatically. The programmatic API exposes the raw low-level objects with no equivalent safeguards, and these required steps are completely undocumented.
The table below shows what the XML loader does automatically vs. what Java code must do manually:
| Done automatically by XML loader | Source | Required manually in Java |
|---|---|---|
filterComponent.setConditionModificationDelegated(true) |
GenericFilterLoader.java:221 |
Must be called on every child component |
filterComponent.setDataLoader(dataLoader) |
GenericFilterLoader.java:222 |
Must be called on every child component |
config.setFilterComponentDefaultValue(param, value) |
GenericFilterLoader.java:206 |
Must be called manually after every setValue() |
FilterUtils.updateDataLoaderInitialCondition(...) |
GenericFilterLoader.java:96 |
Must be called manually; the method is @Internal |
| Validation of filter path uniqueness | GenericFilterLoader.java:142 |
Not validated at all |
action.refreshState() after adding actions |
GenericFilterLoader.java:252 |
Must be called manually |
filter.loadConfigurationsAndApplyDefault() |
GenericFilterLoader.java:157 |
Must be called manually, and only after setDataLoader |
Calling filter.apply() when default values exist |
GenericFilterLoader.java:266 |
Must be called manually |
In practice, this means that what takes one line of XML requires a fragile sequence of 5–7 method calls in Java, with no documentation and no compile-time safety. The programmatic API is essentially the internal machinery of the XML loader exposed without any wrapper.
Most Painful Bug: Configuration.setModified(boolean) Is Order-Dependent
setModified(true) is a silent no-op if called before components are added to the configuration. This is almost certainly the most common mistake when using the API programmatically:
RunTimeConfiguration config = new RunTimeConfiguration("id", root, filter);
config.setModified(true); // NO-OP — no components exist yet
root.add(propertyFilter); // Added, but NOT marked — the remove button will never appearThe correct order is never documented. On top of that, the method name is misleading: it does not mark "the configuration as modified" in any business sense. What it actually does is control the visibility of the per-condition remove buttons in the UI — while simultaneously affecting the enabled state of the Save action (GenericFilterSaveAction). Two completely different concerns, one poorly named method.
Other Issues Found by the Analysis
-
LSP violation: the
Configurationinterface declares mutating methods (setModified,setName,setRootLogicalFilterComponent, etc.) thatDesignTimeConfigurationcannot implement — they all throwUnsupportedOperationException. The compiler gives no warning when calling these on aConfigurationreference. -
No public
refresh()method: after programmatically modifying the current configuration, there is no public API to update the UI. The internalrefreshCurrentConfigurationLayout()isprotected. The only workaround is to callsetCurrentConfiguration(currentConfig)again, which is a hack with unclear semantics. -
Silent no-op in
setCurrentConfiguration: if the configuration has not been registered withaddConfiguration()first, the call silently does nothing — no exception, no log entry. -
One-time setters without compile-time protection:
setProperty,setDataLoader,setParameterClassthrowRuntimeExceptionif called more than once (via Guava'scheckState). There is no builder pattern, no fluent API, and no compile-time enforcement of call order. -
GenericFilterSupportis@Internalbut is the only way to programmatically save a filter configuration to the database. There is no stable public API for this operation. -
Ambiguous terminology: the word "condition" refers to three different things — a
FilterComponent(UI component), aFilterCondition(entity for serialization), and an entry infilter.getConditions()(the list for the Add Condition dialog).filter.addCondition(fc)androot.add(fc)do completely different things despite similar names.
Proposed Direction
The full proposal is in the collapsible section below. The key ideas, all of which are purely additive and backward-compatible:
-
FilterComponentBuilder— a factory tied to aGenericFilterinstance that handles all mandatory initialization steps automatically (setConditionModificationDelegated,setDataLoader, ordering of property/operation/value). -
ConfigurationBuilder— a fluent API for creating and registering configurations that automatically callssetFilterComponentDefaultValuefor each added condition. -
Auto-marking
modifiedonadd()—RunTimeConfigurationshould listen toFilterComponentsChangeEventfrom its root component and automatically track which components have been added, eliminating the order-dependency ofsetModified. -
Split
Configuration/MutableConfigurationinterfaces —DesignTimeConfigurationimplements the read-onlyConfiguration;RunTimeConfigurationimplementsMutableConfiguration. Eliminates the LSP violation at compile time. -
Public
GenericFilterConfigurationsservice — a stable, non-@Internalreplacement for the parts ofGenericFilterSupportthat are needed for programmatic save/load/remove operations.
Full Analysis
All content below was produced by AI-assisted source code analysis (Claude Code).
The finding that the API is problematic and needs improvement is mine;
the technical depth of the analysis is the AI's.
Part 1: Detailed API problem analysis (11 issues with file/line references)
Problem 1: setModified(true) before add() is a no-op (Critical)
RunTimeConfiguration.java:93:
public void setModified(boolean modified) {
for (FilterComponent filterComponent : rootLogicalFilterComponent.getOwnFilterComponents()) {
setFilterComponentModified(filterComponent, modified);
}
}Iterates only over components that already exist. If no components have been added yet — the call does nothing, silently.
Problem 2: LSP violation in the Configuration interface (High)
DesignTimeConfiguration.java:94–139 throws UnsupportedOperationException for: setName, setRootLogicalFilterComponent, setModified, setFilterComponentModified, resetFilterComponentDefaultValue, resetAllDefaultValues. All are declared in the Configuration interface. The compiler does not warn.
Problem 3: No public API to refresh the filter layout (High)
GenericFilter.java:615 — refreshCurrentConfigurationLayout() is protected. After programmatically adding a component to the current configuration, the only way to trigger a UI refresh is to call setCurrentConfiguration(currentConfig) again. This works (because clearValues() is only called on configuration change), but it is semantically incorrect.
Problem 4: Silent no-op in setCurrentConfiguration (High)
GenericFilter.java:594 — if the configuration is not in the configurations list and is not the emptyConfiguration, the method does nothing. No exception, no log.
Problem 5: One-time setters without compile-time safety (Medium)
PropertyFilter.java:142, SingleFilterComponentBase.java:153:
public void setProperty(String property) {
checkState(getProperty() == null, "Property has already been initialized");
...
}No builder pattern, no fluent API, no enforcement of call order at compile time.
Problem 6: Initialization order for PropertyFilter is undocumented (Medium)
setDataLoader should be called before setProperty so that initOperationSelectorActions runs with the correct MetaClass. If called in the wrong order, the operation selector silently uses defaults. No documentation.
Problem 7: Three different things called "condition" (Medium)
| Term | Meaning |
|---|---|
FilterComponent |
UI component (PropertyFilter, JpqlFilter, etc.) |
FilterCondition (entity) |
Model class for DB serialization |
filter.addCondition(fc) |
Adds to the Add Condition dialog list only — does NOT activate it |
root.add(fc) |
Adds to the active configuration |
Problem 8: GenericFilterSupport is @Internal but required for save (Medium)
GenericFilterSupport.java:61 — annotated @Internal, but saveConfigurationModel, getConfigurationsMap, saveCurrentFilterConfiguration have no stable public equivalent.
Problem 9: removeConfiguration silently ignores DesignTimeConfiguration (Low)
GenericFilter.java:876 — the guard condition silently skips DesignTimeConfiguration. No exception, no documentation.
Problem 10: setModified has an unexpected side-effect on the Save button (Low)
GenericFilterSaveAction.java:62 — isApplicable() checks currentConfiguration.isModified(). Calling setModified(false) programmatically disables the Save button even when the configuration has active conditions.
Problem 11: Shallow copy of LogicalCondition in copy() (Low)
GenericFilter.java:1001 — nested LogicalCondition objects inside initialDataLoaderCondition are not deep-copied; child conditions are added by reference.
Part 2: XML declarative vs. programmatic API — detailed comparison
What XML does automatically, Java requires manually
| XML loader step | Source | Java equivalent |
|---|---|---|
filterComponent.setConditionModificationDelegated(true) |
GenericFilterLoader.java:221 |
Manual, on every child |
filterComponent.setDataLoader(dataLoader) |
GenericFilterLoader.java:222 |
Manual, on every child |
config.setFilterComponentDefaultValue(param, value) |
GenericFilterLoader.java:206 |
Manual, after every setValue() |
FilterUtils.updateDataLoaderInitialCondition(...) |
GenericFilterLoader.java:96 |
Manual (@Internal) |
| Filter path uniqueness validation | GenericFilterLoader.java:142 |
Not validated |
action.refreshState() |
GenericFilterLoader.java:252 |
Manual |
loadConfigurationsAndApplyDefault() |
GenericFilterLoader.java:157 |
Manual |
filter.apply() with default values |
GenericFilterLoader.java:266 |
Manual |
Example: one <conditions> block vs. its Java equivalent
XML (8 lines):
<conditions>
<propertyFilter property="number" operation="EQUAL" defaultValue="1337"/>
<jpqlFilter parameterClass="java.lang.Void">
<condition><c:jpql><c:where>{E}.number = '1337'</c:where></c:jpql></condition>
</jpqlFilter>
</conditions>Java equivalent (17 lines, and easy to get wrong):
PropertyFilter<Integer> pf = uiComponents.create(PropertyFilter.class);
pf.setConditionModificationDelegated(true); // not obvious, undocumented
pf.setDataLoader(filter.getDataLoader()); // not obvious, undocumented
pf.setProperty("number");
pf.setOperation(PropertyFilter.Operation.EQUAL);
pf.setValue(1337);
filter.addCondition(pf);
JpqlFilter<Boolean> jf = uiComponents.create(JpqlFilter.class);
jf.setConditionModificationDelegated(true);
jf.setDataLoader(filter.getDataLoader());
jf.setParameterClass(Void.class);
jf.setCondition("{E}.number = '1337'", null);
filter.addCondition(jf);Summary scorecard
| Criterion | XML | Java (programmatic) |
|---|---|---|
| Conciseness | ✅ | ❌ |
| Initialization order guaranteed | ✅ | ❌ on the developer |
setConditionModificationDelegated |
✅ automatic | ❌ manual, easy to miss |
setDataLoader on children |
✅ automatic | ❌ manual, easy to miss |
setFilterComponentDefaultValue |
✅ automatic | ❌ manual, easy to miss |
| Filter path validation | ✅ | ❌ not validated |
refreshState() for actions |
✅ automatic | ❌ manual |
Dynamic configurations (RunTimeConfiguration) |
❌ not supported | ✅ only here |
| Runtime condition list changes | ❌ not supported | ✅ only here |
Part 3: Proposed API improvements
All proposals are purely additive — no existing classes or methods are changed.
FilterComponentBuilder
FilterComponentBuilder builder = filter.componentBuilder();
PropertyFilter<String> nameFilter = builder.propertyFilter()
.property("name")
.operation(PropertyFilter.Operation.CONTAINS)
.defaultValue("Acme")
.build();
// Automatically handles: setConditionModificationDelegated(true),
// setDataLoader(...), correct call ordering
JpqlFilter<Boolean> activeFilter = builder.jpqlFilter()
.where("{E}.status = 'ACTIVE'")
.build();ConfigurationBuilder
filter.configurationBuilder()
.id("searchByStatus")
.name("Search by Status")
.operation(LogicalFilterComponent.Operation.AND)
.add(nameFilter) // auto: root.add() + setFilterComponentDefaultValue()
.add(statusFilter, "NEW") // override default value
.asDefault()
.buildAndRegister(); // addConfiguration() + setCurrentConfiguration() in correct orderAuto-marking modified on add()
RunTimeConfiguration subscribes to FilterComponentsChangeEvent from its root component and automatically tracks added/removed components — eliminating the order-dependency of setModified.
Split Configuration / MutableConfiguration
interface Configuration { ... } // read-only — DesignTimeConfiguration
interface MutableConfiguration // mutable — RunTimeConfiguration
extends Configuration { ... }Public GenericFilterConfigurations service
@Autowired
private GenericFilterConfigurations filterConfigurations; // replaces @Internal GenericFilterSupport
filterConfigurations.save(genericFilter, configuration);
filterConfigurations.remove(genericFilter, configuration);
filterConfigurations.load(genericFilter);