#2985426: Spec compliance: `related` routes should return 200, not 403, if field access is allowed but the related resources are forbidden makes related resources behave just like collections. The scaffolding is there to make them filterable, sortable and paginated.

This primarily just requires that tests be written.

CommentFileSizeAuthor
#12 2986169-12-do-not-test.patch1.28 KBwim leers

Comments

gabesullice’s picture

gabesullice
Primary language English
commented

gabesullice created an issue.

[PP-1] because #2985426: Spec compliance: `related` routes should return 200, not 403, if field access is allowed but the related resources are forbidden has not yet landed.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Support sortable, paginated and filterable related resources. » Support sortable, paginated and filterable related resources.
Status: Postponed » Active

#2985426: Spec compliance: `related` routes should return 200, not 403, if field access is allowed but the related resources are forbidden landed!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#2993654: Provide JSON Pointer as a link on a resource to ease matching with included items perhaps makes this less necessary.

I also wonder what the consequences are of paginated includes: how would one even specify which page of includes for which field to load? It'd still result in multiple requests then, mostly defeating the purpose of includes?

I think it may make sense to remove the "paginated" portion of the issue's intent/scope.

I misinterpreted. See below.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2993654: Provide JSON Pointer as a link on a resource to ease matching with included items
gabesullice’s picture

gabesullice
Primary language English
commented

This issue is not about includes, but the resource objects at a path like /jsonapi/node/article/{uuid}/tags. So, if the article has 200 tags it would be nice to be able to filter, sort and page them.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: -#2993654: Provide JSON Pointer as a link on a resource to ease matching with included items

Hah, whoops! I was triaging the issue queue, and clearly misinterpreted this one. Struck through all of #3.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

How would this work?

I think it'd mean duplicating much of \Drupal\jsonapi\Controller\EntityResource::getCollection() in getRelated() and just automatically adding conditions to the query that ensure it's limited to those resources related to the base resource.

gabesullice’s picture

gabesullice
Primary language English
commented

#7: Yep. See #2985426-7: Spec compliance: `related` routes should return 200, not 403, if field access is allowed but the related resources are forbidden where I did exactly that. It was reverted in #35 because it was out of scope. This issue is the followup for it.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review

Alright, cool, then whenever we do this we don't need to start from scratch. Marking NW to reflect that state.

gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Needs work

FTFY

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

😅

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
StatusFileSize
new 2986169-12-do-not-test.patch1.28 KB

There are two steps to be taken here:

  1. \Drupal\jsonapi\Controller\EntityResource::getRelated() just calls $entity->get($related_field_name)->referencedEntities() today, and doesn't do an Entity Field Query. Which means we actually cannot "just" do filtering, sorting and pagination. We have to convert this to an entity query first.
  2. Once we've done that, we need to make that entity query respect filter, page and sort parameters.

    That's where we run into a similar problem to what we encountered in #3022531-2: (Re)move _json_api_params from JsonApiParamEnhancer and do not use denormalization for them, and we solved it there by moving the creation of the Filter value object out of a route enhancer (\Drupal\jsonapi\Routing\JsonApiParamEnhancer) and into the controller (\Drupal\jsonapi\Controller\EntityResource) where it is about to be used.

    JsonApiParamEnhancer assumes it operates on the resource type of the route, and for the related route that is the host resource type, not the related one. So the sort and page handling in \Drupal\jsonapi\Routing\JsonApiParamEnhancer::enhance() cannot possibly give us the right results.

    For filter, we'd need to call the existing helper method. But … when dealing with related data, there can be multiple resource types. How do we sort and paginate across resource types?

    Given the above, I'm not so sure that this is truly worth doing. This is hugely complex. And despite this issue turning six months old today, zero people besides the two of us are following this. I think we should consider closing this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3022531: (Re)move _json_api_params from JsonApiParamEnhancer and do not use denormalization for them
gabesullice’s picture

gabesullice
Primary language English
commented
Title: Support sortable, paginated and filterable related resources. » [PP-2] Support sortable, paginated and filterable related resources.
Status: Needs work » Postponed
Related issues: +#3031173: FieldResolver::resolveInternalEntityQueryPath should accept an array of resource types, not a single resource type

Despite this issue turning six months old today, zero people besides the two of us are following this. I think we should consider closing this.

The underlying problem here is really that we can't yet support collections of mixed-bundles. Once that is possible, this will be trivial.

I think it's naive to assume that this feature is not worth doing because no-one is following this issue. If we supported this, we'd be a lot closer to being able to automatically support reverse relationship routes. This would be hugely valuable.

The two blockers are:
#3022531: (Re)move _json_api_params from JsonApiParamEnhancer and do not use denormalization for them
#3031173: FieldResolver::resolveInternalEntityQueryPath should accept an array of resource types, not a single resource type

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

If we supported this, we'd be a lot closer to being able to automatically support reverse relationship routes.

It's late over here and perhaps that's why I'm not making the connection. Can you explain this? That alone would make this Major IMHO.

gabesullice’s picture

gabesullice
Primary language English
commented

Imagine you run an American sports magazine website. You have a vocabulary of sports (Basketball, Football, Soccer, etc). And 100,000 articles that are tagged with one of these sports.

Right now, you can visit /jsonapi/node/article?filter[field_sport]={uuid of the football term}

This route obviously supports pagination and filtering. That's good because you're potentially dealing with tens of thousands of results.

A reverse relationship route like this: /jsonapi/taxonomy_term/sports/{uuid of the football term}/node/field_sport

Would be catastrophic unless we supported filtering and pagination. In our example, it would almost always time out (since there's no mechanism to page the results).

Currently, related routes can suffer from this too, it's just that most applications don't have entity reference fields with hundreds or thousands of referenced entities.

You might think, well, can't we just make reverse references behave just like collections? Those work, right? Unfortunately not, because related routes can have >1 resource type in them and the field resolver can't handle that. They're effectively mixed-bundle collections (without an entity query).

So my logic is: if we can "support sortable, paginated and filterable related resources", that means we can support reverse relationships safely without timing out or crashing servers.

vincenzo gambino’s picture

vincenzo gambino commented

Hi,

I am having this issue right now. I would like to paginate or limit relationships.
I would like to implement the pagination/limit in the relationship endpoint? /jsonapi/node/uuid/relationships/field_name?page[limit]=1

Is this what you are planning to do here?

gabesullice’s picture

gabesullice
Primary language English
commented

Is this what you are planning to do here?

@Vincenzo Gambino, not really. This issue would address:

/jsonapi/node/uuid/field_name?page[limit]=1

not

/jsonapi/node/uuid/relationships/field_name?page[limit]=1

So, the route that list the related entities, not the relationships to those entities.

Perhaps you can open another feature request for your use case? Knowing the underlying implementation, I think it would be difficult to handle both routes in this issue.

gabesullice’s picture

gabesullice
Primary language English
commented
Title: [PP-2] Support sortable, paginated and filterable related resources. » [PP-1] Support sortable, paginated and filterable related resources.
Project: JSON:API » Drupal core
Version: 8.x-2.x-dev » 9.1.x-dev
Component: Code » jsonapi.module

#3022531: (Re)move _json_api_params from JsonApiParamEnhancer and do not use denormalization for them landed.

Moving this to the Drupal core JSON:API component.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.