Author: ceki Date: Wed Jul 8 12:14:59 2009 New Revision: 2324 Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/DuplicateMessageFilter.java logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/turbo/DuplicateMessageFilterTest.java Log: Fix LBCLASSIC-134 Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/DuplicateMessageFilter.java ============================================================================== --- logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/DuplicateMessageFilter.java (original) +++ logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/DuplicateMessageFilter.java Wed Jul 8 12:14:59 2009 @@ -55,7 +55,7 @@ @Override public FilterReply decide(Marker marker, Logger logger, Level level, String format, Object[] params, Throwable t) { - int count = msgCache.getMessageCount(format); + int count = msgCache.getMessageCountAndThenIncrement(format); if (count <= allowedRepetitions) { return FilterReply.NEUTRAL; } else { Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java ============================================================================== --- logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java (original) +++ logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java Wed Jul 8 12:14:59 2009 @@ -14,6 +14,8 @@ class LRUMessageCache extends LinkedHashMap<String, Integer> { + // LinkedHashMap permits null elements to be inserted + private static final long serialVersionUID = 1L; final int cacheSize; @@ -26,7 +28,12 @@ this.cacheSize = cacheSize; } - int getMessageCount(String msg) { + int getMessageCountAndThenIncrement(String msg) { + // don't insert null elements + if(msg == null) { + return 0; + } + Integer i = super.get(msg); if(i == null) { i = 0; Modified: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/turbo/DuplicateMessageFilterTest.java ============================================================================== --- logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/turbo/DuplicateMessageFilterTest.java (original) +++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/turbo/DuplicateMessageFilterTest.java Wed Jul 8 12:14:59 2009 @@ -15,49 +15,71 @@ import ch.qos.logback.core.spi.FilterReply; - public class DuplicateMessageFilterTest { - @Test public void smoke() { DuplicateMessageFilter dmf = new DuplicateMessageFilter(); dmf.setAllowedRepetitions(0); dmf.start(); - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "x", null, null)); - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "y", null, null)); - assertEquals(FilterReply.DENY, dmf.decide(null, null, null, "x", null, null)); - assertEquals(FilterReply.DENY, dmf.decide(null, null, null, "y", null, null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "x", null, + null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "y", null, + null)); + assertEquals(FilterReply.DENY, dmf + .decide(null, null, null, "x", null, null)); + assertEquals(FilterReply.DENY, dmf + .decide(null, null, null, "y", null, null)); } - + @Test public void memoryLoss() { DuplicateMessageFilter dmf = new DuplicateMessageFilter(); dmf.setAllowedRepetitions(1); dmf.setCacheSize(1); dmf.start(); - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a", null, null)); - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "b", null, null)); - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a", null, null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a", null, + null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "b", null, + null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a", null, + null)); } - @Test public void many() { DuplicateMessageFilter dmf = new DuplicateMessageFilter(); dmf.setAllowedRepetitions(0); int cacheSize = 10; - int margin = 2; + int margin = 2; dmf.setCacheSize(cacheSize); dmf.start(); - for(int i = 0; i < cacheSize+margin; i++) { - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a"+i, null, null)); + for (int i = 0; i < cacheSize + margin; i++) { + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a" + i, + null, null)); } - for(int i = cacheSize-1; i >= margin; i--) { - assertEquals(FilterReply.DENY, dmf.decide(null, null, null, "a"+i, null, null)); + for (int i = cacheSize - 1; i >= margin; i--) { + assertEquals(FilterReply.DENY, dmf.decide(null, null, null, "a" + i, + null, null)); } - for(int i = margin-1; i >= 0; i--) { - assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a"+i, null, null)); + for (int i = margin - 1; i >= 0; i--) { + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, "a" + i, + null, null)); } } + + @Test + // isXXXEnabled invokes decide with a null format + // http://jira.qos.ch/browse/LBCLASSIC-134 + public void nullFormat() { + DuplicateMessageFilter dmf = new DuplicateMessageFilter(); + dmf.setAllowedRepetitions(0); + dmf.setCacheSize(10); + dmf.start(); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, null, null, + null)); + assertEquals(FilterReply.NEUTRAL, dmf.decide(null, null, null, null, null, + null)); + } + }