Add equivalent of AttrCapabilities for relationships

[bkoelman]

Is your feature request related to a problem?

This design is based on the community request at https://gitter.im/json-api-dotnet-core/Lobby?at=63199f777ccf6b6d45c52943.

Describe the solution you'd like

Currently, developers can control which operations are allowed declaratively per JSON:API attribute in the following way:

public class Customer : Identifiable<long>
{
    [Attr(Capabilities = AttrCapabilities.AllowFilter | AttrCapabilities.AllowSort)]
    public string Name { get; set; }
}

If no capabilities are set in [Attr], what's set in IJsonApiOptions.DefaultAttrCapabilities (which defaults to AttrCapabilities.All) is used.

The proposal here is to add a similar feature for relationships. It would enable the next example:

public class Customer : Identifiable<long>
{
    [Attr(Capabilities = AttrCapabilities.AllowFilter | AttrCapabilities.AllowSort)]
    public string Name { get; set; }

    [HasOne(Capabilities = HasOneCapabilities.AllowView | HasOneCapabilities.AllowSet)]
    public Address? Address { get; set; }

    [HasMany(Capabilities = HasManyCapabilities.AllowView | HasManyCapabilities.AllowAdd)]
    public ISet<Order> Orders = new HashSet<Order>();
}

The effective relationship capabilities would likewise be determined by falling back to what's set on IJsonApiOptions, defaulting to All. When an incoming request violates these constraints, an appropriate HTTP error is returned for one of the violations (the order is undefined and we won't aggregate all violations; I'm assuming that's how AttrCapabilities works today). Similarly to how AttrCapabilities is implemented, custom logic in resource definitions is not constrained by relationship capabilities. In case AllowView is blocked, we need to take precautions to never return the relationship in any response bodies.

Proposed capabilities to add:

/// <summary>
/// Indicates capabilities that can be performed on a <see cref="HasOneAttribute" />.
/// </summary>
[Flags]
public enum HasOneCapabilities
{
    None = 0,

    /// <summary>
    /// Whether or not GET requests can retrieve the to-one relationship. Attempts to retrieve when disabled
    /// will return an HTTP 400 response.
    /// </summary>
    AllowView = 1,

    /// <summary>
    /// Whether or not POST and PATCH requests can assign or clear the to-one relationship. Attempts to assign
    /// or clear when disabled will return an HTTP 400 response from relationship endpoints and an HTTP 422
    /// response from resource endpoints.
    /// </summary>
    AllowSet = 2,

    All = AllowView | AllowSet
}

/// <summary>
/// Indicates capabilities that can be performed on a <see cref="HasManyAttribute" />.
/// </summary>
[Flags]
public enum HasManyCapabilities
{
    None = 0,

    /// <summary>
    /// Whether or not GET requests can retrieve the to-many relationship. Attempts to retrieve when disabled
    /// will return an HTTP 400 response.
    /// </summary>
    AllowView = 1,

    /// <summary>
    /// Whether or not POST and PATCH requests can replace the to-many relationship. Attempts to replace
    /// when disabled will return an HTTP 400 response from relationship endpoints and an HTTP 422 response
    /// from resource endpoints.
    /// </summary>
    AllowSet = 2,

    /// <summary>
    /// Whether or not POST requests can add to the to-many relationship. Attempts to add when disabled
    /// will return an HTTP 400 response.
    /// </summary>
    AllowAdd = 4,

    /// <summary>
    /// Whether or not DELETE requests can remove from the to-many relationship. Attempts to remove when
    /// disabled will return an HTTP 400 response.
    /// </summary>
    AllowRemove = 8,

    All = AllowView | AllowSet | AllowAdd | AllowRemove
}

Alternative solutions and open questions

1. List of capabilities
   We could split Has[One|Many]Capabilities.AllowSet into .Clear and .Assign, or add these while keeping Set (which implies both).

   • .Clear for a to-one relationship means assigning null and for a to-many relationship it means assigning an empty array.
   • .Assign for a to-one relationship means assigning a non-null value and for a to-many relationship it means assigning a non-empty array.

   We can't further distinguish between "assign from empty" vs "replace the existing value" for a to-one relationship, because it would require fetching the currently stored value first, which we're trying to avoid for performance reasons.

   The split could be done at a later time by adding the Clear/Assign members next to the existing Set member. On the other hand, there's no good reason to postpone it if it's considered useful to have them today. I can't think of a reason why the need for them would increase in the future.

2. HTTP status code
   Today JsonApiDotNetCore returns 400 for bad requests, but 422 when the problem is in the request body. Because setting a relationship could be done from:

   • POST /customers
   • PATCH /customers/1
   • PATCH /customers/1/relationships/orders

   adhering to the existing rule means to return 422 in the first two cases, but 400 in the last one. The alternative is to break with the rule and always return HTTP 400 on capabilities violations. I'm not sure what the right choice is here.

3. Meaning of AllowView
   When blocked, this could either mean hiding the relationship data (type + id), or the relationship itself (data + links). And this raises the question of how it interacts with includes from query strings. Today we have the boolean CanInclude property on HasOne/HasMany relationships, which determines whether ?include= can contain the relationship. Should blocking AllowView implicitly mean to also block ?include=, or should we leave this up to the user? The latter means more flexibility, but at the cost of unintentionally forgetting to set CanInclude = false in addition to blocking AllowView, resulting in the wrong outcome. And should we deprecate CanInclude in favor of adding .AllowInclude to the enumerations?

[bkoelman]

Design update

In the proposal above, HasManyCapabilities.AllowFilter is missing. It affects the count() and has() filter functions, which accept a to-many relationship.

Splitting AllowSet

The used terminology is becoming non-intuitive and requires explanation. That's not a good sign.
So I decided to leave this out, also because there's no AttrCapabilities equivalent for setting or clearing an attribute.

HTTP status code

As it turns out, AttrCapabilities is already using 400 and 422 status codes (but not both in the same setting). I decided to follow the pattern used there, to keep it simple.

Generally, failures during write operations return 422, while blocked query string parameters return 400. The latter is required by JSON:API.

AllowView / CanInclude

Turning off AllowView silently hides the field. But when explicitly asked for using ?fields[]=, it returns an error. That's how AttrCapabilities works and the new capabilities should work the same.

Hiding a relationship should hide the entire relationship, not just its data. Rendering links to something you're trying to hide is counterproductive.

Regarding hiding related resources when ?include= is used, in addition to the relationship itself: Yes we should, because this is required by JSON:API full linkage:

Every included resource object MUST be identified via a chain of relationships originating in a document’s primary data.
This means that compound documents require “full linkage” and that no resource object can be included without a direct or indirect relationship to the document’s primary data.

The only exception to the full linkage requirement is when relationship fields that would otherwise contain linkage data are excluded due to sparse fieldsets requested by the client (emphasis mine).

However, note that blocking AllowView does not prevent fetching the related resources directly via a primary resource request.

I've obsoleted CanInclude and added Capabilities.AllowInclude. But to remain backwards-compatible, when CanInclude is explicitly set, it takes precedence over AllowInclude.

So there's no interaction between AllowView and CanInclude/AllowInclude, they affect separate things. The full linkage requirement for AllowView makes the concern of unintentionally including related resources go away.