Pages with default includes are not cached

Great work, that will save quite some request/cpu time.

Any change you could add a line to the tests for that module? I think using:

$this->assertSession()->responseHeaderContains() should be usefull for that to see if the cache has been hit if a second request is done.

It seems that the responses in these tests do not contain headers, just a raw JSON string.

You need to use:

    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');

    // 1a. Check if second request is cached.
    $response = $this->drupalGet('/api/articles');
    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'HIT');

Or something similar. When looking at the html output in files//simpletest/browser_output

array (
  'Date' => 'Sat, 18 Nov 2023 11:41:16 GMT',
  'Server' => 'Apache/2.4.56 (Debian)',
  'X-Powered-By' => 'PHP/8.1.25',
  'X-Drupal-Assertion-0' => 'a%3A3%3A%7Bi%3A0%3BO%3A25%3A%22Drupal%5CCore%5CRender%5CMarkup%22%3A1%3A%7Bs%3A9%3A%22%00%2A%00string%22%3Bs%3A196%3A%22Since%20symfony%2Fserializer%206.3%3A%20%22Drupal%5Cjsonapi%5CNormalizer%5CImpostorFrom%5Cjsonapi_extras%5CFieldItemNormalizerImpostor%22%20should%20implement%20%22NormalizerInterface%3A%3AgetSupportedTypes%28%3Fstring%20%24format%29%3A%20array%22.%22%3B%7Di%3A1%3Bs%3A24%3A%22User%20deprecated%20function%22%3Bi%3A2%3Ba%3A3%3A%7Bs%3A8%3A%22function%22%3Bs%3A21%3A%22trigger_deprecation%28%29%22%3Bs%3A4%3A%22file%22%3Bs%3A61%3A%22%2Fgcl-builds%2Fvendor%2Fsymfony%2Fdeprecation-contracts%2Ffunction.php%22%3Bs%3A4%3A%22line%22%3Bi%3A25%3B%7D%7D',
  'X-Drupal-Assertion-1' => 'a%3A3%3A%7Bi%3A0%3BO%3A25%3A%22Drupal%5CCore%5CRender%5CMarkup%22%3A1%3A%7Bs%3A9%3A%22%00%2A%00string%22%3Bs%3A205%3A%22Since%20symfony%2Fserializer%206.3%3A%20%22Drupal%5Cjsonapi%5CNormalizer%5CImpostorFrom%5Cjsonapi_extras%5CResourceIdentifierNormalizerImpostor%22%20should%20implement%20%22NormalizerInterface%3A%3AgetSupportedTypes%28%3Fstring%20%24format%29%3A%20array%22.%22%3B%7Di%3A1%3Bs%3A24%3A%22User%20deprecated%20function%22%3Bi%3A2%3Ba%3A3%3A%7Bs%3A8%3A%22function%22%3Bs%3A21%3A%22trigger_deprecation%28%29%22%3Bs%3A4%3A%22file%22%3Bs%3A61%3A%22%2Fgcl-builds%2Fvendor%2Fsymfony%2Fdeprecation-contracts%2Ffunction.php%22%3Bs%3A4%3A%22line%22%3Bi%3A25%3B%7D%7D',
  'X-Drupal-Assertion-2' => 'a%3A3%3A%7Bi%3A0%3BO%3A25%3A%22Drupal%5CCore%5CRender%5CMarkup%22%3A1%3A%7Bs%3A9%3A%22%00%2A%00string%22%3Bs%3A201%3A%22Since%20symfony%2Fserializer%206.3%3A%20%22Drupal%5Cjsonapi%5CNormalizer%5CImpostorFrom%5Cjsonapi_extras%5CResourceObjectNormalizerImpostor%22%20should%20implement%20%22NormalizerInterface%3A%3AgetSupportedTypes%28%3Fstring%20%24format%29%3A%20array%22.%22%3B%7Di%3A1%3Bs%3A24%3A%22User%20deprecated%20function%22%3Bi%3A2%3Ba%3A3%3A%7Bs%3A8%3A%22function%22%3Bs%3A21%3A%22trigger_deprecation%28%29%22%3Bs%3A4%3A%22file%22%3Bs%3A61%3A%22%2Fgcl-builds%2Fvendor%2Fsymfony%2Fdeprecation-contracts%2Ffunction.php%22%3Bs%3A4%3A%22line%22%3Bi%3A25%3B%7D%7D',
  'X-Drupal-Assertion-3' => 'a%3A3%3A%7Bi%3A0%3BO%3A25%3A%22Drupal%5CCore%5CRender%5CMarkup%22%3A1%3A%7Bs%3A9%3A%22%00%2A%00string%22%3Bs%3A202%3A%22Since%20symfony%2Fserializer%206.3%3A%20%22Drupal%5Cjsonapi%5CNormalizer%5CImpostorFrom%5Cjsonapi_extras%5CContentEntityDenormalizerImpostor%22%20should%20implement%20%22NormalizerInterface%3A%3AgetSupportedTypes%28%3Fstring%20%24format%29%3A%20array%22.%22%3B%7Di%3A1%3Bs%3A24%3A%22User%20deprecated%20function%22%3Bi%3A2%3Ba%3A3%3A%7Bs%3A8%3A%22function%22%3Bs%3A21%3A%22trigger_deprecation%28%29%22%3Bs%3A4%3A%22file%22%3Bs%3A61%3A%22%2Fgcl-builds%2Fvendor%2Fsymfony%2Fdeprecation-contracts%2Ffunction.php%22%3Bs%3A4%3A%22line%22%3Bi%3A25%3B%7D%7D',
  'X-Drupal-Assertion-4' => 'a%3A3%3A%7Bi%3A0%3BO%3A25%3A%22Drupal%5CCore%5CRender%5CMarkup%22%3A1%3A%7Bs%3A9%3A%22%00%2A%00string%22%3Bs%3A201%3A%22Since%20symfony%2Fserializer%206.3%3A%20%22Drupal%5Cjsonapi%5CNormalizer%5CImpostorFrom%5Cjsonapi_extras%5CConfigEntityDenormalizerImpostor%22%20should%20implement%20%22NormalizerInterface%3A%3AgetSupportedTypes%28%3Fstring%20%24format%29%3A%20array%22.%22%3B%7Di%3A1%3Bs%3A24%3A%22User%20deprecated%20function%22%3Bi%3A2%3Ba%3A3%3A%7Bs%3A8%3A%22function%22%3Bs%3A21%3A%22trigger_deprecation%28%29%22%3Bs%3A4%3A%22file%22%3Bs%3A61%3A%22%2Fgcl-builds%2Fvendor%2Fsymfony%2Fdeprecation-contracts%2Ffunction.php%22%3Bs%3A4%3A%22line%22%3Bi%3A25%3B%7D%7D',
  'Cache-Control' => 'must-revalidate, no-cache, private',
  'X-Drupal-Dynamic-Cache' => 'MISS',
  'Content-language' => 'en',
  'X-Content-Type-Options' => 'nosniff',
  'X-Frame-Options' => 'SAMEORIGIN',
  'X-Drupal-Cache-Tags' => 'config:filter.format.plain_text config:jsonapi_extras.settings config:jsonapi_resource_config_list config:user.role.anonymous http_response node:1 node:2 node_list taxonomy_term:2 taxonomy_term:3 taxonomy_term:4 taxonomy_term:5',
  'X-Drupal-Cache-Contexts' => 'languages:language_interface theme url.query_args:fields url.query_args:filter url.query_args:include url.query_args:page url.query_args:resourceVersion url.query_args:sort url.site user.permissions',
  'X-Drupal-Cache-Max-Age' => '-1 (Permanent)',
  'Expires' => 'Sun, 19 Nov 1978 05:00:00 GMT',
  'X-Generator' => 'Drupal 10 (https://www.drupal.org)',
  'X-Drupal-Cache' => 'MISS',
  'Transfer-Encoding' => 'chunked',
  'Content-Type' => 'application/vnd.api+json',
)

