Skip to content

Commit

Permalink
Fix SelectExpandNode for navigation properties on derived/complex types
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikepizzo
mikepizzo committed Dec 12, 2018
1 parent 6d99730 commit 0988a28
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 87 deletions.
126 changes: 63 additions & 63 deletions src/Microsoft.OData.Core/SelectedPropertiesNode.cs
View file
Expand Up @@ -101,7 +101,7 @@ private void Parse(string selectClause)
{
string[] segments = null;
int idxLP = t.IndexOf('(');
if ( -1 == idxLP )
if (-1 == idxLP)
{
segments = t.Split(PathSeparator);
}
Expand All @@ -117,10 +117,23 @@ private void Parse(string selectClause)
this.ParsePathSegment(segments, 0);
}

if ( (this.selectedProperties == null || this.selectedProperties.Count == 0)
&& this.children.Values.All( n => n.selectionType == SelectionType.EntireSubtree))
DetermineSelectionType(this);
}

private static void DetermineSelectionType(SelectedPropertiesNode node)
{
if (node.children != null)
{
this.selectionType = SelectionType.EntireSubtree;
foreach (SelectedPropertiesNode childNode in node.children.Values)
{
DetermineSelectionType(childNode);
}
}

if ((node.selectedProperties == null || node.selectedProperties.Count == 0)
&& (node.children == null || node.children.Values.All(n => n.selectionType == SelectionType.EntireSubtree)))
{
node.selectionType = SelectionType.EntireSubtree;
}
}

Expand Down Expand Up @@ -364,7 +377,7 @@ internal SelectedPropertiesNode GetSelectedPropertiesForNavigationProperty(IEdmS
}

