Skip to content

Fix wrong links generated for derived type with navigation property when odata.metadata=full #2592

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

Closed
gathogojr wants to merge 2 commits into from
Closed

Fix wrong links generated for derived type with navigation property when odata.metadata=full #2592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6762f75
Fix wrong links generated for derived type with navigation property w…
gathogojr Jan 11, 2023
7016331
Fix failing tests
gathogojr Jan 24, 2023
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
31 changes: 28 additions & 3 deletions src/Microsoft.OData.Core/Evaluation/ODataConventionalEntityMetadataBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace Microsoft.OData.Evaluation
using System.Text;
using Microsoft.OData.Edm;
using Microsoft.OData.Edm.Vocabularies.V1;
using Microsoft.OData.UriParser;
#endregion

/// <summary>
Expand Down Expand Up @@ -405,10 +406,34 @@ private Uri ComputeEditLink()
{
Uri uri = this.ResourceMetadataContext.Resource.HasNonComputedId ? this.ResourceMetadataContext.Resource.NonComputedId : this.ComputedId;

Debug.Assert(this.ResourceMetadataContext != null && this.ResourceMetadataContext.TypeContext != null, "this.resourceMetadataContext != null && this.resourceMetadataContext.TypeContext != null");
if (this.ResourceMetadataContext.ActualResourceTypeName != this.ResourceMetadataContext.TypeContext.NavigationSourceEntityTypeName)
Debug.Assert(this.ResourceMetadataContext != null && this.ResourceMetadataContext.TypeContext != null,
"this.resourceMetadataContext != null && this.resourceMetadataContext.TypeContext != null");

string actualResourceTypeName= this.ResourceMetadataContext.ActualResourceTypeName;

if (actualResourceTypeName != this.ResourceMetadataContext.TypeContext.NavigationSourceEntityTypeName)
{
uri = this.UriBuilder.AppendTypeSegment(uri, this.ResourceMetadataContext.ActualResourceTypeName);
IEdmStructuredType actualResourceType = this.ResourceMetadataContext.ActualResourceType;

// In some scenarios, the resource id will already contain the cast segment. For example,

@KenitoInc KenitoInc Jan 19, 2023

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.

Have you considered a scenario where the id comes after the typesegment?
e.g ~/Customers/My.Customer(1)

This is supported in the library

odata.net/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/PathFunctionalTests.cs

Line 779 in ecd0850

public void KeysExpressionsCanAppearOnTypeSegments()

@gathogojr gathogojr Jan 24, 2023

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.

@KenitoInc I made use of the ODataUriParser and ODataPath classes to take care of this scenario. Please check the updated logic

// when full metadata is requested and a derived entity containing one or more navigation properties
// is being serialized, and the association and navigation links need to be serialized as well, the cast
// segment will already be contained in the resource id.
ODataUriParser odataUriParser = new ODataUriParser(this.MetadataContext.Model, this.MetadataContext.ServiceBaseUri, uri);
ODataPath odataPath = odataUriParser.ParsePath();

// Accomodate two scenarios:
// 1. ~/Customers(1)/NS.EnterpriseCustomer
// 2. ~/Customers/NS.EnterpriseCustomer(1) - key expression on type segment
if (!((odataPath.LastSegment is TypeSegment typeSegment1
&& typeSegment1.TargetEdmType == actualResourceType)
|| (odataPath.LastSegment is KeySegment
&& odataPath.Segments.Count >= 2
&& odataPath.Segments[odataPath.Segments.Count - 2] is TypeSegment typeSegment2
&& typeSegment2.TargetEdmType == actualResourceType)))
{
uri = this.UriBuilder.AppendTypeSegment(uri, actualResourceTypeName);
}

@gathogojr gathogojr Jan 11, 2023 •
edited
Loading

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.

I considered using the following logic as an alternative but was concerned about increase in allocations and the appearance of overkill:

var odataUriParser = new OData.UriParser.ODataUriParser(this.MetadataContext.Model, this.MetadataContext.ServiceBaseUri, uri);
var odataPath = odataUriParser.ParsePath();
if (!(odataPath.LastSegment is OData.UriParser.TypeSegment typeSegment && typeSegment.TargetEdmType == this.ResourceMetadataContext.ActualResourceType))
{
	uri = this.UriBuilder.AppendTypeSegment(uri, this.ResourceMetadataContext.ActualResourceTypeName);
}

@gathogojr gathogojr Jan 24, 2023

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.

Eventually adopted this approach since it made it easy to cater for the scenario raised here

}

return uri;
Expand Down
Loading