Problem/Motivation
There is a problem when inserting/updating a node when this module runs alongside Menu Node API
In Menu Node API's _menu_node_invoke
function, there's
...
module_invoke_all('menu_node_' . $hook, $link, $node);
Where $hook
can contain 'update/insert/delete' resulting in a module_invoke_all('menu_node_update/insert/delete')
.
The problem is that Drupal thinks that the og_menu module implements those hooks whereas it only implements hook_node_update/insert/delete
(that's because there is a module called og
and a function called og_menu_node_update
even though the code does not belong to the og
module)
Since a hook_node_update/insert/delete
expect a $node
as its only parameter, the $link
provided by menu_node's module_invoke_all('menu_node_' . $hook, $link, $node)
ends up in a EntityMalformedException.
I already created a ticket in Menu Node API's issue queue for this problem (see EntityMalformedException is thrown due to an unwanted call to hook_node_update in og_menu) but I think the issue lies here and not in their module.
Proposed resolution
In the aforementioned ticket, I didn't know what was the best option to resolve this issue, but now I think you should test the parameters you receive in your hook_node_update/instert/delete
implementations to avoid any issues.
Remaining tasks
n/a
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#6 | 2601170-6.patch | 1.34 KB | dasginganinja |
#2 | og_menu_-_EntityMalformedException_due_to_wrong_hooks_-_2601170_-_1.patch | 10.02 KB | Babou |
Comments
Comment #2
Babou CreditAttribution: Babou commentedHere is a first attempt to patch the issue.
The patch is bit "weird" for the
hook_node_delete
implementation as I just added a line on top to test the parameters and a closing brace at the end.Comment #3
rv0 CreditAttribution: rv0 commented#2184289: Rename module hooks to avoid conflict with *menu modules implementing hook_node_*
Comment #4
dasginganinjaShouldn't this just be fixed in core? The
module_invoke_all($hook)
implementation in includes/module.inc loops though the output ofmodule_implements($hook)
so the issue really seems that is should be fixed there.@rv0 It seems like og_menu could at least put checks in to check the number of arguments / validate that it's not a mlid that is being passed to it.
Thoughts? I just got burned in a bad way by this. This still exists in og_menu-7.x-3.1.
Comment #5
rv0 CreditAttribution: rv0 commentedNot sure how this could get fixed in core. The problem is poor naming on menu_api's side
I agree though about some light weight checks, feel free to suggest a patch.
Comment #6
dasginganinja@rv0 this could be handled by fixing core's implementation of
module_implements($hook)
to check which module the hook is really defined in.For the time being here is a patch that checks if menu_node is enabled and that the number of arguments match the expected signature.
Comment #7
dasginganinjaComment #8
dasginganinja