Skip to content

issue #568: Error when function/action name contains keyword "On" #576

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

Merged
xuzhg merged 4 commits into from
Apr 27, 2022
Merged

issue #568: Error when function/action name contains keyword "On" #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f78a4e2
issue #568: Error when function/action name contains keyword "On"
xuzhg Apr 22, 2022
2dd6f26
Add the comments
xuzhg Apr 25, 2022
a7e8ada
Address the comments.
xuzhg Apr 26, 2022
7803404
Address the comments.
xuzhg Apr 27, 2022
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
3 changes: 2 additions & 1 deletion src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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