entity_metadata_wrapper does not load correct revisions

Am running into this too with an entityreference field when used with #1837650: Allow referencing a specific revision ID.

Changing title, since this is for all entities

Initial patch. This might need work though. There is no caching mechanism for entity_revision_load. If you consider following code with this patch:

$wrapper = entity_metadata_wrapper('node', array('id' => 1, 'vid' => 1));

// this code runs in the same request.
$wrapper2 = entity_metadata_wrapper('node', array('id' => 1, 'vid' => 1));

$wrapper2 will re-load the node (not fetch it from cache) because entity_revision_load has no caching whatsoever.

Anyway, here's the patch:

Improved patch, the updateparent logic wasn't correct in the previous patch

This one should pass tests.

Another try, this time it *really* should work :-)

Just appled patch from #9, working fine, I think, it should definetely be in contrib!

Doesn't quite apply anymore:

patching file includes/entity.property.inc
patching file includes/entity.wrapper.inc
Hunk #1 succeeded at 566 (offset -8 lines).
Hunk #2 succeeded at 602 (offset -8 lines).
Hunk #3 succeeded at 619 (offset -8 lines).
Hunk #4 succeeded at 718 (offset -8 lines).
Hunk #5 succeeded at 757 (offset -8 lines).
Hunk #6 succeeded at 765 (offset -8 lines).
Hunk #7 FAILED at 776.
Hunk #8 succeeded at 793 (offset -8 lines).
Hunk #9 succeeded at 874 (offset 19 lines).
Hunk #10 succeeded at 1053 (offset 19 lines).
Hunk #11 FAILED at 1102.
2 out of 11 hunks FAILED -- saving rejects to file includes/entity.wrapper.inc.rej

Lovely! This patch worked for me when trying to get field collections working with workbench moderation (and as such the currently published version of the node actually references non-current revisions of field collections in certain cases). The patch needed a re-roll though - attached.

@jamsilver: Thanks for the reroll.

I tested this patch while working on #1837650: Allow referencing a specific revision ID. It still needs work. If a new revision for an entity is created the cache of EntityMetaDataWrapper is not cleared, causing the old revision to still be returned. This also needs tests to prove that it works.

Found an issue while testing the patch at #12 while also working with Organic Groups (7.x-2.7). When attempting to save a node that's configured as group content a fatal error is thrown - ( ! ) Fatal error: Maximum function nesting level of '100' reached, aborting! in /sites/all/modules/contrib/entity/includes/entity.property.inc on line 250

Patch works fine, tho we can't relay on the "vid" name. We have to use the name from the

"entity keys" => "revision"

anon, actually it doesn't matter I think. It's just how EntityMetadataWrapper names its property to store revision id.

artis.bajars, Drupal usualy needs PHP with xdebug running with this limit increased to 140 or even 200.
For doing this you need to add ini_set('xdebug.max_nesting_level', '200'); in your settings.php file.
P.S: This is not PHP limit, It is xdebug's limit.

Not sure if related but running this code:
* Note: field_products is a field_collection

$sub = node_load(3);
$sub_wrapper = entity_metadata_wrapper('node', $sub);
foreach($sub->field_products[LANGUAGE_NONE] as $index=>$sub_product)
{
  dpm($sub_product['revision_id'], 'correct revision ID');
}
dpm($sub_wrapper->field_products[0]->value()->revision_id, 'entity_metadata_wrapper revision ID');

Gives 2 different revision IDs:

1. correct revision ID => 103611
2. entity_metadata_wrapper revision ID => 92235

The 2nd revision ID by entity_metadata_wrapper actually links to a totally different entity.

@artis.bajars I am also experiencing the max function nesting level reached with Organic Groups. There appears to be an infinite loop resulting from this patch's addition of the call to entity_load_single() within EntityDrupalWrapper->set() on line 781 of includes/entity.wrapper.inc.

My basic understanding is that when entity_load() is called to load a node that is group content, this in turn calls NodeController->attachLoad() which leads to entity_reference_field_load() being invoked for Organic Group reference fields. OG's Entity Reference behavior handler's load function OgBehaviourHandler->load() uses entity_metadata_wrapper() (see og/plugins/entityreference/behavior/OgBehaviorHandler.class.php:23) which gets us to EntityDrupalWrapper->set() and with the addition of entity_load_single() we loop back to entity_load().

