og_views.inc sometimes not included [#102526]

It seems the code that includes og_views.inc is wrong.
When the cache is emptied (and views tables and default views are deleted), views does not have access to the og definitions, because og_views.inc is not included.

This breaks a lot of things and makes og rather unusable...

For now, I corrected this on my install by always loading og_views.inc, (like cck currently does...) :

if (module_exists('views')) {
  include_once(drupal_get_path('module', 'content') .'/content_views.inc');
}

at the top-level of the .module file

There might be a more clever way to do it, but it's not the current one :-)

Comments

Comment #1

yched CreditAttribution: yched commented 10 December 2006 at 20:07

Or maybe it has to do with the fact that my og install is under 'sites/all/modules/' ?
I don't get the initial './' in

require './'. drupal_get_path('module', 'og'). '/og_views.inc';

Comment #2

moshe weitzman CreditAttribution: moshe weitzman commented 11 December 2006 at 03:29

how can i reproduce this? ... we worked hard over the past years to elimitate all code that is in global scope like that CCK include. i'd rather not add more.

that leading ./ is an performance optimization that is used every time we include a file (see all of core for examples). the benefit of adding that seemingly useless bit is that php need not search its include_path for the specified file. the full path has already been specified.

Comment #3

yched CreditAttribution: yched commented 29 December 2006 at 02:15

Moshe, sorry for throwing this in and then remain silent for a while - I've been over-busy on un-drupal stuff...

I see that you have (temporarily ?) moved loading og_views.inc on global scope - I assume you intend to switch back to your optimized code at some point.
Anyway, here's what I came up with on the original code (og_views.inc loaded in og_init) :

The issue is actually caused by statistics.module (!), and by the fact that my contrib modules are in sites/all/modules (which is the recommended location)
Steps to reproduce :
- enable statistics module
- go to Groups Directory page (?q=og) : ok, groups are listed.
- empty the cache (devel block...)
- ?q=og now displays "page not found"
- go to ?q=admin/views : og-defined views are not listed

What happens is this - took me some time with a PHP debugger to figure it out :
- after cache is cleared, the next page call (redirection) triggers _init hooks
- statistics_init is called before og_init, because module_implements() orders modules by weights *and by filename* (and core "/modules" comes before contrib "/sites/all/modules")
- statistics_init contains a call to drupal_set_title, which triggers a series of menu-related calls that end in a call _menu_build, and _menu hooks being executed (cache is empty).
- views_menu is executed and builds _views_get_default_views, and caches them. BUT og_init has not been executed yet (we are still in statistics_init), so og_views.inc has not been included and og-defined views info are not available :-)

This being said, I don't exactly know what should be corrected :-)
I guess og module should be given a lesser weight than statistics module - but that seems a strange fix.
Also, I'm not sure the hooks order depending on the folder the module sits in is a right thing to begin with.

Comment #4

moshe weitzman CreditAttribution: moshe weitzman commented 29 December 2006 at 02:41

Status:

Active

» Fixed

thats some terrific debugging. i had no idea that hook_menu() could fire before hook_init(). since drupal6 will not even have a hook_menu on every page, i'm inclined to keep this in global scope for drupal5.

Comment #5

yched CreditAttribution: yched commented 29 December 2006 at 14:03

Priority:	Critical	» Normal
Status:	Fixed	» Active

Actually, statistics logging was moved to hook_init by http://drupal.org/node/77971, and apparently it is about to be rolled back to hook_exit.
Once done, the original code (invlude views code in og_init) can be restored (and we should probably include this trick in CCK too :-) )
Setting back to active as a reminder ?

Comment #6

moshe weitzman CreditAttribution: moshe weitzman commented 2 January 2007 at 15:05

Status:

Active

» Fixed

Comment #7

yched CreditAttribution: yched commented 5 January 2007 at 21:54

Just to be sure how to do the same kind of optimization in cck :
Why do you use hook_init + if (function_exists('drupal_set_content')) { instead of hook_menu($may_cache == FALSE) ?

Comment #8

moshe weitzman CreditAttribution: moshe weitzman commented 6 January 2007 at 02:38

lat i checked, views does its aggregating of tables/fields during hook_menu() so all modules better have their files included by then. so hook_menu() is safe if you are sure cck has lower weight than views in system table. since i had to use init anyway, i just stuck mine there.

Comment #9

yched CreditAttribution: yched commented 6 January 2007 at 19:28

Got it - I guess I figured that when I debugged the 'missing og_views.inc' thing, and then forgot.
Thanks :-)

Comment #10

m3avrck CreditAttribution: m3avrck commented 12 January 2007 at 18:49

Hmm, the hook_init() can be problematic because it can interfere with aggressive caching, see this issue that affects CCK as well:

http://drupal.org/node/108688

Comment #11

moshe weitzman CreditAttribution: moshe weitzman commented 12 January 2007 at 19:52

yeah, i have been following that issue. yched asked a good question. awaiting answer there. i don'tg get why this stuff has to be loaded for an aggressive cache page. the whole point of that is that the page is cached.