Starting on line 95:
**
* Constants defining cache granularity for blocks and renderable arrays.
*
* Modules specify the caching patterns for their blocks using binary
* combinations of these constants in their hook_block_info():
* $block[delta]['cache'] = DRUPAL_CACHE_PER_ROLE | DRUPAL_CACHE_PER_PAGE;
* DRUPAL_CACHE_PER_ROLE is used as a default when no caching pattern is
* specified. Use DRUPAL_CACHE_CUSTOM to disable standard block cache and
* implement
*
* The block cache is cleared in cache_clear_all(), and uses the same clearing
* policy than page cache (node, comment, user, taxonomy added or updated...).
* Blocks requiring more fine-grained clearing might consider disabling the
* built-in block cache (DRUPAL_NO_CACHE) and roll their own.
*
* Note that user 1 is excluded from block caching.
*/
1. In the first example, what does it actually mean, in terms of Drupal caching, to use DRUPAL_CACHE_PER_ROLE | DRUPAL_CACHE_PER_PAGE? More examples are needed (look at error handling in php.ini for a good example of good documentation)
2. What is DRUPAL_CACHE_CUSTOM, and what am I supposed to do with it?
3. What's the difference between DRUPAL_CACHE_CUSTOM and rolling your own and DRUPAL_NO_CACHE and rolling your own?
Comment | File | Size | Author |
---|---|---|---|
#27 | drupal_cache_docs-863428-27.patch | 1.94 KB | Albert Volkman |
#24 | drupal_cache_docs-863428-24.patch | 2.2 KB | Albert Volkman |
#24 | interdiff.txt | 2 KB | Albert Volkman |
#22 | drupal_cache_docs-863428-22.patch | 1.17 KB | Albert Volkman |
#19 | drupal_cache_docs-863428-19.patch | 1.6 KB | Albert Volkman |
Comments
Comment #1
robertDouglass CreditAttribution: robertDouglass commentedFurthermore, advice such as "The DRUPAL_CACHE_PER_ROLE and DRUPAL_CACHE_PER_USER constants should not be combined with the bitwise OR operator (|), as the two caching modes are mutually exclusive." (from Pro Drupal Development) would be useful here.
Comment #2
robertDouglass CreditAttribution: robertDouglass commentedCan I use the & bitwise operator?
Comment #3
apadernoConsidering the values used for
DRUPAL_CACHE_PER_ROLE
,DRUPAL_CACHE_PER_USER
,DRUPAL_CACHE_PER_PAGE
, andDRUPAL_CACHE_GLOBAL
, the value ofDRUPAL_CACHE_PER_USER & DRUPAL_CACHE_PER_PAGE
is0
.The constant
DRUPAL_CACHE_CUSTOM
is described asThe difference is that using
DRUPAL_CACHE_CUSTOM
, the module is using its own block cache it handles with custom code; when a module usesDRUPAL_NO_CACHE
, it says the block should not be cached (and it doesn't implement a custom cache).Comment #4
robertDouglass CreditAttribution: robertDouglass commentedThanks kiamlaluno. Want to take a crack at turning that into a patch for the D7 code comments in common.inc?
Comment #5
jhodgdonThis documentation block is also not useful as it is. Although it starts with /**, it will not appear on api.drupal.org, because it isn't documenting anything (it isn't followed by a function, constant, etc., and it isn't a @defgroup, etc.).
So it needs to be either incorporated into one of the contsants' doc (and then the others can have a line saying something like "See WHATEVER_CONSTANT for more information on using this constant), or it needs to be made into a group.
Comment #6
Mile23Seems like there should already be a caching API group, but there isn't. Sadly, I don't have time to properly research and write about it, so here's a patch where one constant gets the larger treatment, and the others get 'See that other one for more details.'
Also a poke in the eye of block.api.php, which omits DRUPAL_CACHE_CUSTOM from hook_block_info(), causing confusion here: #1200794: Block Example involving custom caching
Having just read the code, I seriously doubt there will ever be a consequence to distinguishing between DRUPAL_NO_CACHE and DRUPAL_CACHE_CUSTOM, and anything with the word 'custom' in it should be removed from Drupal forever. But nevertheless, here we are. :-)
Comment #7
Mile23Comment #8
jhodgdonThe docblocks in this patch do not conform to our docblock standards:
http://drupal.org/node/1354
In particular, they all need to start with a one-line summary of the particular item that is being documented. That's as far as I got in reviewing the patch. There are standards on that page for how to document constants.
Comment #9
Mile23This leads me to two conclusions: 1) The original documentation shouldn't have passed muster, and 2) Someone needs to define a group, probably in cache.inc, since there is no way to create a one-sentence summary of both the need for DRUPAL_NO_CACHE and also a more general discussion of these flags.
Comment #10
jhodgdonThe original doc probably wasn't reviewed properly. I can't help that. Sorry. But this patch is not going in until the doc conforms to our doc standards. Sorry.
And yes, there probably does need to be a group.
Comment #11
Albert Volkman CreditAttribution: Albert Volkman commentedHow's this?
Comment #12
jhodgdonLooks pretty good, thanks!
But we need to define a @defgroup, and then all the generic "about block caching" text should be taken out of the BLOCK_NO_CACHE docs and put in there, rather than linking all the other constant docs to BLOCK_NO_CACHE.
Comment #13
Albert Volkman CreditAttribution: Albert Volkman commentedAh, gotcha. Like this?
Comment #15
jhodgdonPerfect! I'm not sure what that test failure was... I'm going to upload the patch again to preserve that test result (so I can file another issue) and I'll get the patch committed shortly.
Comment #16
Albert Volkman CreditAttribution: Albert Volkman commentedYeah, it seems like a false-positive. I had that same issue over here-
http://drupal.org/node/1186582
Comment #17
jhodgdonI filed an issue for that test failure:
#1783656: Intermittent test failure in testBulkImportUpdateExisting
Since you've seen it elsewhere I'll up the priority.
Comment #18
jhodgdonThanks! Committed to 8.x. Needs port for 7.x (I'm not sure if the constants are exactly the same and in any case this patch does not apply there as it is).
Comment #19
Albert Volkman CreditAttribution: Albert Volkman commentedBackported.
Comment #20
jhodgdonLooks good -- I'll get it committed shortly -- thanks!
Comment #21
jhodgdonI committed this patch to 7.x, thanks!
But I realized we need to go back to 8.x now that we have this nice doc group and fix up the docs a bit. The group docs currently say:
Obviously that first paragraph is missing something... Or actually probably that last partial sentence should be removed (since that information is in the DRUPAL_CACHE_CUSTOM doc block).
Also, looking at the DRUPAL_CACHE_CUSTOM block that is there now, I think we should remove the line that says it's the same as DRUPAL_NO_CACHE, given the information in comments at the top of that issue. It's not the same -- custom means the module is implementing its own system (as stated in the docs now), and no cache means no caching. They are semantically different; let's not confuse the issue by saying they're the same, even if the block system does the same thing with them. What I'm talking about is this sentence from the DRUPAL_CACHE_CUSTOM docs:
Finally, I think that the statement in the group docs about binary combinations should be modified slightly. The no cache and custom constants are negative and they aren't intended to be binary-combined at all. I'm also not sure what the binary combinations would mean.. for instance what would global | per role really do in the block system? Global says it's the same for every user and page, but per role says it can change per role... So this whole idea of binary combination needs to be figured out.
Comment #22
Albert Volkman CreditAttribution: Albert Volkman commentedThis patch resolves everything but your binary combination question.
Comment #23
jhodgdonYes, that looks good so far. One typo:
seful -> useful. doh! :)
I also noticed this at the end of the group docs:
I think this should suggest DRUPAL_CACHE_CUSTOM, right?
Regarding the binary combination wording... I looked in core at all existing hook_block_info() implementations, and there are two (book and profile modules) that use this combination:
DRUPAL_CACHE_PER_PAGE | DRUPAL_CACHE_PER_ROLE
Which seems to mean that the block has to be cached by combination of page and role... hah! I found the function where this caching stuff is done: drupal_render_cid_parts()
So. It looks like you can combine either
per page or per user with per role(oops, edit) per role or per user with per page (end of edit), but that is the only combining that actually does anything, and what it does is to make sure that the unique cache ID contains both the user/role and the page URL (which is what you'd expect).So how can we say this? Maybe replace this:
with something like this:
Modules specify how their blocks can be cached in their hook_block_info() implementations. Caching can be turned off (DRUPAL_NO_CACHE), managed by the module declaring the block (DRUPAL_CACHE_CUSTOM), or managed by the core Block module. If the Block module is managing the cache, you can specify that the block is the same for every page and user (DRUPAL_CACHE_GLOBAL), or that it can change depending on the page (DRUPAL_CACHE_PER_PAGE) or by user (DRUPAL_CACHE_PER_ROLE or DRUPAL_CACHE_PER_USER). Page and user settings can be combined with a bitwise-binary or operator; for example, DRUPAL_CACHE_PER_ROLE | DRUPAL_CACHE_PER_PAGE means that the block can change depending on the user role or page it is on.
Is that too wordy? It's going on a group/topic page so I think it's OK...
Comment #24
Albert Volkman CreditAttribution: Albert Volkman commentedTypo fixed. You're right, that should be DRUPAL_CACHE_CUSTOM, I think. Also, I don't think that's too wordy. Pretty useful for someone who is looking for information on these caching options. Great job :)
Comment #25
jhodgdonThis now seems ready to go... I'll get it committed. Thanks!
Comment #26
jhodgdonThanks! Committed to 8.x. This one needs a reroll for 7.x.
Comment #27
Albert Volkman CreditAttribution: Albert Volkman commentedRe-rolled.
Comment #29
jhodgdon#27: drupal_cache_docs-863428-27.patch queued for re-testing.
Comment #30
jhodgdonThanks! Committed to 7.x. I think this one is now done. :)