Skip to content

Fix issue for nested next page link with singleton containment navigation property #710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lisicase merged 17 commits into from
Oct 6, 2022
Merged

Fix issue for nested next page link with singleton containment navigation property #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e5786e5
Created repro of issue 701
lisicase Sep 23, 2022
71acab0
Process segments contained by a singleton with a path
lisicase Sep 23, 2022
5333a9d
Cleaned up code
lisicase Sep 23, 2022
99f5657
Created singleton test with new sample data model
lisicase Sep 28, 2022
3e8aa4f
Added unit test
lisicase Sep 30, 2022
bb120c6
Removed unnecessary code
lisicase Sep 30, 2022
863f4e1
Updated target frameowkr
lisicase Sep 30, 2022
909f2d9
Created test using modified existing models
lisicase Sep 30, 2022
be1611e
Removed E2E test based on client's repro
lisicase Sep 30, 2022
23218a2
Removed client's repro
lisicase Sep 30, 2022
a4ed6e9
Cleaned up code based on PR review feedback
lisicase Sep 30, 2022
66007f9
Update syntax
lisicase Oct 3, 2022
a433304
Update test syntax
lisicase Oct 4, 2022
7818913
Update creation of disposable objects
lisicase Oct 4, 2022
aeb8e39
Test validity of E2E next link
lisicase Oct 5, 2022
05a450e
Validate nested next link in E2E test
lisicase Oct 5, 2022
012d13c
Build update
lisicase Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AspNetCoreOData.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31815.197
VisualStudioVersion = 17.3.32901.215
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{2F0E102B-EB33-4025-BE56-7B8F9D2C4B8A}"
EndProject
Expand Down
22 changes: 15 additions & 7 deletions src/Microsoft.AspNetCore.OData/Formatter/LinkGenerationHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,13 +349,13 @@ internal static IList<ODataPathSegment> GenerateBaseODataPathSegments(this Resou
return odataPath;
}