Regarding the infinite loop when using Organic Groups, I've created a new issue in OG queue #2363599: Infinite loop after applying patch to Entity API to fix revision loading.

Current patch breaks saving organic groups, as og group behaviour calls the entity wrapper functionality early in insert, and the entity_load_single logic fires and the cache gets set way too early for field (e.g. before field values are present), meaning an empty field load until those caches get cleared.

what is the purpose of the parent logic around that entity load single (which is also causing the other logic issues)? Don't already have the entity being passed in, can't use that?

For now added to check to see if the entity was actually being changed by seeing if previous_id was set.

@hefox thanks for the new patch, could you provide an interdiff? That change was tricky to see as much of the patch was exactly the same.

Queuing hefox's patch for review.

Has this fallen into a black hole? A year and the test still hasn't run?

@NancyDru: The tests show green for me? It might be worth re-uploading it to have the tests run again.

In #22 they show green, but #24 says "queuing". I'm just getting tired of running into this every couple weeks.

Trying out this patch (together with #1837650: Allow referencing a specific revision ID.) and seems to work so far. Will post and update in case I run into any issues. Would be nice if this could be committed.

I've found a possible issue - new (unsaved) entities without IDs are not well handled, as every item in the list gets entity-loaded or as a revision, whereas those cannot have that done, as there is no entity to load yet, but it could still be passed through for use elsewhere. Patch to follow...

As an example of when this might happen, if there is code using the EMW in a hook_node_presave, and there are referenced entities that have been created on the fly, such as a new field collection item, they can be accessed by directly accessing each one, one at a time, but not as a whole list. So this can be made to work fine:

  foreach ($host_entity->{$field_collection_field}->getIterator() as $individual_field_collection) {
    $value = $individual_field_collection->{$sub_field}->value();
  }

...but the following will not work, causing a fatal error:

  $all_values = $host_entity->{$field_collection_field}->value();

(For field collection, this still needs a property callback anyway - #2393627 is dealing with that - but the point here is that there is no way any property callback could work anyway with this patch's changes if the latter snippet above is used.)

Here's a patch to solve the issue I outlined in my previous comment. This will only try and load numeric IDs (including revision IDs), so any unsaved entities (or other data) is left untouched.

Had similar issue with referenced paragraphs, the patch seems to work fine except for the fact that i had to make a small change to the paragraphs_field_property_get function. The function was returning entity id without revision id which would cause the entity_metadata_wrapper property to load the default revision

-$values[$delta] = $data['value'];
+$values[$delta] = array('id' => $data['value'], 'vid' => $data['revision_id']);

I had the same problem for field_collection module.

field_collection_field_property_get() function:

-$values[$delta] = $data['value'];
+$values[$delta] = array('id' => $data['value'], 'vid' => $data['revision_id']);

I've raised this as issues on both paragraphs and field_collection modules:

#2830714: Incorrect revision loading when using entity_metadata_wrapper
#2830713: Incorrect revision loading when using entity_metadata_wrapper

This caused major issues with CER
EntityMetadataWrapperException: Unable to get the data property chain as the parent data structure is not set. in EntityStructureWrapper->getPropertyValue() (line 458 of modules/all/entity/includes/entity.wrapper.inc).

I get the above error: admin/config/content/cer/add when I try to add a left field.

The patch works and fixes the issues with revisions of referenced entities.

@SocialNicheGuru if it doesn't work with combination of another contrib module, maybe the contrib module should also be updated/patched to get it work, as it is the case with the Field Collection module. Maybe it is better to open an issue on the mentioned module's page.

Patch makes sense, however I dislike the 'vid' key. That's a node-centric key and does not make sense generally. Let's use 'id' and 'revision_id' as keys.

Also, as the tag suggests - this new functionality needs test coverage.

+1

Not sure when to look for 'vid', so just took the last patch and replaced all vid changes to revision_id.

This still needs tests but also needs a double check where I should have left the vid, created this patch to help keep this issue moving.

+1 for #39. I've been using it to help fix up some issues in RESTWS, RESTWS Entity Reference and Paragraphs, so I'd be excited to see this one make it through.

The year old patch in #39 still applied cleanly to the latest 7.x-1.x dev snapshot. As there are several other issues dependent on this issue being fixed/committed, is anyone here able review/test this patch and RTBC?

Please be careful with the tags, it can be easy to accidentally remove them. Happy holidays!