reiern70 opened a new issue, #1614:
URL: https://github.com/apache/wicket/issues/1614
h3. Problem
{{LoadableDetachableModel#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 internal state. The method is overridable, so a subclass can
run cleanup on
either side of {{super.detach()}} — or forget the {{super}} call entirely
and leave the model
attached for good. Nothing in the class enforces the invariant the method
exists to maintain.
Separately, a subclass that needs the loaded object while detaching has to
keep a reference of
its own: {{onDetach()}} takes no arguments and the field holding the object
is private. This is
a common need — releasing a resource, closing an iterator, unregistering a
listener tied to the
object that was loaded.
h3. Proposed change
Make {{detach()}} final and offer two hooks instead:
* {{protected void onDetach()}} — unchanged semantics, invoked only when the
model was attached.
* {{protected void onDetachAlways()}} — new; invoked on every {{detach()}}
call, attached or not.
This is what an override of {{detach()}} was in practice, and is where
cleanup that is 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.
Add {{protected void onDetach(T object)}}, handed the object the model was
holding before it is
discarded. The default {{onDetach()}} delegates to it, so an override of
{{onDetach()}} that does
not call {{super}} suppresses it. The object is {{null}} when the loaded
value was {{null}}, and
also when the model is detached while still attaching (i.e. {{load()}}
threw).
h3. Why two hooks
{{StringResourceModel}} is the case that forces it. Per WICKET-5176 it has
to detach its
substitution models even when the model itself was never attached — it
achieves that today by
overriding {{detach()}} and doing the work outside {{onDetach()}}. An
attached-only hook cannot
reproduce that, hence {{onDetachAlways()}}.
h3. Compatibility and migration
Making a public method final is source-incompatible, so this is for master
(11.x) only; it cannot
be backported. It stays binary compatible — {{detach()}} still resolves, on
{{LoadableDetachableModel}} — so already-compiled subclasses keep running,
but a subclass that
overrides {{detach()}} no longer compiles.
Migration for such a subclass: move the body of the override into one of the
hooks and drop the
{{super.detach()}} call.
* {{onDetachAlways()}} reproduces the old behaviour exactly.
* {{onDetach()}} narrows it to the case where the model was actually
attached.
Which one applies is a judgement per subclass, so this is not automated as
an OpenRewrite recipe;
it is documented as a manual step in the 11.x section of
{{wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml}}, and
belongs in the migration
guide on the wiki.
Three subclasses in the tree overrode {{detach()}}, each to detach something
unconditionally, and
all three move to {{onDetachAlways()}}: {{StringResourceModel}}, its
{{AssignmentWrapper}}, and
{{SessionIdentifiersModel}} in wicket-devutils.
--
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]
