GenericFilter: activating a configuration in onInit breaks subsequent configuration switching #5400#5403
GenericFilter: activating a configuration in onInit breaks subsequent configuration switching #5400#5403fractal3000 wants to merge 3 commits into
Conversation
… lazily, before the first filter contribution #5400 A configuration activated during onInit (the base-API setCurrentConfiguration, the builder's makeCurrent, or a standalone GroupFilter) polluted the recorded initial data-loader condition: it was re-captured in a post-onInit init task, by which point the loader already carried the activated configuration's condition. Switching configurations afterwards then combined them (base AND cfg1 AND cfg2), yielding an empty/wrong result. Capture the loader's own condition lazily, once, at the start of updateDataLoaderCondition() - before the first filter contribution - so it is always clean regardless of when a configuration is activated. Remove the eager capture in setDataLoader and the deferred re-capture init tasks in the loaders; keep the copy-on-use that preserves ResetAction behavior (#2406). The now-unused updateDataLoaderInitialCondition helpers are deprecated for backward compatibility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Data-level integration tests (seeded rows, asserting the loaded data, not just the condition object): - switching configurations applies only the target configuration's condition; - with a DataLoadCoordinator the initial load is filtered; - plain setCurrentConfiguration in onInit no longer pollutes the baseline; - ResetAction (#2406) no-accumulation guard (repeated apply/switch stays correct). Also covers builder activation phases and extends the builder API tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| * in {@link #updateDataLoaderCondition()} before the first filter contribution. Retained for | ||
| * backward compatibility. | ||
| */ | ||
| @Deprecated |
| * in {@link #updateDataLoaderCondition()} before the first filter contribution. Retained for | ||
| * backward compatibility. | ||
| */ | ||
| @Deprecated |
| * @deprecated no longer used internally; {@link GroupFilter} now captures the initial data loader | ||
| * condition lazily. Retained for backward compatibility. | ||
| */ | ||
| @Deprecated |
| dataLoader.getCondition()); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Breaking changes is not documented. Was this approved?
The base loader condition must now be set before a configuration is activated. Previously the loader init task ran after onInit, so this captured myBaseCondition:
// in onInit
genericFilter.setCurrentConfiguration(c1);
ordersDl.setCondition(myBaseCondition); // set AFTER activationNow the first updateDataLoaderCondition() snapshots the baseline without it, and it's lost on the next switch — a silent regression for anyone relying on the old timing.
There was a problem hiding this comment.
Thanks, valid point. Removed the regression: the filter now adopts the loader's condition as its baseline if it was replaced externally (a different object, not the filter's own output). So setting the base condition after activating a configuration in onInit works again (and the same for a standalone GroupFilter), while configuration switching stays fixed. Covered by tests.
There was a problem hiding this comment.
Separately: we need to remember to add a clarification to the documentation. To change the base condition, set a new object via DataLoader.setCondition(). If instead, after the first updateDataLoaderCondition (for GenericFilter or GroupFilter), you obtain a reference to the Condition from DataLoader and modify it in place without changing the reference in DataLoader, the filter ignores it: the change is not picked up and is overwritten on the next condition rebuild (the exception being a direct DataLoader.load() before the rebuild, which reads the mutated object, but the filter does not preserve it). This behavior is not changed by this PR — it was the case before as well.
Adopt the loader condition as the new baseline when it was replaced externally (e.g. DataLoader.setCondition() after a configuration was activated in onInit), instead of only capturing it before the first filter contribution. The filter's own output is never adopted (tracked by identity), so configuration switching is still not polluted. This preserves backward compatibility for code that sets the base condition after activating a configuration, and for a standalone GroupFilter, while keeping the onInit-activation fix. Applied symmetrically to GenericFilter and GroupFilter. Add since/forRemoval to the deprecated updateDataLoaderInitialCondition methods. Tests cover: base condition set after activation, base condition revised after activation, standalone GroupFilter, and an explicitly set initial condition. #5400 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fractal3000
force-pushed
the
bug/5400-generic-filter-oninit-condition
branch
from
5c5ca62 to
c1a90b3
Compare
3 participants
#5400