Skip to content

Send general log to SLF4J + cleanup and Junit4 migration, #5198 #5202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 18, 2025
Merged

Send general log to SLF4J + cleanup and Junit4 migration, #5198 #5202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88de2ca
Also send general log to Slf4j, fixes #5198
hansva Apr 17, 2025
dc74056
migrate junit 4 to 5 + sonar cleanup, #5198
hansva Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,15 @@ public class Slf4jLoggingEventListener implements IHopLoggingEventListener {
@VisibleForTesting
Logger pipelineLogger = LoggerFactory.getLogger("org.apache.hop.pipeline.Pipeline");

@VisibleForTesting Logger jobLogger = LoggerFactory.getLogger("org.apache.hop.workflow.Workflow");
@VisibleForTesting
Logger workflowLogger = LoggerFactory.getLogger("org.apache.hop.workflow.Workflow");

@VisibleForTesting Logger diLogger = LoggerFactory.getLogger("org.apache.hop");
@VisibleForTesting Logger hopLogger = LoggerFactory.getLogger("org.apache.hop");

@VisibleForTesting
Function<String, ILoggingObject> logObjProvider =
objId -> LoggingRegistry.getInstance().getLoggingObject(objId);

private static final String SEPARATOR = "/";

public Slf4jLoggingEventListener() {
// Do nothing
}
Expand All @@ -60,14 +59,16 @@ public void eventAdded(HopLoggingEvent event) {
if (loggingObject == null) {
// this can happen if logObject has been discarded while log events are still in flight.
logToLogger(
diLogger, message.getLevel(), message.getSubject() + " " + message.getMessage());
hopLogger, message.getLevel(), message.getSubject() + " " + message.getMessage());
} else if (loggingObject.getObjectType() == PIPELINE
|| loggingObject.getObjectType() == TRANSFORM
|| loggingObject.getObjectType() == DATABASE) {
logToLogger(pipelineLogger, message.getLevel(), loggingObject, message);
} else if (loggingObject.getObjectType() == WORKFLOW
|| loggingObject.getObjectType() == ACTION) {
logToLogger(jobLogger, message.getLevel(), loggingObject, message);
logToLogger(workflowLogger, message.getLevel(), loggingObject, message);
} else {
logToLogger(hopLogger, message.getLevel(), loggingObject, message);
}
}
}
Expand Down Expand Up @@ -107,7 +108,7 @@ private String getDetailedSubject(ILoggingObject loggingObject) {
while (loggingObject != null) {
if (loggingObject.getObjectType() == PIPELINE || loggingObject.getObjectType() == WORKFLOW) {
String filename = loggingObject.getFilename();
if (filename != null && filename.length() > 0) {
if (filename != null && !filename.isEmpty()) {
subjects.add(filename);
}
}
Expand Down
59 changes: 30 additions & 29 deletions core/src/test/java/org/apache/hop/core/logging/Slf4jLoggingEventListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,23 @@

import static org.apache.hop.core.logging.LogLevel.BASIC;
import static org.apache.hop.core.logging.LogLevel.ERROR;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;

import java.util.function.Function;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.junit.jupiter.MockitoExtension;
import org.slf4j.Logger;

@RunWith(MockitoJUnitRunner.class)
public class Slf4jLoggingEventListenerTest {
@ExtendWith(MockitoExtension.class)
class Slf4jLoggingEventListenerTest {

@Mock private Logger pipelineLogger, jobLogger, diLogger;
@Mock private Logger pipelineLogger, workflowLogger, hopLogger;
@Mock private HopLoggingEvent logEvent;
@Mock private ILoggingObject loggingObject;
@Mock private LogMessage message;
Expand All @@ -47,61 +48,61 @@ public class Slf4jLoggingEventListenerTest {

private Slf4jLoggingEventListener listener = new Slf4jLoggingEventListener();

@Before
public void before() {
@BeforeEach
void before() {
listener.pipelineLogger = pipelineLogger;
listener.jobLogger = jobLogger;
listener.diLogger = diLogger;
listener.workflowLogger = workflowLogger;
listener.hopLogger = hopLogger;
listener.logObjProvider = logObjProvider;
when(logEvent.getMessage()).thenReturn(message);
when(message.getLogChannelId()).thenReturn(logChannelId);
when(message.getLevel()).thenReturn(logLevel);
when(message.getMessage()).thenReturn(msgText);
when(message.getSubject()).thenReturn(messageSub);
lenient().when(logEvent.getMessage()).thenReturn(message);
lenient().when(message.getLogChannelId()).thenReturn(logChannelId);
lenient().when(message.getLevel()).thenReturn(logLevel);
lenient().when(message.getMessage()).thenReturn(msgText);
lenient().when(message.getSubject()).thenReturn(messageSub);
}

@Test
public void testAddLogEventNoRegisteredLogObject() {
void testAddLogEventNoRegisteredLogObject() {
listener.eventAdded(logEvent);
verify(diLogger).info(messageSub + " " + msgText);
verify(hopLogger).info(String.format("%s %s", messageSub, msgText));

when(message.getLevel()).thenReturn(ERROR);
listener.eventAdded(logEvent);
verify(diLogger).error(messageSub + " " + msgText);
verify(hopLogger).error(String.format("%s %s", messageSub, msgText));
verifyNoInteractions(pipelineLogger);
verifyNoInteractions(jobLogger);
verifyNoInteractions(workflowLogger);
}

@Test
public void testAddLogEventPipeline() {
void testAddLogEventPipeline() {
when(logObjProvider.apply(logChannelId)).thenReturn(loggingObject);
when(loggingObject.getObjectType()).thenReturn(LoggingObjectType.PIPELINE);
when(loggingObject.getFilename()).thenReturn("filename");
when(message.getLevel()).thenReturn(LogLevel.BASIC);
listener.eventAdded(logEvent);

verify(pipelineLogger).info("[filename] " + msgText);
verify(pipelineLogger).info(String.format("[filename] %s", msgText));
when(message.getLevel()).thenReturn(LogLevel.ERROR);
listener.eventAdded(logEvent);
verify(pipelineLogger).error("[filename] " + msgText);
verifyNoInteractions(diLogger);
verifyNoInteractions(jobLogger);
verify(pipelineLogger).error(String.format("[filename] %s", msgText));
verifyNoInteractions(hopLogger);
verifyNoInteractions(workflowLogger);
}

@Test
public void testAddLogEventJob() {
void testAddLogEventJob() {
when(logObjProvider.apply(logChannelId)).thenReturn(loggingObject);
when(loggingObject.getObjectType()).thenReturn(LoggingObjectType.WORKFLOW);
when(loggingObject.getFilename()).thenReturn("filename");
when(message.getLevel()).thenReturn(LogLevel.BASIC);
listener.eventAdded(logEvent);

verify(jobLogger).info("[filename] " + msgText);
verify(workflowLogger).info(String.format("[filename] %s", msgText));

when(message.getLevel()).thenReturn(LogLevel.ERROR);
listener.eventAdded(logEvent);
verify(jobLogger).error("[filename] " + msgText);
verifyNoInteractions(diLogger);
verify(workflowLogger).error(String.format("[filename] %s", msgText));
verifyNoInteractions(hopLogger);
verifyNoInteractions(pipelineLogger);
}
}
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@
<version>${mockito-core.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>${mockito-core.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.objenesis</groupId>
<artifactId>objenesis</artifactId>
Expand Down