-
Notifications
You must be signed in to change notification settings - Fork 473
Description
If you have a composable unbound function GetAppStats that returns a collection of entities AppUsage, and you make a request that expands navigation properties of the entity type, like so:
GET /GetAppUsage()/{appId}?$expand=CredentialsGET /GetAppSuage()/{appId}/Credentials
Then you'll get an exception like the following:
Microsoft.OData.ODataException: The Path property in ODataMessageWriterSettings.ODataUri must be set when writing contained elements.
at Microsoft.OData.ODataWriterCore.EnterScope(WriterState newState, ODataItem item)
at Microsoft.OData.ODataWriterCore.WriteStartNestedResourceInfoImplementation(ODataNestedResourceInfo nestedResourceInfo)
at Microsoft.OData.ODataWriterCore.<>c.<WriteStartAsync>b__64_0(ODataWriterCore thisParam, ODataNestedResourceInfo nestedResourceInfoParam)
at Microsoft.OData.TaskUtils.GetTaskForSynchronousOperation[TArg1,TArg2](Action`2 synchronousOperation, TArg1 arg1, TArg2 arg2)
--- End of stack trace from previous location ---
at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteExpandedNavigationPropertiesAsync(SelectExpandNode selectExpandNode, ResourceContext resourceContext, ODataWriter writer) in C:\Users\clhabins\source\repos\WebApi\src\Microsoft.AspNet.OData.Shared\Formatter\Serialization\ODataResourceSerializer.cs:line 1889
at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteResourceAsync(Object graph, ODataWriter writer, ODataSerializerContext writeContext, IEdmTypeReference expectedType) in C:\Users\clhabins\source\repos\WebApi\src\Microsoft.AspNet.OData.Shared\Formatter\Serialization\ODataResourceSerializer.cs:line 1497
at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteObjectAsync(Object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext) in C:\Users\clhabins\source\repos\WebApi\src\Microsoft.AspNet.OData.Shared\Formatter\Serialization\ODataResourceSerializer.cs:line 81
at Microsoft.AspNet.OData.Formatter.ODataOutputFormatterHelper.WriteToStreamAsync(Type type, Object value, IEdmModel model, ODataVersion version, Uri baseAddress, MediaTypeHeaderValue contentType, IWebApiUrlHelper internaUrlHelper, IWebApiRequestMessage internalRequest, IWebApiHeaders internalRequestHeaders, Func`2 getODataMessageWrapper, Func`2 getEdmTypeSerializer, Func`2 getODataPayloadSerializer, Func`1 getODataSerializerContext) in C:\Users\clhabins\source\repos\WebApi\src\Microsoft.AspNet.OData.Shared\Formatter\ODataOutputFormatterHelper.cs:line 229
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
...
Reproducing the error
Here's a link to a sample repro project I created on a fork of this repo to make it easier to demonstrate the error.
Run the project then execute the following request and check the debug output logs to see the error
GET http://localhost:5287/odata/GetAppUsage()/App1?$expand=KeyCredentials
Error details and root cause
The error is thrown by ODataWriterCore.EnterScope in ODL when serializing an expanded navigation property when the navigation property is contained and the OData path is empty (has no segments):
case EdmNavigationSourceKind.ContainedEntitySet:
// Containment cannot be written alone without odata uri.
if (!odataPath.Any())
{
throw new ODataException(SRResources.ODataWriterCore_PathInODataUriMustBeSetWhenWritingContainedElement);
}But why is the ODataPath empty at this point despite that our request url definitely has path segments?
Well, it turns out the issue is that WebAPI 7.x has a different ODataPath type from ODL's ODataPath. And when it starts serialization, it converts its ODataPath to ODL's ODataPath. During the conversion, it sets the path to null if it contains an operation segment:
// This function is used to determine whether an OData path includes operation (import) path segments.
// We use this function to make sure the value of ODataUri.Path in ODataMessageWriterSettings is null
// when any path segment is an operation. ODL will try to calculate the context URL if the ODataUri.Path
// equals to null.
private static bool IsOperationPath(ODataPath path)
{
// omitted for brevity...
}
private static Microsoft.OData.UriParser.ODataPath ConvertPath(ODataPath path)
{
if (path == null)
{
return null;
}
if (IsOperationPath(path))
{
var lastSegment = path.Segments.Last();
OperationSegment operation = lastSegment as OperationSegment;
if (operation != null && operation.EntitySet != null)
{
return GeneratePath(operation.EntitySet);
}
OperationImportSegment operationImport = lastSegment as OperationImportSegment;
if (operationImport != null && operationImport.EntitySet != null)
{
return GeneratePath(operationImport.EntitySet);
}
return null;
}
return path.Path;
}Despite the justification in the comments, I don't understand why this is necessary. If you comment out this code, the request returns successfully.
Furthermore, AspNetCoreOData 8+ does not this, it doesn't define a separate ODataPath type, and it doesn't do any conversion. It just sends the path as is to the writer, including the operation segment.
Why was this approach taken in WebAPI 7? Is there any unwanted side-effects that would occur if this the offending code were to be simply removed?