Use case/Motivation

I have a node with a body field, an image field, a caption field, and a credit field.

I also have a short-code (or a token) that I want to be able to insert into the middle of my body field. This short code represents a placeholder for my photo, credit, and caption from fields on the same node.

When the body field is being "filtered" my short-code should be removed and replaced with the rendered output of these three fields.

Problem

The 1st page load after a cache clear, the above short-code will not be replaced.
The 2nd page load after a cache clear, it will.

The reason this happens is because the filter replacements are being run on the body field during the first call to node_load. Before the node is done loading, the filter process callback attempts to call node_load again, to grab the values for the fields for photo, credit, and caption. Even though you are on a node page, and expect to have node available, the static cache for that node has not yet been populated, so there is no node available (yet). The second call to node_load starts recursion.

Since rendering things in filters inside other things is also likely to create a recursive situation, static safeties are often added to keep the recursion from getting out of control. (see #1266064: The media filter allows infinite recursion via media_token_to_markup()). It was while I was adding a static safety that I noticed Drupal detecting recursion on a single node, where I had assumed there wouldn't be any. However, since the filters are called before the node is done loading, my second call to node_load was, in fact, creating recursion.

Proposed resolution

We have a handy static cache for entities after they are loaded. We have a use-case here for a static cache of entities before they have filters run. I propose that we store the entity (as loaded from the database) into the static cache first, so that the entity can be available while filters are being run. Once the entity is completely loaded, we can overwrite the entity in the static cache with the completely loaded and filtered entity.

Remaining tasks

Do it.

User interface changes

none

API changes

none.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jenlampton’s picture

Status: Active » Needs review
FileSize
845 bytes

Here's a fix that solves the recursive entity load problem for filters.

Status: Needs review » Needs work

The last submitted patch, 1: core-add_early_entity_static_cache-1266064-1.patch, failed testing.

jenlampton’s picture

Status: Needs work » Needs review
FileSize
829 bytes

remove docroot :)

acbramley’s picture

Status: Needs review » Reviewed & tested by the community

I've run into a similar issue where my filters are loading nodes first to validate the "shortcode" and then again to grab fields, if I put a shortcode on a node page which contained that node's nid I would get recursion errors. The patch at #3 fixed it for me, thanks a lot!

David_Rothstein’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

Interesting idea. I think it might make sense; I'm a little worried about storing not-fully-loaded objects in the cache, since someone calling entity_load() in the code that follows might expect and rely on getting a completely loaded entity. But their code would completely fail to run at all right now...

This looks like it needs to be considered for Drupal 8 first though; there is very similar code in EntityStorageBase::loadMultiple().

+    // Add an early static cache so the entity will be available to filters.

Maybe change that comment to be a bit more generic - i.e., to refer to any code that needs to load the entity during the attachLoad() step (rather than specifically to filters).

Berdir’s picture

Version: 8.0.x-dev » 7.x-dev
Issue tags: -Needs backport to D7

While the same code exists, the overall situation in D8 is completely different.

hook_field_load() is gone, filtering happens during rendering and *not* when loading entities. While something like that could still happen while entity-level hooks, I think it is way less likely and not something that *should* work, so I'd vote against changing this in 8.x since we don't need a workaround like that.

Moving back to 7.x for now, needs work for the comment thing.

jenlampton’s picture

Status: Needs work » Needs review
FileSize
887 bytes

fixed the comment thing. Still applying patch to 7.56.

jenlampton’s picture

Patch still applies cleanly to 7.58.

jenlampton’s picture

Patch still applies to Drupal 7.59.

jenlampton’s picture

Patch still applies to Drupal 7.63.

jenlampton’s picture

Patch still applies to Drupal 7.69 through 7.94.