Skip to content
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

Make FilteredLog thread-safe #744

Merged
merged 3 commits into from
Apr 15, 2023
Merged

Make FilteredLog thread-safe #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f22f7f
Make FilteredLog thread-safe.
uhafner Apr 14, 2023
e383502
Return an array list on all paths.
uhafner Apr 14, 2023
7f89fb5
Remove duplication.
uhafner Apr 15, 2023
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
143 changes: 111 additions & 32 deletions src/main/java/edu/hm/hafner/util/FilteredLog.java
View file
Open in desktop
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
Open in desktop
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();
}
}