Skip to content

Commit

Permalink
issue #568: Error when function/action name contains keyword "On" (#576)
Browse files Browse the repository at this point in the history 
* issue #568: Error when function/action name contains keyword "On"

* Add the comments

* Address the comments.

* Address the comments.
  • Loading branch information
@xuzhg
xuzhg authored Apr 27, 2022
1 parent cae9277 commit 14d327c
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 27 deletions.
3 changes: 2 additions & 1 deletion src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12427,13 +12427,14 @@
<param name="entityType">The Edm entity type.</param>
<param name="navigationSource">The Edm navigation source.</param>
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Conventions.OperationRoutingConvention.SplitActionName(System.String,System.String@,System.Boolean@)">
<member name="M:Microsoft.AspNetCore.OData.Routing.Conventions.OperationRoutingConvention.SplitActionName(System.String,System.String@,System.Boolean@,System.StringComparison)">
<summary>
Split the action based on supporting pattern.
</summary>
<param name="actionName">The input action name.</param>
<param name="cast">The out of cast type name.</param>
<param name="isOnCollection">The out of collection binding flag.</param>
<param name="comparison">The case comparision flag.</param>
<returns>The operation name.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Routing.Conventions.OperationRoutingConvention.IsOperationParameterMatched(Microsoft.OData.Edm.IEdmOperation,Microsoft.AspNetCore.Mvc.ApplicationModels.ActionModel)">
Expand Down
82 changes: 58 additions & 24 deletions src/Microsoft.AspNetCore.OData/Routing/Conventions/OperationRoutingConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,31 +65,17 @@ protected void ProcessOperations(ODataControllerActionContext context, IEdmEntit
return;
}

// OperationNameOnCollectionOfEntityType
string operationName = SplitActionName(actionName, out string cast, out bool isOnCollection);

bool isOnCollection = false;
IEdmEntityType castTypeFromActionName = null;
if (cast != null)
{
if (cast.Length == 0)
{
// Early return for the following cases:
// - {OperationName}On
// - {OperationName}OnCollectionOf
return;
}

castTypeFromActionName = entityType.FindTypeInInheritance(context.Model, cast, context.Options?.RouteOptions?.EnableActionNameCaseInsensitive == true) as IEdmEntityType;
if (castTypeFromActionName == null)
{
return;
}
IEdmOperation[] candidates = FindCandidates(context, actionName);
if (candidates.Length == 0)
{
// If we can't find any Edm operation using the action name directly,
// Let's split the action name and use part of it to search again.
candidates = FindCandidates(context, entityType, actionName, out castTypeFromActionName, out isOnCollection);
}

// TODO: refactor here
// If we have multiple same function defined, we should match the best one?
StringComparison actionNameComparison = context.Options?.RouteOptions?.EnableActionNameCaseInsensitive == true ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
IEnumerable<IEdmOperation> candidates = context.Model.SchemaElements.OfType<IEdmOperation>().Where(f => f.IsBound && f.Name.Equals(operationName, actionNameComparison));
foreach (IEdmOperation edmOperation in candidates)
{
IEdmOperationParameter bindingParameter = edmOperation.Parameters.FirstOrDefault();
Expand Down Expand Up @@ -172,14 +158,62 @@ protected void ProcessOperations(ODataControllerActionContext context, IEdmEntit
}
}

private static IEdmOperation[] FindCandidates(ODataControllerActionContext context, string operationName)
{
// TODO: refactor here
// If we have multiple same function defined, we should match the best one?

StringComparison actionNameComparison = context.Options?.RouteOptions?.EnableActionNameCaseInsensitive == true ?
StringComparison.OrdinalIgnoreCase :
StringComparison.Ordinal;

return context.Model.SchemaElements
.OfType<IEdmOperation>()
.Where(f => f.IsBound && f.Name.Equals(operationName, actionNameComparison))
.ToArray();
}

private static IEdmOperation[] FindCandidates(ODataControllerActionContext context, IEdmEntityType entityType, string actionName,
out IEdmEntityType castTypeFromActionName, out bool isOnCollection)
{
// OperationNameOnCollectionOfEntityType
StringComparison caseComparision = context.Options?.RouteOptions?.EnableActionNameCaseInsensitive == true ?
StringComparison.OrdinalIgnoreCase :
StringComparison.Ordinal;

string operationName = SplitActionName(actionName, out string cast, out isOnCollection, caseComparision);

castTypeFromActionName = null;
if (cast != null)
{
if (cast.Length == 0)
{
// Early return for the following cases:
// - {OperationName}On
// - {OperationName}OnCollectionOf
return Array.Empty<IEdmOperation>();
}

castTypeFromActionName = entityType.FindTypeInInheritance(context.Model, cast, context.Options?.RouteOptions?.EnableActionNameCaseInsensitive == true) as IEdmEntityType;
if (castTypeFromActionName == null)
{
return Array.Empty<IEdmOperation>();
}
}

return FindCandidates(context, operationName);
}

/// <summary>
/// Split the action based on supporting pattern.
/// </summary>
/// <param name="actionName">The input action name.</param>
/// <param name="cast">The out of cast type name.</param>
/// <param name="isOnCollection">The out of collection binding flag.</param>
/// <param name="comparison">The case comparision flag.</param>
/// <returns>The operation name.</returns>
internal static string SplitActionName(string actionName, out string cast, out bool isOnCollection)
internal static string SplitActionName(string actionName, out string cast, out bool isOnCollection,
StringComparison comparison = StringComparison.Ordinal)
{
Contract.Assert(actionName != null);

Expand All @@ -190,7 +224,7 @@ internal static string SplitActionName(string actionName, out string cast, out b
cast = null;
isOnCollection = false;
string operation;
int index = actionName.IndexOf("OnCollectionOf", StringComparison.Ordinal);
int index = actionName.LastIndexOf("OnCollectionOf", comparison);
if (index > 0)
{
operation = actionName.Substring(0, index);
Expand All @@ -199,7 +233,7 @@ internal static string SplitActionName(string actionName, out string cast, out b
return operation;
}

index = actionName.IndexOf("On", StringComparison.Ordinal);
index = actionName.LastIndexOf("On", comparison);
if (index > 0)
{
operation = actionName.Substring(0, index);
Expand Down
62 changes: 60 additions & 2 deletions test/Microsoft.AspNetCore.OData.Tests/Routing/Conventions/FunctionRoutingConventionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,47 @@ public static TheoryDataSet<Type, string, string[]> FunctionRoutingConventionTes
"/Customers/NS.GetWholeSalary(minSalary={minSalary},maxSalary={maxSalary},aveSalary={aveSalary})",
"/Customers/GetWholeSalary(minSalary={minSalary},maxSalary={maxSalary},aveSalary={aveSalary})"
}
},
{
typeof(CustomersController),
"GetStatusOnLineOfflineUser",
new[]
{
"/Customers/NS.GetStatusOnLineOfflineUser()",
"/Customers/GetStatusOnLineOfflineUser()"
}
},
{
typeof(CustomersController),
"GetStatusOnLineOfflineUser",
new[]
{
"/Customers/NS.GetStatusOnLineOfflineUser()",
"/Customers/GetStatusOnLineOfflineUser()"
}
},
{
typeof(CustomersController),
"StatusLineOfflineUserOn",
new[]
{
"/Customers/NS.StatusLineOfflineUserOn()",
"/Customers/StatusLineOfflineUserOn()"
}
},
{
typeof(CustomersController),
"GetStatusOnLineOfflineUserOnVipCustomer",
new[]
{
"/Customers/NS.VipCustomer/NS.GetStatusOnLineOfflineUser(param={param})",
"/Customers/NS.VipCustomer/GetStatusOnLineOfflineUser(param={param})"
}
}
};
}
}
}

