You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2016/10/03 12:31:35 UTC
Re: [1/2] wicket git commit: Detaches LDM after exception during load()
Answer to myself: the reason is that a previous change that replaces
'attached' boolean with a 'state' enum has not been downported to 6.x.
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Wed, Sep 28, 2016 at 8:35 PM, Martin Grigorov <mg...@apache.org>
wrote:
> This fix looks useful for 6.x as well.
> Any particular reason it is not there ?
>
> On Sep 27, 2016 4:43 PM, <da...@apache.org> 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 <da...@apache.org>
> > Authored: Tue Sep 27 15:41:29 2016 +0200
> > Committer: Martijn Dashorst <da...@apache.org>
> > 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;
> >
> >
>
Re: [1/2] wicket git commit: Detaches LDM after exception during load()
Posted by Martijn Dashorst <ma...@gmail.com>.
On Mon, Oct 3, 2016 at 2:31 PM, Martin Grigorov <mg...@apache.org> wrote:
> Answer to myself: the reason is that a previous change that replaces
> 'attached' boolean with a 'state' enum has not been downported to 6.x.
Yup, that was my conclusion as well :-P
Martijn