Fix Auto-Expand Regression (#2585)

Author: ificator

src/Microsoft.AspNet.OData.Shared/Query/AutoSelectExpandHelper.cs

@@ -6,6 +6,7 @@
 //------------------------------------------------------------------------------
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics.Contracts;
 using System.Linq;
@@ -38,11 +39,7 @@ public static bool HasAutoSelectProperty(this IEdmModel edmModel, IEdmStructured
                 throw Error.ArgumentNull(nameof(structuredType));
             }
 
-            List<IEdmStructuredType> structuredTypes = new List<IEdmStructuredType>();
-            structuredTypes.Add(structuredType);
-            structuredTypes.AddRange(edmModel.FindAllDerivedTypes(structuredType));
-
-            foreach (IEdmStructuredType edmStructuredType in structuredTypes)
+            foreach (IEdmStructuredType edmStructuredType in new SelfAndDerivedEnumerator(structuredType, edmModel))
             {
                 // for top type, let's retrieve its properties and the properties from base type of top type if has.
                 // for derived type, let's retrieve the declared properties.
@@ -92,11 +89,8 @@ private static bool HasAutoExpandProperty(this IEdmModel edmModel, IEdmStructure
             }
             visited.Add(structuredType);
 
-            List<IEdmStructuredType> structuredTypes = new List<IEdmStructuredType>();
-            structuredTypes.Add(structuredType);
-            structuredTypes.AddRange(edmModel.FindAllDerivedTypes(structuredType));
 
-            foreach (IEdmStructuredType edmStructuredType in structuredTypes)
+            foreach (IEdmStructuredType edmStructuredType in new SelfAndDerivedEnumerator(structuredType, edmModel))
             {
                 // for top type, let's retrieve its properties and the properties from base type of top type if has.
                 // for derived type, let's retrieve the declared properties.
@@ -144,7 +138,7 @@ private static bool HasAutoExpandProperty(this IEdmModel edmModel, IEdmStructure
         /// <param name="pathProperty">The property from path, it can be null.</param>
         /// <param name="querySettings">The query settings.</param>
         /// <returns>The auto select paths.</returns>
-        public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel, IEdmStructuredType structuredType,
+        public static IEnumerable<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel, IEdmStructuredType structuredType,
             IEdmProperty pathProperty, ModelBoundQuerySettings querySettings = null)
         {
             if (edmModel == null)
@@ -157,13 +151,8 @@ public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel,
                 throw Error.ArgumentNull(nameof(structuredType));
             }
 
-            List<SelectModelPath> autoSelectProperties = new List<SelectModelPath>();
-
-            List<IEdmStructuredType> structuredTypes = new List<IEdmStructuredType>();
-            structuredTypes.Add(structuredType);
-            structuredTypes.AddRange(edmModel.FindAllDerivedTypes(structuredType));
-
-            foreach (IEdmStructuredType edmStructuredType in structuredTypes)
+            List<SelectModelPath> autoSelectProperties = null;
+            foreach (IEdmStructuredType edmStructuredType in new SelfAndDerivedEnumerator(structuredType, edmModel))
             {
                 // for top type, let's retrieve its properties and the properties from base type of top type if has.
                 // for derived type, let's retrieve the declared properties.
@@ -175,6 +164,11 @@ public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel,
                 {
                     if (IsAutoSelect(property, pathProperty, edmStructuredType, edmModel, querySettings))
                     {
+                        if (autoSelectProperties == null)
+                        {
+                            autoSelectProperties = new List<SelectModelPath>(1);
+                        }
+
                         if (edmStructuredType == structuredType)
                         {
                             autoSelectProperties.Add(new SelectModelPath(new[] { property }));
@@ -187,7 +181,7 @@ public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel,
                 }
             }
 
-            return autoSelectProperties;
+            return autoSelectProperties ?? Enumerable.Empty<SelectModelPath>();
         }
 
         /// <summary>
@@ -199,7 +193,7 @@ public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel,
         /// <param name="isSelectPresent">Is $select presented.</param>
         /// <param name="querySettings">The query settings.</param>
         /// <returns>The auto expand paths.</returns>
-        public static IList<ExpandModelPath> GetAutoExpandPaths(this IEdmModel edmModel, IEdmStructuredType structuredType,
+        public static IEnumerable<ExpandModelPath> GetAutoExpandPaths(this IEdmModel edmModel, IEdmStructuredType structuredType,
             IEdmProperty property, bool isSelectPresent = false, ModelBoundQuerySettings querySettings = null)
         {
             if (edmModel == null)
@@ -290,11 +284,7 @@ private static void GetAutoExpandPaths(this IEdmModel edmModel, IEdmStructuredTy
             }
             visited.Add(structuredType);
 
-            List<IEdmStructuredType> structuredTypes = new List<IEdmStructuredType>();
-            structuredTypes.Add(structuredType);
-            structuredTypes.AddRange(edmModel.FindAllDerivedTypes(structuredType));
-
-            foreach (IEdmStructuredType edmStructuredType in structuredTypes)
+            foreach (IEdmStructuredType edmStructuredType in new SelfAndDerivedEnumerator(structuredType, edmModel))
             {
                 IEnumerable<IEdmProperty> properties;
 
@@ -347,5 +337,103 @@ private static void GetAutoExpandPaths(this IEdmModel edmModel, IEdmStructuredTy
                 }
             }
         }
+
+        /// <summary>
+        /// This is a helper that allows us to avoid inefficiencies in the previous pattern:
+        ///     var structuredTypes = new List&lt;IEdmStructuredType&gt;();
+        ///     structuredTypes.Add(structuredType);
+        ///     structuredTypes.AddRange(edmModel.FindAllDerivedTypes(structuredType));
+        ///
+        /// Specifically, the allocation of the list and the resizing driven by the "AddRange" call are
+        /// avoided by leveraging a struct with a simple state machine for enumerating over a type
+        /// and its derived types.
+        /// </summary>
+        private struct SelfAndDerivedEnumerator : IEnumerator<IEdmStructuredType>
+        {
+            private enum Stage : byte
+            {
+                Initial,
+                Self,
+                Derived,
+            }
+
+            private readonly IEnumerator<IEdmStructuredType> derivedEnumerator;
+            private readonly IEdmStructuredType structuredType;
+
+            private Stage stage;
+
+            public SelfAndDerivedEnumerator(IEdmStructuredType structuredType, IEdmModel edmModel)
+            {
+                if (structuredType == null)
+                {
+                    throw new ArgumentNullException(nameof(structuredType));
+                }
+
+                if (edmModel == null)
+                {
+                    throw new ArgumentNullException(nameof(edmModel));
+                }
+
+                this.stage = Stage.Initial;
+                this.derivedEnumerator = edmModel.FindAllDerivedTypes(structuredType).GetEnumerator();
+                this.structuredType = structuredType;
+            }
+
+            public IEdmStructuredType Current
+            {
+                get
+                {
+                    switch (stage)
+                    {
+                        case Stage.Self:
+                            return this.structuredType;
+
+                        case Stage.Derived:
+                            return this.derivedEnumerator.Current;
+
+                        default:
+                            throw new InvalidOperationException("Enumeration is an invalid state");
+                    }
+                }
+            }
+
+            object IEnumerator.Current => this.Current;
+
+            public void Dispose()
+            {
+                this.derivedEnumerator.Dispose();
+            }
+
+            public SelfAndDerivedEnumerator GetEnumerator()
+            {
+                return this;
+            }
+
+            public bool MoveNext()
+            {
+                switch (this.stage)
+                {
+                    case Stage.Initial:
+                        this.stage = Stage.Self;
+                        return true;
+
+                    case Stage.Self:
+                        this.stage = Stage.Derived;
+                        goto case Stage.Derived;
+
+                    case Stage.Derived:
+                        return this.derivedEnumerator.MoveNext();
+
+                    default:
+                        return false;
+                }
+            }
+
+            public void Reset()
+            {
+                this.stage = Stage.Initial;
+                this.derivedEnumerator.Reset();
+            }
+        }
     }
 }

src/Microsoft.AspNet.OData.Shared/Query/ODataQueryOptions.cs

@@ -838,8 +838,7 @@ private string GetAutoSelectRawValue()
             var selectRawValue = RawValues.Select;
             if (String.IsNullOrEmpty(selectRawValue))
             {
-                IList<SelectModelPath> autoSelectProperties = null;
-
+                IEnumerable<SelectModelPath> autoSelectProperties = null;
                 if (Context.TargetStructuredType != null && Context.TargetProperty != null)
                 {
                     autoSelectProperties = Context.Model.GetAutoSelectPaths(Context.TargetStructuredType, Context.TargetProperty);
@@ -874,7 +873,7 @@ private string GetAutoExpandRawValue()
         {
             var expandRawValue = RawValues.Expand;
 
-            IList<ExpandModelPath> autoExpandNavigationProperties = null;
+            IEnumerable<ExpandModelPath> autoExpandNavigationProperties = null;
             if (Context.TargetStructuredType != null && Context.TargetProperty != null)
             {
                 autoExpandNavigationProperties = Context.Model.GetAutoExpandPaths(Context.TargetStructuredType, Context.TargetProperty, !string.IsNullOrEmpty(RawValues.Select));

src/Microsoft.AspNet.OData.Shared/Query/SelectExpandQueryOption.cs

@@ -379,7 +379,7 @@ private void GetAutoSelectExpandItems(
                 return;
             }
 
-            IList<SelectModelPath> autoSelectProperties = model.GetAutoSelectPaths(baseEntityType, null, modelBoundQuerySettings);
+            IEnumerable<SelectModelPath> autoSelectProperties = model.GetAutoSelectPaths(baseEntityType, null, modelBoundQuerySettings);
             foreach (var autoSelectProperty in autoSelectProperties)
             {
                 ODataSelectPath odataSelectPath = BuildSelectPath(autoSelectProperty, navigationSource);
@@ -393,7 +393,7 @@ private void GetAutoSelectExpandItems(
                 return;
             }
 
-            IList<ExpandModelPath> autoExpandNavigationProperties = model.GetAutoExpandPaths(baseEntityType, null, !isAllSelected, modelBoundQuerySettings);
+            IEnumerable<ExpandModelPath> autoExpandNavigationProperties = model.GetAutoExpandPaths(baseEntityType, null, !isAllSelected, modelBoundQuerySettings);
             foreach (ExpandModelPath itemPath in autoExpandNavigationProperties)
             {
                 string navigationPath = itemPath.NavigationPropertyPath;