You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "reiern70 (via GitHub)" <gi...@apache.org> on 2026/09/23 16:55:48 UTC
[PR] Seal LoadableDetachableModel#detach() and add object-aware detach hooks [wicket]
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Seal LoadableDetachableModel#detach() and add object-aware detach hooks [wicket]
Posted by "reiern70 (via GitHub)" <gi...@apache.org>.
reiern70 commented on PR #1615:
URL: https://github.com/apache/wicket/pull/1615#issuecomment-5890129102
> 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 😁
:-)
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Seal LoadableDetachableModel#detach() and add object-aware detach hooks [wicket]
Posted by "reiern70 (via GitHub)" <gi...@apache.org>.
reiern70 merged PR #1615:
URL: https://github.com/apache/wicket/pull/1615
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Seal LoadableDetachableModel#detach() and add object-aware detach hooks [wicket]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1615:
URL: https://github.com/apache/wicket/pull/1615#issuecomment-5799564798
## [Codecov](https://app.codecov.io/gh/apache/wicket/pull/1615?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `90.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 61.89%. Comparing base ([`6550afd`](https://app.codecov.io/gh/apache/wicket/commit/6550afd842ff15dc807dce8f549f87dbeb4523b1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`40270ea`](https://app.codecov.io/gh/apache/wicket/commit/40270ea898af6409885f650ff7137e45199df754?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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
```
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org