WI01086104 - Fix Group 1 NRE: empty-parent AddNew on RelatedCurrencyManager

[FahmiFuzi]

Summary

Fixes the .NET 10 Group 1 NullReferenceException failures (39 failures in crikey run 27ca6b55) by replicating the .NET Framework ZBindingContext.ParentCurrentChangedHandler empty-parent behavior in the fork's rewired CurrentChanged handler path.

When a parent currency manager has no current row (Count == 0), the child manager now binds an empty list (AllowNew=false) instead of calling AddNew(), which materializes orphaned business objects whose SetDefaultsForNewChild/property getters dereference null parent navigations.

Root Cause — why .NET Framework and .NET 8 worked but .NET 10 fails

The dangerous code path is identical in every version: RelatedCurrencyManager.ParentManager_CurrentItemChanged has an "Everett appcompat" branch that, when the parent manager is empty (Count == 0), calls AddNew() on the parent's list to materialize a dummy row, then CancelCurrentEdit(). On CargoWise business-object collections that AddNew() runs SetCollectionRelationships/SetDefaultsForNewChild against an orphaned collection (Parent == null) → NRE.

What differs is whether CargoWise can intercept manager registration and suppress that path:

Runtime	BindingContext store	Interception	Result
.NET Framework	private Hashtable listManagers	ZBindingContext reflection-swaps the field for BindingContextHashtable; Hashtable.Add is virtual, so every registration is intercepted → empty parent rebound to inert TempList, never AddNew	works
.NET 8	Winzor's own System.Windows.Forms kept private readonly Hashtable _listManagers for exactly this reason (#if NETFRAMEWORK || WINZOR)	same trick works	works
.NET 10 (this fork / MS WinForms)	Dictionary<HashKey, WeakReference>	impossible: the field can't hold a Hashtable subclass and Dictionary<,>.Add is not virtual	Everett AddNew runs against real CargoWise collections for the first time → NRE

Note: Microsoft's WinForms switched to the Dictionary in Jan 2023 (dotnet#8235), so even stock .NET 8 WinForms would have failed — .NET 8 worked only because the client ran on Winzor's implementation.

The earlier WI01076374 fix added BindingContext.OnListManagerAdded + RewireParentChangeHandler as the .NET 10 replacement for the Hashtable interception, but it only replicated the CurrentItemChanged → CurrentChanged event swap (the Group 7 stack-overflow fix). The empty-parent "bind an inert list instead of AddNew" behavior of ParentCurrentChangedHandler was not ported — this PR completes that port. See the PR conversation for the step-by-step execution trace.

Changes

RelatedCurrencyManager.cs:

1. RewireParentChangeHandler: Wires parent CurrentChanged to new ParentManager_CurrentChanged (instead of stock handler) and primes through it.

2. New ParentManager_CurrentChanged (port of FW ParentCurrentChangedHandler):

   • Empty parent (Count == 0) → SetDataSource(new BindingList<object> { AllowNew=false, AllowEdit=false, AllowRemove=false }) + listposition = -1 + raise OnPositionChanged/OnCurrentChanged/OnCurrentItemChanged
   • Non-empty parent → delegate to stock ParentManager_CurrentItemChanged

3. ParentManager_CurrentItemChanged: One-line guard change (combines with LV1 WI01086017):

   • else → else if (currencyManager.List is IBindingList { AllowNew: true })
   • Skips AddNew on read-only lists (including the empty placeholder for rewired managers)
   • Everett body unchanged

Why This Works

Ctor-priming coverage: EnsureListManager is recursive — each parent manager is registered + rewired before the next child is constructed. By the time a child's constructor primes, its empty parent already holds the AllowNew=false placeholder and the guard skips the Everett AddNew. This is the same inter-construction interception window that .NET Framework's BindingContextHashtable.Add used.

Guard+placeholder are interdependent: Without the guard, AddNew on the placeholder BindingList<object> would insert a bare object() and _fieldInfo.GetValue would throw.

Test Plan

• Customs.DE ExitSummaryPlugInTest.TestBashUserControlOfPlugIn (was NRE in SetDefaultsForNewChild) — PASS
• MarketingManager GlbCompanyCampaignFormTest.TestBashingForm (was NRE in get_TrackingStatusDescription) — NRE eliminated (0 signatures in TRX)
• Earlier broad run: 23/23 tests across 11 assemblies NRE-free (12 full PASS, 11 fail on pre-existing Group 7 ZGrid/DataGrid issues)

Residual Issues

Tests that still fail after the fix fail on previously-masked issues (not Group 1):

• Legacy DataGrid GridColumnStylesCollection index out-of-range
• ZGrid ColumnName validation (Group 7)
• Layout/translation assertions (Group 9)

These are separate from the Group 1 NRE and can be addressed in follow-up work.

Notes

• StyleCop SA1513: comments cannot sit between } and else if — moved inside the block
• To ship: pack WTG.Windows.Forms (use Create Release GH workflow per src/System.Windows.Forms.Legacy/how-to-release-nuget-package.md) and bump CargoWise Directory.Packages.props from 0.2.2-pr.17.1

Co-authored-by: Fahmi Fuzi fahmi.fuzi@wisetechglobal.com

[FahmiFuzi]

Root cause analysis — why .NET Framework and .NET 8 work, and .NET 10 doesn't

