Don't have the cache rely on $_GET [#636128]

Currently the cache system are always using GET-parameters in the hashing of the cache key when they're available. This results in all views getting a new cache key when one view on a page are using a pager, table sort or similar.

I'm attaching a patch that removes the use of $_GET in the cache plugins and instead relies on data from the View itself to create the hash for the cache key.

I've only created a special solution for the pager since exposed_info, sort and order are already included in the query and/or the arguments and that way are already included in the hash.

This patch is dependent on #635990: Make the cache respect db_rewrite_sql() and substitutions but this issue isn't in any way dependent on that issue so if someone else would like to make a patch that isn't dependent on that issue - please feel free to do so.

Comment	File	Size	Author
#2	views_cache_get_independent2.patch	2.89 KB	voxpelli
#2
	views_cache_get_independent.patch	2.94 KB	voxpelli

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

voxpelli CreditAttribution: voxpelli commented 3 December 2009 at 18:59

Version:

6.x-2.7

» 6.x-2.8

Reroll against the first patch in #635990: Make the cache respect db_rewrite_sql() and substitutions

Comment #2

voxpelli CreditAttribution: voxpelli commented 3 December 2009 at 19:00

File	Size
views_cache_get_independent2.patch	2.89 KB

Comment #3

merlinofchaos CreditAttribution: merlinofchaos commented 5 January 2010 at 01:22

I'm going to need patches against both Views 2 and Views 3 in order to be able to commit this. Views 3 does this differently since paging is handled by plugins now.

Comment #4

merlinofchaos CreditAttribution: merlinofchaos commented 11 March 2010 at 23:30

Status:

Needs review

» Needs work

Comment #5

dawehner

German

CreditAttribution: dawehner commented 23 June 2010 at 22:37

+++ plugins/views_plugin_cache.inc
@@ -253,11 +253,6 @@ class views_plugin_cache extends views_plugin {
-      foreach (array('exposed_info', 'page', 'sort', 'order') as $key) {
-        if (isset($_GET[$key])) {
-          $key_data[$key] = $_GET[$key];
-        }
-      }

We shouldn't remove sort order here, or?

Powered by Dreditor.

This needs somehow a total redone for views 3 because there is both pluggable query and pagers.

Comment #6

voxpelli CreditAttribution: voxpelli commented 26 June 2010 at 14:22

#5: If I remember correctly then those sort and order had already been taken care of in another place and therefor was redundant.

Regarding Views 3 - yes this needs to be totally redone - which I unfortunately haven't had time to do yet which is the main reason why this hasn't been committed I guess.

Comment #7

dawehner

German

CreditAttribution: dawehner commented 13 October 2010 at 18:54

Which was the other issue? There was a patch for views3

Comment #8

voxpelli CreditAttribution: voxpelli commented 14 October 2010 at 10:05

@dereine: Do you mean #635990: Make the cache respect db_rewrite_sql() and substitutions? It has a Views 3 patch in it

Comment #9

dawehner

German

CreditAttribution: dawehner commented 14 October 2010 at 14:01

Yes. Is this what we spoke about on drupalcon?

Comment #10

voxpelli CreditAttribution: voxpelli commented 14 October 2010 at 17:29

@dereine: Yes

Comment #11

Kars-T CreditAttribution: Kars-T commented 26 November 2011 at 10:59

Status:

Needs work

» Fixed

Does this still apply? #635990: Make the cache respect db_rewrite_sql() and substitutions still seems to be running but this is a Views 2 issue, the texts say that Views 3 is totally different and it is over a year old.

I am setting this to "fixed" so the issue closes in 2 Weeks. Please set it to "needs work" again if the problem still applies to views 3.