You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by da...@apache.org on 2016/09/27 13:43:37 UTC
[1/2] wicket git commit: Detaches LDM after exception during load()
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;
[2/2] wicket git commit: Merge branch 'wicket-7.x' of
https://git-wip-us.apache.org/repos/asf/wicket into wicket-7.x
Posted by da...@apache.org.
Merge branch 'wicket-7.x' of https://git-wip-us.apache.org/repos/asf/wicket into wicket-7.x
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6c6f08a3
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6c6f08a3
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6c6f08a3
Branch: refs/heads/wicket-7.x
Commit: 6c6f08a30352bb44b8fe0080340d2049bdf98b22
Parents: 6045149 bdcf928
Author: Martijn Dashorst <da...@apache.org>
Authored: Tue Sep 27 15:43:26 2016 +0200
Committer: Martijn Dashorst <da...@apache.org>
Committed: Tue Sep 27 15:43:26 2016 +0200
----------------------------------------------------------------------
.../java/org/apache/wicket/settings/ApplicationSettings.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
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
Re: [1/2] wicket git commit: Detaches LDM after exception during load()
Posted by Martin Grigorov <mg...@apache.org>.
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 Martin Grigorov <mg...@apache.org>.
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;
>
>