laravel icon indicating copy to clipboard operation
laravel copied to clipboard

Improvements required in include path iterator for polymorphic relations

Open lindyhopchris opened this issue 4 years ago • 1 comments

The IncludePathIterator is the class the works out what include paths are allowed using a max depth integer.

At the moment, if the iterator encounters a polymorphic relationship at depth (i.e. not the first relationship) the path is terminated at this depth.

We need to actually change the handling of this scenario.

For an Eloquent morphTo relation, eager load paths will work through the relation if the next relationship in the path is valid on all the different inverse models. For example, in our application this path: requests.activity.venue, the activity is a morphTo relation. However all three models that can be returned from the activity relationship have a venue relation. So we can actually support including through morph-to relations where the next relation is common to all inverse schemas. By common, we mean:

  1. The JSON:API relation name is the same; AND
  2. The Eloquent relation name is the same: AND
  3. The inverse schema is the same.

So in our example, this is true for venue because all three model classes have a venue relationship, that returns a Venue model, that are all venue JSON:API relations that all return venues resources.

For our polymorphic to-many field (MorphToMany), including through the relationship at depth would actually work. This is because the implmentation maps the JSON:API include path to the eager load paths for each sub-relation, ignoring any invalid paths.

So in both these example, the IncludePathIterator polymorphic at-depth logic is no longer correct.

Because the logic of whether you can include through a relation is implementation specific, the IncludePathIterator should probably delegate to the Relation interface to know what it can include next. At the moment isIncludePath() is the only include path method on the Relation interface... but this is now probably too simple. We need some sort of method on the relation that returns the list of schemas and relationships on those schemas that can be included next in a relationship path. This is actually two different questions:

  1. If the relation is at the start of an include path (index = 0), what relationships can be next?
  2. If the relation is at depth in the include path (index > 0), what relationships can be next?

So the interface method effectively asks for the next paths, providing the index to indicate where it is in the path.

lindyhopchris avatar Apr 01 '21 13:04 lindyhopchris

Removed from milestone as deferring this to a subsequent beta release.

lindyhopchris avatar Apr 20 '21 12:04 lindyhopchris