Remaining pubternal APIs in the shared framework

[davidfowl]

We still have to look at packages but I haven't done that yet: I want to make sure these were intentional and not forgotten. If they were intentional I'd like to understand why they weren't made public APIs

• - Microsoft.AspNetCore.Authentication.Internal (@HaoK @Tratcher, @Pilchie)
• - Microsoft.AspNetCore.Builder.Internal (@Tratcher @davidfowl, @anurse)
• - Microsoft.AspNetCore.DataProtection.Internal (@natemcmaster @blowdart)
• - Microsoft.AspNetCore.DataProtection.Cng.Internal (@blowdart )
• - Microsoft.AspNetCore.Hosting.Internal (@Tratcher @davidfowl @anurse )
• - Microsoft.AspNetCore.Http.Internal (@Tratcher @davidfowl @anurse)
• - Microsoft.AspNetCore.Mvc.Core.Infrastructure (@rynowak @mkArtakMSFT )
• - Microsoft.AspNetCore.Mvc.Razor.Internal (@rynowak @NTaylorMullen @pranavkm @mkArtakMSFT )
• - Microsoft.AspNetCore.Rewrite.Internal (@jkotalik @anurse )
• - Microsoft.AspNetCore.Routing.Internal (@rynowak @JamesNK)
• - Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal (@halter73 @davidfowl @anurse)
• - Microsoft.AspNetCore.Server.Kestrel.Https.Internal (@anurse , @halter73 @Tratcher )
• - Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http (@halter73, @davidfowl @anurse )
• - Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure (@davidfowl, @halter73 , @anurse )

[natemcmaster]

For DataProtection, see #6975 (comment). It's the only context I have on that one.

[pranavkm]

• All of the Razor.Internal ones are runtime compilation specific. We made specific announcements for it.
• The Mvc.Core.Infrastructure is rather unfortunate. It's still there in 3.0: https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.Core/src/Infrastructure/IAntiforgeryValidationFailedResult.cs

[HaoK]

For auth, it looks like we only have one RequestPathBaseCookieBuilder class which seems harmless enough to make public along side the rest of the Authentication Core code. Its shared between Cookies, OpenIdConnect and the base RemoteAuthentication classes.

@Tratcher any opposition to just making this regular public? This class hasn't been touched since @natemcmaster added this 2 years ago when he introduced the cookie builder stuff.

[davidfowl]

The Mvc.Core.Infrastructure is rather unfortunate. It's still there in 3.0: https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.Core/src/Infrastructure/IAntiforgeryValidationFailedResult.cs

@pranavkm Why not move it out of the internal namespace?

[davidfowl]

For DataProtection, see #6975 (comment). It's the only context I have on that one.

@blowdart besides "leave DataProtection alone", do you know if we can make this type internal now or make it properly public? There are 2 types:

public class DataProtectionBuilder : IDataProtectionBuilder {
         public DataProtectionBuilder(IServiceCollection services);
         public IServiceCollection Services { get; }
     }

This could be public or internal.

public interface IActivator {
    object CreateInstance(Type expectedBaseType, string implementationTypeName);
}

This should be internal.

[blowdart]

Why should it be internal?

@GrabYourPitchforks can you remember why this was written in this way?

[Tratcher]

@HaoK Yes, RequestPathBaseCookieBuilder can be public.

[Tratcher]

For sanity sake, let there only be one breaking change announcement that we add a few comments to.

[davidfowl]

Why should it be internal?

It's a utility interface that's an internal implementation detail that doesn't need to be exposed to external users. We're doing mass clean up of pubternal types in ASP.NET Core 3.0, data protection is part of that clean up.

[davidfowl]

@pranavkm All of the Razor.Internal ones are runtime compilation specific. We made specific announcements for it.

Can we move the code gen helpers to a CompilerServices namespace?

[pranavkm]

Can we move the code gen helpers to a CompilerServices namespace?

Not super sure I follow. Are you talking about CSharpCompiler? For a lot of the others, the actual interface types are still public, as a rule MVC does not expose the actual implementation type.

[davidfowl]

Not super sure I follow. Are you talking about CSharpCompiler? For a lot of the others, the actual interface types are still public, as a rule MVC does not expose the actual implementation type.

No, the types in Razor.Internal, like RazorInject attribute and the other things you said are required for code generation.

[pranavkm]

We decided to leave RazorInject in the pubinternal namespace. We wanted to avoid breaking 2.2 RCLs with generated content. We're treating it as part of our public API surface even though it's in the .Internal namespace.

[davidfowl]

We're making great progress on this!

[davidfowl]

@Tratcher can you look at Microsoft.AspNetCore.Server.Kestrel.Https.Internal?
@rynowak can you make a call on the types in ** Microsoft.AspNetCore.Routing.Internal** - You can leave make DfaWriter as public API with a comment about the fact it can break (similar to how EF has the warning on pubternal types)
I'll take Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http, this is the http parser we use in the platform benchmark.
@blowdart Microsoft.AspNetCore.DataProtection.Internal - I'm going to look at making things internal.

[davidfowl]

@GrabYourPitchforks Do you have any feedback on making Microsoft.AspNetCore.DataProtection.Internal and Microsoft.AspNetCore.DataProtection.Cng.Internal actually internal?

[rynowak]

#11752

[davidfowl]

Spoke to @GrabYourPitchforks and he says make them internal!

[NicolasDorier]

EnableRewind is quite useful, why was it removed?

[Tratcher]

@NicolasDorier because we made a properly public version named EnableBuffering.

[NicolasDorier]

Just another feedback:

I was using AnsiLogConsole/WindowsLogConsole/IConsole directly in the past as I did a custom CustomConsoleLogger, which is almost the same as the official one but with little adjustment. I have a bad memory about the initial reason, I think it is because I only wanted [Info] and nothing more in my logs. (No timestamp) and some custom stuff with log padding.

Since Internal namespace is now internal, my code broke.

I don't think it is a big deal really, as I can just copy/paste those classes directly in my project. It is quite corner case and does not require me too much maintenance. Just wanted to share feedback.

[Samirat]

@Tratcher @davidfowl Can RequestCookieCollection be public? If implementing a custom IRequestCookieCollection or IRequestCookieFeature, there's not much reason to reimplement this vs inherit or wrap it.

I think you can get an instance of one anyway like this:
(new RequestCookiesFeature(new FeatureCollection())).Cookies;

[analogrelay]

@Samirat can you file a new issue for API requests like this?

I'm going to close this as we've finished the work on removing pubternal APIs. Feel free to open new issues to discuss particular APIs. We can triage those as appropriate.