Problem/Motivation

The documentation of the parameter $cid in DrupalCacheInterface::clear() is not sufficient enough. This leads to issues like this one: #1871006: Flaw in the implementation of DrupalCacheInterface ::clear() - missing cid array handling

Proposed resolution

Enhance documentation about what data types are accepted.

Remaining tasks

reviews needed - enhancements welcome :)

User interface changes

none

API changes

none

Files: 
CommentFileSizeAuthor
#7 drupal-enhance-documentation-cache-clear-1871028-7.patch2.43 KBfreblasty
PASSED: [[SimpleTest]]: [MySQL] 40,441 pass(es).
[ View ]
#2 drupal-enhance-documentation-cache-clear-1871028-2.patch2.11 KBfreblasty
PASSED: [[SimpleTest]]: [MySQL] 40,098 pass(es).
[ View ]
drupal-cache-enhance-DrupalCacheInterface-clear-documentation.patch701 bytesdas-peter
PASSED: [[SimpleTest]]: [MySQL] 39,609 pass(es).
[ View ]

Comments

Status:Needs review» Needs work

Thanks for reporting and patching!

I would like to offer a couple of suggestions:

a) I think the documentation could be reworded a bit better. It starts out implying it has to be a string, and then in the new sentence says "Oh, it can actually also be an array".

b) I think the documentation should mention in $cid that it must be a string if $wildcard is true (arrays will not work in that case).

c) Maybe it should also mention in $wildcard that if $cid is empty, $wildcard is ignored.

And as a note, I guess this is D7 only... not sure what/where the corresponding classes are in D8?

Assigned:Unassigned» freblasty
Status:Needs work» Needs review
StatusFileSize
new2.11 KB
PASSED: [[SimpleTest]]: [MySQL] 40,098 pass(es).
[ View ]

Created new patch based upon the suggestions made by jhodgdon. This patch only applies to D7 because of the new cache API in D8 (#81461: Clean up the cache API (stop overloading function arguments, remove procedural wrappers)).

Status:Needs review» Needs work

Better, thanks!

However... Since cache_clear_all() is calling the clear() method, passing in $cid and $wildcard, the documentation for $cid and $wildcard should, I think, be the same for both functions. Right? It looks like $cid matches, but not $wildcard. Let's pick one... I think the one in cache_clear_all() is clearer.

Also, the behavior of cache_clear_all() when both $bin and $cid are NULL should be documented, which it isn't at all at the moment.

Status:Active» Needs review

@jhodgdon: It seems that the behavior with the arguments $cid and $bin is already documented but a little vague:

If called without arguments, expirable entries will be cleared from the cache_page and cache_block bins.

What about the following text:

If called with the arguments $cid and $bin set to NULL or omitted then expirable entries will be cleared from the cache_page and cache_block bins and the $wildcard argument is ignored.

Status:Needs work» Active

Status:Needs review» Needs work

#4 sounds fine; may need a comma or two. :)

And restoring correct status.

Status:Needs work» Needs review
StatusFileSize
new2.43 KB
PASSED: [[SimpleTest]]: [MySQL] 40,441 pass(es).
[ View ]

Patch which contains changes from #2 and #4.

Status:Needs review» Reviewed & tested by the community

Looks good to me, thanks!

Status:Reviewed & tested by the community» Fixed

Thanks again! Committed to 7.x.

@freblasty, @jhodgdon Awesome, thank you very much!

Status:Fixed» Closed (fixed)

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