Test Failures on Autofac 4.0.1 or Later about autofac.webapi HOT 8 CLOSED

As we update the tests, make sure there's no order dependency. That's actually a flaw in how ASP.NET Core ended up getting designed - instead of ordering by a property on the object in the IEnumerable they came to depend on the order of things being resolved. If we can fix the tests in such a way that the tests pass regardless of resolution order, that'd be best.

from autofac.webapi.

I might be able to do that, but it's going to be pretty difficult because all of the filter ordering tests (which are checking everything runs in the right order and sequence for each stage) are entirely dependent upon a predictable order.

I guess I could match a manually resolved set against the executed filters for order, but personally that feels pretty messy and it's going to be quite a bit of code.

from autofac.webapi.

As an example, this test:

            builder.RegisterType<TestActionFilter>()
                .AsWebApiActionFilterFor<TestController>(c => c.Get())
                .InstancePerRequest()
                .GetMetadata(out var testFilter1Meta);
            builder.RegisterType<TestActionFilter2>()
                .AsWebApiActionFilterFor<TestController>(c => c.Get())
                .InstancePerRequest()
                .GetMetadata(out var testFilter2Meta);

            // some other orchestration

            Assert.Equal("TestActionFilter.OnActionExecutingAsync", order[0]);
            Assert.Equal("TestActionFilter2.OnActionExecutingAsync", order[1]);
            Assert.Equal("TestActionFilter2.OnActionExecutedAsync", order[2]);
            Assert.Equal("TestActionFilter.OnActionExecutedAsync", order[3]);

I'm not sure how to make it non-order dependent without perhaps manually resolving the filters directly and using that to build the set of strings?

from autofac.webapi.

Let me step back a bit. If you update to Autofac 4.0.1 or later, does that change the behavior of this library? That is, under Autofac 4.0.0 you see one filter order; under 4.0.1 you see a totally different filter order?

If that's the case, it sounds like part of this may be issuing a major version that forces Autofac 4.0.1 or later because that's a breaking behavior change.

If that's not the case and the behavior of the Web API integration remains stable, then let me be slightly more clear about what I meant - I understand that there is a need for filters to run in a particular order and that tests such as the above are necessary. I'm trying to avoid fragile tests where we register a bunch of stuff, resolve those things, and just assume that the order is otherwise guaranteed. If there is a way to do that, great. If not, it's unfortunate, but it is what it is.

from autofac.webapi.

The effective filter execution order after 4.0.1 is now reversed, but the integration itself doesn't depend on the order to function correctly (in other words the behaviour of the library is still correct). It's only the tests that depend on that order.

In the wild, people who use this integration with Autofac 4.0.1+ installed would have to register their filters in the order they want them executed; anyone using a version prior to 4.0.1 would have to register them in the reverse order of how they want them executed.

I'm genuinely not sure whether upping the dependent version of Autofac in the nuspec constitutes a breaking change for the library, since no-one post 4.0.1 would see any change? It feels strange to up it by a major version when there haven't been any changes...

One other option might be to just up the version we test against, but leave the nuspec the same? That also doesn't feel quite right.

I've had a go at changing a test to not hardcode the expected resolution order, but it makes it much harder to understand what the test is doing (and this is one of the simpler ordering tests, I dread to think what the test involving the order of 40 different filters would look like):

using (var scope = container.BeginLifetimeScope("AutofacWebRequest"))
{
    var resolvedFilters = scope.Resolve<IEnumerable<Meta<Lazy<IAutofacActionFilter>>>>();

    resolvedOrder = resolvedFilters.Select(x => x.Value.Value.GetType().Name).ToArray();
}

await wrapper.ExecuteActionFilterAsync(
    actionContext,
    CancellationToken.None,
    () => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)));

Assert.Equal($"{resolvedOrder[0]}.OnActionExecutingAsync", order[0]);
Assert.Equal($"{resolvedOrder[1]}.OnActionExecutingAsync", order[1]);
Assert.Equal($"{resolvedOrder[1]}.OnActionExecutedAsync", order[2]);
Assert.Equal($"{resolvedOrder[0]}.OnActionExecutedAsync", order[3]);

So on balance I think I'd rather leave the order assertions in there as they currently sit.

from autofac.webapi.

The "breaking change" I'm proposing would be to update the minimum required version of Autofac from 3.5.2 to 4.0.1 to force anyone taking the update to see the new order. This is definitely one of those gray areas where semver fails to explain what to do, but since it's going to break someone and not no one it seems like that'd be a major change. I've taking major updates to Newtonsoft.Json and never seen any problems, but that doesn't mean no one got broken, right? :)

If it's too hard to change, don't worry about it. We can just fix the tests again if needed.

from autofac.webapi.

Understood; I'll up the nuspec version to 5.0 of this integration (which conveniently but entirely accidentally lines us up with the next Autofac release).

from autofac.webapi.

Fixed

from autofac.webapi.