Skip to content

Commit

Permalink
Merge pull request #744 from uhafner/filtered-log-thread-safe
Browse files Browse the repository at this point in the history 
Make FilteredLog thread-safe
  • Loading branch information
@uhafner
uhafner committed Apr 15, 2023
2 parents 6e506fe + 7f89fb5 commit be9cf4e
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 51 deletions.
143 changes: 111 additions & 32 deletions src/main/java/edu/hm/hafner/util/FilteredLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.locks.ReentrantLock;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;

import com.google.errorprone.annotations.FormatMethod;
Expand All @@ -15,7 +16,8 @@

/**
* Provides a log of info messages and a limited number of error messages. If the number of errors exceeds this limit,
* then subsequent error messages will be skipped.
* then subsequent error messages will be skipped. This class is thread-safe and can be used in a distributed
* environment.
*
* @author Ullrich Hafner
*/
Expand All @@ -31,6 +33,17 @@ public class FilteredLog implements Serializable {
private final List<String> infoMessages = new ArrayList<>();
private final List<String> errorMessages = new ArrayList<>();

private transient ReentrantLock lock = new ReentrantLock();

/**
* Creates a new {@link FilteredLog}. The error messages will not have a pre-defined title, you need to make sure
* that there is a meaningful title before each error message. The maximum number of printed errors is given
* by {@link #DEFAULT_MAX_LINES}.
*/
public FilteredLog() {
this(StringUtils.EMPTY, DEFAULT_MAX_LINES);
}

/**
* Creates a new {@link FilteredLog}. The maximum number of printed errors is given by {@link #DEFAULT_MAX_LINES}.
*
Expand All @@ -54,55 +67,80 @@ public FilteredLog(final String title, final int maxLines) {
this.maxLines = maxLines;
}

/**
* Called after de-serialization to improve the memory usage.
*
* @return this
*/
protected Object readResolve() {
lock = new ReentrantLock();

return this;
}

/**
* Logs the specified information message. Use this method to log any useful information when composing this log.
*
* @param message
* the message to log
*/
public void logInfo(final String message) {
infoMessages.add(message);
lock.lock();
try {
infoMessages.add(message);
}
finally {
lock.unlock();
}
}

/**
* Logs the specified information message. Use this method to log any useful information when composing this log.
*
* @param format
* A <a href="../util/Formatter.html#syntax">format string</a>
* a <a href="../util/Formatter.html#syntax">format string</a>
* @param args
* Arguments referenced by the format specifiers in the format string. If there are more arguments than
* format specifiers, the extra arguments are ignored. The number of arguments is variable and may be
* zero.
*/
@FormatMethod
public void logInfo(final String format, final Object... args) {
infoMessages.add(String.format(format, args));
logInfo(String.format(format, args));
}

/**
* Logs the specified error message. Use this method to log any error when composing this log.
*
* @param message
* the error message
*/
public void logError(final String message) {
lock.lock();
try {
if (lines < maxLines) {
errorMessages.add(message);
}
lines++;
}
finally {
lock.unlock();
}
}

/**
* Logs the specified error message. Use this method to log any error when composing this log.
*
* @param format
* A <a href="../util/Formatter.html#syntax">format string</a>
* a <a href="../util/Formatter.html#syntax">format string</a>
* @param args
* Arguments referenced by the format specifiers in the format string. If there are more arguments than
* format specifiers, the extra arguments are ignored. The number of arguments is variable and may be
* zero.
*/
@FormatMethod
public void logError(final String format, final Object... args) {
printTitle();

if (lines < maxLines) {
errorMessages.add(String.format(format, args));
}
lines++;
}

private void printTitle() {
if (lines == 0) {
errorMessages.add(title);
}
logError(String.format(format, args));
}

/**
Expand All @@ -119,13 +157,17 @@ private void printTitle() {
*/
@FormatMethod
public void logException(final Exception exception, final String format, final Object... args) {
printTitle();

if (lines < maxLines) {
errorMessages.add(String.format(format, args));
errorMessages.addAll(Arrays.asList(ExceptionUtils.getRootCauseStackTrace(exception)));
lock.lock();
try {
if (lines < maxLines) {
errorMessages.add(String.format(format, args));
errorMessages.addAll(Arrays.asList(ExceptionUtils.getRootCauseStackTrace(exception)));
}
lines++;
}
finally {
lock.unlock();
}
lines++;
}

/**
Expand All @@ -140,11 +182,12 @@ public int size() {
/**
* Writes a summary message to the reports' error log that denotes the total number of errors that have been
* reported.
*
* @deprecated not useful anymore
*/
@Deprecated
public void logSummary() {
if (lines > maxLines) {
errorMessages.add(String.format(" ... skipped logging of %d additional errors ...", lines - maxLines));
}
// do nothing
}

/**
Expand All @@ -153,7 +196,13 @@ public void logSummary() {
* @return the info messages
*/
public List<String> getInfoMessages() {
return Collections.unmodifiableList(infoMessages);
lock.lock();
try {
return List.copyOf(infoMessages);
}
finally {
lock.unlock();
}
}

/**
Expand All @@ -162,7 +211,24 @@ public List<String> getInfoMessages() {
* @return the error messages
*/
public List<String> getErrorMessages() {
return Collections.unmodifiableList(errorMessages);
lock.lock();
try {
var messages = new ArrayList<String>();
if (errorMessages.isEmpty()) {
return messages;
}
if (StringUtils.isNotBlank(title)) {
messages.add(title);
}
messages.addAll(errorMessages);
if (lines > maxLines) {
messages.add(String.format(" ... skipped logging of %d additional errors ...", lines - maxLines));
}
return messages;
}
finally {
lock.unlock();
}
}

/**
Expand All @@ -171,7 +237,13 @@ public List<String> getErrorMessages() {
* @return {@code true} if error messages have been recorded, {@code false} otherwise
*/
public boolean hasErrors() {
return !errorMessages.isEmpty();
lock.lock();
try {
return !errorMessages.isEmpty();
}
finally {
lock.unlock();
}
}

/**
Expand All @@ -181,8 +253,15 @@ public boolean hasErrors() {
* the log to merge
*/
public void merge(final FilteredLog other) {
infoMessages.addAll(other.infoMessages);
errorMessages.addAll(other.errorMessages);
lock.lock();
try {
infoMessages.addAll(other.getInfoMessages());
errorMessages.addAll(other.getErrorMessages());
lines += other.lines;
}
finally {
lock.unlock();
}
}

@Override @Generated
Expand Down
69 changes: 50 additions & 19 deletions src/test/java/edu/hm/hafner/util/FilteredLogTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package edu.hm.hafner.util;

import java.util.concurrent.locks.ReentrantLock;

import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.Test;

import nl.jqno.equalsverifier.EqualsVerifier;
import nl.jqno.equalsverifier.Warning;

import static edu.hm.hafner.util.assertions.Assertions.*;

Expand All @@ -18,9 +22,11 @@ class FilteredLogTest extends SerializableTest<FilteredLog> {
void shouldLogNothing() {
var filteredLog = new FilteredLog(TITLE, 5);

assertThat(filteredLog).hasNoErrorMessages();
filteredLog.logSummary();
assertThat(filteredLog).hasNoErrorMessages();
assertThat(filteredLog).hasNoErrorMessages().hasNoInfoMessages();
assertThat(filteredLog.size()).isEqualTo(0);

var empty = new FilteredLog();
assertThat(empty.size()).isEqualTo(0);
}

@Test
Expand All @@ -36,17 +42,28 @@ void shouldLogAllErrors() {
filteredLog.logError("4");
filteredLog.logError("5");

assertThatExactly5MessagesAreLogged(filteredLog);

filteredLog.logSummary();

assertThatExactly5MessagesAreLogged(filteredLog);
assertThat(filteredLog.size()).isEqualTo(5);
}

@Test
void shouldSkipAdditionalErrors() {
var filteredLog = new FilteredLog(TITLE, 5);
FilteredLog filteredLog = create5ErrorsLogWithTitle(StringUtils.EMPTY);

assertThat(filteredLog).hasOnlyErrorMessages("1", "2", "3", "4", "5",
" ... skipped logging of 2 additional errors ...");
}

@Test
void shouldSkipAdditionalErrorsWithTitle() {
FilteredLog filteredLog = create5ErrorsLogWithTitle(TITLE);

assertThat(filteredLog).hasOnlyErrorMessages(TITLE, "1", "2", "3", "4", "5",
" ... skipped logging of 2 additional errors ...");
}

private FilteredLog create5ErrorsLogWithTitle(final String title) {
var filteredLog = new FilteredLog(title, 5);

filteredLog.logError("1");
filteredLog.logError("2");
Expand All @@ -56,13 +73,9 @@ void shouldSkipAdditionalErrors() {
filteredLog.logError("6");
filteredLog.logError("7");

assertThatExactly5MessagesAreLogged(filteredLog);

filteredLog.logSummary();

assertThat(filteredLog).hasOnlyErrorMessages(TITLE, "1", "2", "3", "4", "5",
" ... skipped logging of 2 additional errors ...");
assertThat(filteredLog.size()).isEqualTo(7);

return filteredLog;
}

private void assertThatExactly5MessagesAreLogged(final FilteredLog filteredLog) {
Expand All @@ -82,8 +95,9 @@ void shouldMergeLogger() {

parent.merge(child);

assertThat(parent).hasOnlyInfoMessages("parent Info 1", "child Info 1");
assertThat(parent).hasOnlyErrorMessages("Parent Errors", "parent Error 1", "Child Errors", "child Error 1");
assertThat(parent).hasOnlyInfoMessages("parent Info 1", "child Info 1")
.hasOnlyErrorMessages("Parent Errors", "parent Error 1", "Child Errors", "child Error 1");
assertThat(parent.size()).isEqualTo(2);
}

@Test
Expand All @@ -101,8 +115,20 @@ void shouldLogExceptions() {
void shouldLog20ErrorsByDefault() {
FilteredLog filteredLog = createLogWith20Elements();

assertThat(filteredLog.getErrorMessages()).hasSize(21).contains("error19").doesNotContain("error20");
assertThat(filteredLog.getInfoMessages()).hasSize(25).contains("info0").contains("info24");
assertThat(filteredLog.getErrorMessages()).hasSize(22)
.contains(TITLE)
.contains("error19")
.doesNotContain("error20")
.contains(" ... skipped logging of 5 additional errors ...");
assertThat(filteredLog.getInfoMessages()).hasSize(25)
.contains("info0")
.contains("info24");
}

@Override
protected void assertThatRestoredInstanceEqualsOriginalInstance(final FilteredLog original,
final FilteredLog restored) {
assertThat(original).isEqualTo(restored);
}

private FilteredLog createLogWith20Elements() {
Expand Down Expand Up @@ -133,6 +159,11 @@ void shouldManuallyUseSerializationHelpers() {

@Test
void shouldObeyEqualsContract() {
EqualsVerifier.simple().forClass(FilteredLog.class).verify();
EqualsVerifier.forClass(FilteredLog.class)
.usingGetClass()
.withPrefabValues(ReentrantLock.class, new ReentrantLock(), new ReentrantLock())
.withRedefinedSuperclass()
.suppress(Warning.NONFINAL_FIELDS)
.verify();
}
}

0 comments on commit be9cf4e

Please sign in to comment.