Closed (won't fix)
Project:
Entity API
Version:
7.x-1.x-dev
Component:
Entity CRUD controller
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
28 Jan 2011 at 16:05 UTC
Updated:
1 Feb 2011 at 10:20 UTC
Through inspection of the controller code for #1043492: Problems with default (exported) entities I just now found out that reverting an entity will first delete it from the database, and then immediately insert it again. This also leads to the delete and insert CRUD hooks being called, even though the basic operation (viewed as its "outside" effect) is more like an update. And since the re-insertion will assign a new ID to the entity, there also isn't really a good way around this, without building extra-checks into the module for changed IDs.
I think it would be cleaner if, for reverting entities in code, you'd just look-up the defaults and then do an UPDATE (including, of course, a new status of ENTITY_IN_CODE). The way it is implemented now requires a lot more knowledge of the Entity API internals for correctly using the CRUD hooks for exportable entities (at least in more complex cases, such as mine above).
Comments
Comment #1
fagoI'm not sure about that. Having a delete + an insert makes sure the entity is really initialized from scratch, so that way it is ensured it's really defaulted. Else modules might still keep stuff for the entity although the stuff should be reset to the defaults. Also I don't think an 'update' would be really that what folks expect to happen on revert, especially as revert is triggered by delete() action for exportables which would be consistent to the 'delete' hook being invoked.
The problem with indexes and servers is very special, as there is strong link between those. Maybe it should even require re-indexing? Anyway, you could treat them special by just checking the exportable status of the entity, if required.
Comment #2
drunken monkeyWell, yes, if you know that delete() was called, or a revert done, this is what you might expect. However, from an outside point of view, the entity's values are just changed (if they are changed to the defaults, or to something else, usually doesn't matter a bit for other modules), doing a DELETE + INSERT in some cases for that doesn't make sense from that view point.
If a module stores old data even though the original entity was updated, this is more a bug in that module's code than something the Entity API should prevent.
However, if you really don't think that doing just an update would be appropriate here, I'll just work around it in my module, as you say. However, there is no really good way to detect a revert in the insert hook, or is there? (For the delete hook, I can of course just check the entity status.)
Comment #3
fagoWhat's the best in that case, will differ from situation to situation. However, still I think the current behaviour is the most logical one and does well in most cases, so let's stay with that.
>However, if you really don't think that doing just an update would be appropriate here, I'll just work around it in my module, as you say. However, there is no really good way to detect a revert in the insert hook, or is there? (For the delete hook, I can of course just check the entity status.)
Indeed, but I guess you could just lookup whether you have already data for the entity on insert?
Comment #4
drunken monkeyOK, then I'll adapt the Search API code for that …