Add views render caching on views ajax requests

I have a first pass at this ready. It depends on the patch from https://www.drupal.org/node/956186#comment-11498819.

Here's what I did:

1) Made a SqlBase pager setting that lets you choose GET or POST.
2) Fixed ViewAjaxController so that it gets query parameters in a way that respects request method.
3) Changed the way that it renders the view so that render caching works.

Let me know what you think. This should work on 8.1 stable if you get the 8.1.8 version of the patch from that other core issue.

I'm going to need to look at some of those tests. We probably need to wait until https://www.drupal.org/node/956186 is committed to get past some of the AJAX paging related ones, but here is a diff that should fix the schema errors.

I'm very interested in getting these tests to pass. Can anyone point me in the right direction for how to move forward with this?

I've gotten to the point where it's failing because the $view->display_handler is empty in ViewAjaxController::ajaxView. I'm thinking this has something to do with the mock view not being built correctly under the new conditions.

Anyone have a guess about this, or some advice?

Actually I figured it out. I needed to a) set the query params in a different way for GET and b) set up a mock expectation for buildRenderable instead of preview. It will take a bit more legwork but I should have these tests passing soon ...

All tests should be green now. I added separate tests for paging via post vs. via get.

You're right. Here's a summary of what I did:

1) Added render caching to ajax views
2) Added support for GET requests for ajax views
3) Added tests

How testable is this with our current testing suite? Could one of the VDC maintainers please chime in to clarify what sort of tests they would like to see? Thanks.

@dpolant: Do you have any manual testing instructions?

Re-rolled against 8.3x

Fixed JS coding standards error.

Do we need more tests for this? See Drupal\Tests\views\Unit\Controller\testAjaxViewWithPager - this will now test the page endpoint as a GET request and it is pass.

Rerolled.

There's already a patch for 8.4.x, but I still needed one for 8.3.x as of 8.3.7 & figured someone else might, as well; so here's the patch from #24 re-rolled for 8.3.7.

I used the patch and still was not able to see the headers properly set, I then checked the module ajax_cached_get but no luck, I than checked the differences in use of CacheableResponseTrait and it seemed that all the conditions were failing as response object was not instance of CacheableResponseInterface.

Updated the patch, now ViewAjaxResponse implements CacheableResponseInterface and CacheableResponseTrait was already used. Confirmed for my case the headers are properly set now.

Patch is only for 8.3.x as of now and once I confirm it on Acquia cloud I will work on 8.4.x

And now I've run into real issues, ViewExecutable->unserialize requires \Drupal::request() which is null as Drupal is trying to render from cache

Bottom line - don't use my patch till further notice.

After updating to 8.4.2, the patch from 33 no longer appears to apply cleanly due to a small code change in core. This is a reroll of 33 against 8.4.2.

Here is a new re-roll of the patch for 8.5.x.

And I made the patch against the wrong directory. Attaching a new patch and trying against with automated testing.

Thanks for pushing this forward again, @merauluka! 👌 Hopefully Views maintainers can soon review this :)

We have been using this patch on a Project since past few Drupal 8 releases & while upgrading to 8.7, it failed to apply. Re-rolled this patch for 8.7.x. Attaching patch file here.

+ for #47:
I would love to use cached views ajax reloads on a facets filter or exposed filter. What was the reason to only do this for paging?

I also feel that #47 is an important consideration. Getting exposed filters using GET for Ajax is critical for performance.

+1 for this getting this on exposed filters!

Applied the patch from #49 with 3 way merge and update the code a bit:

• Add CacheableResponseInterface so the response can be cached by the dynamic page cache
• Improve the getParam by using isMethod on the request object
• SqlBase is update after 3 way merge

I tested this on 8.8.5 with the latest patch and the patch mentioned in the IS from #956186: Allow AJAX to use GET requests. The first uncached ajax request works, but the next cached request throws this error:

Error: Call to a member function get() on null in Drupal\views\Plugin\views\display\DisplayPluginBase->getHandlers() (line 860 of /web/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php)

@Lukas von Blarer, I had the same error. Currently working on a new patch that will fix this issue. Serializing the view is the problem, if that is not serialized the caching will work as expected.

Updated previous patch to solve the serialization problem when caching the response.

Tests still need work, I think they should be extended to test both GET & POST requests.

The serialisation of the ViewExecutable is quite broken, so for now just don't cache it.

Updated to be inline with the other patch in the mentioned issue.

patch rerol for drupal 9.0

It works for pager but not for exposed filters.
I agree with #50, seems weird to have caching enabled when using pagination but not when using an exposed filter.

Here is a new patch that use the setting in pagination also for exposed filters. Maybe this request type setting should be in the display settings and not in the pagination settings.

This patch will fix the issue on exposed filter in 8.x

Reroll #62 patch and try fix tests.

Fixed typo in patch.

Re-rolled for 8.9.x
and fixed CS issues in 9.2.x

Add missing JS code.

Fixed JS codes.

Reroll.

JS coding standard.

Change media test.

reroll

Rerolling..
Can't seem to generate the interdiff as its throwing error interdiff: Whitespace damage detected in input

Adding new patch for custom commands failure.

I just ported the code of #87 to make it work with 9.5.x branch.

Fixed CCF for #89.

I've re-rolled this to work with 9.5.1. It wasn't applying due to changes from "will()" to "willReturn()". I fixed the conflicts manually. I have not yet tested the patch aside from seeing that it applies.

I'm going to keep this at needs work because core patches should be going against D10, now, but the client needs it for D9 so that's what I did. If I can, I'll see if I can circle back and get it to D10.

Fixed CCF for #91.

(I apologize: The status change was wrong.)

Postponing on #956186: Allow AJAX to use GET requests.

Unblocked!

I think we actually don't need to do this any more - #956186: Allow AJAX to use GET requests already converted Views (without configuration) since it was the main (only) use-case for non-form AJAX in core, so Views is using GET for pagers etc. in Drupal 10.1 already.

It's using GET AJAX requests indeed, but the scope of this issue seems to also stress the need to add render caching in that situation. But perhaps that's already happening? 😄

(I could investigate/verify that too but it's been ages since I've looked at Views' render pipeline, it'd probably take you only a few dozen seconds to check? 🤓😅)

If that's already happening too, then I definitely agree this can be closed (as a duplicate?).

Views render caching is just normal render caching now, that's why this issue was opened in the first place!

Drupal 7: Views has its own HTML caching pipeline, which works on POST requests, but doesn't handle any of the things Drupal 8+ render caching does like cache tags etc.

Drupal 8+: Views uses core render caching, which is great for normal GET requests, but skips caching on POST requests.

Drupal 10.1: cache cache cache cache cache cache!

Closing as outdated.