[mule-dev] Re: [mule-scm] [mule][24750] branches/mule-3.3.x: MULE-6399: MessageCollection elements are not modifiable when the elements were created in another thread

[Pablo Kraan]

Is really needed to add the boolean deepMessageCopy parameter?

I understand that using deepMessageCopy=false will perform better than deepMessageCopy=true.

But can't be that a source of other bugs similar to the one you fixed?

Pablo

On Thu, Aug 16, 2012 at 2:47 PM, <sva...@codehaus.org> wrote:

Revision

24750

Author

svacas

Date

2012-08-16 12:47:24 -0500 (Thu, 16 Aug 2012)

Log Message

MULE-6399: MessageCollection elements are not modifiable when the elements were created in another thread

Modified Paths

• branches/mule-3.3.x/core/src/main/java/org/mule/DefaultMessageCollection.java
• branches/mule-3.3.x/tests/integration/src/test/java/org/mule/test/routing/ForeachTestCase.java
• branches/mule-3.3.x/tests/integration/src/test/resources/foreach-test.xml

Diff

Modified: branches/mule-3.3.x/core/src/main/java/org/mule/DefaultMessageCollection.java (24749 => 24750)

--- branches/mule-3.3.x/core/src/main/java/org/mule/DefaultMessageCollection.java	2012-08-15 22:13:17 UTC (rev 24749)
+++ branches/mule-3.3.x/core/src/main/java/org/mule/DefaultMessageCollection.java	2012-08-16 17:47:24 UTC (rev 24750)
@@ -56,6 +56,17 @@
      */
     public DefaultMessageCollection(DefaultMessageCollection msg, MuleContext muleContext)
     {
+        this(msg, muleContext, false);
+    }
+
+    /**
+     * Performs a shallow or deep copy of the messages
+     * @param msg
+     * @param muleContext
+     * @param deepMessageCopy
+     */
+    public DefaultMessageCollection(DefaultMessageCollection msg, MuleContext muleContext, boolean deepMessageCopy)
+    {
         this(muleContext);
         setUniqueId(msg.getUniqueId());
         setMessageRootId(msg.getMessageRootId());
@@ -65,7 +76,22 @@
         {
             for (int i = 0; i < msg.getMessagesAsArray().length; i++)
             {
-                addMessage(msg.getMessagesAsArray()[i]);
+                MuleMessage currentMsg = msg.getMessagesAsArray()[i];
+                if (deepMessageCopy)
+                {
+                    if (currentMsg instanceof MuleMessageCollection)
+                    {
+                        addMessage(new DefaultMessageCollection((DefaultMessageCollection) currentMsg, muleContext, true));
+                    }
+                    else
+                    {
+                        addMessage(new DefaultMuleMessage(currentMsg, muleContext));
+                    }
+                }
+                else
+                {
+                    addMessage(currentMsg);
+                }
             }
         }
         else
@@ -284,7 +310,7 @@
     public ThreadSafeAccess newThreadCopy()
     {
         checkValidPayload();
-        return new DefaultMessageCollection(this, muleContext);
+        return new DefaultMessageCollection(this, muleContext, true);
     }
 
     /**

Modified: branches/mule-3.3.x/tests/integration/src/test/java/org/mule/test/routing/ForeachTestCase.java (24749 => 24750)

--- branches/mule-3.3.x/tests/integration/src/test/java/org/mule/test/routing/ForeachTestCase.java	2012-08-15 22:13:17 UTC (rev 24749)
+++ branches/mule-3.3.x/tests/integration/src/test/java/org/mule/test/routing/ForeachTestCase.java	2012-08-16 17:47:24 UTC (rev 24750)
@@ -186,7 +186,6 @@
     public void testMessageCollectionConfiguration() throws Exception
     {
         MuleMessageCollection msgCollection = new DefaultMessageCollection(muleContext);
-        msgCollection.setInvocationProperty("totalMessages", 0);
         for (int i = 0; i < 10; i++)
         {
             MuleMessage msg = new DefaultMuleMessage("message-" + i, muleContext);
@@ -201,6 +200,21 @@
     }
 
     @Test
+    public void testMessageCollectionConfigurationOneWay() throws Exception
+    {
+        MuleMessageCollection msgCollection = new DefaultMessageCollection(muleContext);
+        for (int i = 0; i < 10; i++)
+        {
+            MuleMessage msg = new DefaultMuleMessage("message-" + i, muleContext);
+            msg.setProperty("out", "out" + (i+1), PropertyScope.OUTBOUND);
+            msgCollection.addMessage(msg);
+        }
+
+        client.dispatch("vm://input-71", msgCollection);
+        FlowAssert.verify("message-collection-config-one-way");
+    }
+
+    @Test
     public void testMapPayload() throws Exception
     {
         final Map<String, String> payload = new HashMap<String, String>();

Modified: branches/mule-3.3.x/tests/integration/src/test/resources/foreach-test.xml (24749 => 24750)

--- branches/mule-3.3.x/tests/integration/src/test/resources/foreach-test.xml	2012-08-15 22:13:17 UTC (rev 24749)
+++ branches/mule-3.3.x/tests/integration/src/test/resources/foreach-test.xml	2012-08-16 17:47:24 UTC (rev 24750)
@@ -84,16 +84,22 @@
         <vm:inbound-endpoint path="input-7" exchange-pattern="request-response"/>
         <foreach>
             <test:assert expression="#[message.inboundProperties['out']=='out'+counter]" />
-            <test:component/>
-            <message-properties-transformer scope="invocation">
-            	<add-message-property key="totalMessages" value="#[variable:counter]"/>
-            </message-properties-transformer>
+            <set-variable variableName="totalMessages" value="#[counter]"/>
         </foreach>
-        <message-properties-transformer scope="outbound">
-        	<add-message-property key="totalMessages" value="#[variable:totalMessages]"/>
-        </message-properties-transformer>
+        <test:assert expression="#[totalMessages==10]" />
+        <set-property propertyName="totalMessages" value="#[totalMessages]"/>
     </flow>
 
+    <flow name="message-collection-config-one-way">
+        <vm:inbound-endpoint path="input-71" exchange-pattern="one-way"/>
+        <foreach>
+            <test:assert expression="#[message.inboundProperties['out']=='out'+counter]" />
+            <set-variable variableName="totalMessages" value="#[counter]"/>
+        </foreach>
+        <logger level="WARN" message="variable:totalMessages: #[variable:totalMessages]"/>
+        <test:assert expression="#[totalMessages==10]" />
+    </flow>
+
     <flow name="map-config">
         <vm:inbound-endpoint path="input-8" exchange-pattern="request-response"/>
         <foreach>

---

To unsubscribe from this list please visit:

http://xircles.codehaus.org/manage_email

[Santiago Vacas]

right, the performance issue is the main concern, though if sb tries to set a property in a message belonging to the collection it may find the same issue depending on which thread created the message