Skip to content

Commit

Permalink
Let's log the invalid key or function parameter if we have the logger…
Browse files Browse the repository at this point in the history
… enabled in the TryMatch. Otherwise, keep to throw exception.
  • Loading branch information
xuzhg committed Mar 1, 2024
1 parent 6dc9066 commit 5372813
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 23 deletions.
30 changes: 16 additions & 14 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1102,20 +1102,6 @@
<param name="clrType">The type to test.</param>
<returns>True if the type is a DateTime; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsDateOnly(System.Type)">
<summary>
Determine if a type is a <see cref="T:System.DateOnly"/>.
</summary>
<param name="clrType">The type to test.</param>
<returns>True if the type is a DateOnly; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsTimeOnly(System.Type)">
<summary>
Determine if a type is a <see cref="T:System.TimeOnly"/>.
</summary>
<param name="clrType">The type to test.</param>
<returns>True if the type is a TimeOnly; false otherwise.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Common.TypeHelper.IsTimeSpan(System.Type)">
<summary>
Determine if a type is a TimeSpan.
Expand Down Expand Up @@ -15593,3 +15579,19 @@
</member>
</members>
</doc>
ummary>
<param name="segment">The value segment.</param>
</member>
<member name="P:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.Segment">
<summary>
Gets the value segment.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.GetTemplates(Microsoft.AspNetCore.OData.Routing.ODataRouteOptions)">
<inheritdoc />
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Template.ValueSegmentTemplate.TryTranslate(Microsoft.AspNetCore.OData.Routing.Template.ODataTemplateTranslateContext)">
<inheritdoc />
</member>
</members>
</doc>
6 changes: 0 additions & 6 deletions src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ abstract Microsoft.AspNetCore.OData.Formatter.MediaType.MediaTypeMapping.TryMatc
abstract Microsoft.AspNetCore.OData.Formatter.MediaType.ODataRawValueMediaTypeMapping.IsMatch(Microsoft.OData.UriParser.PropertySegment propertySegment) -> bool
abstract Microsoft.AspNetCore.OData.Formatter.Value.EdmDeltaLinkBase.Kind.get -> Microsoft.AspNetCore.OData.Deltas.DeltaItemKind
abstract Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.Bind(Microsoft.OData.UriParser.QueryNode node) -> System.Linq.Expressions.Expression
abstract Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.Parameter.get -> System.Linq.Expressions.ParameterExpression
abstract Microsoft.AspNetCore.OData.Query.SkipTokenHandler.ApplyTo(System.Linq.IQueryable query, Microsoft.AspNetCore.OData.Query.SkipTokenQueryOption skipTokenQueryOption, Microsoft.AspNetCore.OData.Query.ODataQuerySettings querySettings, Microsoft.AspNetCore.OData.Query.ODataQueryOptions queryOptions) -> System.Linq.IQueryable
abstract Microsoft.AspNetCore.OData.Query.SkipTokenHandler.ApplyTo<T>(System.Linq.IQueryable<T> query, Microsoft.AspNetCore.OData.Query.SkipTokenQueryOption skipTokenQueryOption, Microsoft.AspNetCore.OData.Query.ODataQuerySettings querySettings, Microsoft.AspNetCore.OData.Query.ODataQueryOptions queryOptions) -> System.Linq.IQueryable<T>
abstract Microsoft.AspNetCore.OData.Query.SkipTokenHandler.GenerateNextPageLink(System.Uri baseUri, int pageSize, object instance, Microsoft.AspNetCore.OData.Formatter.Serialization.ODataSerializerContext context) -> System.Uri
Expand Down Expand Up @@ -891,11 +890,6 @@ Microsoft.AspNetCore.OData.Query.ETag<TEntity>.ApplyTo(System.Linq.IQueryable<TE
Microsoft.AspNetCore.OData.Query.ETag<TEntity>.ETag() -> void
Microsoft.AspNetCore.OData.Query.Expressions.BinderExtensions
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.BindArguments(System.Collections.Generic.IEnumerable<Microsoft.OData.UriParser.QueryNode> nodes) -> System.Linq.Expressions.Expression[]
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.EnsureFlattenedPropertyContainer(System.Linq.Expressions.ParameterExpression source) -> void
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.ExpressionBinderBase(System.IServiceProvider requestContainer) -> void
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.GetDynamicPropertyContainer(Microsoft.OData.UriParser.SingleValueOpenPropertyAccessNode openNode) -> System.Reflection.PropertyInfo
Microsoft.AspNetCore.OData.Query.Expressions.ExpressionBinderBase.GetFlattenedPropertyExpression(string propertyPath) -> System.Linq.Expressions.Expression
Microsoft.AspNetCore.OData.Query.Expressions.FilterBinder
Microsoft.AspNetCore.OData.Query.Expressions.FilterBinder.FilterBinder() -> void
Microsoft.AspNetCore.OData.Query.Expressions.IFilterBinder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
// </copyright>
//------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Diagnostics.Contracts;
using System.Linq;
using Microsoft.AspNetCore.OData.Common;
using Microsoft.AspNetCore.OData.Formatter;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.UriParser;
Expand Down Expand Up @@ -204,7 +205,16 @@ public override bool TryTranslate(ODataTemplateTranslateContext context)
catch (ODataException ex)
{
string message = Error.Format(SRResources.InvalidKeyInUriFound, strValue, edmType.FullName());
throw new ODataException(message, ex);
ILoggerFactory loggerFactory = context.HttpContext?.RequestServices?.GetService<ILoggerFactory>();
if (loggerFactory != null)
{
loggerFactory.CreateLogger<KeySegmentTemplate>().LogError(message, ex);
return false;
}
else
{
throw new ODataException(message, ex);
}
}

// for non FromODataUri, so update it, for example, remove the single quote for string value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Formatter;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;
Expand Down Expand Up @@ -95,7 +97,16 @@ public static IList<OperationSegmentParameter> Match(ODataTemplateTranslateConte
catch (ODataException ex)
{
string message = Error.Format(SRResources.InvalidParameterValueInUriFound, originalStrValue, edmParameter.Type.FullName());
throw new ODataException(message, ex);
ILoggerFactory loggerFactory = context.HttpContext?.RequestServices?.GetService<ILoggerFactory>();
if (loggerFactory != null)
{
loggerFactory.CreateLogger("ODataFunctionParameterMatcher").LogError(message, ex);
return null;
}
else
{
throw new ODataException(message, ex);
}
}

// for without FromODataUri, so update it, for example, remove the single quote for string value.
Expand Down
159 changes: 159 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Routing/ODataRoutingTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
//-----------------------------------------------------------------------------
// <copyright file="ODataRoutingTests.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.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.OData.Routing.Controllers;
using Microsoft.AspNetCore.OData.TestCommon;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OData.ModelBuilder;
using Xunit;

namespace Microsoft.AspNetCore.OData.E2E.Tests.Routing
{
public class ODataRoutingTests : WebApiTestBase<ODataRoutingTests>
{
public ODataRoutingTests(WebApiTestFixture<ODataRoutingTests> fixture)
: base(fixture)
{
}

// following the Fixture convention.
protected static void UpdateConfigureServices(IServiceCollection services)
{
services.ConfigureControllers(typeof(ProductsController), typeof(CategoriesController));

ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
builder.EntitySet<Product>("Products");
builder.EntitySet<Category>("Categories");
var model = builder.GetEdmModel();

services.AddControllers().AddOData(opt => opt.AddRouteComponents("odata", model));
}

[Theory]
[InlineData("1", true, "\"Id\":1,\"Name\":\"OData Product\"}")]
[InlineData("11", true, "\"Id\":11,\"Name\":\"OData Product\"}")]
[InlineData("42", true, "\"Id\":42,\"Name\":\"OData Product\"}")]
[InlineData("other", false, "{\"id\":11,\"name\":\"Non-OData Product\"}")]
public async Task RequestContainsValidValues_CanRouteToCorrectEndpoint(string key, bool isODataRouting, string expect)
{
// Arrange
HttpClient client = CreateClient();

// Act
var response = await client.GetAsync($"/odata/products/{key}");

// Assert
response.EnsureSuccessStatusCode();

string payload = await response.Content.ReadAsStringAsync();
if (isODataRouting)
{
expect = $"{{\"@odata.context\":\"http://localhost/odata/$metadata#Products/$entity\",{expect}";
}

Assert.Equal(expect, payload);
}

[Theory]
[InlineData("1", true, "\"Id\":1,\"Name\":\"OData Category\"}")]
[InlineData("11", true, "\"Id\":11,\"Name\":\"OData Category\"}")]
[InlineData("42", true, "\"Id\":42,\"Name\":\"OData Category\"}")]
[InlineData("other", false, "{\"id\":11,\"name\":\"Non-OData Category\"}")]
public async Task RequestContainsValidValues_CanRouteToCorrectEndpoint_UsingDifferentOrder(string key, bool isODataRouting, string expect)
{
// Arrange
HttpClient client = CreateClient();

// Act
var response = await client.GetAsync($"/odata/categories/{key}");

// Assert
response.EnsureSuccessStatusCode();

string payload = await response.Content.ReadAsStringAsync();
if (isODataRouting)
{
expect = $"{{\"@odata.context\":\"http://localhost/odata/$metadata#Categories/$entity\",{expect}";
}

Assert.Equal(expect, payload);
}

[Theory]
[InlineData("odata/products/\"1\"")]
[InlineData("odata/products/\"other\"")]
[InlineData("odata/products/other1")]
[InlineData("odata/products/a1other")]
[InlineData("odata/categories/\"1\"")]
[InlineData("odata/categories/\"other\"")]
[InlineData("odata/categories/other1")]
[InlineData("odata/categories/a1other")]
public async Task RequestContainsInValidValues_CanRouteToCorrectEndpoint(string request)
{
// Arrange
HttpClient client = CreateClient();

// Act
var response = await client.GetAsync(request);

// Assert
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
}
}

public class ProductsController : ODataController
{
// This method will have the following routing:
// ~/odata/products({key})
// ~/odata/products/{key}
public IActionResult Get(int key)
{
return Ok(new Product { Id = key, Name = "OData Product" });
}

// Be NOTED: "GetOther" is added after Get() by test, don't change the order
[HttpGet("/odata/products/other")]
public IActionResult GetOther()
{
return Ok(new Product { Id = 11, Name = "Non-OData Product" });
}
}

public class CategoriesController : ODataController
{
// Be NOTED: "GetOther" is added before Get() by test, don't change the order
[HttpGet("/odata/categories/other")]
public IActionResult GetOther()
{
return Ok(new Category { Id = 11, Name = "Non-OData Category" });
}

// This method will have the following routing:
// ~/odata/categories({key})
// ~/odata/categories/{key}
public IActionResult Get(int key)
{
return Ok(new Category { Id = key, Name = "OData Category"});
}
}

public class Product
{
public int Id { get; set; }
public string Name { get; set; }
}

public class Category
{
public int Id { get; set; }
public string Name { get; set; }
}
}

0 comments on commit 5372813

Please sign in to comment.