// $select=Orders will include the entire subtree when there are no same expanded entity.
if (this.selectedProperties.Contains(navigationPropertyName) &&
if (this.selectedProperties != null && this.selectedProperties.Contains(navigationPropertyName) &&
(this.children == null || !this.children.Any(n => n.Key.Equals(navigationPropertyName) && n.Value.isExpandedNavigationProperty)))
{
return new SelectedPropertiesNode(SelectionType.EntireSubtree);
Expand Down Expand Up @@ -411,7 +424,7 @@ internal IEnumerable<IEdmNavigationProperty> GetSelectedNavigationProperties(IEd
}

if (this.selectionType == SelectionType.EntireSubtree || this.hasWildcard
|| (this.selectedProperties.Count() == 0 && this.children.Values.All( n => n.isExpandedNavigationProperty)))
|| ((this.selectedProperties == null || this.selectedProperties.Count() == 0) && this.children.Values.All( n => n.isExpandedNavigationProperty)))
{
return structuredType.NavigationProperties();
}
Expand All @@ -420,8 +433,7 @@ internal IEnumerable<IEdmNavigationProperty> GetSelectedNavigationProperties(IEd
// NOTE: the assumption is that the number of selected properties usually is a lot smaller
// than the number of all properties on the type and that FindProperty for each selected
// property is faster than iterating through all the properties on the type.
Debug.Assert(this.selectedProperties != null, "selectedProperties != null");
IEnumerable<string> navigationPropertyNames = this.selectedProperties;
IEnumerable<string> navigationPropertyNames = this.selectedProperties ?? CreateSelectedPropertiesHashSet();
if (this.children != null)
{
navigationPropertyNames = this.children.Keys.Concat(navigationPropertyNames);
Expand Down Expand Up @@ -465,13 +477,14 @@ internal IEnumerable<IEdmNavigationProperty> GetSelectedNavigationProperties(IEd
return entityType.StructuralProperties().Where(sp => sp.Type.IsStream()).ToDictionary(sp => sp.Name, StringComparer.Ordinal);
}

Debug.Assert(this.selectedProperties != null, "selectedProperties != null");

IDictionary<string, IEdmStructuralProperty> selectedStreamProperties = this.selectedProperties
.Select(entityType.FindProperty)
.OfType<IEdmStructuralProperty>()
.Where(p => p.Type.IsStream())
.ToDictionary(p => p.Name, StringComparer.Ordinal);
IDictionary<string, IEdmStructuralProperty> selectedStreamProperties =
this.selectedProperties == null ?
new Dictionary<string, IEdmStructuralProperty>() :
this.selectedProperties
.Select(entityType.FindProperty)
.OfType<IEdmStructuralProperty>()
.Where(p => p.Type.IsStream())
.ToDictionary(p => p.Name, StringComparer.Ordinal);

// gather up the selected stream from any child nodes that have type segments matching the current type and add them to the dictionary.
foreach (SelectedPropertiesNode typeSegmentChild in this.GetMatchingTypeSegments(entityType))
Expand Down Expand Up @@ -585,7 +598,7 @@ private static IEnumerable<string> GetPossibleMatchesForSelectedOperation(IEdmOp

private bool IsValidExpandToken(string item)
{
// Check #1: Expand token must contain '(' and end with ')', must be related to navigation property.
// Expand token must contain '(' and end with ')', must not contain a dot, and must be related to navigation property.
int idxLP = item.IndexOf('(');
if (idxLP == -1
|| !item.EndsWith(")", StringComparison.Ordinal)
Expand All @@ -595,27 +608,7 @@ private bool IsValidExpandToken(string item)
return false;
}

// Check #2: parentheses must be balanced.
int count = 0;
foreach (char c in item)
{
if (c == '(')
{
++count;
}
else if (c == ')')
{
--count;
}

if (count < 0)
{
// Malformed parentheses detected ==> not expand token.
return false;
}
}

return count == 0;
return true;
}

/// <summary>
Expand All @@ -628,23 +621,29 @@ private bool IsNavigationPropertyToken(string token)
const char nameSpaceSeparator = '.';

/* Decision tree:
#1. if it matches a defined navigation property => treat it as a navigation property
#2. otherwise, if the name contains a dot => it's not a navigation property
#1. if the name contains a dot => it's not a navigation property
#2. otherwise, if it matches a defined navigation property => treat it as a navigation property
#3. otherwise, if it matches an unqualified bound operation name => it's not a navigation property
#4. otherwise, it's a navigation property
*/

// For better readability, set the value in if-else branches corresponding to decision tree above.
bool found;
if (this.structuredType.NavigationProperties().Any(_ => _.Name.Equals(token, StringComparison.Ordinal)))
if (token.IndexOf(nameSpaceSeparator) != -1)
{
// #1
found = false;
}
else if (this.structuredType == null || this.structuredType.NavigationProperties().Any(_ => _.Name.Equals(token, StringComparison.Ordinal)))
{
// #2
// Note that action and function names in a contextUrl *SHOULD* always be qualified,
// So if we can't validate against the structured type, should assume it's a nav prop
found = true;
}
else if (token.IndexOf(nameSpaceSeparator) != -1 ||
this.edmModel.FindBoundOperations(this.structuredType).Any(op => op.Name.Equals(token, StringComparison.Ordinal)))
else if (this.edmModel != null && this.edmModel.FindBoundOperations(this.structuredType).Any(op => op.Name.Equals(token, StringComparison.Ordinal)))
{
// #2, #3
//#3
found = false;
}
else
Expand Down Expand Up @@ -694,18 +693,13 @@ private void ParsePathSegment(string[] segments, int index)
// NOTE: Each path is the name of a property or a series of property names
// separated by slash ('/'). The special star ('*') character is only supported at the end of a path.
string currentSegment = segments[index].Trim();
if (this.selectedProperties == null)
{
this.selectedProperties = CreateSelectedPropertiesHashSet();
}

bool isStar = string.CompareOrdinal(StarSegment, currentSegment) == 0;
int idxLP = currentSegment.IndexOf('(');

if (idxLP != -1 && IsValidExpandToken(currentSegment))
{
string token = currentSegment.Substring(0, idxLP);
SelectedPropertiesNode childNode = this.EnsureChildAnnotation(token, /* isExpandedNavigationProperty */ true);
SelectedPropertiesNode childNode = this.EnsureChildNode(token, /* isExpandedNavigationProperty */ true);
childNode.edmModel = this.edmModel;

if (idxLP < currentSegment.Length - 2)
Expand Down Expand Up @@ -741,11 +735,16 @@ private void ParsePathSegment(string[] segments, int index)
throw new ODataException(ODataErrorStrings.SelectedPropertiesNode_StarSegmentNotLastSegment);
}

SelectedPropertiesNode childNode = this.EnsureChildAnnotation(currentSegment, false);
SelectedPropertiesNode childNode = this.EnsureChildNode(currentSegment, false);
childNode.ParsePathSegment(segments, index + 1);
}
else
{
if (this.selectedProperties == null)
{
this.selectedProperties = CreateSelectedPropertiesHashSet();
}

this.selectedProperties.Add(currentSegment);
}
}
Expand All @@ -754,13 +753,13 @@ private void ParsePathSegment(string[] segments, int index)
}

/// <summary>
/// Ensures that a child annotation for the specified segment name already exists; if not creates one.
/// Ensures that a child node for the specified segment name already exists; if not creates one.
/// </summary>
/// <param name="segmentName">The segment name to get the child annotation for.</param>
/// <param name="segmentName">The segment name to get the child node for.</param>
/// <param name="isExpandedNavigationProperty">Boolean flag indicating whether this is an expanded navigation property.</param>

/// <returns>The existing or newly created child annotation for the <paramref name="segmentName"/>.</returns>
private SelectedPropertiesNode EnsureChildAnnotation(string segmentName, bool isExpandedNavigationProperty)
/// <returns>The existing or newly created child node for the <paramref name="segmentName"/>.</returns>
private SelectedPropertiesNode EnsureChildNode(string segmentName, bool isExpandedNavigationProperty)
{
Debug.Assert(segmentName != null, "segmentName != null");

Expand Down Expand Up @@ -791,16 +790,17 @@ private bool IsOperationSelectedAtThisLevel(IEdmOperation operation, bool mustBe
{
Debug.Assert(operation != null, "operation != null");

if (this.selectionType == SelectionType.Empty)
if (this.selectionType == SelectionType.EntireSubtree)
{
return false;
return true;
}

if (this.selectionType == SelectionType.EntireSubtree)
if (this.selectionType == SelectionType.Empty || this.selectedProperties == null )
{
return true;
return false;
}


return GetPossibleMatchesForSelectedOperation(operation, mustBeNamespaceQualified).Any(possibleMatch => this.selectedProperties.Contains(possibleMatch));
}

Expand Down Expand Up @@ -829,22 +829,22 @@ private static SelectedPropertiesNode ProcessSubExpand(string nodeName, Selected
}

/// <summary>Create SelectedPropertiesNode using selected name list and expand node list.</summary>
/// <param name="selectList">A list of selected item names.</param>
/// <param name="expandList">A list of sub expanded nodes.</param>
/// <param name="selectList">An enumerable of selected item names.</param>
/// <param name="expandList">An enumerable of sub expanded nodes.</param>
/// <returns>The generated SelectedPropertiesNode.</returns>
private static SelectedPropertiesNode CombineSelectAndExpandResult(IList<string> selectList, IList<SelectedPropertiesNode> expandList)
private static SelectedPropertiesNode CombineSelectAndExpandResult(IEnumerable<string> selectList, IEnumerable<SelectedPropertiesNode> expandList)
{
List<string> rawSelect = selectList.ToList();
rawSelect.RemoveAll(expandList.Select(m => m.nodeName).Contains);

if (selectList.Count == 0 && expandList.All( n => n.IsEntireSubtree()))
if (rawSelect.Count == 0 && expandList.All( n => n.IsEntireSubtree()))
{
return new SelectedPropertiesNode(SelectionType.EntireSubtree);
}

SelectedPropertiesNode node = new SelectedPropertiesNode(SelectionType.PartialSubtree)
{
selectedProperties = CreateSelectedPropertiesHashSet(),
selectedProperties = rawSelect.Count > 0 ? CreateSelectedPropertiesHashSet() : null,
children = new Dictionary<string, SelectedPropertiesNode>(StringComparer.Ordinal)
};

Expand Down

0 comments on commit 0988a28

Please sign in to comment.