Dries indicated this might be required for RDF module. Here's a patch. It has basic API docs, better examples and test coverage should be handled by RDF.

This would also disentangle entity API from field API if we wanted - since field.module could implement hook_entity_load() to do it's field attach calls. IMO modules should implement APIs defined by includes as far as possible, but that's not a task for today

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Needs review
FileSize
8.49 KB

Fixed namespace collision with field_test_entity_load().

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Needs review
FileSize
8.49 KB

Missed the menu loader callbac in field_test_menu().

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

FileSize
8.82 KB

It's sleepytime out east. Hopefully this doesn't contain any more stupidity, like not re-rolling the patch before posting the new version.

scor’s picture

Status: Needs work » Needs review

thanks catch, that's exactly what we need for #493030: RDF #1: core RDF module

Dries’s picture

This looks straightforward to me, but it would be great to get some feedback from yched or bjaspan.

bjaspan’s picture

I think this patch is a quick hack to enable a feature. But it is a feature worth having and it is a fairly safe and self-contained hack.

I don't like that we call hook_entity_load() and hook_TYPE_load(), both in module_invoke_all() mode. It's redundant; I don't see why we need hook_TYPE_load() any more. It's almost like we are trying to de-opify hook_entity_load() (before it even exists), except the very nature of the entity loader is that it acts on arbitrary types so its hooks have to specify the type. Also, why do we pass hookLoadArguments to hook_entity_load() but not hook_TYPE_load()? Seems inconsistent.

field_test_entity_load() clearly needs to be renamed due to the collision. I don't like field_test_entity_test_load() because it sounds like it is performing a test load instead of loading a test entity. I think field_test_test_entity_load() would be better, and I even re-rolled the patch that way, but that is bikeshedding that can be addressed more carefully later.

So, I think the patch is flawed, and if it were not the very last minute I'd mark it CNW. It clearly won't kill Drupal to commit it, though. What I expect is that it will be committed as is so RDF can get it in and then later we'll fix the inconsistency/redundancy with hook_entity_load() and hook_TYPE_load(), which of course will violate the rules of code freeze, but hey, it's not like there's an actual law about that.

I can't bring myself to mark it RTBC even though I fully expect it to be committed momentarily... :-)

pwolanin’s picture

Status: Needs review » Reviewed & tested by the community

patch applies cleanly and the code change is minimal and includes tests. Plus ... we want RDF

catch’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
8.8 KB

Re-rolled for taxonomy.test hunk failure.

catch’s picture

Status: Needs review » Reviewed & tested by the community

rrtbc.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs documentation

I see what Barry's saying. But in a way this is analogous to hook_form_alter() vs. hook_form_FORM_ID_alter().

And while we could go and rip out hook_node_load/user_load/etc. in favour of this, I would -1 that, to be honest. There are more than enough paradigm changes in D7. Let's wait and see how this entity stuff pans out "in the wild" before limiting people to only this most generic top-layer to interact with entities.

Committed to HEAD! This will need documenting in the upgrade guide.

bjaspan’s picture

I retract my objection. Having hook_entity_load() and hook_TYPE_load() allows hook_TYPE_load() to called with consistently typed arguments, whereas imagine a hook_entity_load() without hook_TYPE_load() that wanted to perform entity-type-specific operations on two or more entity types that use hookLoadArguments. This hook_entity_load() would be a return to the bad old days of arguments that vary based on $op, or in this case $type.

catch’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)
Issue tags: -RDF, -Needs documentation, -Entity system

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