From fe1975f467cf25aea8d44bdf7de3798a48e3d484 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 18 Feb 2011 08:47:31 +0000 Subject: [PATCH] INT-1799: fix index out of bounds in SequenceSizeReleaseStrategy --- .../SequenceSizeReleaseStrategy.java | 30 ++++++++------ .../SequenceSizeReleaseStrategyTests.java | 40 ++++++++++--------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategy.java b/spring-integration-core/src/main/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategy.java index bc4075fc76..b3fea73515 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategy.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategy.java @@ -22,13 +22,14 @@ import org.springframework.integration.Message; import org.springframework.integration.store.MessageGroup; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; /** - * An implementation of {@link ReleaseStrategy} that simply compares the - * current size of the message list to the expected 'sequenceSize'. + * An implementation of {@link ReleaseStrategy} that simply compares the current size of the message list to the + * expected 'sequenceSize'. * * @author Mark Fisher * @author Marius Bogoevici @@ -42,7 +43,7 @@ public class SequenceSizeReleaseStrategy implements ReleaseStrategy { private volatile Comparator> comparator = new SequenceNumberComparator(); private volatile boolean releasePartialSequences; - + public SequenceSizeReleaseStrategy() { this(false); } @@ -63,17 +64,20 @@ public class SequenceSizeReleaseStrategy implements ReleaseStrategy { public boolean canRelease(MessageGroup messages) { if (releasePartialSequences) { - if(logger.isTraceEnabled()){ - logger.trace("Considering partial release of group [" + messages + "]"); + Collection> unmarked = messages.getUnmarked(); + if (!unmarked.isEmpty()) { + if (logger.isTraceEnabled()) { + logger.trace("Considering partial release of group [" + messages + "]"); + } + List> sorted = new ArrayList>(unmarked); + Collections.sort(sorted, comparator); + int tail = sorted.get(0).getHeaders().getSequenceNumber() - 1; + boolean release = tail == messages.getMarked().size(); + if (logger.isTraceEnabled() && release) { + logger.trace("Release imminent because tail [" + tail + "] is next in line."); + } + return release; } - List> sorted = new ArrayList>(messages.getUnmarked()); - Collections.sort(sorted, comparator); - int tail = sorted.get(0).getHeaders().getSequenceNumber() - 1; - boolean release = tail == messages.getMarked().size(); - if (logger.isTraceEnabled() && release) { - logger.trace("Release imminent because tail [" + tail + "] is next in line."); - } - return release; } return messages.isComplete(); } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategyTests.java b/spring-integration-core/src/test/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategyTests.java index 40f0dbba48..3217e47e2b 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategyTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/aggregator/SequenceSizeReleaseStrategyTests.java @@ -16,15 +16,15 @@ package org.springframework.integration.aggregator; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import org.junit.Test; import org.springframework.integration.Message; import org.springframework.integration.store.MessageGroup; import org.springframework.integration.store.SimpleMessageGroup; import org.springframework.integration.support.MessageBuilder; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - /** * @author Mark Fisher * @author Iwein Fuld @@ -33,8 +33,7 @@ public class SequenceSizeReleaseStrategyTests { @Test public void testIncompleteList() { - Message message = MessageBuilder.withPayload("test1") - .setSequenceSize(2).build(); + Message message = MessageBuilder.withPayload("test1").setSequenceSize(2).build(); SimpleMessageGroup messages = new SimpleMessageGroup("FOO"); messages.add(message); SequenceSizeReleaseStrategy releaseStrategy = new SequenceSizeReleaseStrategy(); @@ -43,10 +42,8 @@ public class SequenceSizeReleaseStrategyTests { @Test public void testCompleteList() { - Message message1 = MessageBuilder.withPayload("test1") - .setSequenceSize(2).build(); - Message message2 = MessageBuilder.withPayload("test2") - .setSequenceSize(2).build(); + Message message1 = MessageBuilder.withPayload("test1").setSequenceSize(2).build(); + Message message2 = MessageBuilder.withPayload("test2").setSequenceSize(2).build(); SimpleMessageGroup messages = new SimpleMessageGroup("FOO"); messages.add(message1); messages.add(message2); @@ -60,7 +57,18 @@ public class SequenceSizeReleaseStrategyTests { assertTrue(releaseStrategy.canRelease(new SimpleMessageGroup("FOO"))); } - @Test + @Test + public void testEmptyUnmarked() { + SequenceSizeReleaseStrategy releaseStrategy = new SequenceSizeReleaseStrategy(); + releaseStrategy.setReleasePartialSequences(true); + SimpleMessageGroup messages = new SimpleMessageGroup("FOO"); + Message message = MessageBuilder.withPayload("test1").setSequenceSize(1).build(); + messages.add(message); + messages.mark(message); + assertTrue(releaseStrategy.canRelease(messages)); + } + + @Test public void shouldReleaseHeadOfSequenceDeliveredInOrder() { SequenceSizeReleaseStrategy releaseStrategy = new SequenceSizeReleaseStrategy(); releaseStrategy.setReleasePartialSequences(true); @@ -71,10 +79,8 @@ public class SequenceSizeReleaseStrategyTests { } private SimpleMessageGroup groupWithFirstMessagesOfIncompleteSequence(SimpleMessageGroup messages) { - Message message1 = MessageBuilder.withPayload("test1") - .setSequenceSize(3).setSequenceNumber(1).build(); - Message message2 = MessageBuilder.withPayload("test2") - .setSequenceSize(3).setSequenceNumber(2).build(); + Message message1 = MessageBuilder.withPayload("test1").setSequenceSize(3).setSequenceNumber(1).build(); + Message message2 = MessageBuilder.withPayload("test2").setSequenceSize(3).setSequenceNumber(2).build(); messages.add(message1); messages.add(message2); @@ -94,10 +100,8 @@ public class SequenceSizeReleaseStrategyTests { private MessageGroup groupWithLastAndFirstMessagesOfIncompleteSequence() { SimpleMessageGroup messages = new SimpleMessageGroup("FOO"); - Message message1 = MessageBuilder.withPayload("test1") - .setSequenceSize(3).setSequenceNumber(3).build(); - Message message2 = MessageBuilder.withPayload("test2") - .setSequenceSize(3).setSequenceNumber(1).build(); + Message message1 = MessageBuilder.withPayload("test1").setSequenceSize(3).setSequenceNumber(3).build(); + Message message2 = MessageBuilder.withPayload("test2").setSequenceSize(3).setSequenceNumber(1).build(); messages.add(message1); messages.add(message2);