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:45:38 UTC

wicket git commit: Detaches LDM after exception during load()

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 <da...@apache.org>
Authored: Tue Sep 27 15:41:29 2016 +0200
Committer: Martijn Dashorst <da...@apache.org>
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;