LOG4J2-2428: run message actions (invoking setters, validations, logging) in a block that routes exception to the defined handler
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/commit/bbeb4564 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/tree/bbeb4564 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/diff/bbeb4564 Branch: refs/heads/master Commit: bbeb456478fd836bcc31d417c68c7a1962491d33 Parents: dc83a07 Author: Andrei Ivanov <[email protected]> Authored: Mon Oct 1 00:38:27 2018 +0300 Committer: Andrei Ivanov <[email protected]> Committed: Mon Oct 1 00:38:27 2018 +0300 ---------------------------------------------------------------------- .../logging/log4j/audit/LogEventFactory.java | 39 +++++++++------ .../logging/log4j/audit/TransferTest.java | 51 ++++++++++++++++---- 2 files changed, 66 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/blob/bbeb4564/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/LogEventFactory.java ---------------------------------------------------------------------- diff --git a/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/LogEventFactory.java b/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/LogEventFactory.java index 9174b67..ad1e874 100644 --- a/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/LogEventFactory.java +++ b/log4j-audit/log4j-audit-api/src/main/java/org/apache/logging/log4j/audit/LogEventFactory.java @@ -71,6 +71,10 @@ public class LogEventFactory { defaultExceptionHandler = (exceptionHandler == null) ? NOOP_EXCEPTION_HANDLER : exceptionHandler; } + static void resetDefaultHandler() { + defaultExceptionHandler = DEFAULT_HANDLER; + } + /** * Constructs an Event object from its interface. * @param intrface The Event interface. @@ -83,8 +87,8 @@ public class LogEventFactory { Class<?>[] interfaces = new Class<?>[] { intrface }; AuditMessage msg = buildAuditMessage(intrface); - AuditEvent audit = (AuditEvent) Proxy.newProxyInstance(intrface - .getClassLoader(), interfaces, new AuditProxy(msg, intrface)); + AuditEvent audit = (AuditEvent) Proxy.newProxyInstance(intrface.getClassLoader(), interfaces, + new AuditProxy(msg, intrface, defaultExceptionHandler)); return (T) audit; } @@ -170,16 +174,20 @@ public class LogEventFactory { * @param handler Class that gets control when an exception occurs logging the event. */ public static void logEvent(AuditMessage msg, AuditExceptionHandler handler) { - try { - AUDIT_LOGGER.logEvent(msg); - } catch (Throwable ex) { - if (handler == null) { - handler = defaultExceptionHandler; - } - handler.handleException(msg, ex); - } + runMessageAction(() -> AUDIT_LOGGER.logEvent(msg), msg, handler); } + private static void runMessageAction(Runnable action, AuditMessage msg, AuditExceptionHandler handler) { + try { + action.run(); + } catch (Throwable ex) { + if (handler == null) { + handler = defaultExceptionHandler; + } + handler.handleException(msg, ex); + } + } + public static List<String> getPropertyNames(String className) { Class<?> intrface = getClass(className); List<String> names; @@ -248,11 +256,12 @@ public class LogEventFactory { private final AuditMessage msg; private final Class<?> intrface; - private AuditExceptionHandler auditExceptionHandler = defaultExceptionHandler; + private AuditExceptionHandler auditExceptionHandler; - AuditProxy(AuditMessage msg, Class<?> intrface) { + AuditProxy(AuditMessage msg, Class<?> intrface, AuditExceptionHandler auditExceptionHandler) { this.msg = msg; this.intrface = intrface; + this.auditExceptionHandler = auditExceptionHandler; } public AuditMessage getMessage() { @@ -260,7 +269,6 @@ public class LogEventFactory { } @Override - @SuppressWarnings("unchecked") public Object invoke(Object o, Method method, Object[] objects) { if (method.getName().equals("toString") && method.getParameterCount() == 0) { return msg.toString(); @@ -268,7 +276,7 @@ public class LogEventFactory { if (method.getName().equals("logEvent")) { - validateEvent(intrface, msg); + runMessageAction(() -> validateEvent(intrface, msg), msg, auditExceptionHandler); logEvent(msg, auditExceptionHandler); return null; @@ -295,13 +303,14 @@ public class LogEventFactory { } if (method.getName().startsWith("set")) { - setProperty(method, objects); + runMessageAction(() -> setProperty(method, objects), msg, auditExceptionHandler); return null; } return null; } + @SuppressWarnings("unchecked") private void setProperty(Method method, Object[] objects) { String name = NamingUtils.lowerFirst(NamingUtils.getMethodShortName(method.getName())); if (objects == null || objects[0] == null) { http://git-wip-us.apache.org/repos/asf/logging-log4j-audit/blob/bbeb4564/log4j-audit/log4j-audit-api/src/test/java/org/apache/logging/log4j/audit/TransferTest.java ---------------------------------------------------------------------- diff --git a/log4j-audit/log4j-audit-api/src/test/java/org/apache/logging/log4j/audit/TransferTest.java b/log4j-audit/log4j-audit-api/src/test/java/org/apache/logging/log4j/audit/TransferTest.java index 7e920cc..c4b2076 100644 --- a/log4j-audit/log4j-audit-api/src/test/java/org/apache/logging/log4j/audit/TransferTest.java +++ b/log4j-audit/log4j-audit-api/src/test/java/org/apache/logging/log4j/audit/TransferTest.java @@ -32,6 +32,7 @@ import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.core.config.AbstractConfiguration; import org.apache.logging.log4j.test.appender.AlwaysFailAppender; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -48,8 +49,33 @@ public class TransferTest extends BaseEventTest { private final String failingAppenderName = "failingAppenderName"; - @Test(expected = ConstraintValidationException.class) + @After + public void cleanup() { + LogEventFactory.resetDefaultHandler(); + } + + @Test + public void testValidationFailureForInvalidRequestContextAttribute() { + MutableBoolean exceptionHandled = new MutableBoolean(false); + LogEventFactory.setDefaultHandler((message, ex) -> { + assertThat(ex, instanceOf(ConstraintValidationException.class)); + exceptionHandled.setTrue(); + }); + + Transfer transfer = LogEventFactory.getEvent(Transfer.class); + transfer.setToAccount(0); + + assertTrue("Should have thrown a ConstraintValidationException", exceptionHandled.isTrue()); + } + + @Test public void testValidationFailureForMissingRequestContextAttribute() { + MutableBoolean exceptionHandled = new MutableBoolean(false); + LogEventFactory.setDefaultHandler((message, ex) -> { + assertThat(ex, instanceOf(ConstraintValidationException.class)); + exceptionHandled.setTrue(); + }); + Transfer transfer = LogEventFactory.getEvent(Transfer.class); ThreadContext.put("companyId", "12345"); ThreadContext.put("ipAddress", "127.0.0.1"); @@ -61,11 +87,18 @@ public class TransferTest extends BaseEventTest { transfer.setFromAccount(111111); transfer.setAmount(new BigDecimal(111.55)); transfer.logEvent(); - fail("Should have thrown an AuditException"); + + assertTrue("Should have thrown a ConstraintValidationException", exceptionHandled.isTrue()); } - @Test(expected = ConstraintValidationException.class) + @Test public void testValidationFailureForMissingEventAttribute() { + MutableBoolean exceptionHandled = new MutableBoolean(false); + LogEventFactory.setDefaultHandler((message, ex) -> { + assertThat(ex, instanceOf(ConstraintValidationException.class)); + exceptionHandled.setTrue(); + }); + Transfer transfer = LogEventFactory.getEvent(Transfer.class); ThreadContext.put("accountNumber", "12345"); ThreadContext.put("companyId", "12345"); @@ -78,7 +111,8 @@ public class TransferTest extends BaseEventTest { transfer.setToAccount(123456); transfer.setFromAccount(111111); transfer.logEvent(); - fail("Should have thrown an AuditException"); + + assertTrue("Should have thrown a ConstraintValidationException", exceptionHandled.isTrue()); } @Test @@ -161,11 +195,10 @@ public class TransferTest extends BaseEventTest { AbstractConfiguration config = setUpFailingAppender(); MutableBoolean exceptionHandled = new MutableBoolean(false); - AuditExceptionHandler exceptionHandler = (message, ex) -> { - assertThat(ex, instanceOf(LoggingException.class)); - exceptionHandled.setTrue(); - }; - LogEventFactory.setDefaultHandler(exceptionHandler); + LogEventFactory.setDefaultHandler((message, ex) -> { + assertThat(ex, instanceOf(LoggingException.class)); + exceptionHandled.setTrue(); + }); Transfer transfer = setUpMinimumEvent(); transfer.logEvent();
