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.
Comment | File | Size | Author |
---|---|---|---|
#21 | views-694094-21.patch | 567 bytes | dawehner |
#20 | views-static-cache-resets-694094-20.patch | 5.85 KB | smokris |
#17 | 694094-drupal_static_17.patch | 3.18 KB | Letharion |
#15 | 694094-drupal_static.patch | 4.93 KB | dawehner |
#12 | views-static-cache-resets-694094-d7.patch | 4.19 KB | aspilicious |
Comments
Comment #1
dawehnerI always used views_invalidate_cache() but it seems to be te wrong function for it.
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 :)
Comment #2
dawehnerPatch 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.
Comment #3
merlinofchaos CreditAttribution: merlinofchaos commentedCommitted 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.
Comment #4
yhahn CreditAttribution: yhahn commentedbooo : P
Guess we'll have to start using 3.x soon anyway.
Comment #6
yhahn CreditAttribution: yhahn commentedFor anyone maintaining functional/integration tests for distros against Views 2.x, here is an updated patch.
Comment #7
Ian Ward CreditAttribution: Ian Ward commentedAttached is a new patch which carries the ability to reset more static caches, which has proved necessary w/in simpletests for an install profile.
Comment #8
dawehnerHere 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
Comment #9
bojanz CreditAttribution: bojanz commentedLooks good.
In D7, can't we just use drupal_static? Or is reinventing the wheel justified in this case?
Comment #10
dawehnerGood point. I will rerole this patch for d7 when it's commited for 6.x-3.x
Comment #11
merlinofchaos CreditAttribution: merlinofchaos commentedCommitted to 6.x-3.x -- needs porting to D7 using drupal_static.
Comment #12
aspilicious CreditAttribution: aspilicious commentedHopefully I didn't miss something
Comment #13
dawehnerWow. You rock!
Thanks. Commited this to the 7.x version.
Comment #14
aspilicious CreditAttribution: aspilicious commentedSrry dereine, I don't think my own patch is correct after reading this issue again
I just ported it, without using D7 specific stuff
Comment #15
dawehnerHere is some progress for this issue
Comment #16
Letharion CreditAttribution: Letharion commentedI 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.
Comment #17
Letharion CreditAttribution: Letharion commentedI'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.
Comment #18
dawehnerPersonally 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
Comment #19
dawehnerI 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
Comment #20
smokris(Re-rolled #7 so it applies cleanly to Views 6.x-2.16.)
Comment #21
dawehnerThis just fixes views_fetch_handler_data for 6.x-3.x
Comment #22
EugenMayer CreditAttribution: EugenMayer commentedbasically, 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
Comment #23
dawehnerThis does not affect the patch for drupal 7, and maybe we potentially still need a backport. Pushed #22 to 6.x-3.x
Comment #24
EugenMayer CreditAttribution: EugenMayer commentedi guess d7 is different, since its handled using drupal_static right?
Comment #25
dawehnerWell, no idea whether it is encouraged to use drupal_static_reset.