In the code of EntityDependencyIterator.inc I found the following comment:
* @todo
* We need to throw an exception when we detect a circular dependency.
I strongly encourage you to NOT do this. Circular dependencies are a perfectly valid use case. I have not had a need for them myself, but I have worked on various 'reference' modules and such and I am aware people's usage often relies on a two-way or binary relationship. A common usage I am aware of is what people call backreferencing, which though it could be technically avoided by using custom code, is often used because of the advantages that come with views integration and such.
Some modules are even designed specifically for two-way relationships like the Relation module, which I bet someone will want to integrate with this module one day. A node could conceivably be dependent on it's own dependency, depending on the intentions of the use-case.
Some circular dependencies may be indirect. A node may reference a user, an the user may in turn reference the node. The chain could be even longer, and the site administrator may not be aware of the complexity of the dependencies.
The current behaviour of ignoring this situation is, IMO, the correct way to handle it.
Therefore, the fact that dependencies are returned prior to the current entity, while convenient, is not to be relied upon. I suspect since this module was originally designed with/for Deploy, that may be something to think about with the entity_dependency integration in that module (though I'm not aware of how it is done).
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | force-id-when-uuid-ref-not-found-1592580.patch | 602 bytes | darrenmothersele |
Comments
Comment #1
aspilicious commentedWouldn't this break the entire module? If you have circualr dependencies the module keeps procesing forever.
Please tell me what I'm missing. Want to know :)
Comment #2
danielb commentedAs a test I set up two nodes with node references going both ways. There was no endless loop or anything. The hasChildren() method appears to skip entities where the type+id already exist in the $checked property, this might be the thing that prevents problems.
Each entity is returned just once, however the only issue is that the expectation that dependencies will be returned first doesn't hold true in that scenario.
I have developed a system to handle dependencies that are out of order, or might not be present until some future event, using the UUID module. However it requires #1590312: Collect useful data in $dependency property to have enough info to manipulate the entity at that time to update the values inside the entity.
Comment #3
darrenmothersele commentedI have an entity, for arguments sake let's call it "artist", with an entity reference field called "related artists". So obviously this entity reference field points to other entities of the same type. It's very easy in this case to see circular entity dependencies, not just X relates to Y, which relates to X, but I also have quite a complex graph, were the circular dependencies are not obvious.
Anyway, when I try to deploy this it gets stuck, and think due to the circular dependencies. Am I right in thinking that to deploy entity X I first need to deploy entity Y, but to deploy entity Y, I first need to deploy entity X?
I'm thinking that on the destination site (the one receiving the entities via Deploy module), in order to create X (which has the entity reference to Y) I need the local ID for Y in order to complete the entity reference field in X before saving X. To do this I need to look up the UUID from the entity reference field and convert to local ID, so Y has to have been deployed first to do this. But Y relates to X, thus this can never complete?
Comment #4
darrenmothersele commentedWhen I try and deploy entities with circular dependencies (for example in entity reference fields), I get SQL errors because the UUIDs do not translate to local IDs in the entity_property_uuid_to_id() function in uuid module.
The attached patch (which is for uuid module, not entity dependency module) fixes this, in that it changes the UUID to 0, so the entities can still be saved (without the correct reference). In my testing the deploy module updates the entities anyway, so the references are correctly set once the whole deploy process has completed.
I'm 100% sure this is not the correct way to solve this, and a more general solution for deploying entities with circular dependencies still needs to be found.