The hazard is ancient and identical in all versions

RelatedCurrencyManager.ParentManager_CurrentItemChanged carries an "Everett appcompat" branch: when the parent manager has no current row (Count == 0), it calls AddNew() on the parent's list to materialize a dummy row (so child metadata can be resolved), then CancelCurrentEdit() to remove it. On CargoWise business-object collections that AddNew() is not inert — it runs SetCollectionRelationships + SetDefaultsForNewChild, and in nested bindings the collection being added to is itself an orphaned dummy from the same dance one level up, so its Parent navigation is null → NRE. The 12 distinct Group 1 "origin" getters are all this one failure.

What changed between runtimes is not this code — it's whether CargoWise could intercept manager registration and suppress it.

.NET Framework — interception works

BindingContext stores managers in a private Hashtable listManagers field, and Hashtable.Add is virtual.

1. ZBindingContext's constructor reflection-swaps that field for BindingContextHashtable (a Hashtable subclass).
2. EnsureListManager is recursive: for A.B.C it ensures the manager for A first, then constructs B's manager, registers it (→ our Add override runs synchronously, before C's manager is constructed), and so on.
3. The Add override unhooks the parent's CurrentItemChanged from the stock handler, hooks CurrentChanged → ParentCurrentChangedHandler, and re-primes immediately.
4. ParentCurrentChangedHandler with an empty parent binds the child to TempList — Count == 0, AllowNew == false, and its IBindingList.AddNew() simply returns null — and sets listposition = -1. No business object is ever created.
5. When the next child's constructor primes through the stock handler and reaches the Everett branch, the AddNew() it issues lands on the inert TempList → harmless no-op. No NRE possible.

.NET 8 — same trick, different WinForms

On .NET 8 the CargoWise client does not run on Microsoft's WinForms — it runs on Winzor's own System.Windows.Forms implementation, whose BindingContext deliberately kept private readonly Hashtable _listManagers (Winzor/BArchitecture.GUI/System.Windows.Forms/src/BindingContext.cs). The ZBindingContext swap is compiled under #if NETFRAMEWORK || WINZOR, so .NET 8 works for exactly the same reason as .NET Framework.

Notably, Microsoft's WinForms switched to Dictionary<HashKey, WeakReference> back in Jan 2023 (dotnet#8235) — it was already in v8.0. Stock .NET 8 WinForms would have failed identically; CargoWise just never ran on it.

.NET 10 — interception impossible, the suppressed path runs for the first time

The fork inherits the upstream Dictionary<HashKey, WeakReference> store. The interception is dead twice over: the field cannot hold a Hashtable subclass, and Dictionary<,>.Add is not virtual. WI01076374 added BindingContext.OnListManagerAdded + RewireParentChangeHandler as the replacement hook, but the initial rewire only replicated the CurrentItemChanged → CurrentChanged event swap (which fixed the Group 7 re-entrant stack overflow). The empty-parent TempList branch of ParentCurrentChangedHandler was not ported — so the Everett AddNew() that the interception had suppressed for ~20 years finally executed against real CargoWise collections.

Execution on .NET 10 before this PR (binding CusExitDetails.CusExitItems.Packages on an empty form data source):

1. EnsureListManager recurses; the root CusExitDetails manager has Count == 0.
2. The CusExitItems RelatedCurrencyManager constructor primes through the stock handler → Everett AddNew() on the root collection creates a dummy CusExitDetail; the re-entrant CurrentChanged binds the child to dummy.CusExitItems; CancelCurrentEdit() then removes the dummy → the still-bound CusExitItemCollection is now orphaned (Parent == null).
3. OnListManagerAdded fires → the (old) rewire swaps events and re-primes — still through the stock handler, so nothing replaces the orphaned list.
4. The Packages RelatedCurrencyManager constructor primes: parent Count == 0, parent list is the real orphaned collection (AllowNew == true) → AddNew() → SetCollectionRelationships wires child.CusExitDetail = collection.Parent (= null) → SetDefaultsForNewChild dereferences it → NRE (CusExitItemCollection.cs:19).
5. Variant of the same root cause: where the dummy survives until handle creation, Binding.PushData invokes getters on the half-initialized current item — e.g. GlbCompanyCampaignItem.get_TrackingStatusDescription → CompanyCampaign.IsTargetList with CompanyCampaign == null → NRE.

How this PR restores the old behavior

RewireParentChangeHandler now wires the parent's CurrentChanged to the new ParentManager_CurrentChanged — a direct port of ParentCurrentChangedHandler: empty parent ⇒ bind a placeholder BindingList<object> { AllowNew = false } + listposition = -1 (the TempList equivalent); non-empty ⇒ delegate to the stock handler. The Everett branch is additionally guarded with is IBindingList { AllowNew: true }.

Constructor-time priming is covered by the same ordering argument as .NET Framework: because EnsureListManager is recursive, each parent is registered and rewired before the next child is constructed, so by the time a child's constructor primes, its empty parent already holds the AllowNew=false placeholder and the guard skips the AddNew. The guard and the placeholder are interdependent — without the guard, AddNew on the placeholder would insert a bare object() and _fieldInfo.GetValue would throw.

(The code comments in RelatedCurrencyManager.cs / BindingContext.cs were updated in 05a2126 to record this analysis.)