This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch feature/WW-5414-after
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 7c271f69db8c4df546b89712c1731e380baefc03
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Sat May 11 08:23:56 2024 +0200
WW-5414 Always call afterInvocation even in case of exception
---
core/pom.xml | 6 +++
.../interceptor/exec/StrutsBackgroundProcess.java | 18 +++++--
.../exec/StrutsBackgroundProcessTest.java | 61 +++++++++++++++++++---
pom.xml | 7 +++
4 files changed, 82 insertions(+), 10 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index 8165d191a..4027d37e7 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -260,6 +260,12 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <scope>test</scope>
+ </dependency>
+
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
index 42237262f..0088ebf2e 100644
---
a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
+++
b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
@@ -20,6 +20,8 @@ package org.apache.struts2.interceptor.exec;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.ActionInvocation;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import java.io.Serializable;
@@ -30,6 +32,8 @@ public class StrutsBackgroundProcess implements
BackgroundProcess, Serializable
private static final long serialVersionUID = 3884464776311686443L;
+ private static final Logger LOG =
LogManager.getLogger(StrutsBackgroundProcess.class);
+
private final String threadName;
private final int threadPriority;
@@ -44,8 +48,8 @@ public class StrutsBackgroundProcess implements
BackgroundProcess, Serializable
/**
* Constructs a background process
*
- * @param invocation The action invocation
- * @param threadName The name of background thread
+ * @param invocation The action invocation
+ * @param threadName The name of background thread
* @param threadPriority The priority of background thread
*/
public StrutsBackgroundProcess(ActionInvocation invocation, String
threadName, int threadPriority) {
@@ -61,11 +65,17 @@ public class StrutsBackgroundProcess implements
BackgroundProcess, Serializable
try {
beforeInvocation();
result = invocation.invokeActionOnly();
- afterInvocation();
} catch (Exception e) {
+ LOG.warn("Exception during invokeActionOnly() execution",
e);
exception = e;
} finally {
- done = true;
+ try {
+ afterInvocation();
+ } catch (Exception ex) {
+ exception = ex;
+ LOG.warn("Exception during afterInvocation()
execution", ex);
+ }
+ done = true;
}
});
processThread.setName(threadName);
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
index 5906c995a..a350d3021 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
@@ -41,6 +41,8 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
+import static org.awaitility.Awaitility.await;
+
/**
* Test case for BackgroundProcessTest.
*/
@@ -59,9 +61,9 @@ public class StrutsBackgroundProcessTest extends
StrutsInternalTestCase {
invocation.setInvocationContext(ActionContext.getContext());
StrutsBackgroundProcess bp = (StrutsBackgroundProcess) new
StrutsBackgroundProcess(
- invocation,
- "BackgroundProcessTest.testSerializeDeserialize",
- Thread.MIN_PRIORITY
+ invocation,
+ "BackgroundProcessTest.testSerializeDeserialize",
+ Thread.MIN_PRIORITY
).prepare();
executor.execute(bp);
@@ -120,6 +122,33 @@ public class StrutsBackgroundProcessTest extends
StrutsInternalTestCase {
assertEquals(100, mutableState.get());
}
+ public void testErrorableProcesses1() {
+ MockActionInvocationWithActionInvoker invocation = new
MockActionInvocationWithActionInvoker(() -> {
+ throw new IllegalStateException("boom");
+ });
+
+ BackgroundProcess bp = new ErrorableBackgroundProcess(invocation,
null).prepare();
+ executor.execute(bp);
+
+ await().atLeast(100, TimeUnit.MILLISECONDS).until(bp::isDone);
+
+ assertTrue("afterInvocation not called in case of exception",
((ErrorableBackgroundProcess) bp).isDoneAfter());
+ }
+
+ public void testErrorableProcesses2() {
+ MockActionInvocationWithActionInvoker invocation = new
MockActionInvocationWithActionInvoker(() -> {
+ throw new IllegalArgumentException("boom!");
+ });
+
+ IllegalStateException expected = new IllegalStateException("after!");
+ BackgroundProcess bp = new ErrorableBackgroundProcess(invocation,
expected).prepare();
+ executor.execute(bp);
+
+ await().atLeast(100, TimeUnit.MILLISECONDS).until(bp::isDone);
+
+ assertEquals(expected, bp.getException());
+ }
+
public void testUnpreparedProcess() throws ExecutionException,
InterruptedException, TimeoutException {
// given
MockActionInvocationWithActionInvoker invocation = new
MockActionInvocationWithActionInvoker(() -> "done");
@@ -147,7 +176,8 @@ public class StrutsBackgroundProcessTest extends
StrutsInternalTestCase {
}
private static class NotSerializableException extends Exception {
- private MockHttpServletRequest notSerializableField;
+ @SuppressWarnings("unused")
+ private final MockHttpServletRequest notSerializableField;
NotSerializableException(MockHttpServletRequest notSerializableField) {
this.notSerializableField = notSerializableField;
@@ -170,10 +200,29 @@ class LockBackgroundProcess extends
StrutsBackgroundProcess {
super.run();
}
}
+}
+
+class ErrorableBackgroundProcess extends StrutsBackgroundProcess {
+
+ private final Exception afterException;
+ private boolean doneAfter;
+
+ public ErrorableBackgroundProcess(ActionInvocation invocation, Exception
afterException) {
+ super(invocation, "errorabale process", Thread.NORM_PRIORITY);
+ this.afterException = afterException;
+ }
@Override
protected void afterInvocation() throws Exception {
- super.afterInvocation();
- lock.notify();
+ if (afterException != null) {
+ throw afterException;
+ } else {
+ super.afterInvocation();
+ doneAfter = true;
+ }
+ }
+
+ public boolean isDoneAfter() {
+ return doneAfter;
}
}
diff --git a/pom.xml b/pom.xml
index 69eea6f95..0f27bd29c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -774,6 +774,13 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <version>4.2.1</version>
+ <scope>test</scope>
+ </dependency>
+
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
