Regardless of whether Drupal core thinks that static caching should be disabled for comments by default or not,

there is hook_entity_info_alter(), and the 'static cache' property can be changed freely,

so comment loading functions should follow other entity loaders and have a $reset argument.

Files: 
CommentFileSizeAuthor
#4 1000282-comment-load-reset-4.patch1.6 KBdixon_
PASSED: [[SimpleTest]]: [MySQL] 32,921 pass(es).
[ View ]
drupal.comment-load-reset.0.patch1.74 KBsun
PASSED: [[SimpleTest]]: [MySQL] 31,509 pass(es).
[ View ]

Comments

Assigned:Unassigned» sun

drupal.comment-load-reset.0.patch queued for re-testing.

Version:7.x-dev» 8.x-dev

StatusFileSize
new1.6 KB
PASSED: [[SimpleTest]]: [MySQL] 32,921 pass(es).
[ View ]

Re-rolled patch because of offset.

Status:Needs review» Reviewed & tested by the community
Issue tags:+needs backport to D7

Thanks!

Patch applies to D7 without offset, so should be ready for straight backport.

Version:8.x-dev» 7.x-dev

Committed to 8.x, moving back to 7.x for webchick. This is an extra argument, so an API addition, and you could technically get around it via calling entity_load() directly, but it's good for consistency with the other entity functions and arguably a bug that we allow you to change the static caching via alter but no obvious way to clear that static if you do.

No tests here but everything gets passed straight down to entity_load() which does have them.

file_load() is also missing this, opened a followup at #1292374: Enable static caching for File entities.

Status:Reviewed & tested by the community» Fixed

Yeah, this looks harmless. Committed and pushed to 7.x. Thanks!

(Note: I thought the entire point of the drupal_static() obfuscation so we didn't have to do this $reset parameter crap everywhere?)

Ah that's because entity static caching is inside the load controller class. But the only way to directly access that method is via _entity_get_controller(). So in D8 we'll be able to remove all the $reset for entities if the controller methods are exposed rather than wrapped in node_load() etc., but there's no nice way to do that yet.

Status:Fixed» Closed (fixed)
Issue tags:-needs backport to D7

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