Currently, _views_discover_default_views() has no mechanism for clearing its static cache. This generally does not pose problems as changes in the default views are usually accompanied by a new page request (e.g. a module is enabled or disabled). However there is one important case where it's turning out to be a real showstopper: testing. In running simpletests against an install profile, it's very frequent for menu_rebuild() to occur before all modules for the install profile have been installed. If this occurs, the views default static cache that is populated for the rest of the simpletest is stale and cannot be cleared even by a subsequent menu_rebuild(), meaning that a whole set of page views become untestable.

The attached patch provides a static cache reset for the views default views and propagates the change into the various API functions for retrieving views. This means when you do set the reset flag you really will get a fresh copy of the view including any possible changes in the defaults.

Patch generated from DRUPAL-6--2 branch, applies to both 2.x and 3.x.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

I always used views_invalidate_cache() but it seems to be te wrong function for it.

+++ includes/cache.inc	24 Jan 2010 18:08:38 -0000
@@ -115,10 +115,9 @@ function _views_discover_default_views()
-      $defaults = module_invoke_all('views_default_views');

Is it right ,that this reduces one run to the views_default_views? Thats cool

Powered by Dreditor.

From code perspective this looks fine. I will do some testing later. Perhaps someone could also write a simpletest for it :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Patch is fine for me. Manually testing and adding a view to a hook_views_default_views cheching whether the function returns it, rebuilding the permission with reset.

merlinofchaos’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 3.x for both D6 and D7. Since 2.x does not have testing there's not a lot of value in adding this to 2.x.

yhahn’s picture

booo : P

Guess we'll have to start using 3.x soon anyway.

Status: Fixed » Closed (fixed)

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

yhahn’s picture

For anyone maintaining functional/integration tests for distros against Views 2.x, here is an updated patch.

Ian Ward’s picture

Attached is a new patch which carries the ability to reset more static caches, which has proved necessary w/in simpletests for an install profile.

dawehner’s picture

Status: Closed (fixed) » Needs review
FileSize
4.04 KB

Here is a updated patch which also adds the features to the plugins helper functions. This is a patch against 6.x-3.x, perhaps it has to be rerolled against 6.x-2.x

bojanz’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

In D7, can't we just use drupal_static? Or is reinventing the wheel justified in this case?

dawehner’s picture

Good point. I will rerole this patch for d7 when it's commited for 6.x-3.x

merlinofchaos’s picture

Version: 6.x-3.x-dev » 7.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to 6.x-3.x -- needs porting to D7 using drupal_static.

aspilicious’s picture

Status: Patch (to be ported) » Needs review
FileSize
4.19 KB

Hopefully I didn't miss something

dawehner’s picture

Status: Needs review » Fixed

Wow. You rock!

Thanks. Commited this to the 7.x version.

aspilicious’s picture

Status: Fixed » Needs work

Srry dereine, I don't think my own patch is correct after reading this issue again

Committed to 6.x-3.x -- needs porting to D7 using drupal_static.

I just ported it, without using D7 specific stuff

dawehner’s picture

FileSize
4.93 KB

Here is some progress for this issue

Letharion’s picture

Assigned: Unassigned » dawehner

I took a look at the patch and even tried updating it, but so much has changed. I'm not even sure it's necessary anymore since atleast parts of it seems to have found it's way into Views anyway.

Letharion’s picture

Status: Needs work » Needs review
FileSize
3.18 KB

I've tried to follow the spirit of the original patch. There are $reset parameters in various functions that appear to have been added by different patches, so what I've done is replaced a number of static $cache = NULL/array() with drupal_static equivalents.

dawehner’s picture

Status: Needs review » Needs work
+++ b/plugins/views_plugin_display.incundefined
@@ -698,7 +698,7 @@ class views_plugin_display extends views_plugin {
+    static $cache = &drupal_static(__FUNCTION__, NULL);

+++ b/plugins/views_plugin_exposed_form_input_required.incundefined
@@ -33,7 +33,7 @@ class views_plugin_exposed_form_input_required extends views_plugin_exposed_form
+    static $cache = &drupal_static(__FUNCTION__, NULL);

Personally i would prefer to add something in front to know that this is not a normal function but in a class. __CLASS_ might not work, because this functions could be overriden

dawehner’s picture

Assigned: dawehner » Unassigned

I think it would make sense to convert the statics to drupal_statics. Then it's a question whether $reset is still needed or just clearing the drupal_statics would help

smokris’s picture

(Re-rolled #7 so it applies cleanly to Views 6.x-2.16.)

dawehner’s picture

FileSize
567 bytes

This just fixes views_fetch_handler_data for 6.x-3.x

EugenMayer’s picture

basically, this feature would be awesome for every distribution. Since you can run into issues enabling views with new handlers ( feature ) of a module getting enabled in the same bootstrap, which make the update-process fail, without any possible solution left. There is absolutely no other way to get arround this, not even a workarround.

Would love to see that one getting applied

dawehner’s picture

This does not affect the patch for drupal 7, and maybe we potentially still need a backport. Pushed #22 to 6.x-3.x

EugenMayer’s picture

i guess d7 is different, since its handled using drupal_static right?

dawehner’s picture

Well, no idea whether it is encouraged to use drupal_static_reset.