reiern70 opened a new pull request, #1615:
URL: https://github.com/apache/wicket/pull/1615
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: https://github.com/apache/wicket/issues/1614
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
