You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2020/11/14 23:36:24 UTC
[wicket] branch wicket-8.x updated: WICKET-6847 WICKET-6848 flush
before detach fixes
This is an automated email from the ASF dual-hosted git repository.
svenmeier pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/wicket-8.x by this push:
new a1c9476 WICKET-6847 WICKET-6848 flush before detach fixes
a1c9476 is described below
commit a1c94760fdaf5769211c47edfd1a4ac6077e998f
Author: Sven Meier <sv...@apache.org>
AuthorDate: Mon Nov 9 21:48:30 2020 +0100
WICKET-6847 WICKET-6848 flush before detach fixes
---
pom.xml | 7 ++++
.../main/java/org/apache/wicket/Application.java | 6 +++
.../src/main/java/org/apache/wicket/Session.java | 40 ++++++++----------
.../apache/wicket/page/AbstractPageManager.java | 5 +++
.../java/org/apache/wicket/page/IPageManager.java | 6 +++
.../apache/wicket/page/PageManagerDecorator.java | 5 +++
.../org/apache/wicket/page/RequestAdapter.java | 48 +++++++++++++++++-----
.../apache/wicket/protocol/http/WicketFilter.java | 8 +---
.../request/cycle/IRequestCycleListener.java | 2 +-
.../apache/wicket/request/cycle/RequestCycle.java | 24 ++++++-----
.../wicket/protocol/http/SessionDestroyTest.java | 14 +++----
11 files changed, 105 insertions(+), 60 deletions(-)
diff --git a/pom.xml b/pom.xml
index 1703d46..bad7ad6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1054,6 +1054,13 @@
<comparisonVersion>8.0.0</comparisonVersion>
<failOnError>true</failOnError>
<logResults>true</logResults>
+ <ignored>
+ <difference>
+ <differenceType>7012</differenceType>
+ <className>org/apache/wicket/page/IPageManager</className>
+ <method>void endRequest()</method>
+ </difference>
+ </ignored>
</configuration>
<executions>
<execution>
diff --git a/wicket-core/src/main/java/org/apache/wicket/Application.java b/wicket-core/src/main/java/org/apache/wicket/Application.java
index 5178f3b..ef6601a 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Application.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Application.java
@@ -1599,6 +1599,12 @@ public abstract class Application implements UnboundListener, IEventSink
requestCycle.getListeners().add(new IRequestCycleListener()
{
@Override
+ public void onEndRequest(RequestCycle cycle)
+ {
+ internalGetPageManager().endRequest();
+ }
+
+ @Override
public void onDetach(final RequestCycle requestCycle)
{
if (Session.exists())
diff --git a/wicket-core/src/main/java/org/apache/wicket/Session.java b/wicket-core/src/main/java/org/apache/wicket/Session.java
index 066c2cb..6316f85 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Session.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Session.java
@@ -501,6 +501,7 @@ public abstract class Session implements IClusterable, IEventSink, IFeedbackCont
{
sessionStore.invalidate(RequestCycle.get().getRequest());
sessionStore = null;
+
id = null;
RequestCycle.get().setMetaData(SESSION_INVALIDATED, false);
clientInfo = null;
@@ -520,6 +521,9 @@ public abstract class Session implements IClusterable, IEventSink, IFeedbackCont
invalidate();
}
+ // clear all pages possibly pending in the request
+ getPageManager().clear();
+
destroy();
feedbackMessages.clear();
setStyle(null);
@@ -551,24 +555,7 @@ public abstract class Session implements IClusterable, IEventSink, IFeedbackCont
*/
public final boolean isSessionInvalidated()
{
- RequestCycle requestCycle = RequestCycle.get();
- return isSessionInvalidated(requestCycle);
- }
-
- /**
- * Whether the session is invalid now, or will be invalidated by the end of the request. Clients
- * should rarely need to use this method if ever.
- *
- * @param requestCycle
- * The current request cycle
- * @return Whether the session is invalid when the current request is done
- *
- * @see #invalidate()
- * @see #invalidateNow()
- */
- public static boolean isSessionInvalidated(RequestCycle requestCycle)
- {
- return Boolean.TRUE.equals(requestCycle.getMetaData(SESSION_INVALIDATED));
+ return Boolean.TRUE.equals(RequestCycle.get().getMetaData(SESSION_INVALIDATED));
}
/**
@@ -679,13 +666,9 @@ public abstract class Session implements IClusterable, IEventSink, IFeedbackCont
}
/**
- * Any detach logic for session subclasses. This is called on the end of handling a request,
- * when the RequestCycle is about to be detached from the current thread.
+ * End the current request.
*/
- public void detach()
- {
- detachFeedback();
-
+ public void endRequest() {
if (isSessionInvalidated())
{
invalidateNow();
@@ -697,6 +680,15 @@ public abstract class Session implements IClusterable, IEventSink, IFeedbackCont
}
}
+ /**
+ * Any detach logic for session subclasses. This is called on the end of handling a request,
+ * when the RequestCycle is about to be detached from the current thread.
+ */
+ public void detach()
+ {
+ detachFeedback();
+ }
+
private void detachFeedback()
{
final int removed = feedbackMessages.clear(getApplication().getApplicationSettings()
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/AbstractPageManager.java b/wicket-core/src/main/java/org/apache/wicket/page/AbstractPageManager.java
index 03b26aa..bc8ee52 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/AbstractPageManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/AbstractPageManager.java
@@ -71,6 +71,11 @@ public abstract class AbstractPageManager implements IPageManager
}
@Override
+ public void endRequest() {
+ getRequestAdapter().endRequest();
+ }
+
+ @Override
public void commitRequest()
{
getRequestAdapter().commitRequest();
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java b/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
index a19a0e3..e12130e 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/IPageManager.java
@@ -81,6 +81,12 @@ public interface IPageManager
boolean supportsVersioning();
/**
+ * End the current request.
+ */
+ default void endRequest() {
+ }
+
+ /**
* Commits the changes to external storage if the manager uses it.
*
* Should also detach all pages that were touched during this request.
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageManagerDecorator.java b/wicket-core/src/main/java/org/apache/wicket/page/PageManagerDecorator.java
index 8f719f8..efeb739 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/PageManagerDecorator.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/PageManagerDecorator.java
@@ -74,6 +74,11 @@ public class PageManagerDecorator implements IPageManager
}
@Override
+ public void endRequest() {
+ delegate.endRequest();
+ }
+
+ @Override
public void commitRequest()
{
delegate.commitRequest();
diff --git a/wicket-core/src/main/java/org/apache/wicket/page/RequestAdapter.java b/wicket-core/src/main/java/org/apache/wicket/page/RequestAdapter.java
index 3894e23..4a1464c 100644
--- a/wicket-core/src/main/java/org/apache/wicket/page/RequestAdapter.java
+++ b/wicket-core/src/main/java/org/apache/wicket/page/RequestAdapter.java
@@ -172,26 +172,38 @@ public abstract class RequestAdapter
/**
*
*/
- protected void commitRequest()
+ protected void endRequest()
{
- // store pages that are not stateless
+ // check for pages that are not stateless
if (touchedPages.isEmpty() == false)
{
List<IManageablePage> statefulPages = new ArrayList<IManageablePage>(
touchedPages.size());
for (IManageablePage page : touchedPages)
{
- boolean isPageStateless;
- try
- {
- isPageStateless = page.isPageStateless();
- }
- catch (Exception x)
+ if (isPageStateless(page) == false)
{
- log.warn("An error occurred while checking whether a page is stateless. Assuming it is stateful.", x);
- isPageStateless = false;
+ // last opportunity to create a session
+ bind();
+ break;
}
- if (isPageStateless == false)
+ }
+ }
+ }
+
+ /**
+ *
+ */
+ protected void commitRequest()
+ {
+ // store pages that are not stateless
+ if (touchedPages.isEmpty() == false)
+ {
+ List<IManageablePage> statefulPages = new ArrayList<IManageablePage>(
+ touchedPages.size());
+ for (IManageablePage page : touchedPages)
+ {
+ if (isPageStateless(page) == false)
{
statefulPages.add(page);
}
@@ -204,6 +216,20 @@ public abstract class RequestAdapter
}
}
+ private boolean isPageStateless(final IManageablePage page) {
+ boolean isPageStateless;
+ try
+ {
+ isPageStateless = page.isPageStateless();
+ }
+ catch (Exception x)
+ {
+ log.warn("An error occurred while checking whether a page is stateless. Assuming it is stateful.", x);
+ isPageStateless = false;
+ }
+ return isPageStateless;
+ }
+
public void clear() {
touchedPages.clear();
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java
index 5eb4fc6..ccacc37 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java
@@ -271,13 +271,11 @@ public class WicketFilter implements Filter
final FilterChain chain) throws IOException, ServletException
{
boolean reqProcessed;
- boolean respFlushed = false;
try
{
reqProcessed = requestCycle.processRequest();
- if (reqProcessed && !Session.isSessionInvalidated(requestCycle))
+ if (reqProcessed)
{
- respFlushed = true;
webResponse.flush();
}
}
@@ -294,10 +292,6 @@ public class WicketFilter implements Filter
chain.doFilter(httpServletRequest, httpServletResponse);
}
}
- else if (!respFlushed)
- {
- webResponse.flush();
- }
return reqProcessed;
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/request/cycle/IRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/request/cycle/IRequestCycleListener.java
index 43ee75d..563d7e4 100644
--- a/wicket-core/src/main/java/org/apache/wicket/request/cycle/IRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/request/cycle/IRequestCycleListener.java
@@ -107,7 +107,7 @@ public interface IRequestCycleListener
{}
/**
- * Called when the request cycle object has finished its response
+ * Called when the request cycle object has finished before flushing the response
*
* @param cycle
*/
diff --git a/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java b/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java
index 3823f59..19a8112 100644
--- a/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java
+++ b/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java
@@ -260,6 +260,16 @@ public class RequestCycle implements IRequestCycle, IEventSink
}
finally
{
+ try
+ {
+ listeners.onEndRequest(this);
+ onEndRequest();
+ }
+ catch (RuntimeException e)
+ {
+ log.error("Exception occurred during onEndRequest", e);
+ }
+
set(null);
}
@@ -645,16 +655,6 @@ public class RequestCycle implements IRequestCycle, IEventSink
{
try
{
- onEndRequest();
- listeners.onEndRequest(this);
- }
- catch (RuntimeException e)
- {
- log.error("Exception occurred during onEndRequest", e);
- }
-
- try
- {
requestHandlerExecutor.detach();
}
catch (RuntimeException exception)
@@ -782,6 +782,10 @@ public class RequestCycle implements IRequestCycle, IEventSink
*/
protected void onEndRequest()
{
+ if (Session.exists())
+ {
+ Session.get().endRequest();
+ }
}
/**
diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/SessionDestroyTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/SessionDestroyTest.java
index 51ba825..7ef723f 100644
--- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/SessionDestroyTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/SessionDestroyTest.java
@@ -56,21 +56,21 @@ public class SessionDestroyTest extends WicketTestCase
// schedule invalidation
session.invalidate();
- // the invalidation will happen on #detach(), so #destroy() is still not called
- verify(session, never()).invalidateNow();
+ // the invalidation will happen on #end(), so #destroy() is still not called
+ verify(session, never()).endRequest();
assertThat(session.isSessionInvalidated(), is(true));
- session.detach();
+ session.endRequest();
// the session has been detached so #destroy() has been called and 'sessionInvalidated' is reset
- verify(session, times(1)).invalidateNow();
+ verify(session, times(1)).endRequest();
assertThat(session.isSessionInvalidated(), is(false));
// no matter how many times #detach() is called #destroy() should not be called
- session.detach();
+ session.endRequest();
verify(session, times(1)).invalidateNow();
- session.detach();
- session.detach();
+ session.endRequest();
+ session.endRequest();
verify(session, times(1)).invalidateNow();
assertThat(session.isSessionInvalidated(), is(false));