Webapi: AmbiguousActionException when submitting BatchWithSingleChangeset and versioning enabled

[e27182]

When submitting batch with SaveChangesAsync(SaveChangesOptions.BatchWithSingleChangeset) that contains 2+ operations (different controllers\actions) AND versioning support is enabled too, only first operation succeeds. Second one will hang with AmbiguousActionException thrown from Microsoft.AspNetCore.Mvc.Routing.DefaultApiVersionRoutePolicy.AmbiguousActionException.

Assemblies affected

*Microsoft.AspNetCore.OData, Version=7.1.0.21120

Reproduce steps

1. Call batch operation with BatchWithSingleChangeset, that contains modifications in 2+ different entity sets

Expected result

For all batch operations appropriate controller actions (one for each operation) selected by routing and called.

Actual result

AmbiguousActionException when batch switched to 2nd operation.

Additional detail

Root cause

ApiVersioningFeature instance added by the ApiVersioningMiddleware.cs gets copied by the CreateHttpContext(...) to every new copy of HttpContext created for each batch operation. Then, when 1st operation get's into processing, routing adds to the list of ApiVersioningFeature.SelectionResult.CandidateActions matched action and successfully executes it.
After that 2nd operation processing begins, again goes through routing and it adds new item to the list of ApiVersioningFeature.SelectionResult.CandidateActions. So now there are 2 items in there, as both copies of HttpContext shares the same instance of ApiVersioningFeature. And finally we end up in DefaultApiVersionRoutePolicy.OnMultipleMatches that throws exception.

Possible Fix

But it requires reference to Microsoft.AspNetCore.Mvc.Versioning:
on https://github.com/OData/WebApi/blob/master/src/Microsoft.AspNetCore.OData/Batch/ODataBatchReaderExtensions.cs#L193 add check:

kvp.Key == typeof(IApiVersioningFeature)

then on https://github.com/OData/WebApi/blob/master/src/Microsoft.AspNetCore.OData/Batch/ODataBatchReaderExtensions.cs#L224 add new instance:

context.Features.Set( new ApiVersioningFeature(context) );

Workarounds:

1. Custom batch handler (useful especially when you want to add extra functionality, like implicit transaction scope):

    public class TransactionedODataBatchHandler : DefaultODataBatchHandler
    {
        public override async Task ProcessBatchAsync(HttpContext context, RequestDelegate nextHandler)
        {
            using (TransactionScope scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
            {
                await base.ProcessBatchAsync(context, nextHandler);
                scope.Complete();
            }
        }

        public override async Task<IList<ODataBatchRequestItem>> ParseBatchRequestsAsync(HttpContext context)
        {
            var result = await base.ParseBatchRequestsAsync(context);

            foreach (var item in result)
            {
                if (item is ChangeSetRequestItem changeSetRequestItem)
                {
                    foreach (var httpContext in changeSetRequestItem.Contexts)
                    {
                        httpContext.Features.Set<IApiVersioningFeature>(new ApiVersioningFeature(httpContext));
                    }
                }
            }
            return result;
        }
    }

... then in Startup.cs, Configure method:

builder.MapVersionedODataRoutes("odata", "odata/v{version:apiVersion}", models, () => new TransactionedODataBatchHandler());

1. Custom ApiVersionRoutePolicy:

    public class CustomApiVersionRoutePolicy : DefaultApiVersionRoutePolicy
    {
        private const ActionDescriptor NoMatch = default(ActionDescriptor);

        public CustomApiVersionRoutePolicy(IErrorResponseProvider errorResponseProvider, IReportApiVersions reportApiVersions,
            ILoggerFactory loggerFactory, IOptions<ApiVersioningOptions> options)
            : base(errorResponseProvider, reportApiVersions, loggerFactory, options)
        {
        }

        public override ActionDescriptor Evaluate(RouteContext context, ActionSelectionResult selectionResult)
        {
            if (context == null)
                throw new ArgumentNullException(nameof(context));

            if (selectionResult == null)
                throw new ArgumentNullException(nameof(selectionResult));

            switch (selectionResult.MatchingActions.Count)
            {
                case 0:
                    OnUnmatched(context, selectionResult);
                    return NoMatch;

                case 1:
                    return OnSingleMatch(context, selectionResult);

                default:
                    return OnMultipleMatches(context, selectionResult);
            }
        }

        protected new virtual ActionDescriptor OnMultipleMatches(RouteContext context, ActionSelectionResult selectionResult)
        {
            if (context == null)
                throw new ArgumentNullException(nameof(context));

            if (selectionResult == null)
                throw new ArgumentNullException(nameof(selectionResult));

            var odataPath = (string)context.RouteData.Values["odatapath"];
            var bestAction = selectionResult.MatchingActions.SingleOrDefault(x => x.RouteValues["controller"] == odataPath);

            if (bestAction != null)
                return bestAction;

            base.OnMultipleMatches(context, selectionResult);

            return NoMatch;
        }
    }

... then in Startup.cs, ConfigureServices method:

services.AddSingleton<IApiVersionRoutePolicy, CustomApiVersionRoutePolicy>();

Personally I prefer 1st approach, as it looks like less intrusive and contains less custom logic.

[KanishManuja-MS]

@e27182 Thanks for the investigation.

[commonsensesoftware]

After my own investigation on the API Versioning side of things, the issue described is a result of the feature not being copied. This issue isn't specific to API versioning. This could happen to any middleware that registers _features_. In this case, the features are not copied over to the batch request. In fairness, there isn't a simple way to do this since each feature is request-specific and needs to be cloned for each new HttpContext instance.

Solution

API Versioning auto-registers its middleware so you do not have to worry about it. In this particular case, you need to opt out of that behavior and register things manually.

Step 1

Disable auto-registration of the API Versioning middleware. This currently doesn't do anything, but add the IApiVersioningFeature.

Change your setup as follows:

```c#
services.AddApiVersioning( options => options.RegisterMiddleware = false );

### Step 2
Register the API Versioning middleware **after** the OData batching middleware.

Change your setup to:

```c#
public void Configure( IApplicationBuilder app, VersionedODataModelBuilder modelBuilder )
{
    app.UseODataBatching();
    app.UseApiVersioning();
    app.UseMvc(
        routeBuilder =>
        {
            var models = modelBuilder.GetEdmModels();
            Func<ODataBatchHandler> batchHandlerFactory = () => new DefaultODataBatchHandler();
            routeBuilder.MapVersionedODataRoutes( "odata", "api", models, batchHandlerFactory );
        } );
}

Step 3

Run your app and enjoy!

---

The OData team may find a more elegant way to make this transparent to service authors, but for now, at least there is a solution.