Problem/Motivation

Currently, \Drupal\jsonapi\Normalizer\Value\JsonApiDocumentTopLevelNormalizerValue::__construct() associates the following cache contexts with all non-error JSON API responses:

  1. url.query_args:filter
  2. url.query_args:sort
  3. url.query_args:page
  4. url.query_args:fields
  5. url.query_args:include

The last two apply to all JSON API responses. But the other three only apply to collection responses.

First pointed out by @gabesullice in #2984911-48: Remove access to the Request object in the normalization process.

Proposed resolution

Associate the appropriate cache contexts! Good for correctness. But without tangible real-world benefits.

Remaining tasks

Do it.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#7 2992673-7.patch2.27 KBwim leers
#7 interdiff.txt692 byteswim leers
#5 2992673-5.patch1.61 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

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

#2984911: Remove access to the Request object in the normalization process should land first.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Set collection-specific query parameter cache contexts on collection responses instead of all responses » [PP-1] Set collection-specific query parameter cache contexts on collection responses instead of all responses
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Set collection-specific query parameter cache contexts on collection responses instead of all responses » Set collection-specific query parameter cache contexts on collection responses instead of all responses
Status: Postponed » Active

#2984911: Remove access to the Request object in the normalization process landed.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
Related issues: +#2991389: Test coverage: relationship response cacheabiliity
StatusFileSize
new 2992673-5.patch1.61 KB

#2991389: Test coverage: relationship response cacheabiliity already reduced this to only relationship responses.

I think this is what we want to see pass tests.

Status: Needs review » Needs work

The last submitted patch, 5: 2992673-5.patch, failed testing. View results

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt692 bytes
new 2992673-7.patch2.27 KB
gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

  • gabesullice committed 2e11f9d on 8.x-2.x authored by Wim Leers 
    Issue #2992673 by Wim Leers: Set collection-specific query parameter...
gabesullice’s picture

gabesullice
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.