Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Review for System.Web adapters (Part 2): General Usage #42091

Closed
twsouthwick opened this issue Jun 8, 2022 · 9 comments
Closed

API Review for System.Web adapters (Part 2): General Usage #42091

twsouthwick opened this issue Jun 8, 2022 · 9 comments
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@twsouthwick
Copy link
Member

twsouthwick commented Jun 8, 2022

Background and Motivation

We've been developing some adapters to help with migrating from ASP.NET Framework that adapter the System.Web.HttpContext to the Microsoft.AspNetCore.Http.HttpContext. We've released Preview 1 and would like to review the API shape.

This issue focuses on the APIs that will be used on the ASP.NET Core/ASP.NET Framework applications to add functionality in.

Related API Designs:

Proposed API (ASP.NET Core)

Package: Microsoft.AspNetCore.SystemWebAdapters.CoreServices

namespace Microsoft.AspNetCore.SystemWebAdapters
{
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class BufferResponseStreamAttribute : System.Attribute
    {
        public BufferResponseStreamAttribute() { }
        public System.Nullable<long> BufferLimit { get { throw null; } set { } }
        public bool IsDisabled { get { throw null; } set { } }
        public int MemoryThreshold { get { throw null; } set { } }
    }
    public partial interface ISessionManager
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> CreateAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.SystemWebAdapters.SessionAttribute metadata);
    }
    public partial interface ISystemWebAdapterBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial interface ISystemWebAdapterRemoteClientAppBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class PreBufferRequestStreamAttribute : System.Attribute
    {
        public PreBufferRequestStreamAttribute() { }
        public System.Nullable<long> BufferLimit { get { throw null; } set { } }
        public int BufferThreshold { get { throw null; } set { } }
        public bool IsDisabled { get { throw null; } set { } }
    }
    public partial class RemoteAppClientOptions
    {
        public RemoteAppClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKey { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKeyHeader { get { throw null; } set { } }
        public System.Net.Http.HttpMessageHandler BackchannelHandler { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public System.Uri RemoteAppUrl { get { throw null; } set { } }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SessionAttribute : System.Attribute
    {
        public SessionAttribute() { }
        public Microsoft.AspNetCore.SystemWebAdapters.SessionBehavior Behavior { get { throw null; } set { } }
        public bool IsReadOnly { get { throw null; } set { } }
    }
    public enum SessionBehavior
    {
        None = 0,
        [System.ObsoleteAttribute("This will enable session on the endpoint but will resort to sync over async behavior")]
        OnDemand = 2,
        Preload = 1,
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SetThreadCurrentPrincipalAttribute : System.Attribute
    {
        public SetThreadCurrentPrincipalAttribute() { }
        public bool IsDisabled { get { throw null; } set { } }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SingleThreadedRequestAttribute : System.Attribute
    {
        public SingleThreadedRequestAttribute() { }
        public bool IsDisabled { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication
{
    public partial interface IRemoteAppAuthenticationResultProcessor
    {
        System.Threading.Tasks.Task ProcessAsync(Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationResult result, Microsoft.AspNetCore.Http.HttpContext context);
    }
    public partial class RemoteAppAuthenticationClientOptions : Microsoft.AspNetCore.Authentication.AuthenticationSchemeOptions
    {
        public RemoteAppAuthenticationClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public Microsoft.AspNetCore.Http.PathString AuthenticationEndpointPath { get { throw null; } set { } }
        public System.Collections.Generic.ICollection<string> RequestHeadersToForward { get { throw null; } }
        public System.Collections.Generic.ICollection<string> ResponseHeadersToForward { get { throw null; } }
    }
    public static partial class RemoteAppAuthenticationDefaults
    {
        public const string AuthenticationScheme = "Remote";
    }
    public partial class RemoteAppAuthenticationResult
    {
        public RemoteAppAuthenticationResult(System.Security.Claims.ClaimsPrincipal user, int statusCode, System.Net.Http.HttpRequestMessage authRequest) { }
        public System.Net.Http.HttpRequestMessage AuthenticationRequest { get { throw null; } }
        public Microsoft.AspNetCore.Http.IHeaderDictionary ResponseHeaders { get { throw null; } }
        public int StatusCode { get { throw null; } }
        public System.Security.Claims.ClaimsPrincipal User { get { throw null; } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession
{
    public partial class RemoteAppSessionStateClientOptions
    {
        public RemoteAppSessionStateClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string CookieName { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public Microsoft.AspNetCore.Http.PathString SessionEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization
{
    public partial interface ISessionKeySerializer
    {
        bool TryDeserialize(string key, byte[] bytes, out object obj);
        bool TrySerialize(string key, object value, out byte[] bytes);
    }
    public partial interface ISessionSerializer
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token);
        System.Threading.Tasks.Task SerializeAsync(Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState state, System.IO.Stream stream, System.Threading.CancellationToken token);
    }
    public partial class JsonSessionSerializerOptions
    {
        public JsonSessionSerializerOptions() { }
        public System.Collections.Generic.IDictionary<string, System.Type> KnownKeys { get { throw null; } }
        public void RegisterKey<T>(string key) { }
    }
    public partial class SessionSerializerOptions
    {
        public SessionSerializerOptions() { }
        public bool ThrowOnUnknownSessionKey { get { throw null; } set { } }
    }
}
namespace Microsoft.Extensions.DependencyInjection
{
    public static partial class ISystemWebAdapterBuilderSessionExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder WrapAspNetCoreSession(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.Builder.SessionOptions> options = null) { throw null; }
    }
    public static partial class JsonSessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddJsonSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.JsonSessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppAuthenticationExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddAuthenticationClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder builder, bool isDefaultScheme, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, string scheme) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, string scheme, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteClientAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
    }
    public static partial class RemoteAppClientExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddRemoteAppClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.RemoteAppClientOptions> configure) { throw null; }
    }
    public static partial class RemoteAppSessionStateExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddSessionClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession.RemoteAppSessionStateClientOptions> configure = null) { throw null; }
    }
    public static partial class SessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.SessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class SystemWebAdaptersExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSystemWebAdapters(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; }
        public static TBuilder BufferResponseStream<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.BufferResponseStreamAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static TBuilder PreBufferRequestStream<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.PreBufferRequestStreamAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static TBuilder RequireSystemWebAdapterSession<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.SessionAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static void UseSystemWebAdapters(this Microsoft.AspNetCore.Builder.IApplicationBuilder app) { }
    }
}

Usage Examples:

Scenario: Using remote app for auth and session

builder.Services.AddSystemWebAdapters()
    .AddRemoteAppClient(remote => remote
        .Configure(options =>
        {
            options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]);
            options.ApiKey = ClassLibrary.RemoteServiceUtils.ApiKey;
        })
        .AddAuthentication(true)
        .AddSession())
    .AddJsonSessionSerializer(options => ClassLibrary.RemoteServiceUtils.RegisterSessionKeys(options.KnownKeys));

var app = builder.Build();

...

app.UseSystemWebAdapters();

...

app.MapGet("/current-principals-with-metadata", (HttpContext ctx) =>
{
    var user1 = Thread.CurrentPrincipal;
    var user2 = ClaimsPrincipal.Current;

    return "done";
}).WithMetadata(new SetThreadCurrentPrincipalAttribute(), new SingleThreadedRequestAttribute());

app.MapDefaultControllerRoute()
  .RequireSystemWebAdapterSession();

app.MapReverseProxy();

Scenario: Using built-in session manager behind System.Web.HttpContext.Session

builder.Services.AddSystemWebAdapters()
    .WrapAspNetCoreSession()
    .AddJsonSessionSerializer(options => ClassLibrary.RemoteServiceUtils.RegisterSessionKeys(options.KnownKeys));

Proposed API (ASP.NET Framework)

On ASP.NET Framework, we insert a single module (SystemWebAdaptersModule) that then will load additional modules if required. This is done in a fluent syntax that tries to mimic the setup of ASP.NET Core. Users can register their own IHttpModule types that will be run (in order of registration) by SystemWebAdaptersModule.

Package: Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices

namespace Microsoft.AspNetCore.SystemWebAdapters
{
    public partial interface ISystemWebAdapterBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial interface ISystemWebAdapterRemoteServerAppBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial class ProxyOptions
    {
        public ProxyOptions() { }
        public string OriginalHostHeaderName { get { throw null; } set { } }
        public string Scheme { get { throw null; } set { } }
        public string ServerName { get { throw null; } set { } }
        public int ServerPort { get { throw null; } set { } }
        public bool UseForwardedHeaders { get { throw null; } set { } }
    }
    public partial class RemoteAppServerOptions
    {
        public RemoteAppServerOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKey { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKeyHeader { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication
{
    public partial class RemoteAppAuthenticationServerOptions
    {
        public RemoteAppAuthenticationServerOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string AuthenticationEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession
{
    public partial class RemoteAppSessionStateServerOptions
    {
        public RemoteAppSessionStateServerOptions() { }
        public string CookieName { get { throw null; } set { } }
        public string SessionEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization
{
    public partial interface ISessionKeySerializer
    {
        bool TryDeserialize(string key, byte[] bytes, out object obj);
        bool TrySerialize(string key, object value, out byte[] bytes);
    }
    public partial interface ISessionSerializer
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token);
        System.Threading.Tasks.Task SerializeAsync(Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState state, System.IO.Stream stream, System.Threading.CancellationToken token);
    }
    public partial class JsonSessionSerializerOptions
    {
        public JsonSessionSerializerOptions() { }
        public System.Collections.Generic.IDictionary<string, System.Type> KnownKeys { get { throw null; } }
        public void RegisterKey<T>(string key) { }
    }
    public partial class SessionSerializerOptions
    {
        public SessionSerializerOptions() { }
        public bool ThrowOnUnknownSessionKey { get { throw null; } set { } }
    }
}
namespace System.Web
{
    public static partial class JsonSessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddJsonSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.JsonSessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppAuthenticationExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddAuthenticationServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationServerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppServerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddRemoteAppServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.RemoteAppServerOptions> configure) { throw null; }
    }
    public static partial class RemoteAppSessionStateExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddSessionServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession.RemoteAppSessionStateServerOptions> configureRemote = null) { throw null; }
    }
    public static partial class SessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.SessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class SystemWebAdapterConfiguration
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddProxySupport(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.ProxyOptions> configure) { throw null; }
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSystemWebAdapters(this System.Web.HttpApplication application) { throw null; }
    }
}

Usage Examples:

Scenario: Set up remote app for session and auth. Also configures support for proxy headers

public class MvcApplication : System.Web.HttpApplication
{
    protected void Application_Start()
    {
        ...

        SystemWebAdapterConfiguration.AddSystemWebAdapters(this)
            .AddProxySupport(options => options.UseForwardedHeaders = true)
            .AddJsonSessionSerializer(options =>
            {
                options.RegisterKey<int>("test-value");
                options.RegisterKey<SessionDemoModel>("SampleSessionItem");
            })
            .AddRemoteAppServer(options => options.ApiKey = ConfigurationManager.AppSettings["RemoteAppApiKey"])
            .AddAuthenticationServer()
            .AddSessionServer();
    }
}

Alternative Designs

  • The name of Client/Server for the ASP.NET Core application and the ASP.NET Framework application was the best we could come up with. However, it doesn't quite feel right, and we've had feedback that it's confusing since they're both servers. We're consistent at this point in what we've named them, just want to settle on the correct terms
  • Should the ISessionKeySerializer act on byte[] or on IMemory<byte>?
@twsouthwick twsouthwick added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 8, 2022
@twsouthwick twsouthwick changed the title API Review for System.Web adapters: General Usage API Review for System.Web adapters (Part 2): General Usage Jun 8, 2022
@Pilchie Pilchie added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 8, 2022
@Tratcher Tratcher added area-runtime api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Jun 8, 2022
@ghost
Copy link

ghost commented Jun 8, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@BrennanConroy BrennanConroy added this to the 7.0-preview6 milestone Jun 8, 2022
@halter73 halter73 removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 13, 2022
@halter73
Copy link
Member

halter73 commented Jul 14, 2022

API review notes:

  • Can the ASP.NET Core versions of the AddSystemWebAdapters and UseSystemWebAdapters to the Microsoft.AspNetCore.Builder namespace?
  • Let's also move the System.Web versions of these to the System.Web namespace
    • These share code, but we can ifdef the namespaces as well which might help.
    • It might still be easier just to split the files
  • Is it normal to have an Add extension method on HttpApplicationState for AddSystemWebAdapters(this System.Web.HttpApplicationState state)?
    • Probably not super normal for System.Web, but it is similar in functionality to what the ASP.NET Core version AddSystemWebAdapters so having the same name helps.
  • Let's make SystemWebAdapterModule internal.
  • Let's remove the interfaces from the attributes.
  • Let's make IsEnabled IsDisabled instead so the default value is false.
  • PreLoad -> Preload

I'll try to edit this with the final reviewed API shape and mark it api-approved later today.

@halter73
Copy link
Member

halter73 commented Jul 15, 2022

More API review notes:

  • Should we use ValueTask to potentially save allocations?
    • No. It would add an extra dependency to the System.Web version of the adapters.
  • Let's make DelegatingSessionState internal.

@ghost
Copy link

ghost commented Sep 9, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@halter73
Copy link
Member

API Review Notes:

  • RemoteAppClientOptions.RemoteAppUrl is a Uri. Can you just Bind() to Uri directly from config?
  • AddAuthentication() and AddSession() sound too similar to existing IServiceCollection extension methods even though they extend a different builder type.
    • AddAuthentication() -> AddRemoteAuthentication
    • AddSession -> AddRemoteSessions
  • Remove Configure(this ISystemWebAdapterRemoteClientAppBuilder builder, Action<RemoteAppClientOptions> configure) and have AddRemoteAppClient take the Action<RemoteAppClientOptions> directly.
  • Can things like BufferResponseStreamAttribute and PreBufferRequestStreamAttribute be moved into aspnetcore runtime proper?

@halter73
Copy link
Member

We also thought that adding data annotations to the options and adding validation to it is unusual for library code. Normally we just validate in the constructors of services taking the options. Maybe this allows for better errors though. We should double check options in the ctor either way.

@halter73
Copy link
Member

Thanks for responding to the feedback. I'm going to copy the proposed API at this point in time for easier tracking of any potential future updates.

Package: Microsoft.AspNetCore.SystemWebAdapters.CoreServices

namespace Microsoft.AspNetCore.SystemWebAdapters
{
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class BufferResponseStreamAttribute : System.Attribute
    {
        public BufferResponseStreamAttribute() { }
        public System.Nullable<long> BufferLimit { get { throw null; } set { } }
        public bool IsDisabled { get { throw null; } set { } }
        public int MemoryThreshold { get { throw null; } set { } }
    }
    public partial interface ISessionManager
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> CreateAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.SystemWebAdapters.SessionAttribute metadata);
    }
    public partial interface ISystemWebAdapterBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial interface ISystemWebAdapterRemoteClientAppBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class PreBufferRequestStreamAttribute : System.Attribute
    {
        public PreBufferRequestStreamAttribute() { }
        public System.Nullable<long> BufferLimit { get { throw null; } set { } }
        public int BufferThreshold { get { throw null; } set { } }
        public bool IsDisabled { get { throw null; } set { } }
    }
    public partial class RemoteAppClientOptions
    {
        public RemoteAppClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKey { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKeyHeader { get { throw null; } set { } }
        public System.Net.Http.HttpMessageHandler BackchannelHandler { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public System.Uri RemoteAppUrl { get { throw null; } set { } }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SessionAttribute : System.Attribute
    {
        public SessionAttribute() { }
        public Microsoft.AspNetCore.SystemWebAdapters.SessionBehavior Behavior { get { throw null; } set { } }
        public bool IsReadOnly { get { throw null; } set { } }
    }
    public enum SessionBehavior
    {
        None = 0,
        [System.ObsoleteAttribute("This will enable session on the endpoint but will resort to sync over async behavior")]
        OnDemand = 2,
        Preload = 1,
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SetThreadCurrentPrincipalAttribute : System.Attribute
    {
        public SetThreadCurrentPrincipalAttribute() { }
        public bool IsDisabled { get { throw null; } set { } }
    }
    [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method)]
    public sealed partial class SingleThreadedRequestAttribute : System.Attribute
    {
        public SingleThreadedRequestAttribute() { }
        public bool IsDisabled { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication
{
    public partial interface IRemoteAppAuthenticationResultProcessor
    {
        System.Threading.Tasks.Task ProcessAsync(Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationResult result, Microsoft.AspNetCore.Http.HttpContext context);
    }
    public partial class RemoteAppAuthenticationClientOptions : Microsoft.AspNetCore.Authentication.AuthenticationSchemeOptions
    {
        public RemoteAppAuthenticationClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public Microsoft.AspNetCore.Http.PathString AuthenticationEndpointPath { get { throw null; } set { } }
        public System.Collections.Generic.ICollection<string> RequestHeadersToForward { get { throw null; } }
        public System.Collections.Generic.ICollection<string> ResponseHeadersToForward { get { throw null; } }
    }
    public static partial class RemoteAppAuthenticationDefaults
    {
        public const string AuthenticationScheme = "Remote";
    }
    public partial class RemoteAppAuthenticationResult
    {
        public RemoteAppAuthenticationResult(System.Security.Claims.ClaimsPrincipal user, int statusCode, System.Net.Http.HttpRequestMessage authRequest) { }
        public System.Net.Http.HttpRequestMessage AuthenticationRequest { get { throw null; } }
        public Microsoft.AspNetCore.Http.IHeaderDictionary ResponseHeaders { get { throw null; } }
        public int StatusCode { get { throw null; } }
        public System.Security.Claims.ClaimsPrincipal User { get { throw null; } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession
{
    public partial class RemoteAppSessionStateClientOptions
    {
        public RemoteAppSessionStateClientOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string CookieName { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public Microsoft.AspNetCore.Http.PathString SessionEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization
{
    public partial interface ISessionKeySerializer
    {
        bool TryDeserialize(string key, byte[] bytes, out object obj);
        bool TrySerialize(string key, object value, out byte[] bytes);
    }
    public partial interface ISessionSerializer
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token);
        System.Threading.Tasks.Task SerializeAsync(Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState state, System.IO.Stream stream, System.Threading.CancellationToken token);
    }
    public partial class JsonSessionSerializerOptions
    {
        public JsonSessionSerializerOptions() { }
        public System.Collections.Generic.IDictionary<string, System.Type> KnownKeys { get { throw null; } }
        public void RegisterKey<T>(string key) { }
    }
    public partial class SessionSerializerOptions
    {
        public SessionSerializerOptions() { }
        public bool ThrowOnUnknownSessionKey { get { throw null; } set { } }
    }
}
namespace Microsoft.Extensions.DependencyInjection
{
    public static partial class ISystemWebAdapterBuilderSessionExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder WrapAspNetCoreSession(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.Builder.SessionOptions> options = null) { throw null; }
    }
    public static partial class JsonSessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddJsonSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.JsonSessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppAuthenticationExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddAuthenticationClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder builder, bool isDefaultScheme, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, string scheme) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteAppAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, string scheme, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
        public static Microsoft.AspNetCore.Authentication.AuthenticationBuilder AddRemoteClientAuthentication(this Microsoft.AspNetCore.Authentication.AuthenticationBuilder authenticationBuilder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationClientOptions> configureOptions = null) { throw null; }
    }
    public static partial class RemoteAppClientExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddRemoteAppClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.RemoteAppClientOptions> configure) { throw null; }
    }
    public static partial class RemoteAppSessionStateExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder AddSessionClient(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteClientAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession.RemoteAppSessionStateClientOptions> configure = null) { throw null; }
    }
    public static partial class SessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.SessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class SystemWebAdaptersExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSystemWebAdapters(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; }
        public static TBuilder BufferResponseStream<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.BufferResponseStreamAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static TBuilder PreBufferRequestStream<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.PreBufferRequestStreamAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static TBuilder RequireSystemWebAdapterSession<TBuilder>(this TBuilder builder, Microsoft.AspNetCore.SystemWebAdapters.SessionAttribute metadata = null) where TBuilder : Microsoft.AspNetCore.Builder.IEndpointConventionBuilder { throw null; }
        public static void UseSystemWebAdapters(this Microsoft.AspNetCore.Builder.IApplicationBuilder app) { }
    }
}

Package: Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices

namespace Microsoft.AspNetCore.SystemWebAdapters
{
    public partial interface ISystemWebAdapterBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial interface ISystemWebAdapterRemoteServerAppBuilder
    {
        Microsoft.Extensions.DependencyInjection.IServiceCollection Services { get; }
    }
    public partial class ProxyOptions
    {
        public ProxyOptions() { }
        public string OriginalHostHeaderName { get { throw null; } set { } }
        public string Scheme { get { throw null; } set { } }
        public string ServerName { get { throw null; } set { } }
        public int ServerPort { get { throw null; } set { } }
        public bool UseForwardedHeaders { get { throw null; } set { } }
    }
    public partial class RemoteAppServerOptions
    {
        public RemoteAppServerOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKey { get { throw null; } set { } }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string ApiKeyHeader { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication
{
    public partial class RemoteAppAuthenticationServerOptions
    {
        public RemoteAppAuthenticationServerOptions() { }
        [System.ComponentModel.DataAnnotations.RequiredAttribute]
        public string AuthenticationEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession
{
    public partial class RemoteAppSessionStateServerOptions
    {
        public RemoteAppSessionStateServerOptions() { }
        public string CookieName { get { throw null; } set { } }
        public string SessionEndpointPath { get { throw null; } set { } }
    }
}
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization
{
    public partial interface ISessionKeySerializer
    {
        bool TryDeserialize(string key, byte[] bytes, out object obj);
        bool TrySerialize(string key, object value, out byte[] bytes);
    }
    public partial interface ISessionSerializer
    {
        System.Threading.Tasks.Task<Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState> DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token);
        System.Threading.Tasks.Task SerializeAsync(Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState state, System.IO.Stream stream, System.Threading.CancellationToken token);
    }
    public partial class JsonSessionSerializerOptions
    {
        public JsonSessionSerializerOptions() { }
        public System.Collections.Generic.IDictionary<string, System.Type> KnownKeys { get { throw null; } }
        public void RegisterKey<T>(string key) { }
    }
    public partial class SessionSerializerOptions
    {
        public SessionSerializerOptions() { }
        public bool ThrowOnUnknownSessionKey { get { throw null; } set { } }
    }
}
namespace System.Web
{
    public static partial class JsonSessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddJsonSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.JsonSessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppAuthenticationExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddAuthenticationServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.Authentication.RemoteAppAuthenticationServerOptions> configure = null) { throw null; }
    }
    public static partial class RemoteAppServerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddRemoteAppServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.RemoteAppServerOptions> configure) { throw null; }
    }
    public static partial class RemoteAppSessionStateExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder AddSessionServer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterRemoteServerAppBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession.RemoteAppSessionStateServerOptions> configureRemote = null) { throw null; }
    }
    public static partial class SessionSerializerExtensions
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSessionSerializer(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization.SessionSerializerOptions> configure = null) { throw null; }
    }
    public static partial class SystemWebAdapterConfiguration
    {
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddProxySupport(this Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder builder, System.Action<Microsoft.AspNetCore.SystemWebAdapters.ProxyOptions> configure) { throw null; }
        public static Microsoft.AspNetCore.SystemWebAdapters.ISystemWebAdapterBuilder AddSystemWebAdapters(this System.Web.HttpApplication application) { throw null; }
    }
}

I think this looks good. @Tratcher Do you agree? If so, I think we can mark this approved.

@dotnet/aspnet-api-review

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Sep 17, 2022
@halter73
Copy link
Member

Given that there has been no more feedback in the last two days, I'm labeling this api-approved.

@joperezr
Copy link
Member

joperezr commented Jan 27, 2023

This is no longer relevant as it was for 1.0 release of system.web adapters.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 26, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

No branches or pull requests

8 participants