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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Assigned: Unassigned » sun
sun’s picture

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

dixon_’s picture

Version: 7.x-dev » 8.x-dev
dixon_’s picture

Re-rolled patch because of offset.

sun’s picture

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

Thanks!

dixon_’s picture

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

catch’s picture

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.

webchick’s picture

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?)

catch’s picture

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.