Skip to content

Commit

Permalink
Fix Auto-Expand Regression (#2585)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ificator
ificator authored Jan 9, 2022
1 parent 256f591 commit 46a0d5a
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 29 deletions.
136 changes: 112 additions & 24 deletions src/Microsoft.AspNet.OData.Shared/Query/AutoSelectExpandHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//------------------------------------------------------------------------------

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics.Contracts;
using System.Linq;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand All @@ -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 }));
Expand All @@ -187,7 +181,7 @@ public static IList<SelectModelPath> GetAutoSelectPaths(this IEdmModel edmModel,
}
}

return autoSelectProperties;
return autoSelectProperties ?? Enumerable.Empty<SelectModelPath>();
}

/// <summary>
Expand All @@ -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)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
}
}
}
5 changes: 2 additions & 3 deletions src/Microsoft.AspNet.OData.Shared/Query/ODataQueryOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.OData.Shared/Query/SelectExpandQueryOption.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down

0 comments on commit 46a0d5a

Please sign in to comment.