aspnet / HttpAbstractions

[Archived] HTTP abstractions such as HttpRequest, HttpResponse, and HttpContext, as well as common web utilities. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
382 stars 194 forks source link

Copy core endpoint routing types to HttpAbstractions #1030

Closed JamesNK closed 6 years ago

JamesNK commented 6 years ago

Note that this is in master for 3.0 Will type forward from routing abstractions.

https://github.com/aspnet/Routing/issues/607

JamesNK commented 6 years ago

Why is there no package for the feature branch? https://dotnet.myget.org/feed/aspnetcore-dev/package/nuget/Microsoft.AspNetCore.Http.Abstractions

Tratcher commented 6 years ago

So why is all of this going into the Http.Abstractions package? I only see one type reference (RequestDelegate) that even relates to Http. I Also see a lot more implementation than I'd expect in the abstractions package. This seems like it would become its own thing like Microsoft.AspNetCore.Routing.Something.

kichalla commented 6 years ago

Why is there no package for the feature branch? https://dotnet.myget.org/feed/aspnetcore-dev/package/nuget/Microsoft.AspNetCore.Http.Abstractions

@JamesNK We would ultimately want to move to VSTS and it currently does not have support for feature branches, so @natemcmaster enabled feature branch support only for Routing and MVC last week on TeamCity as we were making cross-repo changes.

JamesNK commented 6 years ago

@natemcmaster Is it difficult to add feature branch builds to HttpAbstractions?

natemcmaster commented 6 years ago

Done. http://aspnetci/viewType.html?buildTypeId=Lite_HttpAbstractions&branch_Lite=endpoint-routing-types&tab=buildTypeStatusDiv

JamesNK commented 6 years ago

I have moved Invoker from IEndpointFeature to Endpoint from @davidfowl's offline feedback.

Outstanding decisions:

  1. Namespaces
    • Should Endpoint+EndpointMetadataCollection live in Microsoft.AspNetCore.Routing?
    • Should IEndpointFeature live in Microsoft.AspNetCore.Routing.Features?
  2. Package
    • Should these types (+RouteValueDictionary) be in Http.Abstractions or Http.Features?

These types flow through a lot of repositories and changing them later before 3.0 will be a pain so lets attempt to make the best decision now. If we don't lock these down online in the PR in the next day I'll organize a quick meeting.

davidfowl commented 6 years ago

Consumption:

public interface IEndpointFeature
{
    Endpoint Endpoint { get; set; }
}
public interface IRouteValuesFeature
{
    RouteValueDictionary RouteValues { get; set; }
}
public class HttpRequest
{
    RouteValueDictionary RouteValues { get; set; }
}

Open questions:

Tratcher commented 6 years ago

Put it in AppBuilder and fall back to 404 if it's missing.

Hmm, if you do that then Map can end up executing the same endpoint regardless of the branch taken. Not sure how expected that would be.

JamesNK commented 6 years ago

Everything is still in Http.Abstractions and not Http.Endpoints. FYI RequestDelegate lives in Http.Abstractions so if Endpoint was moved to Http.Endpoints then RequestDelegate would need to be forwarded.

JamesNK commented 6 years ago

🆙 📅

Added implicit call to Endpoint's RequestDelegate.

JamesNK commented 6 years ago

There are a lot of outstanding PRs on routing and MVC for 2.2, some that touch these types. Once there is an all clear to check the PRs in and they're merged to master then I'll flow this through feature branches to routing/MVC/consuming middleware so we can see what it looks like.

JamesNK commented 6 years ago

Should IRouteValuesFeature be in 2.2? We're going to need to remove route values from IEndpointFeature in 2.2 so I'm guessing yes unless we want to keep using IRoutingFeature in 2.2.

rynowak commented 6 years ago

I could be convinced either way. We don't need it in 2.2...... but the other details that we are locking in 2.2 will require the design to be the same. So I think it has the same effect with the only delta being things like naming.

JamesNK commented 6 years ago

🆙 📅

Synced with changes in Routing:

JamesNK commented 5 years ago

I'm confused. The HttpRequest.RouteValues property has disappeared from this commit. Did we decide to remove it, or did I somehow delete it in a rebase gone wrong?

I remember writing it, and there are comments about it on the PR - https://github.com/aspnet/HttpAbstractions/pull/1030#discussion_r210463585

@rynowak @davidfowl

rynowak commented 5 years ago

I seem to remember that we wanted it...

JamesNK commented 5 years ago

As do I. Maybe there was some reason it needed to be left out of this PR.

I'll add it back.

davidfowl commented 5 years ago

How did it disappear?