ApiController conventions

[pranavkm]

Background

The goal of this feature is to provide good API documentation (i.e. ApiExplorer) for idiomatic APIs with minimal attribute soup. In 2.1.0, we took a stab at this - #6936 - conventions were types that
at runtime transformed ApiDescription. One of the difficulties with this approach is that there isn't a very good way to tell if the actual body of an action deviated from the convention. For instance, a convention could state that an action should return a 404, but could not verify the veracity of this claim. It would be nicer, if we could look at returned types in a method and infer what response codes and types are returned by an action.

This brings us to the idea of using analyzers and code fixes. We could leverage analyzers to indicate when actions deviated from their stated conventions and use code fixes to "control + ." their way to success. A hard limit that analyzers enforce is that any convention that needs to be applied has to be available without executing application code. In addition, this needs to be available both at runtime, so that the runtime can transform ApiDescription, and at build time so analyzers \ code fixes could inspect it. One way to go about this is expressing conventions as types.

API conventions as code

• Explicit ApiExplorer attribute, such as ProducesResponseTypeAttribute, take precedence over any conventions. Once an action is associated with an ApiExplorer attribute, no conventions are applied.
  🌮 - We may invent a new attribute that allows specifying an exact convention to apply e.g. ApiConvention(typeof(SomeConvention), nameof(SomeConvention.Name)).

• One or more ApiConventionsAttribute can be applied either to the controller or as an assembly attribute. Each ApiConventionsAttribute accepts a type parameter that is the convention type.

[ApiConventions(typeof(SomeConventions))]

• For the initial go, we want to avoid dealing with complexity around type inheritance, method overloads etc. We'll require the convention type to be static.

public static class SomeConventions
{
    public static TModel Get<TModel>(object id) => null;
}

• Rules for matching a convention method to an action method.

  • The convention method name must be a prefix of the action method name. The method name Get from above is a match for the following actions: Get, GetItem, GetById. We'll use some of the casing based matching from page handler methods and avoid matching things like GetawayFrom(), PostalService as prefixes.
  • Convention parameter names must be a suffix for the action parameter. id from above would match id, personId while not matching grid.
  • Leading underscores may be used to represent multiple parameters with the same suffix e.g. The convention method Get(object id, object _id) to match methods such as GetPersonAddress(int personId, int addressId);
  • object is used as a wildcard parameter type. Any other type requires matching. The convention method Get(object id) matches GetPerson(int personId), bit Get(string id) would not.
  • Generic parameters are treated like wildcards and are not matched by type or name. This allows us to match Put(object id, TModel model) to match Put(int id, Person person).
  • 🌮 - Possibly need a way to match remaining arguments e.g. List(string searchTerm, params object[] any) that matches List(string searchTerm, int top, int page) or List(string searchTerm, PaginationData pagination)

Default API conventions

We'd like to produce good documentation if you use the defaults. To this effect, e'll tailor a default set of conventions around code that Scaffolding produces. Here's a stripped copy of things that shipped with ASP.NET Core 2.1: https://gist.github.com/pranavkm/fbb590e031dbd0d619c3b1e01209691d. This gets us a set of conventions that looks like so:

public static class DefaultApiConventions
{
    [ProducesResponseType(StatusCodes.Status200OK)]
    [ProducesResponseType(StatusCodes.Status404NotFound)]
    [MatchByName(NameMatchBehavior.Prefix)]
    public static void Get(
        [MatchByName(NameMatchBehavior.Suffix)]
        [MatchByType(TypeMatchBehavior.DerivedTypes)]
        object id)
    { }

    [ProducesResponseType(StatusCodes.Status201Created)]
    [ProducesResponseType(StatusCodes.Status400BadRequest)]
    [MatchByName(NameMatchBehavior.Prefix)]
    public static void Post(
        [MatchByName(NameMatchBehavior.Any)]
        [MatchByType(TypeMatchBehavior.Any)]
        object model)
    { }

    [ProducesResponseType(StatusCodes.Status204NoContent)]
    [ProducesResponseType(StatusCodes.Status404NotFound)]
    [ProducesResponseType(StatusCodes.Status400BadRequest)]
    [MatchByName(NameMatchBehavior.Prefix)]
    public static void Put(
        [MatchByName(NameMatchBehavior.Suffix)]
        [MatchByType(TypeMatchBehavior.DerivedTypes)]
        object id,

        [MatchByName(NameMatchBehavior.Any)]
        [MatchByType(TypeMatchBehavior.Any)]
        object model)
    { }

    [ProducesResponseType(StatusCodes.Status200OK)]
    [ProducesResponseType(StatusCodes.Status404NotFound)]
    [ProducesResponseType(StatusCodes.Status400BadRequest)]
    [MatchByName(NameMatchBehavior.Prefix)]
    public static void Delete(
        [MatchByName(NameMatchBehavior.Suffix)]
        [MatchByType(TypeMatchBehavior.DerivedTypes)]
        object id)
    { }
}

StatusCodeAttribute

Statically knowning status codes from an ActionResult type is essential for analyzers to perform a diff of what was specified versus what's in code. We'll introduce a StatusCodeAttribute that indicates what the default status code associated with an ActionResult is:
e.g.

[StatusCode(201)]
public class CreatedAtActionResult : ObjectResult

As long as the compiler can infer this type being returned from an action, we should be able to infer a possible status returned from the action. Using the default set of action results that come with MVC or using helper methods on controller base should do the right thing. That said, there are possible patterns where this approach does not work. We can think about these once we've had more user feedback, but listing them here for posterity:

• Returning custom action result instances without the attribute.
• Status codes calculated at runtime e.g. return StatusCode(204); 🌮 We could tackle this unique case of StatusCodeResult, but not generally so.
• Returning an action result with a different status code than the one by default return new OkResult(result) { StatusCode = 201 };
• If the return type cannot be inferred or is lost e.g. var result = id == 0 ? (ActionResult)NotFound() : Ok("some-content"); return result;

Analyzers and code fixes

• An analyzer that provides info level diagnostics if the matched convention suggests a status code that does not seem to appear in code. e.g. Your convention says that you should be returning a 404, but I don't see one in your code.
• An analyzer that provides a warning if an action returns a status code that does not appear in the convention. e.g. Your convention does not state that you return a 204, but your code does. This can be a warning with code fixes that allow you to either
  • apply the ApiExplorer attributes to the action
  • produce a new convention, or modify an existing convention in code apply it to the controller

---

6/25 - Updated to reflect attributes for matching conventions.

[pranavkm]

Putting this on hold - we have a fairly demoable prototype now. Will start working on it once we have a design

[pranavkm]

cc @rynowak \ @glennc using this issue to drive design and discussions for Api conventions

[pranavkm]

Tracking further work using individual items