Issue 506: ODataConventionModelBuilder should ignore object[] property

6 files changed, 154 insertions, 74 deletions

diff --git a/src/System.Web.Http.OData/OData/Builder/Conventions/ConventionsHelpers.cs b/src/System.Web.Http.OData/OData/Builder/Conventions/ConventionsHelpers.cs
index 7816ff6a..fc01dbba 100644
--- a/src/System.Web.Http.OData/OData/Builder/Conventions/ConventionsHelpers.cs
+++ b/src/System.Web.Http.OData/OData/Builder/Conventions/ConventionsHelpers.cs
@@ -14,8 +14,6 @@ namespace System.Web.Http.OData.Builder.Conventions
 {
     internal static class ConventionsHelpers
     {
-        private static HashSet<Type> _ignoredCollectionTypes = new HashSet<Type>(new Type[] { typeof(string) });
-
         public static string GetEntityKeyValue(EntityInstanceContext entityContext, IEntityTypeConfiguration entityTypeConfiguration)
         {
             // TODO: BUG 453795: reflection cleanup
@@ -96,46 +94,13 @@ namespace System.Web.Http.OData.Builder.Conventions
                 throw Error.ArgumentNull("type");
             }
 
+            Type elementType;
+
             return !(type.IsGenericTypeDefinition
                      || type.IsNested
                      || type.IsPointer
-                     || type == typeof(object));
-        }
-
-        public static bool IsCollection(this Type type)
-        {
-            return type.IsCollection(out type);
-        }
-
-        public static bool IsCollection(this Type type, out Type elementType)
-        {
-            if (type == null)
-            {
-                throw Error.ArgumentNull("type");
-            }
-
-            elementType = type;
-
-            // see if this type should be ignored.
-            if (_ignoredCollectionTypes.Contains(type))
-            {
-                return false;
-            }
-
-            Type collectionInterface
-                = type.GetInterfaces()
-                    .Union(new[] { type })
-                    .FirstOrDefault(
-                        t => t.IsGenericType
-                             && t.GetGenericTypeDefinition() == typeof(IEnumerable<>));
-
-            if (collectionInterface != null)
-            {
-                elementType = collectionInterface.GetGenericArguments().Single();
-                return true;
-            }
-
-            return false;
+                     || type == typeof(object)
+                     || (type.IsCollection(out elementType) && elementType == typeof(object)));
         }
 
         // gets the primitive odata uri representation.
diff --git a/src/System.Web.Http.OData/TypeHelper.cs b/src/System.Web.Http.OData/TypeHelper.cs
index f0fbd277..deed93c2 100644
--- a/src/System.Web.Http.OData/TypeHelper.cs
+++ b/src/System.Web.Http.OData/TypeHelper.cs
@@ -13,6 +13,53 @@ namespace System.Web.Http
 {
     internal static class TypeHelper
     {
+        // Gets the collection element type.
+        public static Type GetInnerElementType(this Type type)
+        {
+            Type elementType;
+            type.IsCollection(out elementType);
+            Contract.Assert(elementType != null);
+
+            return elementType;
+        }
+
+        public static bool IsCollection(this Type type)
+        {
+            Type elementType;
+            return type.IsCollection(out elementType);
+        }
+
+        public static bool IsCollection(this Type type, out Type elementType)
+        {
+            if (type == null)
+            {
+                throw Error.ArgumentNull("type");
+            }
+
+            elementType = type;
+
+            // see if this type should be ignored.
+            if (type == typeof(string))
+            {
+                return false;
+            }
+
+            Type collectionInterface
+                = type.GetInterfaces()
+                    .Union(new[] { type })
+                    .FirstOrDefault(
+                        t => t.IsGenericType
+                             && t.GetGenericTypeDefinition() == typeof(IEnumerable<>));
+
+            if (collectionInterface != null)
+            {
+                elementType = collectionInterface.GetGenericArguments().Single();
+                return true;
+            }
+
+            return false;
+        }
+
         /// <summary>
         /// Determines whether the given type is a primitive type or
         /// is a <see cref="string"/>, <see cref="DateTime"/>, <see cref="Decimal"/>,
diff --git a/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ConventionsHelpersTests.cs b/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ConventionsHelpersTests.cs
index 0d689966..6532c15c 100644
--- a/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ConventionsHelpersTests.cs
+++ b/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ConventionsHelpersTests.cs
@@ -1,7 +1,5 @@
 // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
 
-using System.Collections;
-using System.Collections.Generic;
 using System.Linq;
 using Microsoft.TestCommon;
 using Microsoft.TestCommon.Types;
@@ -64,31 +62,6 @@ namespace System.Web.Http.OData.Builder.Conventions
             }
         }
 
-        [Theory]
-        [InlineData(typeof(ICollection<string>), typeof(string))]
-        [InlineData(typeof(IList<string>), typeof(string))]
-        [InlineData(typeof(List<int>), typeof(int))]
-        [InlineData(typeof(IsCollection_with_Collections_TestClass), typeof(bool))]
-        [InlineData(typeof(IEnumerable<int>), typeof(int))]
-        [InlineData(typeof(int[]), typeof(int))]
-        [InlineData(typeof(MyCustomCollection), typeof(int))]
-        public void IsCollection_with_Collections(Type collectionType, Type elementType)
-        {
-            Type type;
-            Assert.True(collectionType.IsCollection(out type));
-            Assert.Equal(elementType, type);
-        }
-
-        [Theory]
-        [InlineData(typeof(object))]
-        [InlineData(typeof(ICollection))]
-        [InlineData(typeof(IEnumerable))]
-        [InlineData(typeof(string))]
-        public void IsCollection_with_NonCollections(Type type)
-        {
-            Assert.False(type.IsCollection());
-        }
-
         [Fact]
         public void GetProperties_ReturnsProperties_FromBaseAndDerived()
         {
@@ -286,10 +259,6 @@ namespace System.Web.Http.OData.Builder.Conventions
                 ConventionsHelpers.GetUriRepresentationForValue(value));
         }
 
