You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by so...@apache.org on 2018/01/28 05:10:49 UTC

[12/14] wicket git commit: WICKET-6503 reintroduced markRendering

WICKET-6503 reintroduced markRendering

- so rendered page or part checks hierarchy changes while rendering
- stateless pages can be unmarked after beforeRender() in PageAndComponentProvider
- prevent multiple onBeforeRender() calls for auto components that are already added


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/d21bfe32
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/d21bfe32
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/d21bfe32

Branch: refs/heads/WICKET-6498_deferred_javascript_2
Commit: d21bfe32525151a235afb3a6288ea11bcfa15329
Parents: 2e4496e
Author: Sven Meier <sv...@apache.org>
Authored: Sat Jan 27 22:02:46 2018 +0100
Committer: Sven Meier <sv...@apache.org>
Committed: Sat Jan 27 22:47:43 2018 +0100

----------------------------------------------------------------------
 .../main/java/org/apache/wicket/Component.java  | 32 +++++++++
 .../java/org/apache/wicket/MarkupContainer.java | 15 +++++
 .../src/main/java/org/apache/wicket/Page.java   |  2 +
 .../handler/PageAndComponentProvider.java       |  1 +
 .../org/apache/wicket/MarkupContainerTest.java  | 71 +++++++++++++-------
 5 files changed, 97 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/d21bfe32/wicket-core/src/main/java/org/apache/wicket/Component.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java
index fc32b09..c1ae823 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Component.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Component.java
@@ -2117,6 +2117,23 @@ public abstract class Component
 	}
 
 	/**
+	 * THIS METHOD IS NOT PART OF THE WICKET PUBLIC API. DO NOT USE IT!
+	 * 
+	 * Sets the RENDERING flag and removes the PREPARED_FOR_RENDER flag on component and it's
+	 * children.
+	 * 
+	 * @param setRenderingFlag
+	 *            if this is false only the PREPARED_FOR_RENDER flag is removed from component, the
+	 *            RENDERING flag is not set.
+	 * 
+	 * @see #internalPrepareForRender(boolean)
+	 */
+	public final void markRendering(boolean setRenderingFlag)
+	{
+		internalMarkRendering(setRenderingFlag);
+	}
+
+	/**
 	 * Called to indicate that the model content for this component has been changed
 	 */
 	public final void modelChanged()
@@ -2190,6 +2207,8 @@ public abstract class Component
 
 		page.startComponentRender(this);
 
+		markRendering(true);
+
 		render();
 		
 		page.endComponentRender(this);
@@ -3790,6 +3809,7 @@ public abstract class Component
 	 */
 	protected void onBeforeRender()
 	{
+		setRequestFlag(RFLAG_PREPARED_FOR_RENDER, true);
 		onBeforeRenderChildren();
 		setRequestFlag(RFLAG_BEFORE_RENDER_SUPER_CALL_VERIFIED, true);
 	}
@@ -4132,6 +4152,18 @@ public abstract class Component
 	}
 
 	/**
+	 * @param setRenderingFlag
+	 *            rendering flag
+	 */
+	void internalMarkRendering(boolean setRenderingFlag)
+	{
+		// WICKET-5460 no longer prepared for render
+		setRequestFlag(RFLAG_PREPARED_FOR_RENDER, false);
+
+		setRequestFlag(RFLAG_RENDERING, setRenderingFlag);
+	}
+
+	/**
 	 * @return True if this component or any of its parents is in auto-add mode
 	 */
 	public final boolean isAuto()

http://git-wip-us.apache.org/repos/asf/wicket/blob/d21bfe32/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
index aad0aac..038585d 100644
--- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
@@ -1695,6 +1695,21 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp
 	}
 
 	/**
+	 * 
+	 * @see org.apache.wicket.Component#internalMarkRendering(boolean)
+	 */
+	@Override
+	void internalMarkRendering(boolean setRenderingFlag)
+	{
+		super.internalMarkRendering(setRenderingFlag);
+
+		for (Component child : this)
+		{
+			child.internalMarkRendering(setRenderingFlag);
+		}
+	}
+
+	/**
 	 * @return a copy of the children array.
 	 */
 	@SuppressWarnings("unchecked")

http://git-wip-us.apache.org/repos/asf/wicket/blob/d21bfe32/wicket-core/src/main/java/org/apache/wicket/Page.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/Page.java b/wicket-core/src/main/java/org/apache/wicket/Page.java
index 9b8dc7f..ca8a874 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Page.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Page.java
@@ -995,6 +995,8 @@ public abstract class Page extends MarkupContainer
 				delay.release();
 			}
 
+			markRendering(true);
+			
 			render();
 		}
 		finally

