Setting expire values for each cached block

I'll add cache_clear_all() in the blockcache_alter_save_settings() function which will clears all blockcache entries. That's the fastest way and probably the most logical to fix this.

Committed. New release also out in a few moments.

But wouldn't it be better to *just* clear the cache of the block one is changing?

Damn, that's indeed a better option. I was a bit to fast to roll out another release allready ...

Which is why I'm looking for a way to set TTL in a way that they wouldn't all refresh their cache at the same time.

That's impossible indeed without hacking core. However, we could implement our own caching mechanism. What would happen is that we simply copy cache.inc to say cache_block_cache.inc, change the cache_clear_all('cache_block') with a smarter query clearing the cache. Of course, if you use any other cache replacements (like memcache, cacherouter ..) it won't be possible to use those modules.

It shouldn't be that hard to write that, I'd be happy to write that if you can test along with it. Let me know what you think.

Attached is a patch which gives you 2 radio buttons underneath the cache select box on the block configure page.
It lets you choose to clear the cache for that block only or to clear all cache.

Can you test that out ?

Also should I take over the block overview page (where you can place blocks in regions and set their weight), because submitting that page clears everything too as you can see in the snippet underneath. It shouldn't be that hard to override this I guess.

function block_admin_display_form_submit($form, &$form_state) {
  foreach ($form_state['values'] as $block) {
    $block['status'] = $block['region'] != BLOCK_REGION_NONE;
    $block['region'] = $block['status'] ? $block['region'] : '';
    db_query("UPDATE {blocks} SET status = %d, weight = %d, region = '%s', throttle = %d WHERE module = '%s' AND delta = '%s' AND theme = '%s'", $block['status'], $block['weight'], $block['region'], isset($block['throttle']) ? $block['throttle'] : 0, $block['module'], $block['delta'], $block['theme']);
  }
  drupal_set_message(t('The block settings have been updated.'));
  cache_clear_all();
}

What I could do is adding a checkbox or radio buttons near that submit button where you can decide if you want to clear block cache too or not, that sounds logical right ?

What I could do is adding a checkbox or radio buttons near that submit button where you can decide if you want to clear block cache too or not, that sounds logical right ?

Attached patch has that option now too.

I've committed the patch in #7 to the drupal 6 branch. I need to think a bit more about the radio buttons on the block overview page, not sure if it's really interesting enough to keep that going. I'll repost another patch soon with a first test with a custom caching implementation.

I needed this functionality in 6.x, so I wrote the neccessary changes myself. And not only that piece. However there were few problems that could not be solved without patching the core.

The problem with the expire time for blocks is that a block gets cached only during the block_list() funсtion. And this function is called only when a theme region is rendered. And in this function all blocks are set to be cached temporarily so next time cache_clear_all() is called all blocks are rebuilt and there is no workaround for this.

In drupal 5.x block caching was introduced with the great blockcache module, which was sort of ported into the core for drupal6. When I was running 5.x, I've enhanced the blockcache module for myself. For the drupal 6, I have decided to use blockcache_alter as a basis and combine it with blockcache5 features.

Here is what you can do now with my modification:

1) choose the cache type (blockcache_alter original feature)

2) set the block cache expire time, in seconds

3) choose when to re-cache the block: (blockcache5 feature)
a. when a node is added/updated/deleted
b. when a comment is added/updated/deleted
c. when a user is added/updated/deleted
d. when a user logs in or out
This option would overwrite the expire time for the block.

4) When 3.a. and/or 3.b are set you can choose which node types should cause the refresh.
It is very useful when you have a lots of blocks, but they should be refreshed independently. For example, my frontpage have blocks like "Latest blogs", "Latest images", "Latest news", etc and blogs, images and news are all different content types. Obviously it is better to refresh each block only its content type if affected. So, this option does just that. Now "Latest news" are refreshed only when new news are posted, or new comment to news is posted. Same with other blocks. Very-very handy option. Drupal doesn't have to rebiuld the blocks that didn't actually changed.

Combining these options gives you an incredible level of control related to blocks. And i think this is how block caching should be done in drupal. My frontpage (www.liveangarsk.ru) has about 10 blocks of different aggregated contents. Without blockcaching it takes about 1500 db queries to build the frontpage. With my module this number went down to 300 db queries per frontpage load. And it is not just for guests, but for authenticated users! And more over, now single posting doesn't cause the entire page to refresh, only needed parts are refreshed.

However, as I mentioned earlier, there is a patch to the core. You have to patch the block.module, the function to patch is block_list(). It starts at about line 400.

Find this part (about 469 line)

          if (!count(module_implements('node_grants')) && $_SERVER['REQUEST_METHOD'] == 'GET' && ($cid = _block_get_cache_id($block)) && ($cache = cache_get($cid, 'cache_block'))) {
            $array = $cache->data;
          }
          else {
            $array = module_invoke($block->module, 'block', 'view', $block->delta);
            if (isset($cid)) {
              cache_set($cid, $array, 'cache_block', CACHE_TEMPORARY);
            }
          }

