Views cache settings not added to headers

Thanks for the patch, will test and review.

Not sure if separate issue should be opened but proposed patch doesn't fix the cache contexts.
If any of the view plugins implements the CacheableDependencyInterface::getCacheContexts the context won't be added to the response.
So, for example if you have a view with the uid contextual filter that provides default value set to "current user", the [user] context won't be added to a response leading to the potential exposing user's data.

I tried to fix that and found that few things are wrong there.

<?php
    if (!$context->isEmpty()) {
      $bubbleable_metadata = $context->pop();
      BubbleableMetadata::createFromObject($view->result)
        ->merge($bubbleable_metadata);
    }
    else {
      $bubbleable_metadata = BubbleableMetadata::createFromObject($view->result);
    }
?>

$view->result is a Drupal\views\ResultRow object that doesn't implement the CacheableDependencyInterface, so according CacheableMetadata::createFromObject cacheMaxAge will be set to 0.
Also BubbleableMetadata::merge doesn't modify the object, you must use the return result. Seems 2 bugs avoided each other.

So instead of trying to build the cache object from view->result I used $view->element instead (However I'm not sure if it is the proper place to look for the caching metadata).
Patch attached (will use the issue fork the next time).

Wow, tests say that I'm terribly wrong even it seems to work for me

Looks like now the [url] cache context is added and url.query_args and other more specific contexts are ignored during the merging.
Not sure if problem is in tests or cache contexts.

@jesss,

I tested the patch locally and it appears to resolve issue that I can reproduce. However, the patch does still need a test that proves the issue. As this issue needs to be resolved I will start work on the test shortly.

@artem_sylchuk,

It's possible that both solutions resolve the issue, but that your solution may solve additional issues, which is great. I don't have a high enough understanding of your specific issues, but if you are able to provide me with some automated tests or reproducible steps I should be able to help more.

@Deciphered
The simpliest reproducable steps are the following:
1. Create the view showing nodes
2. Add the contextula filter by node author
3. in the WHEN THE FILTER VALUE IS NOT AVAILABLE of the contextual filter select Provide default value: User ID from logged in user
4. Create 2 users A and B. Create a few nodes by each.
5. Confirm that user A sees only own nodes when visits the view page, same for user B
6. Expose the view via Jsonapi:views
7. Check the Jsoapi response for the user A
8. Check the Jsoapi response for the user B (you'll see the same nodes as for the user A in the current version of the module)

I can try providing test for this the next week

Thanks for getting back to me so quickly @artem_sylchuk

I did just push @jesss' patch with an automated test, but I will run through those steps now and see what I can see.

patch #4 bump to be compatible with release 1.0

This is an old issue, but I had problems with caching when different users are logged in, and other times, a 403 error is cached, this patch attempts to fix the issue by adding the dependency using the view render array, I made sure to also add the cache contexts to fix the issue of different users when attempting to use the API. Also, make sure to add cache dependency when the view access is denied.

@yahyaalhamad thanks, could you maybe add a MR with your patch and add .gitlab-ci.yml to see if the tests turn green then?

@deciphered are you still actively maintaining this module?

Sorry for the quiet spell here, I'm back and actively working through the backlog across all my modules, this one included.

MR is up: cache tags, contexts and max-age are now derived from the view's own render array instead of the broken $view->result based collection. The user-context leak artem_sylchuk found is fixed, and 403 responses now carry proper cacheability too. Tests green on D10 and D11.

Thanks all, fixed and committed for the upcoming 8.x-1.2 release.