Add none generic service middleware registration methods in Autofac

[idiotsky]

These changes introduce new methods to register service middleware in the Autofac library. Also, a new test has been added to Autofac.Specification.Test to ensure that the middleware is getting invoked correctly when a service is registered. These methods allow for more precise control of the pipeline during the resolve request process.

[alistairjevans]

Hi @idiotsky, can you please elaborate on the purpose of these new methods? What issue are you trying to address?

Typically we'd raise an issue to discuss a feature request or bug before we open a PR.

[alistairjevans]

Ah, ok, this is for #1416.

[idiotsky]

does my change broken the build, on my side all the test passed? looks the unit test failed not related to my code

[tillig]

I mean, this isn't a question I can answer off the top of my head, it's just what we see:

• The build has been stable for all other PRs.
• The build failed on this PR.

Unclear if it's due to a problem in the test or something in this PR or what. But that's what I see - the same thing as you.

I kicked off the build again to see if it consistently fails.

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.51%. Comparing base (4547d16) to head (2cddfd3).
Report is 35 commits behind head on develop.

Files with missing lines	Patch %	Lines
...Autofac/ServiceMiddlewareRegistrationExtensions.cs	66.66%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1417      +/-   ##
===========================================
- Coverage    78.52%   78.51%   -0.02%     
===========================================
  Files          200      200              
  Lines         5713     5719       +6     
  Branches      1168     1168              
===========================================
+ Hits          4486     4490       +4     
- Misses         714      716       +2     
  Partials       513      513              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tillig]

Build seems to be passing now, but it appears the coverage is unhappy.

I haven't gotten around to reviewing this yet (but it looks like it's reasonable); I did see there was one unit test added to the RegistrationOnlyIfTests which seemed an odd place to add tests for a middleware registration method. After doing some searching, it appears we actually don't have specific tests about RegisterServiceMiddleware, so that seems like a shortcoming.

Here's what I'd say:

• Add a class Autofac.Specification.Test.Registration.ServiceMiddlewareTests.
• Put your tests for your new registrations in there. They should be named like RegisterServiceMiddleware_ScenarioNameHere since that's the method they're testing.
• Add at least one test for each of the new overloads so the overloads get exercised. The test should verify that the registered middleware ran as expected.

That should take care of the coverage issue and give us a place where we can put additional tests as needed.

[tillig]

It looks like this has been sitting for a while. My understanding of the status:

• We need some new tests to validate the methods, as indicated by the missing coverage numbers.
• The title on the PR should be updated a little - I don't know what "none generic service middleware" is (is it "non-generic?" is it a typo for "more generic?"). That's important so we can quantify it in release notes.

We'd love to get this into a new release, but it does need a little more work.

If we don't hear back in, say, a month? By end of October 2024? Then we'll close it and not integrate it.