public static TheoryDataSet<Type, string, string[]> FunctionRoutingConventionCaseInsensitiveTestData
{
get
Expand Down Expand Up @@ -448,6 +484,19 @@ private static IEdmModel GetEdmModel()
getSalaray.AddOptionalParameter("aveSalary", intType, "129");
model.AddElement(getSalaray);

EdmFunction f = new EdmFunction("NS", "GetStatusOnLineOfflineUser", intType, isBound: true, entitySetPathExpression: null, isComposable: false);
f.AddParameter("entityset", new EdmCollectionTypeReference(new EdmCollectionType(new EdmEntityTypeReference(customer, false))));
model.AddElement(f);

EdmFunction f2 = new EdmFunction("NS", "StatusLineOfflineUserOn", intType, isBound: true, entitySetPathExpression: null, isComposable: false);
f2.AddParameter("entityset", new EdmCollectionTypeReference(new EdmCollectionType(new EdmEntityTypeReference(customer, false))));
model.AddElement(f2);

EdmFunction f3 = new EdmFunction("NS", "GetStatusOnLineOfflineUser", intType, isBound: true, entitySetPathExpression: null, isComposable: false);
f3.AddParameter("entityset", new EdmCollectionTypeReference(new EdmCollectionType(new EdmEntityTypeReference(vipCustomer, false))));
f3.AddParameter("param", intType);
model.AddElement(f3);

EdmEntityContainer container = new EdmEntityContainer("NS", "Default");
container.AddEntitySet("Customers", customer);
container.AddEntitySet("CustomersCaseInsensitive", customer);
Expand All @@ -461,6 +510,15 @@ private class CustomersController
public void Get()
{ }

[HttpGet]
public void GetStatusOnLineOfflineUser() { }

[HttpGet]
public void GetStatusOnLineOfflineUserOnVipCustomer(int param) { }

[HttpGet]
public void StatusLineOfflineUserOn() { }

[HttpGet]
public void IsBaseUpgraded(int key, CancellationToken cancellation)
{ }
Expand Down

0 comments on commit 14d327c

Please sign in to comment.