From ab03a7c3c1a8b99bae9ea6e61954211f8e30d49c Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 14 Feb 2020 17:46:14 -0800 Subject: [PATCH] Change internally created StringBuffer and StringBuilder StringAppenderWrapper implementations used by the StringAppender from Singletons to Prototypes. Fixes bug in StringAppender that maintained stale Log messages in the buffers of the wrappers between tests. Resolves gh-73. --- .../logging/slf4j/logback/StringAppender.java | 59 +++++++++++++------ .../logback/StringAppenderUnitTests.java | 6 +- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/spring-geode-starter-logging/src/main/java/org/springframework/geode/logging/slf4j/logback/StringAppender.java b/spring-geode-starter-logging/src/main/java/org/springframework/geode/logging/slf4j/logback/StringAppender.java index 0f0d214f..0e2e63b7 100644 --- a/spring-geode-starter-logging/src/main/java/org/springframework/geode/logging/slf4j/logback/StringAppender.java +++ b/spring-geode-starter-logging/src/main/java/org/springframework/geode/logging/slf4j/logback/StringAppender.java @@ -44,27 +44,19 @@ public class StringAppender extends AppenderBase { @FunctionalInterface interface StringAppenderWrapper { + void append(CharSequence charSequence); + + default void clear() { } + } - protected static final StringAppenderWrapper stringBuilderAppenderWrapper = new StringAppenderWrapper() { + protected static class StringBufferAppenderWrapper implements StringAppenderWrapper { - private final StringBuilder stringBuilder = new StringBuilder(); - - @Override - public void append(CharSequence charSequence) { - this.stringBuilder.append(charSequence); - this.stringBuilder.append(NEWLINE); + protected static StringBufferAppenderWrapper create() { + return new StringBufferAppenderWrapper(); } - @Override - public java.lang.String toString() { - return this.stringBuilder.toString(); - } - }; - - protected static final StringAppenderWrapper stringBufferAppenderWrapper = new StringAppenderWrapper() { - private final StringBuffer stringBuffer = new StringBuffer(); @Override @@ -73,11 +65,41 @@ public class StringAppender extends AppenderBase { this.stringBuffer.append(NEWLINE); } + @Override + public void clear() { + this.stringBuffer.delete(0, this.stringBuffer.length()); + } + @Override public java.lang.String toString() { return this.stringBuffer.toString(); } - }; + } + + protected static class StringBuilderAppenderWrapper implements StringAppenderWrapper { + + protected static StringBuilderAppenderWrapper create() { + return new StringBuilderAppenderWrapper(); + } + + private final StringBuilder stringBuilder = new StringBuilder(); + + @Override + public void append(CharSequence charSequence) { + this.stringBuilder.append(charSequence); + this.stringBuilder.append(NEWLINE); + } + + @Override + public void clear() { + this.stringBuilder.delete(0, this.stringBuilder.length()); + } + + @Override + public java.lang.String toString() { + return this.stringBuilder.toString(); + } + } @SuppressWarnings({ "rawtypes", "unchecked", "unused" }) public static class Builder { @@ -148,7 +170,10 @@ public class StringAppender extends AppenderBase { } private StringAppenderWrapper resolveStringAppenderWrapper() { - return this.useSynchronization ? stringBufferAppenderWrapper : stringBuilderAppenderWrapper; + + return this.useSynchronization + ? StringBufferAppenderWrapper.create() + : StringBuilderAppenderWrapper.create(); } public StringAppender build() { diff --git a/spring-geode-starter-logging/src/test/java/org/springframework/geode/logging/slf4j/logback/StringAppenderUnitTests.java b/spring-geode-starter-logging/src/test/java/org/springframework/geode/logging/slf4j/logback/StringAppenderUnitTests.java index b4036c50..5c26fd6c 100644 --- a/spring-geode-starter-logging/src/test/java/org/springframework/geode/logging/slf4j/logback/StringAppenderUnitTests.java +++ b/spring-geode-starter-logging/src/test/java/org/springframework/geode/logging/slf4j/logback/StringAppenderUnitTests.java @@ -74,7 +74,8 @@ public class StringAppenderUnitTests { assertThat(stringAppender.isStarted()).isFalse(); assertThat(stringAppender.getContext()).isEqualTo(LoggerFactory.getILoggerFactory()); assertThat(stringAppender.getName()).isEqualTo(StringAppender.DEFAULT_NAME); - assertThat(stringAppender.getStringAppenderWrapper()).isSameAs(StringAppender.stringBuilderAppenderWrapper); + assertThat(stringAppender.getStringAppenderWrapper()) + .isInstanceOf(StringAppender.StringBuilderAppenderWrapper.class); } @Test @@ -116,7 +117,8 @@ public class StringAppenderUnitTests { assertThat(stringAppender.isStarted()).isTrue(); assertThat(stringAppender.getContext()).isEqualTo(mockContext); assertThat(stringAppender.getName()).isEqualTo("TestStringAppender"); - assertThat(stringAppender.getStringAppenderWrapper()).isEqualTo(StringAppender.stringBufferAppenderWrapper); + assertThat(stringAppender.getStringAppenderWrapper()) + .isInstanceOf(StringAppender.StringBufferAppenderWrapper.class); assertThat(rootLogger.getAppender("TestStringAppender")).isEqualTo(stringAppender); verify(delegate, times(1)).setAppender(isA(CompositeAppender.class));