Output of render_cache_entity_view_single is not compatible to node_view(), etc. as it has two more levels

h3. Proposed resolution

Audit code and make output compatible to overridden functions

Comments

fabianx’s picture

Status: Active » Fixed

Fixed in GIT.

codycraven’s picture

Status: Fixed » Needs work

I notice this commit added "$node->view = $this->view;" into the views hijack code.

I purposely left that out as any alterations a site makes to a node based on the view object will cause potential issues:

  1. The cached render will be invalid as soon as the node moves within the view's output. The index of the node will change which could break whatever customization the site is doing based on the view object.
  2. A node rendered in multiple views (with the same view mode) will cache only one version and use across all the instances. Again causing issues if the site performs any customization based on the view object.
  3. If the node's view mode is used anywhere other than views we encounter the same issue as 2.

This does restrict sites to not customize based on the view object, however I think it is an acceptable trade off.

What do you think?

fabianx’s picture

Any more advanced site will do customization of nodes based on view_mode.

For example for Display Suite there is hook_ds_views_row_render_entity_alter.

And we do things (on a production site) like:

$content['#node']->context['image_style'] = 'thumbnail';

based on a view name for example.

The solution is and render_cache can't fix this any better way, to set the hash manually.

I have it setup that the serialized $entity->context is taken and saved in the hash and such, I can control on what views re-act or not.

As the $node->view is available, I can just duplicate the logic I did else where to do things (so If I react on $node->view, I will need to use the same logic in the render_cache_render_hash_alter to supply it as a context.

So restriction is not an option IMHO as we loose flexibility that can easily be mapped by implementing the hook_render_cache_entity_hash_alter.

I even thought about changing render_cache to only support core entities by default and activate it using a key within entity_info_alter().

$entity_info['render_cache'] = TRUE;
$entity_info['render_cache_defaults'] = _render_cache_get_defaults();

And then in our last coming render_cache_entity_info_alter() only enable for those entities that have render_cache == TRUE set.

render_caching is complex and and while we can supply some things out of the box, even adding a simple Entity Reference field can already break things and you need to add to the hash manually.

That said I am working on a solution for entity references (including user reference and taxonomy reference) and plan to add a multiple mode to entity_modified_last_multiple().

IMHO, a nice thing is to have:

* Collect all (entity_type, entity_id) pairs of all to be rendered entities.
* Then calculate cids taking into account the fully loaded "modified" and 'entity_id, entity_type' values.

As entity_modified_last_multiple will be very fast (just one cache call), this will make many more scenarios possible.

This is however just a helper of what can be done manually as well and I am already doing this manually.

I am currently working on supporting #attached in nested entities, by recursively collecting. It works well! :)

So my current work plan is:

1. Finish nested #attached support (only works if one entity is given as parent as else we don't know which entity it is part of).
2. Add entity_modified_get() and entity_modified_get_multiple() to entity_modified API. Also add static and cache_modified caching. (as now this will be called a _lot_). Already with just three modified calls it is the majority of my rendered cache time.
3. Add an API to support collecting "entity_references" before calculating cids to mass load "modified" values.

4. Add support module or variable to support ER automatically to a certain depth (1 or 2 levels).

This will probably be configurable per entity_type and of course needs benchmarks.

5. Convert render_cache to be more of an API module and support "core" entities more properly by adding references to comments, etc. (configurable).

Not sure when I get to that, but to 3) probably very soon.

Thanks,

Fabian

codycraven’s picture

Status: Needs work » Fixed

Solid reasoning, I'm marking back to fixed as the other tasks will be addressed in separate issues.

Status: Fixed » Closed (fixed)

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