This is an automated email from the ASF dual-hosted git repository.
reiern70 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/master by this push:
new f6cfc1dc56 Seal LoadableDetachableModel#detach() and add object-aware
detach hooks
f6cfc1dc56 is described below
commit f6cfc1dc56cb49f8a628e837d97e2a701d675f90
Author: reiern70 <[email protected]>
AuthorDate: Wed Sep 23 09:57:28 2026 -0500
Seal LoadableDetachableModel#detach() and add object-aware detach hooks
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. Its override stays
final, as detach() was, so a subclass cannot quietly drop that cleanup.
GitHub issue #1614: https://github.com/apache/wicket/issues/1614
---
.../wicket/model/LoadableDetachableModelTest.java | 112 +++++++++++++++++++++
.../wicket/model/LoadableDetachableModel.java | 41 +++++++-
.../apache/wicket/model/StringResourceModel.java | 23 +++--
.../pagestore/browser/SessionIdentifiersModel.java | 4 +-
.../src/main/resources/META-INF/rewrite/wicket.yml | 8 ++
5 files changed, 175 insertions(+), 13 deletions(-)
diff --git
a/wicket-core-tests/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
b/wicket-core-tests/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
index 6035a083de..8c23939123 100644
---
a/wicket-core-tests/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
+++
b/wicket-core-tests/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
@@ -18,6 +18,7 @@ package org.apache.wicket.model;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayInputStream;
@@ -134,6 +135,117 @@ class LoadableDetachableModelTest extends WicketTestCase
assertEquals(true, ldm.detachCalled);
}
+ @Test
+ void onDetachReceivesTheAttachedObject()
+ {
+ class DetachingLoad extends LoadableDetachableModel<Integer>
+ {
+ private static final long serialVersionUID = 1L;
+
+ private boolean detachCalled = false;
+
+ private Integer detached;
+
+ @Override
+ protected Integer load()
+ {
+ return 42;
+ }
+
+ @Override
+ protected void onDetach(Integer object)
+ {
+ detachCalled = true;
+ detached = object;
+ }
+ }
+
+ DetachingLoad ldm = new DetachingLoad();
+ assertThat(ldm.getObject()).isEqualTo(42);
+
+ ldm.detach();
+
+ assertEquals(true, ldm.detachCalled);
+ assertThat(ldm.detached).isEqualTo(42);
+ assertEquals(false, ldm.isAttached());
+ }
+
+ @Test
+ void onDetachReceivesNullAfterFailedLoad()
+ {
+ class ExceptionalLoad extends LoadableDetachableModel<Integer>
+ {
+ private static final long serialVersionUID = 1L;
+
+ private boolean detachCalled = false;
+
+ private Integer detached = 42;
+
+ @Override
+ protected Integer load()
+ {
+ throw new RuntimeException();
+ }
+
+ @Override
+ protected void onDetach(Integer object)
+ {
+ detachCalled = true;
+ detached = object;
+ }
+ }
+
+ ExceptionalLoad ldm = new ExceptionalLoad();
+ assertThrows(RuntimeException.class, ldm::getObject);
+
+ ldm.detach();
+
+ assertEquals(true, ldm.detachCalled);
+ assertThat(ldm.detached).isNull();
+ }
+
+ @Test
+ void onDetachAlwaysCalledEvenWhenNeverAttached()
+ {
+ class CountingLoad extends LoadableDetachableModel<Integer>
+ {
+ private static final long serialVersionUID = 1L;
+
+ private int detachCount = 0;
+
+ private int alwaysCount = 0;
+
+ @Override
+ protected Integer load()
+ {
+ return 42;
+ }
+
+ @Override
+ protected void onDetach(Integer object)
+ {
+ detachCount++;
+ }
+
+ @Override
+ protected void onDetachAlways()
+ {
+ alwaysCount++;
+ }
+ }
+
+ CountingLoad ldm = new CountingLoad();
+
+ ldm.detach();
+ assertEquals(0, ldm.detachCount);
+ assertEquals(1, ldm.alwaysCount);
+
+ ldm.getObject();
+ ldm.detach();
+ assertEquals(1, ldm.detachCount);
+ assertEquals(2, ldm.alwaysCount);
+ }
+
private static class SerializedLoad extends
LoadableDetachableModel<Integer>
{
private static final long serialVersionUID = 1L;
diff --git
a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
index 872a202ba2..d36464046e 100644
---
a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
+++
b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
@@ -98,7 +98,7 @@ public abstract class LoadableDetachableModel<T> implements
IModel<T>
}
@Override
- public void detach()
+ public final void detach()
{
// even if LDM is in partial attached state (ATTACHING) it
should be detached
if (state != null && state != InternalState.DETACHED)
@@ -115,6 +115,8 @@ public abstract class LoadableDetachableModel<T> implements
IModel<T>
log.debug("removed transient object for '{}'",
this);
}
}
+
+ onDetachAlways();
}
@Override
@@ -178,8 +180,45 @@ public abstract class LoadableDetachableModel<T>
implements IModel<T>
/**
* Detaches from the current request. Implement this method with custom
behavior, such as
* setting the model object to null.
+ * <p>
+ * This implementation delegates to {@link #onDetach(Object)}, passing
the object this model is
+ * currently holding. An override that does not call {@code
super.onDetach()} suppresses that
+ * callback.
*/
protected void onDetach()
+ {
+ onDetach(transientModelObject);
+ }
+
+ /**
+ * Detaches from the current request, handing over the object this
model is holding before it is
+ * discarded. This is the place to release resources tied to that
object.
+ * <p>
+ * The object is the one returned by the last {@link #load()} or passed
to
+ * {@link #setObject(Object)}. It is {@code null} when that value was
{@code null}, and also when
+ * the model is detached while still attaching - {@link #load()} is
then either running or has
+ * thrown.
+ *
+ * @param object
+ * the object that was attached, may be {@code null}
+ * @since 11.0.0
+ */
+ protected void onDetach(T object)
+ {
+ }
+
+ /**
+ * Detaches from the current request, whether this model was attached
or not. Unlike
+ * {@link #onDetach()} this is invoked on every call to {@link
#detach()}, so it is the place for
+ * cleanup that is not tied to the loaded object - detaching models
this one was handed, for
+ * instance, which may have been attached without this model ever
loading.
+ * <p>
+ * When the model was attached, this runs after {@link #onDetach()} and
after the loaded object
+ * has been discarded.
+ *
+ * @since 11.0.0
+ */
+ protected void onDetachAlways()
{
}
diff --git
a/wicket-core/src/main/java/org/apache/wicket/model/StringResourceModel.java
b/wicket-core/src/main/java/org/apache/wicket/model/StringResourceModel.java
index 2364fa2aa3..8aaeb3f16c 100644
--- a/wicket-core/src/main/java/org/apache/wicket/model/StringResourceModel.java
+++ b/wicket-core/src/main/java/org/apache/wicket/model/StringResourceModel.java
@@ -241,23 +241,26 @@ public class StringResourceModel extends
LoadableDetachableModel<String>
this.component = component;
}
- @Override
- public void detach()
- {
- super.detach();
-
- StringResourceModel.this.detach();
- }
-
@Override
protected void onDetach()
{
+ super.onDetach();
+
if (StringResourceModel.this.component == null)
{
+ // without an explicit component the wrapped
model never attaches itself
StringResourceModel.this.onDetach();
}
}
+ @Override
+ protected void onDetachAlways()
+ {
+ super.onDetachAlways();
+
+ StringResourceModel.this.detach();
+ }
+
@Override
protected String load()
{
@@ -619,9 +622,9 @@ public class StringResourceModel extends
LoadableDetachableModel<String>
}
@Override
- public final void detach()
+ protected final void onDetachAlways()
{
- super.detach();
+ super.onDetachAlways();
// detach any model
if (model != null)
diff --git
a/wicket-devutils/src/main/java/org/apache/wicket/devutils/pagestore/browser/SessionIdentifiersModel.java
b/wicket-devutils/src/main/java/org/apache/wicket/devutils/pagestore/browser/SessionIdentifiersModel.java
index 7cd8b4c189..b7183582ca 100644
---
a/wicket-devutils/src/main/java/org/apache/wicket/devutils/pagestore/browser/SessionIdentifiersModel.java
+++
b/wicket-devutils/src/main/java/org/apache/wicket/devutils/pagestore/browser/SessionIdentifiersModel.java
@@ -63,9 +63,9 @@ public class SessionIdentifiersModel extends
LoadableDetachableModel<List<String
}
@Override
- public void detach()
+ protected void onDetachAlways()
{
- super.detach();
+ super.onDetachAlways();
store.detach();
}
diff --git a/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml
b/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml
index 90b1b3a735..7bd0e14305 100644
--- a/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml
+++ b/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml
@@ -81,6 +81,14 @@ recipeList:
# The ChangeType entries below handle the common case; getRememberMe(),
setRememberMe(),
# onSignInRemembered() and the two-argument SignInPanel constructor have no
equivalent and need
# hand-editing. Both are deprecated in 8.19.0, 9.24.0 and 10.11.0.
+#
+# org.apache.wicket.model.LoadableDetachableModel#detach() is final, so a
subclass can no longer
+# step into the attach/detach state machine. An override of it moves its body
to one of two hooks
+# and drops the super.detach() call: onDetachAlways(), which runs on every
detach() just as the
+# override did, or onDetach(), which runs only when the model was attached.
The new onDetach(T) is
+# handed the object the model was holding before that object is discarded; the
default onDetach()
+# delegates to it, so an override of onDetach() that does not call super
suppresses it. These
+# require manual migration.
type: specs.openrewrite.org/v1beta/recipe
name: org.apache.wicket.MigrateToWicket11
displayName: Migrate to Wicket 11.x
