NIFI-869 Fixed SimpleProcessLogger to log correct messages Ensured that SimpleProcessLogger correctly interprets Throwable as discussed in JIRA Added tests

Signed-off-by: Mark Payne <markap14@hotmail.com>

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/scheduling/StandardProcessScheduler.java

@@ -655,7 +655,7 @@ public final class StandardProcessScheduler implements ProcessScheduler {
                             final Throwable cause = e instanceof InvocationTargetException ? e.getCause() : e;
 
                             final ComponentLog componentLog = new SimpleProcessLogger(service.getIdentifier(), service);
-                            componentLog.error("failed to invoke @OnEnabled method due to {}", new Object[]{cause.toString()});
+                            componentLog.error("Failed to invoke @OnEnabled method due to {}", cause);
                             LOG.error("Failed to invoke @OnEnabled method of {} due to {}", service.getControllerServiceImplementation(), cause.toString());
                             if (LOG.isDebugEnabled()) {
                                 LOG.error("", cause);

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/processor/SimpleProcessLogger.java

@@ -62,8 +62,12 @@ public class SimpleProcessLogger implements ProcessorLog {
     }
 
     @Override
-    public void warn(final String msg, final Throwable t) {
+    public void warn(String msg, final Throwable t) {
-        warn("{} " + msg, new Object[]{component}, t);
+        //warn("{} " + msg, new Object[]{component}, t);
+        msg = "{} " + msg;
+        final Object[] os = {component, t.toString(), t};
+        logger.warn(msg, os);
+        logRepository.addLogMessage(LogLevel.WARN, msg, os, t);
     }
 
     @Override
@@ -101,8 +105,8 @@ public class SimpleProcessLogger implements ProcessorLog {
     @Override
     public void trace(String msg, Throwable t) {
         msg = "{} " + msg;
-        final Object[] os = {component};
+        final Object[] os = {component, t.toString(), t};
-        logger.trace(msg, os, t);
+        logger.trace(msg, os);
         logRepository.addLogMessage(LogLevel.TRACE, msg, os, t);
     }
 
@@ -160,7 +164,7 @@ public class SimpleProcessLogger implements ProcessorLog {
     @Override
     public void info(String msg, Throwable t) {
         msg = "{} " + msg;
-        final Object[] os = {component};
+        final Object[] os = {component, t.toString()};
 
         logger.info(msg, os);
         if (logger.isDebugEnabled()) {
@@ -207,12 +211,12 @@ public class SimpleProcessLogger implements ProcessorLog {
     @Override
     public void error(String msg, Throwable t) {
         msg = "{} " + msg;
-        final Object[] os = {component};
+        Object[] os = {component, t.toString()};
-
+        logger.error(msg, os);
-        logger.error(msg, os, t);
         if (logger.isDebugEnabled()) {
             logger.error("", t);
         }
+
         logRepository.addLogMessage(LogLevel.ERROR, msg, os, t);
     }
 

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/processor/TestSimpleProcessLogger.java

@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processor;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.argThat;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.times;
+
+import java.lang.reflect.Field;
+
+import org.apache.nifi.reporting.ReportingTask;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentMatcher;
+import org.mockito.internal.matchers.VarargMatcher;
+import org.slf4j.Logger;
+
+public class TestSimpleProcessLogger {
+	private final Exception e = new RuntimeException("intentional");
+
+	private  ReportingTask task;
+
+	private SimpleProcessLogger componentLog;
+
+	private Logger logger;
+
+	@Before
+	public void before(){
+		task = mock(ReportingTask.class);
+		when(task.getIdentifier()).thenReturn("foo");
+		when(task.toString()).thenReturn("MyTask");
+		componentLog = new SimpleProcessLogger(task.getIdentifier(), task);
+		try {
+			Field loggerField = componentLog.getClass().getDeclaredField("logger");
+			loggerField.setAccessible(true);
+			logger = mock(Logger.class);
+			loggerField.set(componentLog, logger);
+		} catch (Exception e) {
+			e.printStackTrace();
+			fail(e.getMessage());
+		}
+	}
+
+	@Test
+	public void validateDelegateLoggerReceivesThrowableToStringOnError() {
+		componentLog.error("Hello {}", e);
+		verify(logger, times(1)).error(anyString(), argThat(new MyVarargMatcher()));
+	}
+
+	@Test
+	public void validateDelegateLoggerReceivesThrowableToStringOnInfo() {
+		componentLog.info("Hello {}", e);
+		verify(logger, times(1)).info(anyString(), argThat(new MyVarargMatcher()));
+	}
+
+	@Test
+	public void validateDelegateLoggerReceivesThrowableToStringOnTrace() {
+		componentLog.trace("Hello {}", e);
+		verify(logger, times(1)).trace(anyString(), argThat(new MyVarargMatcher()));
+	}
+
+	@Test
+	public void validateDelegateLoggerReceivesThrowableToStringOnWarn() {
+		componentLog.warn("Hello {}", e);
+		verify(logger, times(1)).warn(anyString(), argThat(new MyVarargMatcher()));
+	}
+
+	/**
+	 *
+	 */
+	private class MyVarargMatcher extends ArgumentMatcher<Object[]> implements VarargMatcher {
+		private static final long serialVersionUID = 1L;
+		@Override
+		public boolean matches(Object argument) {
+			Object[] args = (Object[]) argument;
+			assertEquals(task, args[0]);
+			assertEquals(e.toString(), args[1]);
+			return true;
+		}
+	}
+}