-        private sealed class IsCollection_with_Collections_TestClass : List<bool>
-        {
-        }
-
         private class GetKeyProperty_validEntityType_TestClass_Id
         {
             public int Id { get; set; }
@@ -360,8 +329,4 @@ namespace System.Web.Http.OData.Builder.Conventions
     {
         public int A { get; set; }
     }
-
-    class MyCustomCollection : List<int>
-    {
-    }
 }
diff --git a/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ODataConventionModelBuilderTests.cs b/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ODataConventionModelBuilderTests.cs
index 6d23ace5..cc1c0b54 100644
--- a/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ODataConventionModelBuilderTests.cs
+++ b/test/System.Web.Http.OData.Test/OData/Builder/Conventions/ODataConventionModelBuilderTests.cs
@@ -963,6 +963,46 @@ namespace System.Web.Http.OData.Builder.Conventions
             IEdmComplexType complexType = model.AssertHasComplexType(new { ComplexProperty = default(string) }.GetType());
             complexType.AssertHasPrimitiveProperty(model, "ComplexProperty", EdmPrimitiveTypeKind.String, isNullable: true);
         }
+
+        [Theory]
+        [InlineData(typeof(object[]))]
+        [InlineData(typeof(IEnumerable<object>))]
+        [InlineData(typeof(List<object>))]
+        public void ObjectCollectionsAreIgnoredByDefault(Type propertyType)
+        {
+            MockType type =
+                new MockType("entity")
+                .Property<int>("ID")
+                .Property(propertyType, "Collection");
+
+            ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
+            var entityType = builder.AddEntity(type);
+            builder.AddEntitySet("entityset", entityType);
+
+            IEdmModel model = builder.GetEdmModel();
+            Assert.Equal(2, model.SchemaElements.Count());
+            var entityEdmType = model.AssertHasEntitySet("entityset", type);
+        }
+
+        [Fact]
+        public void CanMapObjectArrayAsAComplexProperty()
+        {
+            MockType type =
+                new MockType("entity")
+                .Property<int>("ID")
+                .Property<object[]>("Collection");
+
+            ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
+            var entityType = builder.AddEntity(type);
+            entityType.AddCollectionProperty(type.GetProperty("Collection"));
+            builder.AddEntitySet("entityset", entityType);
+
+            IEdmModel model = builder.GetEdmModel();
+            Assert.Equal(3, model.SchemaElements.Count());
+            var entityEdmType = model.AssertHasEntitySet("entityset", type);
+            model.AssertHasComplexType(typeof(object));
+            entityEdmType.AssertHasCollectionProperty(model, "Collection", typeof(object), isNullable: true);
+        }
     }
 
     public class Product
diff --git a/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj b/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
index 324c1c74..abfd15cf 100644
--- a/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
+++ b/test/System.Web.Http.OData.Test/System.Web.Http.OData.Test.csproj
@@ -232,6 +232,7 @@
     <Compile Include="TestCommon\ReferenceDepthContext.cs" />
     <Compile Include="TestCommon\SupportedTypes.cs" />
     <Compile Include="TestCommon\TypeInitializer.cs" />
+    <Compile Include="TypeHelperTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="BaselineResource.resx">
diff --git a/test/System.Web.Http.OData.Test/TypeHelperTest.cs b/test/System.Web.Http.OData.Test/TypeHelperTest.cs
new file mode 100644
index 00000000..49a62be6
--- /dev/null
+++ b/test/System.Web.Http.OData.Test/TypeHelperTest.cs
@@ -0,0 +1,62 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
+
+using System.Collections;
+using System.Collections.Generic;
+using Microsoft.TestCommon;
+
+namespace System.Web.Http.OData
+{
+    public class TypeHelperTest
+    {
+        public static TheoryDataSet<Type, Type> CollectionTypesData
+        {
+            get
+            {
+                return new TheoryDataSet<Type, Type>
+                {
+                    { typeof(ICollection<string>), typeof(string) },
+                    { typeof(IList<string>), typeof(string) },
+                    { typeof(List<int>), typeof(int) },
+                    { typeof(IsCollection_with_Collections_TestClass), typeof(bool) },
+                    { typeof(IEnumerable<int>), typeof(int) },
+                    { typeof(int[]), typeof(int) },
+                    { typeof(MyCustomCollection), typeof(int) },
+                };
+            }
+        }
+
+        [Theory]
+        [PropertyData("CollectionTypesData")]
+        public void IsCollection_with_Collections(Type collectionType, Type elementType)
+        {
+            Type type;
+            Assert.True(collectionType.IsCollection(out type));
+            Assert.Equal(elementType, type);
+        }
+
+        [Theory]
+        [PropertyData("CollectionTypesData")]
+        public void GetInnerElementType(Type collectionType, Type elementType)
+        {
+            Assert.Equal(elementType, collectionType.GetInnerElementType());
+        }
+
+        [Theory]
+        [InlineData(typeof(object))]
+        [InlineData(typeof(ICollection))]
+        [InlineData(typeof(IEnumerable))]
+        [InlineData(typeof(string))]
+        public void IsCollection_with_NonCollections(Type type)
+        {
+            Assert.False(type.IsCollection());
+        }
+
+        private sealed class IsCollection_with_Collections_TestClass : List<bool>
+        {
+        }
+
+        private class MyCustomCollection : List<int>
+        {
+        }
+    }
+}