private static void GenerateBaseODataPathSegmentsForNonSingletons(
private static void GenerateBaseODataPathSegments(
ODataPath path,
IEdmNavigationSource navigationSource,
IList<ODataPathSegment> odataPath)
{
// If the navigation is not a singleton we need to walk all of the path segments to generate a
// contextually accurate URI.
// If the navigation is a contained property, we need to walk all of the path segments
// to generate a contextually accurate URI.
bool segmentFound = false;
bool containedFound = false;
if (path != null)
Expand All @@ -379,6 +379,14 @@ private static void GenerateBaseODataPathSegmentsForNonSingletons(
{
currentNavigationSource = navigationPathSegment.NavigationSource;
}

var singletonPathSegment = pathSegment as SingletonSegment;

@corranrogue9 corranrogue9 Oct 3, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want to have an INavigationSourceSegment interface or something and have EntitySetSegment, NavigationPropertySegment, and SingletonSegment implement it so that there is a consistent way to retrieve the "navigation source" from a segment. Doing this would also allow us to always cast only 1 time.

@lisicase lisicase Oct 5, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this would involve a change to another repo, this could be a good feature for the future.

@corranrogue9 corranrogue9 Oct 6, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, if you've talked to others and think is the path we actually want to go, can you create a github issue for it? If we aren't going that route, you can resolve this

if (singletonPathSegment != null)
{
currentNavigationSource = singletonPathSegment.Singleton;
}


if (containedFound)
{
odataPath.Add(pathSegment);
Expand Down Expand Up @@ -440,9 +448,9 @@ private static void GenerateBaseODataPathSegmentsForEntity(
this ResourceContext resourceContext,
IList<ODataPathSegment> odataPath)
{
// If the navigation is not a singleton we need to walk all of the path segments to generate a
// contextually accurate URI.
GenerateBaseODataPathSegmentsForNonSingletons(
// If the navigation is a contained property, we need to walk all of the path segments
// to generate a contextually accurate URI.
GenerateBaseODataPathSegments(
resourceContext.SerializerContext.Path, resourceContext.NavigationSource, odataPath);

odataPath.Add(new KeySegment(ConventionsHelpers.GetEntityKey(resourceContext), resourceContext.StructuredType as IEdmEntityType,
Expand All @@ -453,7 +461,7 @@ private static void GenerateBaseODataPathSegmentsForFeed(
this ResourceSetContext feedContext,
IList<ODataPathSegment> odataPath)
{
GenerateBaseODataPathSegmentsForNonSingletons(feedContext.Request.ODataFeature().Path,
GenerateBaseODataPathSegments(feedContext.Request.ODataFeature().Path,
feedContext.EntitySetBase,
odataPath);
}
Expand Down
139 changes: 69 additions & 70 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,22 @@
Helper method to check whether the given object is Delta resource set.
</summary>
<param name="result">The given object.</param>
Object (Equivalent of ODataDeletedResource in ODL).
<returns>True/False.</returns>
</member>
Comment thread
lisicase marked this conversation as resolved.
<member name="T:Microsoft.AspNetCore.OData.Deltas.DeltaItemKind">
<summary>
The Kind of the object within the DeltaPayload used to distinguish between
Resource/DeletedResource/DeltaDeletedLink/AddedLink.
</summary>
</member>
<member name="F:Microsoft.AspNetCore.OData.Deltas.DeltaItemKind.Resource">
<summary>
Corresponds to EdmEntityObject (Equivalent of ODataResource in ODL).
</summary>
</member>
<member name="F:Microsoft.AspNetCore.OData.Deltas.DeltaItemKind.DeletedResource">
<summary>
Corresponds to EdmDeltaDeletedResourceObject (Equivalent of ODataDeletedResource in ODL).
</summary>
</member>
<member name="F:Microsoft.AspNetCore.OData.Deltas.DeltaItemKind.DeltaDeletedLink">
Expand Down Expand Up @@ -1569,20 +1584,6 @@
<member name="P:Microsoft.AspNetCore.OData.Deltas.DeltaLink`1.Kind">
<inheritdoc />
</member>
<member name="T:Microsoft.AspNetCore.OData.Deltas.Delta`1">
<summary>
A class the tracnew instance of <see cref="T:Microsoft.AspNetCore.OData.Deltas.DeltaLink`1"/>.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.DeltaLink`1.#ctor(System.Type)">
<summary>
Initializes a new instance of <see cref="T:Microsoft.AspNetCore.OData.Deltas.DeltaLink`1"/>.
</summary>
<param name="structuralType">The derived structural type for which the changes would be tracked.</param>
</member>
<member name="P:Microsoft.AspNetCore.OData.Deltas.DeltaLink`1.Kind">
<inheritdoc />
</member>
<member name="T:Microsoft.AspNetCore.OData.Deltas.Delta`1">
<summary>
A class the tracks changes (i.e. the Delta) for a particular <typeparamref name="T"/>.
Expand Down Expand Up @@ -1637,7 +1638,21 @@
<summary>
The list of property names that can be updated.
</summary>
<remarks>When the list is modified, ttempts to get the value of the nested Property called <paramref name="name"/> from the underlying resource.
<remarks>When the list is modified, any modified properties that were removed from the list are no longer
considered to be changed.</remarks>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.Clear">
<inheritdoc/>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.TrySetPropertyValue(System.String,System.Object)">
<inheritdoc/>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.TryGetPropertyValue(System.String,System.Object@)">
<inheritdoc/>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.TryGetNestedPropertyValue(System.String,System.Object@)">
<summary>
Attempts to get the value of the nested Property called <paramref name="name"/> from the underlying resource.
<remarks>
Only properties that exist on Entity can be retrieved.
Only modified nested properties can be retrieved.
Expand Down Expand Up @@ -1699,19 +1714,6 @@
</summary>
<param name="original">The entity to be updated.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.TryGetPropertyRef(`0,System.String,System.Object@)">
<summary>
Attempts to get the property by the specmarks>
</summary>
<param name="original">The entity to be updated.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.Put(`0)">
<summary>
Overwrites the <paramref name="original"/> entity with the values stored in this Delta.
<remarks>The semantics of this operation are equivalent to a HTTP PUT operation, hence the name.</remarks>
</summary>
<param name="original">The entity to be updated.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Deltas.Delta`1.TryGetPropertyRef(`0,System.String,System.Object@)">
<summary>
Attempts to get the property by the specified name.
Expand Down Expand Up @@ -2669,7 +2671,16 @@
This is how formatters create links to invoke bound actions or functions.
</summary>
</member>
<member n.OperationLinkBuilder.#ctor(System.Func{Microsoft.AspNetCore.OData.Formatter.ResourceSetContext,System.Uri},System.Boolean)">
<member name="M:Microsoft.AspNetCore.OData.Edm.OperationLinkBuilder.#ctor(System.Func{Microsoft.AspNetCore.OData.Formatter.ResourceContext,System.Uri},System.Boolean)">
<summary>
Create a new <see cref="T:Microsoft.AspNetCore.OData.Edm.OperationLinkBuilder"/> based on an entity link factory.
</summary>
<param name="linkFactory">The link factory this <see cref="T:Microsoft.AspNetCore.OData.Edm.OperationLinkBuilder"/> should use when building links.</param>
<param name="followsConventions">
A value indicating whether the link factory generates links that follow OData conventions.
</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Edm.OperationLinkBuilder.#ctor(System.Func{Microsoft.AspNetCore.OData.Formatter.ResourceSetContext,System.Uri},System.Boolean)">
<summary>
Create a new <see cref="T:Microsoft.AspNetCore.OData.Edm.OperationLinkBuilder"/> based on a feed link factory.
</summary>
Expand Down Expand Up @@ -7981,18 +7992,6 @@
Enables a controller action to support OData query parameters.
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.EnableQueryAttribute.EnsureStableOrdering">
<summary>
Gets or sets a value indicating whether query composition should
alter the original query when necessary to ensure a stable sort order.
</summary>
<value>A <c>true</c> value indicates the original query should
be modified when necessary to guarantee a stable sort order.
A <c>false</c> value indicates the sort order can be considered
stable without modifying the query. Query providers that ensure
a stable sort order shoa controller action to support OData query parameters.
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.EnableQueryAttribute.EnsureStableOrdering">
<summary>
Gets or sets a value indicating whether query composition should
Expand Down Expand Up @@ -10357,7 +10356,19 @@
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.ParameterAliasNodeTranslator.Visit(Microsoft.OData.UriParser.SingleNavigationNode)">
<summary>
">
Translate a SingleNavigationNode.
</summary>
<param name="nodeIn">The node to be translated.</param>
<returns>The translated node.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.ParameterAliasNodeTranslator.Visit(Microsoft.OData.UriParser.SingleValueFunctionCallNode)">
<summary>
Translate a SingleValueFunctionCallNode.
</summary>
<param name="nodeIn">The node to be translated.</param>
<returns>The translated node.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.ParameterAliasNodeTranslator.Visit(Microsoft.OData.UriParser.SingleValueOpenPropertyAccessNode)">
<summary>
Translate a SingleValueOpenPropertyAccessNode.
</summary>
Expand Down Expand Up @@ -10423,18 +10434,6 @@
</summary>
<param name="context">The filter context.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.QueryFilterProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.Filters.FilterProviderContext)">
<summary>
Summary:
Called in decreasing Microsoft.AspNetCore.Mvc.Filters.IFilterProvider.Order,
after all Microsoft.AspNetCore.Mvc.Filters.IFilterProviders have executed once.
</summary>
<param name="context">The Mica.Query.QueryFilterProvider.OnProvidersExecuting(Microsoft.AspNetCore.Mvc.Filters.FilterProviderContext)">
<summary>
Provides filters to apply to the specified action.
</summary>
<param name="context">The filter context.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.QueryFilterProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.Filters.FilterProviderContext)">
<summary>
Summary:
Expand Down Expand Up @@ -10554,7 +10553,21 @@
<summary>
Gets the raw $count value.
</summary>
validation failed.
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.CountQueryOption.Value">
<summary>
Gets the value of the $count in a parsed form.
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.CountQueryOption.Validator">
<summary>
Gets or sets the $count query validator.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.CountQueryOption.Validate(Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings)">
<summary>
Validate the count query based on the given <paramref name="validationSettings"/>.
It throws an ODataException if validation failed.
</summary>
<param name="validationSettings">The <see cref="T:Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings"/> instance
which contains all the validation settings.</param>
Expand Down Expand Up @@ -11590,20 +11603,6 @@
Gets a list of properties one can orderby the result with. Note, by default this list is empty,
it means it can be ordered by any property.

For example, having an empty col: any, all

</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings.AllowedLogicalOperators">
<summary>
Gets or sets a list of allowed logical operators such as 'eq', 'ne', 'gt', 'ge', 'lt', 'le', 'and', 'or', 'not'.
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings.AllowedOrderByProperties">
<summary>
Gets a list of properties one can orderby the result with. Note, by default this list is empty,
it means it can be ordered by any property.

For example, having an empty collection means client can order the queryable result by any properties.
Adding "Name" to this list means that it only allows queryable result to be ordered by Name property.
</summary>
Expand Down
50 changes: 50 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Singleton/MonstersIncController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@ private static void InitData()
Category = CompanyCategory.Electronics,
Partners = new List<Partner>(),
Branches = new List<Office>() { new Office { City = "Shanghai", Address = "Minhang" }, new Office { City = "Xi'an", Address = "Dayanta" } },
Projects = new List<Project>()
{
new Project
{
Id = 1,
Title = "In Closet Scare",
ProjectDetails = new List<ProjectDetail>()
{
new ProjectDetail { Id = 1, Comment = "The original scare" },
new ProjectDetail { Id = 2, Comment = "Leaving the door open is the worst mistake any employee can make" },
new ProjectDetail { Id = 3, Comment = "Leaving the door open could let it not only a draft, but a child" },
new ProjectDetail { Id = 4, Comment = "Has led to the intrusion of a young girl, Boo" }
},
},
new Project
{
Id = 2,
Title = "Under Bed Scare",
ProjectDetails = new List<ProjectDetail>() {
new ProjectDetail { Id = 5, Comment = "Tried and true" },
new ProjectDetail { Id = 6, Comment = "Tip: grab a foot"}
},
},
new Project
{
Id = 3,
Title = "Midnight Snack in Kitchen Scare",
ProjectDetails= new List<ProjectDetail>(),
},
},
};
}

Expand Down Expand Up @@ -70,6 +100,26 @@ public IActionResult GetCompanyRevenue()
return Ok(MonstersInc.Revenue);
}

[HttpGet("Projects")]
[EnableQuery(PageSize = 2)]
public IActionResult GetProjects()
{
return Ok(MonstersInc.Projects);
}

[HttpGet("Projects/{key}/ProjectDetails")]
[EnableQuery]
public IActionResult GetProjectDetails(int key)
{
var project = MonstersInc.Projects.FirstOrDefault(a => a.Id == key);
if (project == null)
{
return NotFound($"Project with given key {key} does not exist.");
}

return Ok(project.ProjectDetails);
}

[HttpGet("Branches/$count")]
public IActionResult GetBranchesCount(ODataQueryOptions<Office> options)
{
Expand Down
31 changes: 30 additions & 1 deletion test/Microsoft.AspNetCore.OData.E2E.Tests/Singleton/SingletonDataModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
//------------------------------------------------------------------------------

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using Microsoft.OData.ModelBuilder;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Singleton
Expand Down Expand Up @@ -48,6 +51,10 @@ public class Company
[NotCountable]
public IList<Partner> Partners { get; set; }
public IList<Office> Branches { get; set; }

[Contained]
[AutoExpand]
public IList<Project> Projects { get; set; }
}

/// <summary>
Expand All @@ -59,6 +66,28 @@ public class Office
public string Address { get; set; }
}

/// <summary>
/// Present a contained navigation property
/// </summary>
public class Project
{
public int Id { get; set; }
public string Title { get; set; }

[AutoExpand]
[Contained]
public IList<ProjectDetail> ProjectDetails { get; set; }
}

/// <summary>
/// Present a nested contained navigation property
/// </summary>
public class ProjectDetail
{
public int Id { get; set; }
public string Comment { get; set; }
}

/// <summary>
/// EntityType derives from "Company"
/// </summary>
Expand All @@ -68,4 +97,4 @@ public class SubCompany : Company
public string Description { get; set; }
public Office Office { get; set; }
}
}
}
Loading