Seal LoadableDetachableModel#detach() and add object-aware detach hooks

[reiern70]

detach() drives the model's attach/detach state machine: it invokes onDetach() when there is something to detach, then discards the transient object and resets the state. It was overridable, so a subclass could run cleanup on either side of super.detach(), or skip the super call and leave the model attached for good. Nothing enforced the invariant the method exists to maintain.

detach() is now final, and cleanup goes into one of two hooks:

onDetach() - as before, only when the model was attached
onDetachAlways() - on every detach() call, attached or not

onDetachAlways() is what an override of detach() in practice was, and is where cleanup not tied to the loaded object belongs: detaching models this one was handed, for instance, which may have been attached without this model ever loading.

A subclass that needed the loaded object while detaching had to keep a reference of its own, because onDetach() took no arguments and the field holding the object is private. The new onDetach(T object) is handed that object before it is discarded. The default onDetach() delegates to it, so an override of onDetach() that does not call super suppresses it.

Making a public method final is source-incompatible, which is why this lands on master only. It stays binary compatible - detach() still resolves, on LoadableDetachableModel - so compiled subclasses keep running, but a subclass that overrides detach() no longer compiles. Moving the override's body to onDetachAlways() and dropping the super.detach() call reproduces the old behaviour exactly; moving it to onDetach() narrows it to the attached case. That rewrite needs a judgement about which hook applies, so it is documented as a manual step in wicket.yml rather than automated.

Three subclasses in the tree overrode detach(), each to detach something unconditionally, and all three move to onDetachAlways(): StringResourceModel and its AssignmentWrapper, and the devutils SessionIdentifiersModel. StringResourceModel is the reason onDetachAlways() exists at all - per WICKET-5176 it has to detach its substitution models even when it was never attached itself, which an attached-only hook cannot do.

GitHub issue #1614: #1614

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.89%. Comparing base (6550afd) to head (e64a2bf).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1615   +/-   ##
=========================================
  Coverage     61.89%   61.89%           
- Complexity    11242    11245    +3     
=========================================
  Files          1247     1247           
  Lines         48367    48372    +5     
  Branches       6788     6788           
=========================================
+ Hits          29935    29941    +6     
  Misses        15725    15725           
+ Partials       2707     2706    -1     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[papegaaij]

I think this is an improvement. It will cause some migration issues though. @dashorst counted 91 cases in our major projects. I think it's worth it, but maybe that's because my project had 0 issues 😁