Block cache should be BLOCK_NO_CACHE

Version with patch bot .module and .install files

This is critical to UI bacause ALL users see same block with COUNT from user who open the page first...

Docs
http://api.drupal.org/api/function/hook_block/6
http://api.drupal.org/api/constant/BLOCK_CACHE_PER_USER/6

@litwol 6.x-1.0-rc2 still defines block without BLOCK_CACHE_PER_USER so update_6002 is needed.

CVS DRUPAL-6--1 already holds BLOCK_CACHE_PER_USER definition for block but there's not update function in .install

This patch should wait till #370960: Remove t() from all schema descriptions

Do we really need to update the cache table ? That should be done automatically when update.php is called, which does clear the cache, not?

@Berdir update block table - not cache

But then your patch is wrong :)

.. + $ret[] = update_sql("UPDATE {cache} SET cache = 2 WHERE module='privatemsg' AND delta='privatemsg-menu'"); ...

Sorry, blocks not cache
patch against -6--1

Instead of 'cache = 2' you need to use combination of 'cache = %d' and BLOCK_CACHE_PER_USER as query argument.

@litwol Agree exactly

There are some ideas - previous way is wrong

1) Quick solution - set block->cache to BLOCK_NO_CACHE and check for new messages on every page request
2) Advanced - leave BLOCK_CACHE_PER_USER but make some logic for sending messages
- clear cache per recipient when message is sent
- clear own cache when user oen a message

Suppose 2 is little hard to implement but it brings some perfomance (I'll try)

But wanna know opinion of maintainers

3) Convert the block to a menu, so that everything except the number of new messages is cached, which is loaded anyway. Devel does this now too, see 'menu_name' option at http://api.drupal.org/api/function/hook_menu/6

the idea behind caching is to avoid executing long queries (those that scan whole pmsg tables). (2) aligns well with what i was brainstorming about last night in IRC with Berdir. we should flush cache per user when they receive messages.

At this moment I see 2 blocks + menuitem they all brings same functionality (notify users about new messages)

All of them call only one function privatemsg_unread_count() which executes query only once and staticaly caches the result so no matter how offen it called.

Menu item updates on *privatemsg_title_callback*

This 2 blocks never updates (I found no cache_clear_all) they should called on send for all recipients

I use this code in some parts but when I need clear cache for different users I need to know there $theme $language
So I change the _block_get_cache_id by my own function

This function work only for current user but clears cache for all user's themes (all themes)
It's easy to change (copy/paste) *_block_get_cache_id* code to make cache_id depending on needs

I tried to get this working, but pretty much failed. It is just too complex. The main issue is that we don't only need to clear the cache for the current user but for all recipients, too. And these users could have another language and theme. And there are multiple blocks, at the moment two, both could be actived.. or not.

Additionaly, I'm not even sure if we gain anything by using the cache at all. Both blocks just display some text with a single query, that is only executed once and, because of the menu callback, executed anyway. So, all needed resources are either already cached (translation), use static cache (unread count, permissions) or are just simple strings. If using a cache, it would execute a query per block unless a cache backend like apc or memcached is used.

My suggestion:
use BLOCK_NO_CACHE for both blocks, patch is attached.

Agree exactly... I just forget about multiple recipients and there themes-languages

Code that generate this blocks are lightweigh so caching is optional

RTBS patch from #14

just updating the title..

Just reroll against current state

Committed

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