You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@reef.apache.org by we...@apache.org on 2017/04/13 17:19:44 UTC

reef git commit: [REEF-1775] ArrayPipelineDataConverter : FullMessage will throw on null data

Repository: reef
Updated Branches:
  refs/heads/master affd9735d -> 4e2e09c4f


[REEF-1775] ArrayPipelineDataConverter : FullMessage will throw on null data

This adds an explicit null-filter on incoming pipeline messages.

JIRA:
  [REEF-1775](https://issues.apache.org/jira/browse/REEF-1775)

Pull Request:
  This closes #1289


Project: http://git-wip-us.apache.org/repos/asf/reef/repo
Commit: http://git-wip-us.apache.org/repos/asf/reef/commit/4e2e09c4
Tree: http://git-wip-us.apache.org/repos/asf/reef/tree/4e2e09c4
Diff: http://git-wip-us.apache.org/repos/asf/reef/diff/4e2e09c4

Branch: refs/heads/master
Commit: 4e2e09c4fbece40ea6364ceb3fd8823552efbe3e
Parents: affd973
Author: roganc <ro...@microsoft.com>
Authored: Wed Apr 12 22:16:15 2017 -0700
Committer: Markus Weimer <we...@apache.org>
Committed: Thu Apr 13 10:18:53 2017 -0700

----------------------------------------------------------------------
 .../GroupCommunication/PipeliningTests.cs       | 56 ++++++++++++++++++++
 .../Impl/ArrayPipelineDataConverter.cs          |  3 ++
 2 files changed, 59 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/reef/blob/4e2e09c4/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/PipeliningTests.cs
----------------------------------------------------------------------
diff --git a/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/PipeliningTests.cs b/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/PipeliningTests.cs
index d777086..9853725 100644
--- a/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/PipeliningTests.cs
+++ b/lang/cs/Org.Apache.REEF.Network.Tests/GroupCommunication/PipeliningTests.cs
@@ -119,6 +119,9 @@ namespace Org.Apache.REEF.Network.Tests.GroupCommunication
             // Verify that the constructor has the proper restrictions
             AssertPositivePipelinePackageElementsRequired<T[], ArrayPipelineDataConverter<T>>();
 
+            // Verify that the FullMessage method properly handles null and zero-length arrays
+            AssertNullAndZeroLengthArraysHandledByFullMessage<T, ArrayPipelineDataConverter<T>>();
+
             // Test the valid case where we break up the array into smaller pieces
             // First determine how many messages to create from originalArray
             int pipelineMessageSize;
@@ -175,6 +178,59 @@ namespace Org.Apache.REEF.Network.Tests.GroupCommunication
         }
 
         /// <summary>
+        /// Verify that the FullMessage method properly handles null and zero-length arrays
+        /// </summary>
+        /// <typeparam name="T">The type of array the IPipelineDataConverter converts</typeparam>
+        /// <typeparam name="DataConverter">The IPipelineDataConverter implementation</typeparam>
+        private static void AssertNullAndZeroLengthArraysHandledByFullMessage<T, DataConverter>()
+            where T : new()
+            where DataConverter : class, IPipelineDataConverter<T[]>
+        {
+            // Test that the valid configuration can be injected
+            IConfiguration config = GetPipelineDataConverterConfig(1);
+            IPipelineDataConverter<T[]> dataConverter = TangFactory.GetTang().NewInjector(config).GetInstance<ArrayPipelineDataConverter<T>>();
+
+            // Test that a null message returns a null object
+            List<PipelineMessage<T[]>> nullMessage = new List<PipelineMessage<T[]>>
+            {
+                new PipelineMessage<T[]>(null, true)
+            };
+            Assert.Null(dataConverter.FullMessage(nullMessage));
+
+            // Test that many null messages returns a null object
+            List<PipelineMessage<T[]>> manyNullMessages = new List<PipelineMessage<T[]>>
+            {
+                new PipelineMessage<T[]>(null, false),
+                new PipelineMessage<T[]>(null, true)
+            };
+            Assert.Null(dataConverter.FullMessage(manyNullMessages));
+
+            // Test that null messages mixed with non-null returns the non-null object
+            List<PipelineMessage<T[]>> someNullMessages = new List<PipelineMessage<T[]>>
+            {
+                new PipelineMessage<T[]>(null, false),
+                new PipelineMessage<T[]>(new T[2], false),
+                new PipelineMessage<T[]>(null, true)
+            };
+            Assert.Equal(2, dataConverter.FullMessage(someNullMessages).Length);
+
+            // Test that a zero-length message returns a zero-length object
+            List<PipelineMessage<T[]>> zeroLengthMessage = new List<PipelineMessage<T[]>>
+            {
+                new PipelineMessage<T[]>(new T[0], true)
+            };
+            Assert.Equal(0, dataConverter.FullMessage(zeroLengthMessage).Length);
+
+            // Test that many zero-length message return a zero-length object
+            List<PipelineMessage<T[]>> manyZeroLengthMessages = new List<PipelineMessage<T[]>>
+            {
+                new PipelineMessage<T[]>(new T[0], false),
+                new PipelineMessage<T[]>(new T[0], true)
+            };
+            Assert.Equal(0, dataConverter.FullMessage(manyZeroLengthMessages).Length);
+        }
+
+        /// <summary>
         /// Validate that the IsLast flag is properly set on a list of pipeline messages
         /// </summary>
         /// <typeparam name="T"></typeparam>

http://git-wip-us.apache.org/repos/asf/reef/blob/4e2e09c4/lang/cs/Org.Apache.REEF.Network/Group/Pipelining/Impl/ArrayPipelineDataConverter.cs
----------------------------------------------------------------------
diff --git a/lang/cs/Org.Apache.REEF.Network/Group/Pipelining/Impl/ArrayPipelineDataConverter.cs b/lang/cs/Org.Apache.REEF.Network/Group/Pipelining/Impl/ArrayPipelineDataConverter.cs
index 12540ac..6d43812 100644
--- a/lang/cs/Org.Apache.REEF.Network/Group/Pipelining/Impl/ArrayPipelineDataConverter.cs
+++ b/lang/cs/Org.Apache.REEF.Network/Group/Pipelining/Impl/ArrayPipelineDataConverter.cs
@@ -96,6 +96,9 @@ namespace Org.Apache.REEF.Network.Group.Pipelining.Impl
         /// <returns>The full constructed message</returns>
         public T[] FullMessage(List<PipelineMessage<T[]>> pipelineMessage)
         {
+            // Filter out messages with null data payloads
+            pipelineMessage = pipelineMessage.Where(message => message.Data != null).ToList();
+
             // Null data corresponds to an empty list of pipeline messages
             if (pipelineMessage.Count == 0)
             {