This fix looks useful for 6.x as well. Any particular reason it is not there ?
On Sep 27, 2016 4:43 PM, <[email protected]> wrote: > > Repository: wicket > Updated Branches: > refs/heads/wicket-7.x bdcf92890 -> 6c6f08a30 > > > Detaches LDM after exception during load() > > When an Exception occurs during the `load()` phase of a > LoadableDetachableModel, the state of the LDM is "ATTACHING", but it > can't ever recover from it through detaching. Detaching should always > happen when the model doesn't have a state (state == null) and the > model hasn't been detached prior during the request. (the inverse > happens for attaching). > > Fixes WICKET-6249 > > > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6045149e > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6045149e > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6045149e > > Branch: refs/heads/wicket-7.x > Commit: 6045149e004efd6073e660473b69f8e0f82cae7a > Parents: 796255d > Author: Martijn Dashorst <[email protected]> > Authored: Tue Sep 27 15:41:29 2016 +0200 > Committer: Martijn Dashorst <[email protected]> > Committed: Tue Sep 27 15:42:55 2016 +0200 > > ---------------------------------------------------------------------- > .../wicket/model/LoadableDetachableModel.java | 3 +- > .../model/LoadableDetachableModelTest.java | 45 ++++++++++++++++++-- > 2 files changed, 44 insertions(+), 4 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/wicket/blob/6045149e/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java > ---------------------------------------------------------------------- > 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 75e91bd..a012245 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 > @@ -103,7 +103,8 @@ public abstract class LoadableDetachableModel<T> implements IModel<T> > @Override > public void detach() > { > - if (state == InternalState.ATTACHED) > + // even if LDM is in partial attached state (ATTACHING) it should be detached > + if (state != null && state != InternalState.DETACHED) > { > try > { > > http://git-wip-us.apache.org/repos/asf/wicket/blob/6045149e/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java > ---------------------------------------------------------------------- > diff --git a/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java b/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java > index 0e96509..fe599fe 100644 > --- a/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java > +++ b/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java > @@ -62,6 +62,35 @@ public class LoadableDetachableModelTest extends WicketTestCase > assertThat(ldm.isAttached(), is(true)); > } > > + @Test > + public void onAttachCalled() > + { > + class AttachingLoadableModel extends LoadableDetachableModel<Integer> > + { > + private static final long serialVersionUID = 1L; > + > + private boolean attachCalled = false; > + > + @Override > + protected Integer load() > + { > + return null; > + } > + > + @Override > + protected void onAttach() > + { > + attachCalled = true; > + } > + } > + > + AttachingLoadableModel m = new AttachingLoadableModel(); > + m.getObject(); > + > + assertThat(m.isAttached(), is(true)); > + assertThat(m.attachCalled, is(true)); > + } > + > /** > * Checks whether the LDM can escape recursive calls. > */ > @@ -72,11 +101,19 @@ public class LoadableDetachableModelTest extends WicketTestCase > { > private static final long serialVersionUID = 1L; > > + private boolean detachCalled = false; > + > @Override > protected Integer load() > { > throw new RuntimeException(); > } > + > + @Override > + protected void onDetach() > + { > + detachCalled = true; > + } > } > > ExceptionalLoad ldm = new ExceptionalLoad(); > @@ -89,8 +126,10 @@ public class LoadableDetachableModelTest extends WicketTestCase > } > catch (RuntimeException e) > { > - assertThat(ldm.isAttached(), is(false)); > } > + ldm.detach(); > + assertThat(ldm.isAttached(), is(false)); > + assertThat(ldm.detachCalled, is(true)); > } > > private static class SerializedLoad extends LoadableDetachableModel<Integer> > @@ -131,8 +170,8 @@ public class LoadableDetachableModelTest extends WicketTestCase > > /** Serialization helper */ > @SuppressWarnings("unchecked") > - private <T> LoadableDetachableModel<T> deserialize(byte[] serialized) throws IOException, > - ClassNotFoundException > + private <T> LoadableDetachableModel<T> deserialize(byte[] serialized) > + throws IOException, ClassNotFoundException > { > LoadableDetachableModel<T> deserialized = null; > >
