From 4dfbe093438d8bda9fed71c227416a6ace22c934 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 11 May 2024 08:23:56 +0200 Subject: [PATCH] WW-5414 Always call afterInvocation even in case of exception --- core/pom.xml | 6 ++ .../exec/StrutsBackgroundProcess.java | 20 +++++-- .../exec/StrutsBackgroundProcessTest.java | 60 +++++++++++++++++-- pom.xml | 7 +++ 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 8165d191a5..4027d37e71 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -260,6 +260,12 @@ test + + org.awaitility + awaitility + test + + junit junit 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 42237262f8..b385c9b8fa 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 @@ 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,19 @@ public BackgroundProcess prepare() { 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) { + if (exception == null) { + 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 5906c995a6..a705c2c7b2 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 @@ -26,6 +26,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.NotSerializableException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.security.SecureRandom; @@ -41,6 +42,8 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import static org.awaitility.Awaitility.await; + /** * Test case for BackgroundProcessTest. */ @@ -59,9 +62,9 @@ public void testSerializeDeserialize() throws Exception { 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 +123,31 @@ public void testMultipleProcesses() throws InterruptedException { 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(() -> "done"); + + 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 +175,8 @@ public String invokeActionOnly() throws Exception { } private static class NotSerializableException extends Exception { - private MockHttpServletRequest notSerializableField; + @SuppressWarnings("unused") + private final MockHttpServletRequest notSerializableField; NotSerializableException(MockHttpServletRequest notSerializableField) { this.notSerializableField = notSerializableField; @@ -170,10 +199,29 @@ public void run() { 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 69eea6f951..0f27bd29c3 100644 --- a/pom.xml +++ b/pom.xml @@ -774,6 +774,13 @@ test + + org.awaitility + awaitility + 4.2.1 + test + + javax.servlet javax.servlet-api