HttpContext and JSON

[rynowak]

HttpContext and JSON

This is a proposal to add extension methods for using HttpContext (and related types) with System.Text.Json. Adding this functionality will make ASP.NET Core developers productive with smaller amounts of code. This is aligned with our goal of making route-to-code a useful programming pattern.

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadFromJsonAsync<WeatherForecast>();
    await UpdateDatabase(weather);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

Goals

• Make this the easiest imperative way to read/write JSON

• Make this fastest way to read/write JSON

• Do HTTP things correct by default (don't cut corners)

• Replace the guts of MVC's JSON formatter with this (validates flexibility)

• Do all of this without requiring service registration/configuration

• NON-GOAL: Create an abstraction for JSON serializers/settings

API Additions

These are the proposed APIs, see the following (annotated) sections for discussion.

Some updates based on recent discussions with @JamesNK and @Tratcher:

• These would be added to the Microsoft.AspNetCore.Http.Extensions assembly.
• JsonOptions would be inherited (used as the base) by MVC and SignalR - but they are copies. So modifying JsonOptions would influence MVC and SignalR - but not the reverse. This is worth a discussion.
• We got rid of APIs that allow specifying an Encoding for writing. Can bring these back if needed.
• Overloads that specify a contentType should specify the value they want to see in the Content-Type header. We will not append a charset. We will not parse the value you give us and change the encoding based on charset. It's your job to get it right.

namespace Microsoft.AspNetCore.Http
{
    public static class HttpRequestJsonExtensions
    {
        public static bool HasJsonContentType(this HttpRequest request);
    }
}

namespace Microsoft.AspNetCore.Http.Json
{
    public static class HttpContextJsonExtensions
    {
        public static ValueTask<TValue> ReadFromJsonAsync<TValue>(
            this HttpRequest request, 
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask<TValue> ReadFromJsonAsync<TValue>(
            this HttpRequest request, 
            JsonSerializerOptions? options, 
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask<object> ReadFromJsonAsync(
            this HttpRequest request, 
            Type type, 
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask<object> ReadFromJsonAsync(
            this HttpRequest request, 
            Type type, 
            JsonSerializerOptions? options, 
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync<TValue>(
            this HttpResponse response, 
            TValue value, 
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync<TValue>(
            this HttpResponse response, 
            TValue value, 
            JsonSerializerOptions? options,
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync<TValue>(
            this HttpResponse response,
            TValue value,
            JsonSerializerOptions? options,
            string? contentType,
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync(
            this HttpResponse response,
            Type type,
            object? value,
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync(
            this HttpResponse response,
            Type type,
            object? value,
            JsonSerializerOptions? options,
            CancellationToken cancellationToken = default) { throw null; }

        public static ValueTask WriteAsJsonAsync(
            this HttpResponse response,
            Type type,
            object? value,
            JsonSerializerOptions? options,
            string? contentType,
            CancellationToken cancellationToken = default) { throw null; }
    }

    public class JsonOptions
    {
        public JsonSerializerOptions SerializerOptions { get; }
    }
}

Design Notes

0: General Shape

There are three primary APIs here:

• Detect if the request body is JSON
• Read JSON from the body
  • Using a static type (TValue)
  • Using a dynamic type (Type type)
• Write JSON to the body
  • Using a static type (TValue value)
  • Using a dynamic type (Type type, object value)

In general I've tried to keep naming consistent with the functionality for reading the form (HasFormContentType, ReadFormAsync()).

1: HasJsonContentType

I tried to make the design of this similar to HasFormContentType which already exists on the HttpRequest. I considered as well making this an extension method, but there's no good reason to deviate from the pattern we established with form.

There's nothing about this property that makes it specific to a serializer, it's a straightforward comparison of the content type.

2: Namespace

One of the challenges in this area is that if we park the best method names to mean System.Text.Json, then this will be confusing to someone using another serializer. It would be really annoying to use JIL and see ReadJsonAsync() show up everywhere, but mean System.Text.Json. For this reason I put these extensions in a different namespace.

You could imagine that another serializer would want to provide similar functionality. I want to make sure the existing serializer ecosystem can coexist with this addition.

---

In my mind there are a few ways to address this challenge:

• Create a serializer abstraction
• Put System.Text.Json in the method names
• Make this an extra package
• Use a namespace to isolate this functionality

---

Creating a serializer abstraction has a bunch of problems:

• The abstraction would have to go in the BCL where they (likely) don't want it
• We'd likely need to require service registration (or coupling in hosting) to use these extensions
• We'd have to remove JsonSerializerOptions from the APIs

I reject this option, I think it defeats enough of our stated goals.

---

Considering other options - the namespace seems like the best, most flexible choice. Putting System.Text.Json in the names would be flexible as well, but would be ugly. So I'm pitching the namespace.

I think any of the non-abstraction options would be reasonable for us to choose while still meeting our goals.

3: ReadJsonAsync and Overload Set

I took the approach of defining lots of overloads here with just the cancellation token as an optional parameter.

The reason why is because of what happens when you using cancellation tokens with other optional parameters. Imagine that I made a single overload with optional parameters for both options and cancellation token.

public static ValueTask<TValue> ReadJsonAsync<TValue>(
    this HttpRequest request, 
    JsonSerializerOptions options = default, 
    CancellationToken cancellationToken = default) { throw null; }

await httpContext.Request.ReadJsonAsync<WeatherForecast>(myCancellationToken); // compile error

await httpContext.Request.ReadJsonAsync<WeatherForecast>(null, myCancellationToken); // works
await httpContext.Request.ReadJsonAsync<WeatherForecast>(cancellationToken: myCancellationToken); // works

We can avoid this ugliness by defining more overloads.

4: Media Type and Encoding

There are a lot of overloads of WriteJsonAsync - I'm interested in ways to cut this down.

We need to expose the media type as an option because it's becoming more common to use suffix content types like application/cloud-events+json. This fills a gap in what traditional media types do - a media type like application/json describes a data-representation, but says nothing about the kind of data being represented. More modern designs like CloudEvents will use suffix content types to describe both.

We need to expose the encoding because we MVC supports it (it's a goal to have MVC call this API), and because we still have the requirement to support things like GB2312.

I don't think either of these things are contraversial, we should debate whether we're happy with the design being proposed here as the best way to express this flexibility.

5: Managing JsonSerializerOptions

We need a strategy to deal with the fact that the default settings of JsonSerializerOptions are bad for the web. We want the serializer to output camelCase JSON by default, and be more tolerant of casing differences on input. Also because we know that we're always outputting text with a character encoding, we can be more relaxed about the set of characters we escape compared to the defaults.

I reject the idea that we'd give the user the default JsonSerializerOptions through these APIs and make it their job to manage, because that conflicts with the goal of this being the easiest way to do JSON - we want these APIs to have good defaults for the web.

There's a couple of options for how we could implement this:

• Have a static instance
• Have a feature on the http context
• Use options

Each of these have a downside. The static is a static, the downside is that its static. Using a feature either allocates a bunch or has wierd coupling (kestrel coupled to a serializer). Using options has some runtime overhead for the service lookup. Of these options seems like the best choice. We could also use the options approach to share the options instance between MVC's JsonOptions and this one for compatibility.

Behavious

HasJsonContentType

This method will return true when a request has a JSON content type, that is:

• application/json
• text/json
• application/*+json

Null or empty content type is not considered a match.

ReadJsonAsync

The overloads of ReadJsonAsync will throw an exception if the request does not have a JSON content type (similar to ReadFormAsync).

Depending on the value of CharSet the method may need to create a stream to wrap the request body and transcode - System.Text.Json only speaks UTF8. We're in discussions with CoreFx about moving our transcoding stream implementations into the BCL. We will assume UTF8 if no CharSet was provided, the serializer/reader will validate the correctness of the bytes.

We'll call the appropriate overload of JsonSerializer.DeserializeAsync to do the heavy lifting.

---

There's a couple of usability concerns here related to error handling. These APIs optimize for the happy path:

• Throws exceptions for non-JSON content type
• Throws exceptions for invalid JSON content

Someone who wants to handle both of these errors and turn them into a 400 would need to write an if for the content type, and a try/catch for the possible exceptions from deserialization.

It would be possible to make a TryXyz set of APIs as well - they would end up handling the exception for you. Since these are extension methods, they can't really log (without service locator).

With a union:

public static ValueTask<(bool success, TValue value)> TryReadJsonAsync<TValue>(this HttpRequest request);

endpoints.MapPost("/weather", async context =>
{
    var result = await context.Request.TryReadJsonAsync<WeatherForecast>();
    if (!result.success)
    {
        context.Response.StatusCode = StatusCodes.Status400BadRequest;
        return;
    }

    await UpdateDatabase(result.value);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

A possibly better version:

public static ValueTask<TValue> ReadJsonOrDefaultAsync<TValue>(this HttpRequest request);

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonOrDefaultAsync<WeatherForecast>();
    if (weather is null)
    {
        context.Response.StatusCode = StatusCodes.Status400BadRequest;
        return;
    }

    await UpdateDatabase(weather);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

WriteJsonAsync

Prior to doing the serialization to the response body, the WriteJsonAsync method will write a content type (with CharSet) to the Content-Type header. If no content type is specified then application/json will be used. If no encoding is specified, then UTF8 will be used.

Serialization will call JsonSerializer.SerializeAsync - and provide a wrapper stream if an encoding other than UTF8 is in use.

Code Samples

Reading JSON from the request.

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonAsync<WeatherForecast>();
    await UpdateDatabase(weather);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

---

Writing JSON to the response.

endpoints.MapGet("/weather/{city}", async context =>
{
    var city = (string)context.Request.RouteValues["city"];
    var weather = GetFromDatabase(city);

    await context.Response.WriteJsonAsync(weather);
});

---

Writing JSON to the response with explicit content type.

endpoints.MapGet("/weather/{city}", async context =>
{
    var city = (string)context.Request.RouteValues["city"];
    var weather = GetFromDatabase(city);

    await context.Response.WriteJsonAsync(weather, options: null, mediaType: "application/weather+json");
});

I'm not completely in love with this one. We might want to think about making more parameters optional.

---

Explicitly handling bad content-type

endpoints.MapPost("/weather", async context =>
{
    if (!context.Request.HasJsonContentType)
    {
        context.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
        return;
    }

    var weather = await context.Request.ReadJsonAsync<WeatherForecast>();
    await UpdateDatabase(weather);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

---

Letting routing handle bad content-type (possible feature)

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonAsync<WeatherForecast>();
    await UpdateDatabase(weather);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
})
.WithRequiredContentType("application/json");

[Tratcher]

1: HasJsonContentType

I tried to make the design of this similar to HasFormContentType which already exists on the HttpRequest. I considered as well making this an extension method, but there's no good reason to deviate from the pattern we established with form.

There's nothing about this property that makes it specific to a serializer, it's a straightforward comparison of the content type.

Not a fan of adding any more stuff directly on HttpRequest. HasFormContentType mainly makes sense because Form is also there, and in hindsight neither should have been.

When all of the other new JSON APIs are extensions, HasJsonContentType might as well be too.

[rynowak]

1: HasJsonContentType

I tried to make the design of this similar to HasFormContentType which already exists on the HttpRequest. I considered as well making this an extension method, but there's no good reason to deviate from the pattern we established with form. There's nothing about this property that makes it specific to a serializer, it's a straightforward comparison of the content type.

Not a fan of adding any more stuff directly on HttpRequest. HasFormContentType mainly makes sense because Form is also there, and in hindsight neither should have been.

When all of the other new JSON APIs are extensions, HasJsonContentType might as well be too.

I think either one of these could be reasonable, I went back and forth on which one to propose. It accomplishes the same goal either way, just slightly inconsistent in terms of syntax.

[davidfowl]

While I agree it would be ideal to not add more virtuals like this to the HttpContext, extension methods for things like this aren't great. Would we still do this if we add the Try methods?

[xsoheilalizadeh]

Why the HttpResponse isn't readable and I can't see any ReadJsonAsync extension method for that in your API proposal.

[davidfowl]

Why the HttpResponse isn't readable and I can't see any ReadJsonAsync extension method for that in your API proposal.

The HttpResponse is write only. You can read the body if you replace the output stream with a readable stream (like a memory stream or something more efficient...)

[xsoheilalizadeh]

You can read the body if you replace the output stream with a readable stream (like a memory stream or something more efficient...)

I did it in this way and not sure it's efficient. Could you show me similar internal usage of reading HttpResponse in ASP.NET Core?

[jstclair]

In the second-to-last example, isn’t that mixing up Content-Type and Accept?

[Yves57]

Request content type check

if (!context.Request.HasJsonContentType)
{
    context.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
    return;
}

seems a little bit painful to write to each endpoint. But .WithRequiredContentType("application/json") breaks the imperative code idea for me (and there is no possibility anymore to set a custom status code).

Extension methods target

But I was asking myself if imperative involved low level. For example attach the extension methods to HttpRequest and HttpResponse is the most logical in a technical point of view, but if the extension methods were attached to HttpContext, the code would be maybe clearer to read? In the same interrogation imperative / low level, are there other methods like WriteStatusCode in the pipe (not mandatory for a low level functionality, but useful in a imperative code)?

endpoints.MapPost("/weather", async context =>
{
    if (await context.TryReadJsonAsync<WeatherForecast>(out var weather))
    {
       await UpdateDatabase(weather);

       await context.WriteJsonAsync(weather);
   }
   else
   {
      context.WriteStatusCode(StatusCodes.Status415UnsupportedMediaType);
      // await context.WriteStatusCodeAsync(StatusCodes.Status415UnsupportedMediaType);
   }
});

[Rinsen]

This looks really good and useful!

Will it be possible to use together with dependency injection in an easy way in a library for example? Also without resolving the whole huge business implementation that I might have in a single huge ctor for all mappings. Will it also be possible to use standard scoped services here in any way?

The only examples I found did this in the Configure() method.

[poke]

it's a goal to have MVC call this API

Wouldn't break that the ability though to use different serializers like Json.NET with MVC? That would be a huge breaking change for anyone that relies on the ability to switch to a serializer with a larger feature set.

Wouldn't it make more sense to use the existing abstractions that exist, to allow users to have the choice?

[nil4]

Rather than coupling these methods to the HTTP request and response objects, couldn't they be targeting the PipeReader/PipeWriter or other abstraction for the request/response bodies? After all, that's what is actually being read/written. These may be more generally useful beyond HttpContext.

[epignosisx]

How would this play with model validation? Will there be a way of getting a hold of the model validation system through DI?

[JamesNK]

Important scenario: Reading and writing JSON DOM. Think of JObject and JArray from Json.NET, but with the new JSON DOM:

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonAsync<JsonObject>();
    await UpdateDatabase(weather["id"], weather["state"]);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

Hopefully System.Text.Json.JsonSerializer will Just Work when given one of these JSON DOM objects, but it is worth double checking with the JSON crew.

[JamesNK]

At risk of starting a bikeshedding sesh, the methods on JsonSerializer were changed from Read/Write to Serialize/Deserialize after a usability study.

ReadJsonAsync -> DeserializeJsonAsync?

Serialize/Deserialize would be more consistent with JsonSerializer.

[davidfowl]

Rather than coupling these methods to the HTTP request and response objects, couldn't they be targeting the PipeReader/PipeWriter or other abstraction for the request/response bodies? After all, that's what is actually being read/written. These may be more generally useful beyond HttpContext.

They should be on the request and response. There's already and API to read from a Stream but not a PipeReader and PipeWriter). If those get added, they should be in corefx, not in ASP.NET Core.

The other reason I prefer them on the HttpRequest and HttpResponse is that it allows us to change the implementation (from Stream to PipeReader) and not break consuming code.

[davidfowl]

At risk of starting a bikeshedding sesh, the methods on JsonSerializer were changed from Read/Write to Serialize/Deserialize after a usability study. ReadJsonAsync -> DeserializeJsonAsync? Serialize/Deserialize would be more consistent with JsonSerializer.

We could do another study 😄 , I still prefer Read/Write since those are the verbs we use today https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.formatters.inputformatter?view=aspnetcore-3.0

[davidfowl]

@poke Wouldn't it make more sense to use the existing abstractions that exist, to allow users to have the choice?

Which abstractions? Anything that exists today live in the wrong layer in the stack. That's another challenge. We don't want to push those things (which have the wrong namespace) lower into the core abstractions.

That said, @rynowak what would it look like if we added an abstraction (even if it lives in corefx)?

[davidfowl]

@Yves57 But I was asking myself if imperative involved low level. For example attach the extension methods to HttpRequest and HttpResponse is the most logical in a technical point of view, but if the extension methods were attached to HttpContext, the code would be maybe clearer to read? In the same interrogation imperative / low level, are there other methods like WriteStatusCode in the pipe (not mandatory for a low level functionality, but useful in a imperative code)?

I think it should be on the request and response, not HttpContext (we don't have much on the HttpContext today). That code sample does make me lean towards the Try* API more though.

[davidfowl]

@Rinsen

Will it be possible to use together with dependency injection in an easy way in a library for example? Also without resolving the whole huge business implementation that I might have in a single huge ctor for all mappings. Will it also be possible to use standard scoped services here in any way?

Are you asking specifically about injecting services into those routing delegates?

[JamesNK]

I still prefer Read/Write since those are the verbs we use today

No one uses inputters/outputters directly. I don't think we should pay them any attention.

More interesting prior art is ReadAsAsync<T> from System.Net.Http.Formatting - https://docs.microsoft.com/en-us/previous-versions/aspnet/hh835763(v%3Dvs.118)

[davidfowl]

And ReadFormAsync as @rynowak mentioned.

[JamesNK]

        public static ValueTask WriteJsonAsync(
            this HttpResponse response,
            Type type,
            object? value,
            CancellationToken cancellationToken = default) { throw null; }

What does the type argument do here? Why an explicit type vs value?.GetType()?

[Tratcher]

You can read the body if you replace the output stream with a readable stream (like a memory stream or something more efficient...)

I did it in this way and not sure it's efficient. Could you show me similar internal usage of reading HttpResponse in ASP.NET Core?

Your usage is adequate but will have issues with larger responses. The main optimization we make in our own components is to do what we call write-through processing. The bytes are processed as they're written rather than copied back at the end. https://github.com/aspnet/AspNetCore/blob/426a70c4506cd000cbf827d8bfa1c4a7ad8aa45d/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs#L108-L123 https://github.com/aspnet/AspNetCore/blob/426a70c4506cd000cbf827d8bfa1c4a7ad8aa45d/src/Middleware/ResponseCaching/src/Streams/ResponseCachingStream.cs#L133-L157

[davidfowl]

What does the type argument do here? Why an explicit type vs value?.GetType()?

Follows the JSON serializer API shape.

[Rinsen]

@Rinsen

Will it be possible to use together with dependency injection in an easy way in a library for example? Also without resolving the whole huge business implementation that I might have in a single huge ctor for all mappings. Will it also be possible to use standard scoped services here in any way?

Are you asking specifically about injecting services into those routing delegates?

Yes!

So the UpdateDatabase method implementation can come from injected services.

[pimbrouwers]

@Rinsen

Will it be possible to use together with dependency injection in an easy way in a library for example? Also without resolving the whole huge business implementation that I might have in a single huge ctor for all mappings. Will it also be possible to use standard scoped services here in any way?

Are you asking specifically about injecting services into those routing delegates?

Yes!

So the UpdateDatabase method implementation can come from injected services.

Why not just wrap your request delegate's in a closure and poor-man/woman inject the services yourself?

[Yves57]

I think it should be on the request and response, not HttpContext (we don't have much on the HttpContext today). That code sample does make me lean towards the Try* API more though.

@davidfowl If I'm not wrong, choosing Feature or options to get the default JsonSerializerOptions will need to access to the parent HttpContext. So the code would be still be linked to the HttpContext. The developer may suppose that the method is limited to the HttpRequest, but it would in fact depend to the parent HttpContext too.

Other question: what about HttpContext.RequestAborted? It may be interesting to automatically integrate it in some methods (Try* ?).

[davidfowl]

@davidfowl If I'm not wrong, choosing Feature or options to get the default JsonSerializerOptions will need to access to the parent HttpContext. So the code would be still be linked to the HttpContext. The developer may suppose that the method is limited to the HttpRequest, but it would in fact depend to the parent HttpContext too.

That's an implementation detail and it doesn't change the semantics of what the method does. I don't think it makes sense to put things on the context that have to do with reading and writing response bodies. We haven't done it to date, and I haven't seen anything that would make us change it.

Other question: what about HttpContext.RequestAborted? It may be interesting to automatically integrate it in some methods (Try* ?).

No, nothing should implicitly use the token.

[davidfowl]

@pimbrouwers Why not just wrap your request delegate's in a closure and poor-man/woman inject the services yourself?

Because that doesn't work when you need to access services with a scoped lifetime.

[Tratcher]

Other question: what about HttpContext.RequestAborted? It may be interesting to automatically integrate it in some methods (Try* ?).

No, nothing should implicitly use the token.

Today reading the request body already throws an exception when the client disconnects, passing the token isn't needed. We'd have think about the expected behavior for a TryRead API. I'm not sure returning false makes sense if it conflates A) The request doesn't have a JSON content-type (try something else?) B) The Body couldn't be parsed as valid JSON (400) or C) The client disconnected and the JSON was truncated (Abort).

[JamesNK]

B would be: invalid JSON, error reading incoming content, or error deserializing object (e.g. deserializing "three" into an int). If JsonSerializer ever added TryDeserialize method then it would be B/C.

I don't think we should do anything around TryReadJson until JsonSerializer does it first.

[JamesNK]

Regarding content type, a way to handle different content types that would be routing content negotiation:

// JSON
endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonAsync<JsonObject>();
    await UpdateDatabase(weather["id"], weather["state"]);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
}).WithMetdata(new ConsumesAttribute("application/json"));

// Forms
endpoints.MapPost("/weather", async context =>
{
    var form = await context.Request.ReadFormAsync();
    await UpdateDatabase(form["id"], form["state"]);

    context.Response.StatusCode = StatusCodes.Status202Accepted;
}).WithMetdata(new ConsumesAttribute("multipart/form-data"));

// Everything else returns 415

@rynowak Might be worth adding nicer extension methods for content negotiation, like we have for authz.

endpoints.MapPost("/weather", async context =>
{
    // ...
}).RequireContent("application/json");

Or even just .RequireJson()

[jchannon]

It's interesting to see these features as its very similar to what Nancy did off and what Carter currently does offer.

Whilst all these features proposed here look good they all exist at the middleware level. The nice thing that Nancy/Carter offers is that it allows you to follow these proposed API changes at the module/controller level.

Would it be possible to bubble these up to the controller in mvc?

[HttpPost("/accounts")]
public async Task<IActionResult> CreateAccount()
{
  var weather = await context.Request.ReadJsonAsync<Account>();

 //validation

//....
}

[rynowak]

wow, there's been a lot of comments on this since I posted it. Thanks for the interest and the good discussion 😀

Responding to comments that haven't gotten an answer yet.

---

@jstclair

In the second-to-last example, isn’t that mixing up Content-Type and Accept?

I don't think so.

If the client sends a request with Content-Type: application/xml to a handler that only knows how to process JSON, then 415 is the right response.

One enhancement that could be asked for here, is an easy way to know the client want to receive JSON (Accept header). That can result in a 406 - however, we generally just don't return 406 today. By the time you're fully checking both input and output content types, this starts to feel a lot like what MVC does.

---

@poke

Wouldn't break that the ability though to use different serializers like Json.NET with MVC? That would be a huge breaking change for anyone that relies on the ability to switch to a serializer with a larger feature set.

Sorry for not being totally clear. MVC's System.Text.Json support would call through to this.

---

@epignosisx

How would this play with model validation? Will there be a way of getting a hold of the model validation system through DI?

This does not play with model validation. It's possible to interact with MVC's validation system through DI, but it's extremely complex.

This is also on my list to look at, but it's separate from this proposal.

---

@JamesNK

Regarding content type, a way to handle different content types that would be routing content negotiation:

What I suggested is pretty similar no? I didn't use the [Consumes] attribute because that's an MVC feature in the MVC namespace - but I imagined similar functionality.

---

@jchannon

Would it be possible to bubble these up to the controller in mvc?

Everything we put at lower levels of the stack is available in MVC.

[jchannon]

Will you add validation hooks too like Carter does?

context.Request.ValidateJsonAsync<Account>(); for example

[pimbrouwers]

@pimbrouwers Why not just wrap your request delegate's in a closure and poor-man/woman inject the services yourself?

Because that doesn't work when you need to access services with a scoped lifetime.

It does if you bring into context the IServiceCollection. I'm also not certain that DI should be the default approach for every project, so it may not even be a concern in all cases.

[Rinsen]

@pimbrouwers Why not just wrap your request delegate's in a closure and poor-man/woman inject the services yourself?

Because that doesn't work when you need to access services with a scoped lifetime.

It does if you bring into context the IServiceCollection. I'm also not certain that DI should be the default approach for every project, so it may not even be a concern in all cases.

If you want to use DI it is really not a god option to not use DI. And injecting a IServiceCollection or some other factory method is not a compelling solution at all. Then I rather go with som other pattern like pure MVC or something else.

[davidfowl]

@pimbrouwers It does if you bring into context the IServiceCollection. I'm also not certain that DI should be the default approach for every project, so it may not even be a concern in all cases.

Not sure what you mean, you don't have to use DI if you don't want, but you can't really use closures (or I'm misunderstanding what you mean).

Consider the following:

endpoints.MapPost("/weather/{id}", async context =>
{
    var id = context.Request.RouteValues["id"];
    var weather = await context.Request.ReadJsonAsync<JsonObject>();

    using var db = new WeatherContext();

    var w = db.Weathers.Find(int.Parse(id));
    w.State = weather.State;
    await db.SaveChangesAsync();

    context.Response.StatusCode = StatusCodes.Status202Accepted;
});

Now you want to inject the WeatherContext so this can be tested. Here are some options:

Task PostWeather(HttpContext context, WeatherContext db) 
{
    var id = context.Request.RouteValues["id"];
    var weather = await context.Request.ReadJsonAsync<JsonObject>();

    var w = db.Weathers.Find(int.Parse(id));
    w.State = weather.State;
    await db.SaveChangesAsync();

    context.Response.StatusCode = StatusCodes.Status202Accepted;
}

// 1.
endpoints.MapPost("/weather/{id}", async context =>
{
    using var db = new WeatherContext();
    await PostWeather(context, db);
});

// 2.
endpoints.MapPost("/weather/{id}", async context =>
{
    // Scoped service
    var db = context.RequestServices.GetRequiredService<WeatherContext>();
    await PostWeather(context, db);
});

// 3. We could add overloads that make this easier

endpoints.MapPost("/weather/{id}", async (HttpContext context, WeatherContext db) =>
{
    // Scoped service
    await PostWeather(context, db);
});

[pimbrouwers]

@davidfowl -- Sorry for the confusion. I should've been more specific, clear and accurate. I only meant to imply that developer's don't always HAVE to jump right into the built-in IoC container. And that dependencies could be provided by others means, in (my opinion) sometimes a more explicit way. Either using closures, or a plain old object. For example:

using System;
using System.Data;
using System.Data.SQLite;
using System.Net;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace WebApp {
    public class Program {
        public static async Task<int> Main(string[] args) {
            try {
                var db = new Db("Data Source=:memory:;Version=3;New=True;");
                var server = new Server(db);

                var host = new WebHostBuilder()
                    .UseKestrel()
                    .UseUrls("http://localhost:5000/;https://localhost:5001")
                    .ConfigureServices(_configureServices)
                    .Configure(_configure(server))
                    .Build();

                await host.RunAsync();

                return 0;
            } catch {
                return -1;
            }
        }

        static void _configureServices(IServiceCollection services) {
            services.AddRouting();
        }

        static Action<WebHostBuilderContext, IApplicationBuilder> _configure(Server server) {
            return (context, app) => {
                app.UseRouting();
                app.UseEndpoints(server.Routes);
                app.Run(server.handleNotFound);
            };
        }
    }

    internal class Server {
        readonly Db _db;

        internal Server(Db db) {
            _db = db;
        }

        internal void Routes(IEndpointRouteBuilder r) {
            r.MapGet("/post/get", handlePostGet);
            r.MapGet("/hello/{name}", handleHelloName);
            r.MapGet("/", handleHello);
        }

        internal Task handleNotFound(HttpContext ctx) {
            ctx.Response.StatusCode = (int)HttpStatusCode.NotFound;
            return ctx.Response.WriteAsync("Post not found.");
        }

        Task handleHello(HttpContext ctx) =>
            ctx.Response.WriteAsync("Hello World!");

        Task handleHelloName(HttpContext ctx) =>
            ctx.Response.WriteAsync("hello " + ctx.Request.RouteValues["name"]);

        Task handlePostGet(HttpContext ctx) {
            using var conn = _db.CreateConnection();
            using var cmd = conn.CreateCommand();
            cmd.CommandText = "SELECT 1 as post_id, 'hello world' as title, 'some text' as body";

            using var rd = cmd.ExecuteReader();
            if (rd.Read()) {
                return ctx.Response.WriteAsync(rd.GetString(rd.GetOrdinal("body")));
            } else {
                return handleNotFound(ctx);
            }
        }
    }

    internal class Db {
        readonly string _connStr;

        internal Db(string connStr) {
            _connStr = connStr ?? throw new ArgumentNullException(nameof(connStr));
        }

        internal IDbConnection CreateConnection() =>
            new SQLiteConnection(_connStr).OpenAndReturn();
    }
}

This example is trivial, uses poor naming (I did this for simplicity) and missing a bunch of boilerplate code (null dep checks, error checking etc.), but demonstrates the point I'm trying to make. This approach I think is testable and clear. I also like that Server can be constructed prior to kestrel's activation, which becomes useful if you want to use precompiled templates (ex: Stubble/Scriban/Nustache etc.) and wish to panic/exit if something goes amuck. You could also assert the db in this manner if desired.

I could be missing something glaring, I definitely don't have a CS background. For whatever that's worth.

As someone who came from outside .NET, I wish the approach in teaching the platform started from this perspective. As opposed to "here is full-blown MVC and EF". I believe that we'd like have greater adoption, and probably better developer's.

Just my two uneducated cents.

p.s. @davidfowl loving the lack of braces on those usings?

[jstclair]

@jstclair

In the second-to-last example, isn’t that mixing up Content-Type and Accept?

I don't think so.

If the client sends a request with Content-Type: application/xml to a handler that only knows how to process JSON, then 415 is the right response.

One enhancement that could be asked for here, is an easy way to know the client want to receive JSON (Accept header). That can result in a 406 - however, we generally just don't return 406 today. By the time you're fully checking both input and output content types, this starts to feel a lot like what MVC does.

Sorry if it feels like I'm bike-shedding here - but in the example, it's a GET and the parameter is via the route, so I'm unclear why you'd care what the Content-Type is? It's fine to explicitly avoid the content-negotiation dance, but the proposed API changes seem to assume that checking Content-Type is the same as checking Accept

[Rinsen]

@davidfowl -- Sorry for the confusion. I should've been more specific, clear and accurate. I only meant to imply that developer's don't always HAVE to jump right into the built-in IoC container. And that dependencies could be provided by others means, in (my opinion) sometimes a more explicit way. Either using closures, or a plain old object. For example: .....

I feel now that I was a bit to hard in my response, I strongly agree with you that it's great to have both options! Thad adds a lot of value to the stack!

[davidfowl]

@pimbrouwers In your example, the Db class is a singleton. That's why I said closures don't work for all scenarios. If you want things created per request and you want to be able to pass them explicitly, you're kinda stuck with the IoC container or code is that creating objects per request (like a factory or using new)

[rynowak]

@jstclair

In the second-to-last example, isn’t that mixing up Content-Type and Accept?

I don't think so. If the client sends a request with Content-Type: application/xml to a handler that only knows how to process JSON, then 415 is the right response. One enhancement that could be asked for here, is an easy way to know the client want to receive JSON (Accept header). That can result in a 406 - however, we generally just don't return 406 today. By the time you're fully checking both input and output content types, this starts to feel a lot like what MVC does.

Sorry if it feels like I'm bike-shedding here - but in the example, it's a GET and the parameter is via the route, so I'm unclear why you'd care what the Content-Type is? It's fine to explicitly avoid the content-negotiation dance, but the proposed API changes seem to assume that checking Content-Type is the same as checking Accept

Ah! Thats the mistake. I'll fix the example, thanks!

[alefranz]

5: Managing JsonSerializerOptions ... Using options has some runtime overhead for the service lookup. Of these options seems like the best choice. We could also use the options approach to share the options instance between MVC's JsonOptions and this one for compatibility.

That feels the best option. The default options as you said are sub-optimal (my standard is: camelCase properties, camelCase enums, omit null properties) so there is a need for an easy way to change the options, in a single place for everything (uncommon to have different options per endpoint).

Writing JSON to the response with explicit content type. ... I'm not completely in love with this one. We might want to think about making more parameters optional.

I think it would be nice to have a way to specify globally a media type, like the options, as it is not uncommon to use same custom media type and serialization options for all endpoints. This would define the default content-type, not sure if it would be needed for the Accept value as well.

Letting routing handle bad content-type (possible feature) ... .WithRequiredContentType("application/json");

It's not clear to me if this would only handle the content-type or also the Accept value.

Anyway, if this is going to happen, and there is going to be a default media type (as per above comment), then there should probably be a method to enforce the default like .WithRequiredContentType() or .WithRequiredDefaultContentType()

---

I really like this as I think it would be really useful to write simple endpoints. However, to be a viable solution out of quick prototyping, there should be some syntactic sugar to have content negotiation and error handling, otherwise creating a Controller would still be more convenient then to write inline handler.

What about having some standard exceptions that can be thrown by this methods and handled by default, so you can either:

• use those methods and handle the exceptions yourself
• use the Try methods and handle the error scenarios
• (NEW) don't handle the error scenario and it would just work

e.g.

endpoints.MapPost("/weather", async context =>
{
    var weather = await context.Request.ReadJsonAsync<WeatherForecast>();
    // Throws exception then translated to 415 if body is not json
    // Throws exception then translated to 400 if it fails to parse

    await UpdateDatabase(weather);

    await context.Response.WriteJsonAsync(weather);
    // Throws exception then translated to 406 if the Accept is not json
    // Not ideal not being able to fail fast, but you can not know upfront is there is going to be a body and it is going to be json.
    // Easy to see the drawbacks in this specific example as you get an error even if you had updated the DB
});

[rynowak]

@JamesNK - updated with our notes

[Tratcher]

You know the other place people have been begging us to add these json extensions? Session. Our docs even have a basic version of them you can paste into your app.

[rynowak]

API approved - we've made some small edits:

ReadJsonAsync -> ReadFromJsonAsync WriteJsonAsync -> WriteAsJsonAsync mediaType -> contentType

Also, it looks like IdentityServer has already done the work to make their extensions scoped: https://github.com/IdentityServer/IdentityServer4/issues/3699

[BrennanConroy]

@JamesNK Can you get to this in preview6?