Remove MODULE_load*() & Co functions in favor of entity_*() functions

hm, I'm not sure about this one. While entity-generic modules certainly need entity-generic hooks, it's much nicer to be able to implement e.g. hook_node_view() instead of hook_entity_view(). Also, I guess a hook_node_view() is faster than hook_entity_view() and a if, in particular when it comes to more of this hook implementations. We might want to benchmark this though.

@#1 I think this issue is not about hooks, which remains totally valid, but about menu helpers/loaders and such.

This isn't about the hooks, it's about the API functions. node_load(), node_revision_load() and so on.

But have the same feeling there, I'm not sure if it's worth it. It's als mucher easier to use %node in hook_menu instead of %entity and then having to deal with load arguments.

As I understood it, it wasn't about the hooks, but the wrapper functions.

I agree with not removing the hooks. It's like having hook_form_FORM_ID_alter and hook_form_BASE_FORM_ID_alter, both are good.

Indeed, to get back to the original issue, I'm definitely more than OK with removing all those duplicated helpers, they make no sense! If we find a way to convert %node in menu trail automatically to entity_get_controller('node')->load(some id); it would be a huge win. But what about non-entity loaders?

>As I understood it, it wasn't about the hooks, but the wrapper functions.

Ah yeah, I see - sry.

I agree that all those entity-type specific helper functions are just a burden. Modules partly define them, partly not - so as a dev you cannot rely on them to be there anyway if you don't know the module. Plus, I've seen developers putting entity-type specific modifications in there - so by not having the functions devs are forced to do it right.

> Consistently use entity system functions only.
Sounds good to me.

Big +1.

I'm fine with doing it as well, wondering if we'll run into some issues when doing this. One thing that I don't really like, as mentioned above, is that hook_menu() definitions get quite a bit more complex (%entity + load arguments instead of %node, does that even work for combined stuff like node/%node/revision/%node_revision?). But the new router system might change that anyway.

And it will be a huge patch that will break tons of things, so maybe we should postpone doing this after feature freeze?

While we're talking about major changes - is it worth considering removing all of the hook_menu entries of the form
%entity_type/%entity

eg node/%node
user/%user

And replacing them with a single entry in the entity system

There's #1743590: Isolated Block Rendering which will probably add block/%block and #1606794: Implement new routing system where we're grappling with removing the number of routes (see comment 26 onwards)

And there's #1026616: Implement an entity render controller and the similar rationalization of entity forms.

All of these take us closer to a more restful approach.

Thoughts?

So... Considering that node_load(), etc. are currently required for the menu loaders to work (we can't change that to %entity yet because the arguments are in reverse ($entity_type, $id vs. $id, $entity_type). What we can do, however, is to replace node_load(), comment_load(), file_load(), etc. wherever they are used in code currently. Does that make sense to do that right now? Or do we want to fully post-pone this issue. We could actually split it up in 2 separate issue:

#1 Change invocations of node_load(), etc. to entity_load() in code
#2 Fix %node, %comment, etc. once we figured out #1801570: DX: Replace hook_route_info() with YAML files and RouteBuildEvent

So.. I've started with removing the entity type specific _load, _load_multiple, _delete, _delete_multiple, _load, _load_multiple and _save functions. Also, I added an entity_delete() function - I think it's reasonable to add that in the scope of this issue.

Let's see what the testbot says. I am pretty sure that I missed some - I did most of this through regex... But it should have covered at least most of it already.

Next try.

#15: 1757586-14-remove-entity-type-specific-functions.patch queued for re-testing.

Accidently removed some of the required loaders for menu paths. Bad regex.

The @todo's in the old MODULE_load() functions should be in the docblock, not inline. maybe @deprecated?

Yes. Good catch. Let's directly mark them deprecated and only have the menu system still rely on them.

Woot! We are green. One more follow-up patch to fix #22 and kill some empty line deletions that got added when I debugged :)

Another current usage for the MODULE_load() functions is for the 'exists' callback for '#type' => 'machine_name'. See #1588422-147: Convert contact categories to configuration system

nice
#24 removes a lot of code
looks like it is very close to RTBC

#24: 1757586-24-remove-entity-type-specific-functions.patch queued for re-testing.

Re-testing that patch was a brave attempt :P

was more of a bump:P

Let's make the issue as meta and split the task to set of issues like:

• #2022509: Deprecate taxonomy_vocabulary_load_multiple() and taxonomy_term_load_multiple() in favour of entity_load_multiple()
• #2054213: Deprecate comment_load_multiple() in favour of entity_load_multiple()

There's debate about whether we want to do this, since entity_load can't type hint what type of entity it will return. I haven't got an exact issue to postpone against, but please check #2010138-12: Remove taxonomy_term_load_multiple() in favour of \Drupal\taxonomy\Entity\Term::loadMultiple(). and #2190313: Add $EntityType::load() and loadMultiple() to simplify loading entities before spending time on this.

Yeah, Closing as duplicate of #2190313: Add $EntityType::load() and loadMultiple() to simplify loading entities and the corresponding create() method.

Hmpf.