Adding condition :min(1) to route parameter breaks OData routing

[AndrewZenith]

Assemblies affected
.NET 9.0
Microsoft.AspNetCore.OData 9.1.0
Microsoft.OData.Core 8.21

Describe the bug
[ApiExplorerSettings(GroupName = "OData")]
[ODataRouteComponent("global")]
[Route("/odata/global")]
public class SystemDataTypeController(GlobalDbContext _db) : ODataController
{
[EnableQuery]
[HttpGet("SystemDataType")]
public IQueryable Get()
{
return _db.SystemDataTypes.AsNoTracking();
}

[EnableQuery]
[HttpGet("SystemDataType/{id:int}")] // <--------------------------------- This works, but id:int:min(1) breaks the routing
[HttpGet("SystemDataType({id:int:min(1)})")] <--------------------------- This works, with or without condition
public SingleResult<SystemDataType> GetById([FromRoute] uint id)
{
    return SingleResult.Create(_db.SystemDataTypes.Where(x => x.Id == id).AsNoTracking());
}

[julealgon]

Can you elaborate what exactly do you mean by "breaks OData routing"? Does it throw an exception when trying to access the endpoint? What is the status code?

Can you share the output of the $odata route debug endpoint with and without the constraint added?

[AndrewZenith]

The endpoint is pushed into the Non-OData Endpoint Mappings in the :min(1) case

The following is seen in the console:

'odata/global/SystemDataType/{id:int:min(1)}' on the action 'GetById' in controller 'SystemDataType' is not a valid OData path template. Bad Request - Error in query syntax.

[julealgon]

Ok, I can repro too. Definitely a bug, apparently caused by the presence of the parenthesis in the constraint, considering something like this still works fine: {id:int:required}.

Even with a single constraint it also triggers the problem, so {id:min(1)} is enough.

This feels to me like an opportunity to unify code between OData and ASPNET Core itself. Apparently, OData is not properly parsing Route Constraints here, meaning it is not using strong-enough abstractions to read the route template string.

Error log is coming from here, on the attribute routing convention logic:

AspNetCoreOData/src/Microsoft.AspNetCore.OData/Routing/Conventions/AttributeRoutingConvention.cs

Lines 159 to 205 in 9a0c7db

	private SelectorModel CreateActionSelectorModel(string prefix, IEdmModel model, IServiceProvider sp,
	string routeTemplate, SelectorModel actionSelectorModel,
	string originalTemplate, string actionName, string controllerName, int? order)
	{
	try
	{
	// Due the uri parser, it will throw exception if the route template is not a OData path.
	ODataPathTemplate pathTemplate = _templateParser.Parse(model, routeTemplate, sp);
	if (pathTemplate != null)
	{
	// Create a new selector model?
	SelectorModel newSelectorModel = new SelectorModel(actionSelectorModel);
	// Shall we remove any certain attributes/metadata?
	ClearMetadata(newSelectorModel);
	// Add OData routing metadata
	ODataRoutingMetadata odataMetadata = new ODataRoutingMetadata(prefix, model, pathTemplate)
	{
	IsConventional = false
	};
	newSelectorModel.EndpointMetadata.Add(odataMetadata);
	// replace the attribute routing template using absolute routing template to avoid appending any controller route template
	newSelectorModel.AttributeRouteModel = new AttributeRouteModel()
	{
	Template = $"/{originalTemplate}", // add a "/" to make sure it's absolute template, don't combine with controller
	Order = order
	};
	return newSelectorModel;
	}
	return null;
	}
	catch (ODataException ex)
	{
	// use the logger to log the wrong odata attribute template. Shall we log the others?
	string warning = string.Format(CultureInfo.CurrentCulture, SRResources.InvalidODataRouteOnAction,
	originalTemplate, actionName, controllerName, ex.Message);
	// Whether we throw exception or mark it as warning is a design pattern.
	// throw new ODataException(warning);
	_logger.LogWarning(warning);
	return null;
	}
	}

But the actual exception is in Microsoft.OData.Core in ODataUriParser:
https://github.com/OData/odata.net/blob/be48a19940bcded91bae4db83b79ec9a50312382/src/Microsoft.OData.Core/UriParser/ODataUriParser.cs#L287-L291