Adding accept header to callsites within SignalR

[MattyLeslie]

Adding accept header to callsites within SignalR

Summary of the changes
As mentioned in #47398, many of the HttpClient call sites within the SignalR library were missing their appropriate accept headers. This change aims to address that issue.

Description
Changes made include:

1. Adding accept header to Negotiate() within HttpConnection to ensure header is present for negotiation requests.
2. Adding accept header to SendMessage() within SendUtils for the send requests.
3. Adding accept header to Poll() within LongPollingTransport for the get request.
4. Test to ensure first request and subsequent polling requests for LongPollingTransport have the correct accept header
5. Test to ensure the request for ServerSentEventsTransport has the correct accept header
6. Test to ensure the negotiate in HttpConnection has the correct accept header.

Fixes #47398 by ensuring the negotiation requests and other HttpClient call sites of SignalR are made with the relevant accept header.

[BrennanConroy]

Are you actually changing anything, or just kicking the build? If it's kicking the build please stop.
Also, could you please look into adding tests, see

aspnetcore/src/SignalR/common/Http.Connections/test/ServerSentEventsTests.cs

Line 34 in 42bc1a1

Assert.Equal("text/event-stream", context.Response.ContentType);

as an example.

[BrennanConroy]

Looks like you're missing negotiate? Otherwise I think that covers all the spots.

[BrennanConroy]

I'm sorry to see this closed, anything I can help with? It looked really close to finished.

[MattyLeslie]

Hello @BrennanConroy,

I've reopened the PR and pushed some new commits.

I'm encountering a couple of challenges that I'm currently addressing:

1. For the ServerSentEventsTransport.cs call site, the test hangs if the accept header is missing from the request in the StartAsync() method.
2. The test I created for the NegotiateAsync() method in HttpConnection.cs isn't passing. It seems that not all dependencies are being adequately mocked.

To better test the NegotiateAsync() method, I modified the unit testing constructor to accept a nullable HttpMessageHandler. This change allows me to mock HttpMessageHandler in the CreateHttpClient() method, enabling me to verify the outgoing requests made by the HttpClient during NegotiateAsync(). I hope this change is appropriate.

I welcome any constructive feedback on the testing approach I've implemented for the various call sites. I'm actively working to resolve the issues mentioned above.

[BrennanConroy]

To better test the NegotiateAsync() method, I modified the unit testing constructor to accept a nullable HttpMessageHandler. This change allows me to mock HttpMessageHandler in the CreateHttpClient() method, enabling me to verify the outgoing requests made by the HttpClient during NegotiateAsync(). I hope this change is appropriate.

Why not use HttpConnectionOptions.HttpMessageHandlerFactory?

aspnetcore/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs

Line 3272 in 0bae45e

HttpMessageHandlerFactory = handler => new GetNegotiateHttpHandler(handler, stream)