Repository: logging-log4j2 Updated Branches: refs/heads/master 10c9e9cf7 -> 798e62514
LOG4J2-2060 AbstractDatabaseManager should make a copy of LogEvents before holding references to them: AsyncLogger log events are mutable Reverting commit that causes Test failures in log4j-taglib Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/798e6251 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/798e6251 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/798e6251 Branch: refs/heads/master Commit: 798e625147ef90514cc99a7d1ffdec1b38167a0a Parents: 10c9e9c Author: rpopma <[email protected]> Authored: Tue Oct 24 00:05:51 2017 +0900 Committer: rpopma <[email protected]> Committed: Tue Oct 24 00:05:51 2017 +0900 ---------------------------------------------------------------------- .../appender/db/AbstractDatabaseManager.java | 2 +- .../db/AbstractDatabaseManagerTest.java | 50 +++++++++++++++----- pom.xml | 2 +- src/changes/changes.xml | 3 ++ 4 files changed, 42 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/798e6251/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java index c36c4d8..b8b9899 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java @@ -162,7 +162,7 @@ public abstract class AbstractDatabaseManager extends AbstractManager implements */ public final synchronized void write(final LogEvent event) { if (this.bufferSize > 0) { - this.buffer.add(event); + this.buffer.add(event.toImmutable()); if (this.buffer.size() >= this.bufferSize || event.isEndOfBatch()) { this.flush(); } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/798e6251/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java index 04d7699..41e7dd8 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java @@ -22,9 +22,7 @@ import org.junit.Test; import static org.junit.Assert.*; import static org.mockito.ArgumentMatchers.same; import static org.mockito.BDDMockito.then; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.*; public class AbstractDatabaseManagerTest { private AbstractDatabaseManager manager; @@ -119,6 +117,16 @@ public class AbstractDatabaseManagerTest { final LogEvent event3 = mock(LogEvent.class); final LogEvent event4 = mock(LogEvent.class); + final LogEvent event1copy = mock(LogEvent.class); + final LogEvent event2copy = mock(LogEvent.class); + final LogEvent event3copy = mock(LogEvent.class); + final LogEvent event4copy = mock(LogEvent.class); + + when(event1.toImmutable()).thenReturn(event1copy); + when(event2.toImmutable()).thenReturn(event2copy); + when(event3.toImmutable()).thenReturn(event3copy); + when(event4.toImmutable()).thenReturn(event4copy); + manager.startup(); then(manager).should().startupInternal(); @@ -128,10 +136,10 @@ public class AbstractDatabaseManagerTest { manager.write(event4); then(manager).should().connectAndStart(); - then(manager).should().writeInternal(same(event1)); - then(manager).should().writeInternal(same(event2)); - then(manager).should().writeInternal(same(event3)); - then(manager).should().writeInternal(same(event4)); + then(manager).should().writeInternal(same(event1copy)); + then(manager).should().writeInternal(same(event2copy)); + then(manager).should().writeInternal(same(event3copy)); + then(manager).should().writeInternal(same(event4copy)); then(manager).should().commitAndClose(); then(manager).shouldHaveNoMoreInteractions(); } @@ -144,6 +152,14 @@ public class AbstractDatabaseManagerTest { final LogEvent event2 = mock(LogEvent.class); final LogEvent event3 = mock(LogEvent.class); + final LogEvent event1copy = mock(LogEvent.class); + final LogEvent event2copy = mock(LogEvent.class); + final LogEvent event3copy = mock(LogEvent.class); + + when(event1.toImmutable()).thenReturn(event1copy); + when(event2.toImmutable()).thenReturn(event2copy); + when(event3.toImmutable()).thenReturn(event3copy); + manager.startup(); then(manager).should().startupInternal(); @@ -153,9 +169,9 @@ public class AbstractDatabaseManagerTest { manager.flush(); then(manager).should().connectAndStart(); - then(manager).should().writeInternal(same(event1)); - then(manager).should().writeInternal(same(event2)); - then(manager).should().writeInternal(same(event3)); + then(manager).should().writeInternal(same(event1copy)); + then(manager).should().writeInternal(same(event2copy)); + then(manager).should().writeInternal(same(event3copy)); then(manager).should().commitAndClose(); then(manager).shouldHaveNoMoreInteractions(); } @@ -168,6 +184,14 @@ public class AbstractDatabaseManagerTest { final LogEvent event2 = mock(LogEvent.class); final LogEvent event3 = mock(LogEvent.class); + final LogEvent event1copy = mock(LogEvent.class); + final LogEvent event2copy = mock(LogEvent.class); + final LogEvent event3copy = mock(LogEvent.class); + + when(event1.toImmutable()).thenReturn(event1copy); + when(event2.toImmutable()).thenReturn(event2copy); + when(event3.toImmutable()).thenReturn(event3copy); + manager.startup(); then(manager).should().startupInternal(); @@ -177,9 +201,9 @@ public class AbstractDatabaseManagerTest { manager.shutdown(); then(manager).should().connectAndStart(); - then(manager).should().writeInternal(same(event1)); - then(manager).should().writeInternal(same(event2)); - then(manager).should().writeInternal(same(event3)); + then(manager).should().writeInternal(same(event1copy)); + then(manager).should().writeInternal(same(event2copy)); + then(manager).should().writeInternal(same(event3copy)); then(manager).should().commitAndClose(); then(manager).should().shutdownInternal(); then(manager).shouldHaveNoMoreInteractions(); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/798e6251/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 2452c7c..d9fdc04 100644 --- a/pom.xml +++ b/pom.xml @@ -179,7 +179,7 @@ <logbackVersion>1.2.3</logbackVersion> <jackson1Version>1.9.13</jackson1Version> <jackson2Version>2.9.1</jackson2Version> - <springVersion>4.3.12.RELEASE</springVersion> + <springVersion>3.2.18.RELEASE</springVersion> <flumeVersion>1.7.0</flumeVersion> <disruptorVersion>3.3.7</disruptorVersion> <conversantDisruptorVersion>1.2.10</conversantDisruptorVersion> <!-- Version 1.2.11 requires Java 8 --> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/798e6251/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 3cb4b35..e86e155 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -31,6 +31,9 @@ - "remove" - Removed --> <release version="2.10.0" date="2017-MM-DD" description="GA Release 2.10.0"> + <action issue="LOG4J2-2060" dev="rpopma" type="fix"> + AbstractDatabaseManager should make a copy of LogEvents before holding references to them: AsyncLogger log events are mutable. + </action> <action issue="LOG4J2-2076" dev="mikes" type="update"> Split up log4j-nosql into one module per appender. </action>
