[RFC] Support for inheritance among resources

[maurei]

Recently a PR was merged which introduces support for updating relationships of a resource when relationship types are abstract. It is supported by accepting for these relationship assignments members of types that are derived from this abstract relationship type. (see #696 and #833). It is said that this implementation allows for heterogeneous relationships because eg Article.Author can be assigned to either Student or Teacher.

Having such heterogeneity in relationships is in accordance with the way how JADNC currently deals with endpoints for abstract resources. Responses of these endpoints are currently heterogeneous too, i.e. the /people endpoint for the abstract Person resource will return entries of types students and teachers, not people.

Having heterogeneous responses from abstract endpoints was however never the result of a conscious design choice. A different approach could be having homogeneous responses where the type is just people, containing only the subset of attributes shared by all derived types. There currently is no consensus in the json:api community as to which approach is best (see json-api/json-api#862 and json-api/json-api#28).

Both approaches have their pros and cons: it mostly depends on the specific use case of the developer. Therefore, I believe deciding on whichever is best (or perhaps even allow for both through configurability) depends on input/use cases of the community. Hence this is a tracking issue for that purpose.

Note that the following features are known to be broken when dealing with resource inheritance (also see #846):

1. /people?include=... for to one, to many and many to many relationships
2. Filtering on nested scopes, since this relies on the previous
3. people?fields=...
4. Pagination and sorting on nested scopes

All of these are broken because they rely on the construction of a Select clause when building the queries. This fails because when creating a new expression as a part of such projection, the resource factory does not take into consideration model inheritance. Instead, it tries to create a new expression using the abstract type, which is not possible. A mechanism would have to be incorporated that takes into account, if present, the discriminator configuration when choosing the type for which to create a new expression. I am currently not sure if this is even possible without prematurely executing the query.

[RichardsonWTR]

I'll argue that first, the JADNC library needs to focus on implementing JSON API spec first, and some options should be evaluated later in the future. I personally don't feel json-api/json-api#28 and json-api/json-api#862 discussions are over, so JADNC team should proceed with caution, to avoid incompatibilities with JSONAPI frontend libraries, etc.

Example

• There are the following classes: Student and Teacher, both inherit from Person, an abstract class.

I agree that the backend should not expose to the client heterogeneous responses. It should be kept simple, predictable. An abstract class inheritance could be implemented as a simple one-to-one relationship.

What to do first

To sum up, in my opinion, JADNC should implement the minimum.

• If the client request GET /people, it should return just people. Just the common attributes.
• If the client requests GET /students, it should return the specific Student attributes. If the user needs all the attributes, it should use an inclusion: GET /students?include=people.
• Since Person is abstract, the server must allow only GET responses from this endpoint.

What to do later, as an option:

• IMO, GET /students returning Person + Student attributes looks nicer, but I think it breaks the simplicity I just said. This could be something JADNC could let the user decide, as an option.

[maurei]

An abstract class inheritance could be implemented as a simple one-to-one relationship.

I disagree that this is simple or evident. I think it is actually confusing and mixing up concepts.

Usually relationships as exposed through json:api content reflect has relationships, eg article has one or many author(s). The relationship you're describing here however is a is relationship: student is a person, so there is no composition here unlike in the has relationship. I don't think it is evident that the include operator should used for both types interchangeably. A client would have to be aware of the type of relationship when including, while the concept has vs is relationships is foreign to the json:api spec. So I think it certainly wouldn't be "simple and predictable" if we were to mix these up. Implementing GET /students?include=people would introduce a whole new concept.

That being said, a different approach would be to introduce a different operator to side-load such is data. But I don't think we should just implement this before it being clear that this is in line with where the json:api spec is going.

I think that this is a sign that heterogeneity might be the better approach. It takes away the clients responsibility of having to differentiate between has and is. I also don't think heterogeneity reduces simplicity, but that its flexibility will introduce simpler use cases. But as mentioned in the OP, I am aware that to be sure about this we need to know more about these actual use cases. As such I think we should not move forward until we know about them.

[bart-degreed]

We could use the .NET type system to our advantage here. In case a resource class is abstract, it has no meaning on itself. For example, while an abstract Vehicle has weight and dimensions, it gets meaning through its derived types like Car, Bike, Truck etc. So a GET /vehicles would return solely derived types with all their subclass-specific fields.

On the other hand, when the base class is not abstract, it has meaning on its own. For example, GET /content would solely return content items with their URLs, while GET /books would return both URLs and number of pages.

This distinction allows API developers to steer what is returned without the need for a configuration option.

To support this, we'd need to enhance building of SELECT clauses based on the discriminator. I've done some experiments in this area and found that EF Core is capable of producing proper SQL for the following:

IQueryable<Human> staticQuery = _appDbContext.Humans
    //.Where(human => human is Man && ((Man)human).HasBeard)
    .Select(human => human is Man ? new Man
    {
        Id = human.Id,
        Name = human.Name,
        Age = human.Age,
        HasBeard = ((Man)human).HasBeard
    } :
    human is WorkingWoman ? (Human)new WorkingWoman
    {
        Id = human.Id,
        Name = human.Name,
        Age = human.Age,
        IsPregnant = ((WorkingWoman)human).IsPregnant,
        WorksPartTime = ((WorkingWoman)human).WorksPartTime,
        Profession = ((WorkingWoman)human).Profession == null ? null : new Profession
        {
            Id = ((WorkingWoman)human).Profession.Id,
            JobName = ((WorkingWoman)human).Profession.JobName
        }
    } :
    human is NonWorkingWoman ? new NonWorkingWoman
    {
        Id = human.Id,
        Age = human.Age,
        Name = human.Name,
        IsPregnant = ((NonWorkingWoman)human).IsPregnant
    } : null);

var humanType = _appDbContext.Model.GetEntityTypes().Single(x => x.ClrType == typeof(Human));
var concreteTypes = humanType.GetConcreteDerivedTypesInclusive();

Human[] staticResult = staticQuery.ToArray();
string staticText = DumpText(staticResult);

Forgive me if the model above looks discriminating to you. I'm not trying to express a political statement of any kind, this is just a model I built some time ago which allowed me to explore the feature.

Once we have these SELECT clause enhancements in place, we could consider to add extra filter functions in order to perform type checks.

While we should learn from the different points of view in json:api spec discussions, I don't think we need to wait for a definitive decision in the form of a spec change before implementing this. The spec currently does not say anything about inheritance, leaving it up to implementations to choose a strategy that best fits their platform. And we already seem to allow inheritance, just not in a very consistent/documented/tested way.

[bart-degreed]

I'm labelling this 'breaking' for two reasons:

• We don't exactly know how the current implementation handles inheritance for abstract and non-abstract types.
• We'll likely want to properly test and support table-per-type mapping, which was added in EF Core 5.