I see cache info. Locally i was not sure how to reproduce the error, i got a hit on the second call.

@bbrala, thank you for the clear explanation. Updating the patch with the corresponding tests. Passed successfully on my local env

Hmm, gitlab is weird.

But when i run those tests locally, when i do only the test (which should fail right?) then the test is still green. So that test doesn't proove the faulty caching...

@bbrala do you have default includes added when you test it locally?

Fixed bug with variable name

For some reason, locally, I have also passed tests by adding a cache tag check: $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'HIT') after the second request to '/api/articles'. However, at the same time, when I check it by visiting the JSON API route with the resource with default includes, I always receive an uncached response.

Update patch with minor code styles fixes

Fixed 500 error for the resources without default includes

Thanks ihor_allin. The patch in comment #16 works for me.

Hey @bbrala,

Any update on the above fix ??

The last submitted patch, 2: 3402388-default-includes-caching-fix.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The last submitted patch, 7: 3402388-default-includes-caching-fix-7.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Review https://git.drupalcode.org/project/jsonapi_extras/-/merge_requests/40

I have found an issue with response caching with this patch.
The isDefaultIncludes method checks only if the request contains included parameters from the jsonapi_extras configuration. So, it can cause problems when the one cached response is used for different includes params.

Say, you have an endpoint for an article that uses 3 taxonomy fields (field_tag, field_type, field_category).
The jsonapi_extras is configured to include by default the field_type field. Then if you send the first request with ?include=field_category this response will be cached and applied to the response without the include param or for ?include=field_tag.

So, instead of checking if the request uses default includes we need to check if the request uses ONLY default includes.

The diff for PR to avoid adding numerous commits to the composer.json.

I realized that made a mistake in my previous commit. Here is the correct version of the patch

Ty for taking the time here to fix this up. Changes seem to work as promised <3

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