Repository: wicket
Updated Branches:
  refs/heads/master 1f801ba7e -> 165faa3f7


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/165faa3f
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/165faa3f
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/165faa3f

Branch: refs/heads/master
Commit: 165faa3f73df4a2e2c13b51b958f05e4d820d3b6
Parents: 1f801ba
Author: Martijn Dashorst <[email protected]>
Authored: Tue Sep 27 15:41:29 2016 +0200
Committer: Martijn Dashorst <[email protected]>
Committed: Tue Sep 27 15:45:26 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/165faa3f/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 032da9f..c08cebc 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,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/165faa3f/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;
 

Reply via email to