From d0caad8f4d198234c91b0e45b7623cca1383f7a4 Mon Sep 17 00:00:00 2001 From: Iwein Fuld Date: Fri, 9 Apr 2010 08:47:41 +0000 Subject: [PATCH] INT-1072: Resolved ConcurrentModificationException in MessageHistory and improved equality testing --- .../integration/history/MessageHistory.java | 87 +++++++------------ ...hodInvokingMessageGroupProcessorTests.java | 2 +- .../history/MessageHistoryTests.java | 7 +- .../json/InboundJsonMessageMapperTests.java | 6 +- 4 files changed, 39 insertions(+), 63 deletions(-) diff --git a/org.springframework.integration/src/main/java/org/springframework/integration/history/MessageHistory.java b/org.springframework.integration/src/main/java/org/springframework/integration/history/MessageHistory.java index d5cbc2cf8d..b7eb7e75e7 100644 --- a/org.springframework.integration/src/main/java/org/springframework/integration/history/MessageHistory.java +++ b/org.springframework.integration/src/main/java/org/springframework/integration/history/MessageHistory.java @@ -16,88 +16,61 @@ package org.springframework.integration.history; -import java.io.Serializable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - import org.springframework.integration.support.ComponentMetadata; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + /** - * Iterable list of {@link MessageHistoryEvent} instances. - * + * Threadsafe Iterable list of {@link MessageHistoryEvent} instances. + * * @author Mark Fisher * @author Oleg Zhurakousky + * @author Iwein Fuld * @since 2.0 */ @SuppressWarnings("serial") public class MessageHistory implements Iterable, Serializable { - private final List events = new ArrayList(); - - private final ReadWriteLock lock = new ReentrantReadWriteLock(); - + private final Queue events = new ConcurrentLinkedQueue(); /** * Add a new event with the provided component metadata. */ - public MessageHistoryEvent addEvent(ComponentMetadata metadata) { - if (metadata != null && metadata.getComponentName() != null) { - try { - this.lock.writeLock().lock(); - MessageHistoryEvent event = new MessageHistoryEvent(metadata); - this.events.add(event); - return event; - } - finally { - this.lock.writeLock().unlock(); - } - } - return null; - } + public MessageHistoryEvent addEvent(ComponentMetadata metadata) { + if (metadata != null && metadata.getComponentName() != null) { + MessageHistoryEvent event = new MessageHistoryEvent(metadata); + this.events.add(event); + return event; + } + return null; + } - /** - * Returns an iterator for an unmodifiable list of the history events. - */ - public Iterator iterator() { - try { - this.lock.readLock().lock(); - return Collections.unmodifiableList(this.events).iterator(); - } - finally { - this.lock.readLock().unlock(); - } - } + /** + * Returns a weakly consistent iterator that will never throw + * ConcurrentModificationException as in {@link java.util.concurrent.ConcurrentLinkedQueue#iterator()}. + */ + public Iterator iterator() { + return this.events.iterator(); + } public boolean equals(Object other) { return (other instanceof MessageHistory - && this.events.equals(((MessageHistory) other).events)); + && this.events.containsAll(((MessageHistory) other).events)) + && ((MessageHistory) other).events.containsAll(this.events); } public int hashCode() { - - try { - this.lock.readLock().lock(); - return 17 * this.events.hashCode(); - } - finally { - this.lock.readLock().unlock(); - } + return 17 * this.events.hashCode(); } /** * Returns a String representation of the history event list. */ public String toString() { - try { - this.lock.readLock().lock(); - return this.events.toString(); - } - finally { - this.lock.readLock().unlock(); - } - } + return new ArrayList(events).toString(); + } } diff --git a/org.springframework.integration/src/test/java/org/springframework/integration/aggregator/MethodInvokingMessageGroupProcessorTests.java b/org.springframework.integration/src/test/java/org/springframework/integration/aggregator/MethodInvokingMessageGroupProcessorTests.java index 263b47e907..ca1fd1ce75 100644 --- a/org.springframework.integration/src/test/java/org/springframework/integration/aggregator/MethodInvokingMessageGroupProcessorTests.java +++ b/org.springframework.integration/src/test/java/org/springframework/integration/aggregator/MethodInvokingMessageGroupProcessorTests.java @@ -60,7 +60,7 @@ public class MethodInvokingMessageGroupProcessorTests { return result; } - public String know() { + public String know(List flags) { return "I'm not the one "; } } diff --git a/org.springframework.integration/src/test/java/org/springframework/integration/history/MessageHistoryTests.java b/org.springframework.integration/src/test/java/org/springframework/integration/history/MessageHistoryTests.java index 065b76173d..fd46e6b296 100644 --- a/org.springframework.integration/src/test/java/org/springframework/integration/history/MessageHistoryTests.java +++ b/org.springframework.integration/src/test/java/org/springframework/integration/history/MessageHistoryTests.java @@ -15,13 +15,13 @@ */ package org.springframework.integration.history; +import org.junit.Test; +import org.springframework.integration.support.ComponentMetadata; + import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import org.junit.Test; -import org.springframework.integration.support.ComponentMetadata; - /** * @author Oleg Zhurakousky * @since 2.0 @@ -29,6 +29,7 @@ import org.springframework.integration.support.ComponentMetadata; public class MessageHistoryTests { private long times = 1000; private ExecutorService executor = Executors.newCachedThreadPool(); + @Test public void testConcurrentModificationsOnObjectMethods() throws Exception{ final MessageHistory history = new MessageHistory(); diff --git a/org.springframework.integration/src/test/java/org/springframework/integration/json/InboundJsonMessageMapperTests.java b/org.springframework.integration/src/test/java/org/springframework/integration/json/InboundJsonMessageMapperTests.java index 54b344b236..1bafdbc09e 100644 --- a/org.springframework.integration/src/test/java/org/springframework/integration/json/InboundJsonMessageMapperTests.java +++ b/org.springframework.integration/src/test/java/org/springframework/integration/json/InboundJsonMessageMapperTests.java @@ -16,7 +16,9 @@ package org.springframework.integration.json; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import java.io.IOException; @@ -50,8 +52,8 @@ public class InboundJsonMessageMapperTests { String jsonMessage = "{\"headers\":{\"$timestamp\":1,\"$id\":\"" + id + "\"},\"payload\":\"myPayloadStuff\"}"; Message expected = MessageBuilder.withPayload("myPayloadStuff").setHeader(MessageHeaders.TIMESTAMP, new Long(1)).setHeader(MessageHeaders.ID, id).build(); InboundJsonMessageMapper mapper = new InboundJsonMessageMapper(String.class); - Message result = mapper.toMessage(jsonMessage); - assertEquals(expected, result); + Message result = (Message) mapper.toMessage(jsonMessage); + assertThat(result, is(expected)); } @Test