Skip to content
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.

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

Resolve ContentID in odata.bind annotation #643

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Serialization;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.OData.Deltas;
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Extensions;
using Microsoft.AspNetCore.OData.Formatter.Value;
using Microsoft.AspNetCore.OData.Formatter.Wrapper;
using Microsoft.AspNetCore.OData.Routing;
using Microsoft.AspNetCore.OData.Routing.Parser;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.UriParser;
Expand All @@ -33,6 +36,8 @@ namespace Microsoft.AspNetCore.OData.Formatter.Deserialization
/// </summary>
public class ODataResourceDeserializer : ODataEdmTypeDeserializer
{
private static readonly Regex ContentIdReferencePattern = new Regex(@"\$\d", RegexOptions.Compiled);

/// <summary>
/// Initializes a new instance of the <see cref="ODataResourceDeserializer"/> class.
/// </summary>
Expand Down Expand Up @@ -376,7 +381,7 @@ public virtual void ApplyNestedProperty(object resource, ODataNestedResourceInfo
}

IList<ODataItemWrapper> nestedItems;
var referenceLinks = resourceInfoWrapper.NestedItems.OfType<ODataEntityReferenceLinkWrapper>().ToArray();
ODataEntityReferenceLinkWrapper[] referenceLinks = resourceInfoWrapper.NestedItems.OfType<ODataEntityReferenceLinkWrapper>().ToArray();
if (referenceLinks.Length > 0)
{
// Be noted:
Expand Down Expand Up @@ -578,7 +583,7 @@ private object ReadNestedResourceInline(ODataResourceWrapper resourceWrapper, IE

IEdmStructuredTypeReference structuredType = edmType.AsStructured();

var nestedReadContext = new ODataDeserializerContext
ODataDeserializerContext nestedReadContext = new ODataDeserializerContext
{
Path = readContext.Path,
Model = readContext.Model,
Expand Down Expand Up @@ -797,7 +802,7 @@ private static ODataResourceWrapper CreateResourceWrapper(IEdmTypeReference edmP
resource.Properties = CreateKeyProperties(refLink.EntityReferenceLink.Url, readContext) ?? Array.Empty<ODataProperty>();
ODataResourceWrapper resourceWrapper = new ODataResourceWrapper(resource);

foreach (var instanceAnnotation in refLink.EntityReferenceLink.InstanceAnnotations)
foreach (ODataInstanceAnnotation instanceAnnotation in refLink.EntityReferenceLink.InstanceAnnotations)
{
resource.InstanceAnnotations.Add(instanceAnnotation);
}
Expand Down Expand Up @@ -835,7 +840,7 @@ private ODataResourceWrapper UpdateResourceWrapper(ODataResourceWrapper resource
else
{
IDictionary<string, ODataProperty> newPropertiesDic = resourceWrapper.Resource.Properties.ToDictionary(p => p.Name, p => p);
foreach (var key in keys)
foreach (ODataProperty key in keys)
{
// Logic: if we have the key property, try to keep the key property and get rid of the key value from ID.
// Need to double confirm whether it is the right logic?
Expand Down Expand Up @@ -870,26 +875,35 @@ private static IList<ODataProperty> CreateKeyProperties(Uri id, ODataDeserialize

try
{
Uri serviceRootUri = null;
if (id.IsAbsoluteUri)
IEdmModel model = readContext.Model;
HttpRequest request = readContext.Request;
IServiceProvider requestContainer = request.GetRouteServices();
Uri resolvedId = id;

string idOriginalString = id.OriginalString;
if (ContentIdReferencePattern.IsMatch(idOriginalString))
Copy link
Contributor

@ElizabethOkerio ElizabethOkerio Jul 25, 2022

Choose a reason for hiding this comment

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

is this Regex Matching? Is it possible to use substring or indexOf to do the matching instead of Regex. I remember some discussions where this Regex was being discouraged because of performance reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ElizabethOkerio We use similar logic in ODataMessageWrapper class. We're looking for a very particular pattern here - a $ followed by an integer. I think using Substring and Index would result into more complex logic

Copy link
Contributor

Choose a reason for hiding this comment

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

On an unrelated note, let's avoid substring in favor Span<char>.Slice() where possible, to avoid string allocations.

{
string serviceRoot = readContext.Request.CreateODataLink();
serviceRootUri = new Uri(serviceRoot, UriKind.Absolute);
// We can expect request.ODataBatchFeature() to not be null
string resolvedUri = ContentIdHelpers.ResolveContentId(
idOriginalString,
request.ODataBatchFeature().ContentIdMapping);
resolvedId = new Uri(resolvedUri, UriKind.RelativeOrAbsolute);
}

var request = readContext.Request;
IEdmModel model = readContext.Model;

// TODO: shall we use the DI to inject the path parser?
DefaultODataPathParser pathParser = new DefaultODataPathParser();
Uri serviceRootUri = new Uri(request.CreateODataLink());
IODataPathParser pathParser = requestContainer?.GetService<IODataPathParser>();
if (pathParser == null) // Seem like IODataPathParser is NOT injected into DI container by default
habbes marked this conversation as resolved.
Show resolved Hide resolved
{
pathParser = new DefaultODataPathParser();
}

IList<ODataProperty> properties = null;
var path = pathParser.Parse(model, serviceRootUri, id, request.GetRouteServices());
ODataPath path = pathParser.Parse(model, serviceRootUri, resolvedId, requestContainer);
KeySegment keySegment = path.OfType<KeySegment>().LastOrDefault();
if (keySegment != null)
{
properties = new List<ODataProperty>();
foreach (var key in keySegment.Keys)
foreach (KeyValuePair<string, object> key in keySegment.Keys)
{
properties.Add(new ODataProperty
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
//-----------------------------------------------------------------------------
// <copyright file="ContentIdToLocationMappingTests.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.OData.Batch;
using Microsoft.AspNetCore.OData.E2E.Tests.Commons;
using Microsoft.AspNetCore.OData.Routing.Controllers;
using Microsoft.AspNetCore.OData.TestCommon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;
using Xunit;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Batch
{
public class ContentIdToLocationMappingTests : WebApiTestBase<ContentIdToLocationMappingTests>
{
private static IEdmModel edmModel;

public ContentIdToLocationMappingTests(WebApiTestFixture<ContentIdToLocationMappingTests> fixture)
: base(fixture)
{
}

protected static void UpdateConfigureServices(IServiceCollection services)
{
services.ConfigureControllers(
typeof(ContentIdToLocationMappingParentsController),
typeof(ContentIdToLocationMappingChildrenController));

edmModel = GetEdmModel();
services.AddControllers().AddOData(opt =>
{
opt.EnableQueryFeatures();
opt.EnableContinueOnErrorHeader = true;
opt.AddRouteComponents("ContentIdToLocationMapping", edmModel, new DefaultODataBatchHandler());
});
}

protected static void UpdateConfigure(IApplicationBuilder app)
{
app.UseODataBatching();
app.UseRouting();
app.UseEndpoints(endpoints =>
{
endpoints.MapControllers();
});
}

protected static IEdmModel GetEdmModel()
{
ODataModelBuilder builder = new ODataConventionModelBuilder();
builder.EntitySet<ContentIdToLocationMappingParent>("ContentIdToLocationMappingParents");
builder.EntitySet<ContentIdToLocationMappingChild>("ContentIdToLocationMappingChildren");
builder.Namespace = typeof(ContentIdToLocationMappingParent).Namespace;

return builder.GetEdmModel();
}

[Fact]
public async Task CanResolveContentIdInODataBindAnnotationAsync()
{
// Arrange
HttpClient client = CreateClient();
string serviceBase = $"{client.BaseAddress}ContentIdToLocationMapping";
string requestUri = $"{serviceBase}/$batch";
string parentsUri = $"{serviceBase}/ContentIdToLocationMappingParents";
string childrenUri = $"{serviceBase}/ContentIdToLocationMappingChildren";
string payload = "{" +
" \"requests\": [" +
" {" +
" \"id\": \"1\"," +
" \"method\": \"POST\"," +
$" \"url\": \"{parentsUri}\"," +
" \"headers\": {" +
" \"OData-Version\": \"4.0\"," +
" \"Content-Type\": \"application/json;odata.metadata=minimal\"," +
" \"Accept\": \"application/json;odata.metadata=minimal\"" +
" }," +
" \"body\": {\"ParentId\":123}" +
" }," +
" {" +
" \"id\": \"2\"," +
" \"method\": \"POST\"," +
$" \"url\": \"{childrenUri}\"," +
" \"headers\": {" +
" \"OData-Version\": \"4.0\"," +
" \"Content-Type\": \"application/json;odata.metadata=minimal\"," +
" \"Accept\": \"application/json;odata.metadata=minimal\"" +
" }," +
" \"body\": {" +
" \"Parent@odata.bind\": \"$1\"" +
" }" +
" }" +
" ]" +
"}";

// Act
HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Post, requestUri);
request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json"));
request.Content = new StringContent(payload);
request.Content.Headers.ContentType = MediaTypeHeaderValue.Parse("application/json");
HttpResponseMessage response = await client.SendAsync(request);

// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);

var stream = await response.Content.ReadAsStreamAsync();
IODataResponseMessage odataResponseMessage = new ODataMessageWrapper(stream, response.Content.Headers);
int subResponseCount = 0;
using (var messageReader = new ODataMessageReader(odataResponseMessage, new ODataMessageReaderSettings(), edmModel))
{
var batchReader = messageReader.CreateODataBatchReader();
while (batchReader.Read())
{
switch (batchReader.State)
{
case ODataBatchReaderState.Operation:
var operationMessage = batchReader.CreateOperationResponseMessage();
subResponseCount++;
Assert.Equal(201, operationMessage.StatusCode);
break;
}
}
}

// NOTE: We assert that $1 is successfully resolved from the controller action
Assert.Equal(2, subResponseCount);
}
}

public class ContentIdToLocationMappingParentsController : ODataController
{
public ActionResult Post([FromBody] ContentIdToLocationMappingParent parent)
{
return Created(new Uri($"{Request.Scheme}://{Request.Host}{Request.Path}/{parent.ParentId}"), parent);
}
}

public class ContentIdToLocationMappingChildrenController : ODataController
{
public ActionResult Post([FromBody] ContentIdToLocationMappingChild child)
{
Assert.Equal(123, child.Parent.ParentId);
habbes marked this conversation as resolved.
Show resolved Hide resolved

return Created(new Uri($"{Request.Scheme}://{Request.Host}{Request.Path}/{child.ChildId}"), child);
}
}

public class ContentIdToLocationMappingParent
{
public ContentIdToLocationMappingParent()
{
Children = new HashSet<ContentIdToLocationMappingChild>();
}

[Key]
public int ParentId
{
get; set;
}

public virtual ICollection<ContentIdToLocationMappingChild> Children
{
get; set;
}
}

public class ContentIdToLocationMappingChild
{
[Key]
public int ChildId
{
get; set;
}

public int? ParentId
{
get; set;
}

public virtual ContentIdToLocationMappingParent Parent
{
get; set;
}
}
}