Feedback from initial API Review

[twsouthwick]

Summary

We had an API review and here are the list of changes we need to make:

• Remove constructors from HttpContext, HttpRequest, HttpResponse Address some feedback from API review #113
• Add [Obsolete] to APIs that will cause sync over async and expose async counterparts on the ASP.NET Core build Add async APIs to push users aways from sync over async patterns #164
• Move builders to System.Web namespace for framework helpers and Microsoft.AspNetCore.Builders for the core helpers
• Remove interfaces from attributes Address some feedback from API review #113
• Rename IsEnabled to IsDisabled from metadata attributes Address some feedback from API review #113
• Rename PreLoad to Preload Address some feedback from API review #113
• Make DelegatingSessionState internal (Relayer packages to remove dependencies for adapters #134)
• Make DefaultRequestHeadersToForward and DefaultResponseHeadersToForward internal (Update remote app builder pattern #120)
• NetworkTimeout could probably be made an HttpClient #138
• AuthenticationEndpointPath should be a PathString on ASP.NET Core. (Update remote app builder pattern #120)
• Avoid using the same name for types that are at all structurally different and will not ever have shared code calling it from top (Update remote app builder pattern #120)
• Rethink builder pattern for remote app functionality (Update remote app builder pattern #120 Separate client/server remote options #165)

Original feedback:

• API Review for System.Web adapters (Part 1): HttpContext aspnetcore#42090
• API Review for System.Web adapters (Part 2): General Usage aspnetcore#42091
• API Review for System.Web adapters (Part 3): Session/Auth aspnetcore#42092

[twsouthwick]

/cc @mjrousos the namespace changes may make sense in your current PR (it at least can't be done until you merge your changes)

[mjrousos]

Yea, I can make the namespace chances in #110. I'll get that updated tomorrow.

[mjrousos]

For the .NET Core namespace for the extension methods, should it be Microsoft.AspNetCore.Builders or Microsoft.Extensions.DependencyInjection? The extension methods start by extending IServiceCollection (rather than by extending the web application builder directly).

[twsouthwick]

Microsoft.AspNetCore.Builders - that's the pattern ASP.NET Core wants us to use

[mjrousos]

Hm... ok. It seems like a lot of the other builders are using MED (AddControllers, AddReverseProxy, etc.) but I'll use Microsoft.AspNetCore.Builders for now and we can ask about it.

[adityamandaleeka]

cc @mjrousos for "NetworkTimeout could probably be made an HttpClient"

[adityamandaleeka]

Looks like the only one remaining is

Add [Obsolete] to APIs that will cause sync over async and expose async counterparts on the ASP.NET Core build #136

[danroth27]

Add [Obsolete] to APIs that will cause sync over async and expose async counterparts on the ASP.NET Core build

Which APIs are we talking about obsoleting? Are we talking about APIs in the System.Web Adapters? We haven't shipped a stable release of the System.Web Adapters package yet, so if we need to make an API change we can just do so without obsoleting anything.

[twsouthwick]

@danroth27 this is about providing a pathway for users using APIs that are currently doing sync over async due to the shape of System.Web APIs. We would need to keep the APIs themselves, but the obsolete would identify what they should use instead. This was an item that was not necessary initially (and does not block us from releasing). I've created a separate issue to discuss what we should do here: #164