and replace it with the following:

         if (!count(module_implements('node_grants')) && $_SERVER['REQUEST_METHOD'] == 'GET' && ($cid = _block_get_cache_id($block)) && ($cache = cache_get($cid, 'cache_block'))) {
            $array = $cache->data;
          }
          else {
            $array = module_invoke($block->module, 'block', 'view', $block->delta);
            if (isset($cid)) {
              $blocklife = variable_get('bc_life_' . $block->module . '_' . $block->delta, '');
		$blocklife = (int)$blocklife;
		if($blocklife > 1) {
		         cache_set($cid, $array, 'cache_block', $blocklife + time());
			}
		else {
              cache_set($cid, $array, 'cache_block', CACHE_TEMPORARY); 
		}


	if (variable_get('bca_debug', FALSE) && user_access('administer nodes')) {
	drupal_set_message('Block re-cached: ' . $block->title . '_' . $block->module . '_' . $block->delta . '_' . $blocklife . '_' . time());
				}

            }
          }

Without this patch nothing would work.

My mod has a settings page (admin/settings/blockcache_alter) where you can enable of disable the output of the debug information. I used the debug to control what gets refreshed and when.

Also, here is one more note.

Sometimes, people do not enable the block itself, but call it with the module_invoke() function to put it somewhere they need. I found out that in this case blocks don't get cached even if all cache settings are set, because the block does not belong to any region. I've worked a solution for this problem - you should set the cache settings for the block as you desire, leave it disabled, and then call the block with the following piece of code:

$block = module_invoke('blockcache_alter', 'block', 'view', 'block,14');
print $block['content'];

The last parameter should consist of the name of original block and its original delta seperated by comma. All blocks could be cached this way. For example:

$block = module_invoke('blockcache_alter', 'block', 'view', 'views,popular-block');
print $block['content'];

This would output the views "Popular" block and cache it according to your settings.

In general, most user would not need it, but still I felt I had to do it.

I used your blockcache_alter-1.1 version as a basis, but since so many code has been added, I think it would be better for me to attach the full module.

I hope everyone will find it useful.

Subscribing. It would be super cool if each block cache could expire independently. :)

Regarding batbugs2's core block.module patch in #11, it seems we need to check that the block is not expired as well. The core code only uses CACHE_TEMPORARY (-1) so it assumes if there's a cache, it's not expired.

Line 469 should be something like this:

          if (!count(module_implements('node_grants')) && $_SERVER['REQUEST_METHOD'] == 'GET' && ($cid = _block_get_cache_id($block)) && ($cache = cache_get($cid, 'cache_block')) && ($cache->expire == CACHE_TEMPORARY || $cache->expire > time())) {

@batbug, impressive! I'll be checking out the code during the DrupalCon this week and probably commit most of them as a patch that comes with the module so everyone can choose individualy how far they want to go with their caching strategy on a production site. I'm going to write some tests for them too and talk a bit with some other core people trying to convince them to consider to introduce some changes into core for next releases!

@swentel, glad to hear this. Feel free to post any questions, if you have any, I read my tracker every day.

Ok, I went ahead and committed most of the changes by batbug and the small fix by ball.in.th to the drupal 6 branch. I also created a dev release available at http://drupal.org/node/395422 (might not be available within 12 hours though, you can always checkout from cvs of course)

Dev version (and the later the new version) includes 2 core patches so blockcaching with lifetime expires will work:

- blockcache_alter_with_node_grants.patch: patches block module with node_grants check still available
- blockcache_alter_no_node_grants.patch: patches block module with node_grants check gone (handle with care)

Work to be done: a variable on the settings page to tell if you have applied the patch or not (depending on that, the caching fieldset will show you more or less options regarding expire times), more documentation and of course TESTING, please go nuts and post follow ups here :)

Thanks for the work so far, I think we're close making this a top caching module! (sadly with a small core patch, but hey)

Update: there is now a variable on the settings page of blockcache alter. When this is checked more options appear on the block configuration page - but be sure to apply the patch of course (there is no check for that and I currently don't see the point for that either)

Get latest cvs or latest dev build (in a few hours) and let's test!

Nice to hear that. I just didn't understand what's the point in node grants check? And i don't understand ball.in.th patch as well. (sorry for my bad english :)

"(sadly with a small core patch, but hey)" -- drupal devs could've used the blockcache5.x approach ;) I still don't get why they did not.

Nice to hear that. I just didn't understand what's the point in node grants check?

Well, the drupal devs have a good point checking on it, but they implemented it completely wrong IMO, as in, there is no way to get around it. Take organic groups for instance, many sites use that module and it implements a hook_node_grants which simply means that blockcaching will be completely disabled at that point.

And i don't understand ball.in.th patch as well. (sorry for my bad english :)

I should test this further more for now, but I indeed had some moments where I really knew a block should refresh but wasn't .. I'm going to write some tests further on this week to get his completely ok !

"(sadly with a small core patch, but hey)" -- drupal devs could've used the blockcache5.x approach ;) I still don't get why they did not.

Don't start about this, I'm getting mad about it too all the time :)

Just a quick update, the development package now comes with tests and I can either conclude two things for now:

1) I fail at writing tests
or
2) The logic in refreshing blocks is flawed especially in the core patches that come with the module now. I've changed them a bit already but I really need todo some more testing as I think there is a logical flaw in checking on the expire > time() right now OR using cache_temporary for the expire time .. the tests reveal these inconsistencies .. will research more this weekend and next week!

Not sure if this is related, but there's a serious bug in cache_clear_all() and cache_set().

@ball.in.th,
very likely to be, but I can't say for sure, because I don't use minimum cache lifetime settings at all, and therefore I never have any problems with blocks not refreshing.

Ok guys, please test out http://drupal.org/node/395422 - check the README that comes with the package which patch you optionally want to use for additional blockcache options.
It also comes with tests: 206 passes, 0 fails, and 0 exceptions :)
I hope to release it somewhere this week!

Ok, I went ahead and create a release, have fun!