http://git-wip-us.apache.org/repos/asf/wicket/blob/d21bfe32/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageAndComponentProvider.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageAndComponentProvider.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageAndComponentProvider.java
index de59ecc..79b8d0d 100644
--- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageAndComponentProvider.java
+++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/PageAndComponentProvider.java
@@ -170,6 +170,7 @@ public class PageAndComponentProvider extends PageProvider implements IPageAndCo
 					Page p = (Page)page;
 					p.internalInitialize();
 					p.beforeRender();
+					p.markRendering(false);
 					component = page.get(componentPath);
 				}
 			}

http://git-wip-us.apache.org/repos/asf/wicket/blob/d21bfe32/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java b/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java
index 4ab69da..8be4fe8 100644
--- a/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java
@@ -25,7 +25,6 @@ import static org.hamcrest.CoreMatchers.sameInstance;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 
 import java.lang.reflect.Field;
-import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
@@ -153,6 +152,43 @@ public class MarkupContainerTest extends WicketTestCase
 		assertEquals(2, page.beforeRenderCalls);
 	}
 
+	@Test
+	public void hierarchyChangeDuringRender()
+	{
+		HierarchyChangePage page = new HierarchyChangePage();
+		try
+		{
+			tester.startPage(page);
+			fail();
+		}
+		catch (WicketRuntimeException expected)
+		{
+			assertEquals(
+				"Cannot modify component hierarchy after render phase has started (page version cant change then anymore)",
+				expected.getMessage());
+		}
+	}
+
+	private static class HierarchyChangePage extends WebPage
+		implements
+			IMarkupResourceStreamProvider
+	{
+
+		@Override
+		protected void onRender()
+		{
+			// change hierarchy during render
+			add(new Label("child"));
+		}
+
+		@Override
+		public IResourceStream getMarkupResourceStream(MarkupContainer container,
+			Class<?> containerClass)
+		{
+			return new StringResourceStream("<html><body></body></html>");
+		}
+	}
+
 	/**
 	 * https://issues.apache.org/jira/browse/WICKET-4012
 	 */
@@ -1274,28 +1310,23 @@ public class MarkupContainerTest extends WicketTestCase
 	public void stream()
 	{
 		LoginPage loginPage = new LoginPage();
-		Optional<Component> first = loginPage.stream()
-			.filter(c -> c.getId().equals("form"))
+		Optional<Component> first = loginPage.stream().filter(c -> c.getId().equals("form"))
 			.findFirst();
 		assertThat(first.isPresent(), is(false));
 
 		loginPage.add(new Form<>("form"));
-		Optional<Component> second = loginPage.stream()
-			.filter(c -> c.getId().equals("form"))
+		Optional<Component> second = loginPage.stream().filter(c -> c.getId().equals("form"))
 			.findFirst();
 		assertThat(second.isPresent(), is(true));
 
 		loginPage.add(new WebMarkupContainer("wmc"));
 
-		Optional<Form> form = loginPage.stream()
-			.filter(Form.class::isInstance)
-			.map(Form.class::cast)
-			.findFirst();
+		Optional<Form> form = loginPage.stream().filter(Form.class::isInstance)
+			.map(Form.class::cast).findFirst();
 		assertThat(form.isPresent(), is(true));
 
 		Optional<WebMarkupContainer> wmc = loginPage.stream()
-			.filter(WebMarkupContainer.class::isInstance)
-			.map(WebMarkupContainer.class::cast)
+			.filter(WebMarkupContainer.class::isInstance).map(WebMarkupContainer.class::cast)
 			.findFirst();
 		assertThat(wmc.isPresent(), is(true));
 	}
@@ -1304,8 +1335,7 @@ public class MarkupContainerTest extends WicketTestCase
 	public void streamChildren()
 	{
 		LoginPage loginPage = new LoginPage();
-		Optional<Component> first = loginPage.stream()
-			.filter(c -> c.getId().equals("form"))
+		Optional<Component> first = loginPage.stream().filter(c -> c.getId().equals("form"))
 			.findFirst();
 		assertThat(first.isPresent(), is(false));
 
@@ -1314,20 +1344,13 @@ public class MarkupContainerTest extends WicketTestCase
 
 		form.add(new TextField<>("field"));
 
-		assertThat(loginPage.streamChildren()
-			.filter(c -> c.getId().equals("form"))
-			.findFirst()
+		assertThat(loginPage.streamChildren().filter(c -> c.getId().equals("form")).findFirst()
 			.isPresent(), is(true));
 
-		assertThat(loginPage.streamChildren()
-			.filter(c -> c.getId().equals("field"))
-			.findFirst()
+		assertThat(loginPage.streamChildren().filter(c -> c.getId().equals("field")).findFirst()
 			.isPresent(), is(true));
 
-		assertThat(loginPage.streamChildren()
-			.filter(TextField.class::isInstance)
-			.filter(c -> c.getId().equals("field"))
-			.findFirst()
-			.isPresent(), is(true));
+		assertThat(loginPage.streamChildren().filter(TextField.class::isInstance)
+			.filter(c -> c.getId().equals("field")).findFirst().isPresent(), is(true));
 	}
 }