Fix filter panel restore race; doc v2 plan & logging

Refactored MgGridFilterPanelTagBox to robustly heal filter criteria vs. async lookup data race, using level-triggered snapshot detection and reload. Added diagnostic logging. Updated MGGRID docs to explain the new logic, rationale, and planned v2 layout envelope solution. Minor unrelated shell command tweaks in settings.local.json.
This commit is contained in:
Loretta 2026-07-17 06:31:41 +02:00
parent d164a99ed9
commit efd4c67cc0
4 changed files with 169 additions and 41 deletions

View File

@ -45,43 +45,111 @@ public class MgGridFilterPanelTagBox<TData, TValue> : MgTagBox<TData, TValue>, I
InheritSizeModeFromGrid();
// Keep the selection in step with our field's active criteria on EVERY parameter pass. The editor
// resolves its tags against the CURRENT Data, and the criteria (layout restore via JS interop) and the
// lookup Data (async load) arrive in arbitrary order — one-shot "did X change" gates proved to get
// consumed in the wrong interleaving and strand an applied-but-invisible selection. This form is
// idempotent: it re-asserts Values (fresh instance => tags re-resolve) whenever the Data instance
// changed or the displayed selection drifted from the criteria, and no-ops otherwise.
if (Grid is not null && !string.IsNullOrWhiteSpace(FieldName)
TrackEditorSnapshot();
Diag("OnParametersSet");
SyncSelectionToCriteria();
}
// ======================= TEMP DIAGNOSTIC — REMOVE after the restore-race investigation =======================
// Purpose: prove/disprove that DevExpress raises ValuesChanged with an EMPTY list around Reload() (dropping
// not-yet-resolvable tags), which OnSelectionChanged would misread as a user edit and clear the criteria.
// Blazor WASM: Console.WriteLine lands in the browser console. Filter the console by "[MgFP".
private static string Fmt(IEnumerable<TValue>? values) => values is null ? "null" : $"[{string.Join(",", values)}]";
private void Diag(string message)
{
string criteria;
try
{
criteria = Grid is not null && !string.IsNullOrWhiteSpace(FieldName)
&& Grid.GetFilterPanelCriteria(FieldName) is { } activeCriteria
&& TryExtractInValues(activeCriteria, out var criteriaValues))
? activeCriteria.ToString()
: "null";
}
catch (Exception ex)
{
// Content signature, not just reference: consumers routinely bind a pre-created EMPTY collection and
// fill the SAME instance later (AcObservableCollection.Replace) — the reference never changes, only
// the count does (0 => N). Count() is O(1) for ICollection-backed lookups.
var dataCount = Data?.Count() ?? -1;
var sameInstanceRefilled = ReferenceEquals(Data, _lastSyncedData) && dataCount != _lastSyncedDataCount;
var dataChanged = !ReferenceEquals(Data, _lastSyncedData) || dataCount != _lastSyncedDataCount;
var selectionDrifted = Values is null || !Values.SequenceEqual(criteriaValues);
criteria = $"ERR:{ex.GetType().Name}";
}
if (dataChanged || selectionDrifted)
var dataCount = Data is null ? -1 : Data.Count();
Console.WriteLine($"[MgFP:{FieldName}] {message} | dataCount={dataCount} snapWasEmpty={_snapshotTakenWhileDataEmpty} " +
$"values={Fmt(Values)} criteria={criteria}");
}
// ===================== /TEMP DIAGNOSTIC =====================================================================
// ---- Editor-snapshot state -----------------------------------------------------------------------------
// A restore has TWO inputs that arrive in arbitrary order: the criteria (async layout restore via JS interop)
// and the lookup Data (async load — routinely filled IN PLACE via AcObservableCollection.Replace on a
// pre-created empty instance). DevExpress snapshots the editor's item list and detects Data changes BY
// REFERENCE ONLY, so an in-place fill leaves the editor holding an EMPTY list and no asserted Values can ever
// resolve into tags.
// This state is LEVEL-triggered by design: it records the standing fact "this editor's snapshot was taken
// while Data was empty" instead of trying to catch the 0->N edge. Edge/one-shot gates were tried twice and
// both systematically stranded the LAST-filled lookup — its arming pass never happens (see ADR 0004).
private IEnumerable<TData>? _snapshotData;
private bool _snapshotTakenWhileDataEmpty;
private bool _reassertAfterReloadPending;
private bool IsDataEmpty => Data is null || !Data.Any();
/// <summary>
/// Records whether the editor's current item snapshot was taken against an EMPTY <c>Data</c>. Runs on every
/// parameter pass REGARDLESS of criteria presence — arming must not depend on the criteria arriving first;
/// that ordering assumption is exactly what stranded the last-filled lookup.
/// </summary>
private void TrackEditorSnapshot()
{
_lastSyncedData = Data;
_lastSyncedDataCount = dataCount;
// DevExpress snapshots the editor's item list and detects Data changes BY REFERENCE — an in-place
// fill is invisible to it, so the re-asserted Values could never resolve into tags (re-rendering
// does not help). Reload() (DxListEditorBase) forces the editor to re-read its Data source.
if (sameInstanceRefilled) Reload();
SetValuesCore(criteriaValues);
}
}
if (!ReferenceEquals(Data, _snapshotData))
{
// A NEW instance is detected by DevExpress, which re-reads the list itself => the editor's snapshot
// matches whatever this instance currently holds.
_snapshotData = Data;
_snapshotTakenWhileDataEmpty = IsDataEmpty;
return;
}
// Last Data instance + count a re-assert ran against — only consumed when an assignment actually happens
// (see OnParametersSet; a change while there is no active criteria must NOT count as handled).
private IEnumerable<TData>? _lastSyncedData;
private int _lastSyncedDataCount = -1;
// Same instance, still empty => the snapshot is (still) an empty list, and a later in-place fill will be
// invisible to DevExpress. Same instance, non-empty => leave the flag alone: it answers "was the snapshot
// taken before the fill?", which only a heal may clear.
if (IsDataEmpty) _snapshotTakenWhileDataEmpty = true;
}
/// <summary>
/// Forces the editor to re-read its <c>Data</c> source when its snapshot is known-stale (taken while empty)
/// and <c>Data</c> has since been filled in place. Returns true when a reload was issued, and queues ONE
/// deferred re-assert: DevExpress documents no settle guarantee for <c>Reload()</c>
/// (<c>DxListEditorBase.Reload</c> is a bare <c>void</c>), so a same-pass <c>Values</c> assignment could still
/// race the item list.
/// </summary>
private bool HealEditorSnapshotIfStale()
{
if (!_snapshotTakenWhileDataEmpty || IsDataEmpty) return false;
_snapshotTakenWhileDataEmpty = false;
Reload();
_reassertAfterReloadPending = true;
return true;
}
/// <summary>
/// Brings the displayed selection in line with our field's active criteria: heals a stale editor snapshot
/// first, then asserts the values when the snapshot was just healed or the selection drifted.
/// </summary>
private void SyncSelectionToCriteria()
{
if (Grid is null || string.IsNullOrWhiteSpace(FieldName)) return;
if (Grid.GetFilterPanelCriteria(FieldName) is not { } criteria) return;
if (!TryExtractInValues(criteria, out var criteriaValues)) return;
var healed = HealEditorSnapshotIfStale();
var drifted = Values is null || !Values.SequenceEqual(criteriaValues);
if (healed || drifted) SetValuesCore(criteriaValues);
}
/// <summary>
/// Defaults <c>SizeMode</c> to the grid's (via the <see cref="Grid"/> cascade) so the filter panel matches the
@ -114,6 +182,22 @@ public class MgGridFilterPanelTagBox<TData, TValue> : MgTagBox<TData, TValue>, I
// registration weakly and reconciles this component immediately — restoring a persisted panel filter
// into the UI, or clearing it while the panel is hidden. See IMgGridFilterPanelComponent / ADR 0004.
if (firstRender) Grid?.RegisterFilterPanelComponent(this);
// Deferred re-assert after a Reload(): the reload-triggered render has completed by now, so the editor's
// item list is settled even though Reload() itself gives no such guarantee. Flag-gated and idempotent —
// the re-assert's own render finds the flag cleared, so this cannot loop.
if (_reassertAfterReloadPending)
{
_reassertAfterReloadPending = false;
if (Grid is not null && !string.IsNullOrWhiteSpace(FieldName)
&& Grid.GetFilterPanelCriteria(FieldName) is { } criteria
&& TryExtractInValues(criteria, out var values))
{
SetValuesCore(values); // fresh instance => the editor re-resolves its tags
StateHasChanged();
}
}
}
private void OnSelectionChanged(IEnumerable<TValue>? values)
@ -147,6 +231,11 @@ public class MgGridFilterPanelTagBox<TData, TValue> : MgTagBox<TData, TValue>, I
if (criteria is not null && TryExtractInValues(criteria, out var values))
{
// The grid may be telling us about a criteria whose lookup data has only just arrived — e.g. the
// consumer calling RefreshFilterPanel() right after an in-place fill. Without healing the editor's
// (empty) snapshot first, the assert below could never resolve into tags: this is what made
// RefreshFilterPanel fall short of its documented contract.
HealEditorSnapshotIfStale();
SetValuesCore(values);
}
else

View File

@ -84,12 +84,13 @@ record which fields were panel-owned.
Reset layout (toolbar) on the affected client, or re-save the layout from a client that has the panel.
### Proposed fix
Wrap the persisted layout in an MgGrid envelope (`{ dxLayout, quickFilterFields[] }`) so the loader can clear
quick-filter fields even with no registered controls. Deliberately deferred until the scenario is actually hit
(YAGNI); referenced from ADR 0004 (AyCode.Core repo).
Wrap the persisted layout in an MgGrid envelope (`MgGridLayoutSave { Version, GridLayoutJson,
FilterPanelLayoutJson }`) so panel state never rides inside the DevExpress layout: an entry no component claims
simply never reaches the grid — this issue is then closed **by construction**, no clearing logic needed. Design in
ADR 0004 (AyCode.Core repo) → "Planned v2".
### Related TODO
None yet.
`MGGRID_TODO.md#acblazor-grid-t-w8q3` (the envelope + pull-model restore — closes this issue).
## Issue entry template

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long