Problem/Motivation
In #2190313: Add $EntityType::load() and loadMultiple() to simplify loading entities the static methods Entity::load() and Entity::loadMultiple() are implemented.
Beta phase evaluation
#2321701-15: Replace all instances of contact_form_load(), entity_load('contact_form') and entity_load_multiple('contact_form') with static method calls @alexpott mentions it is allowed since it is removal of deprecated code.
It might be that not all child issues are as simple as that.
Issue category | Task because it is refactoring code, using new APIs. per https://www.drupal.org/core/issue-category |
---|---|
Issue priority | Major because it is across many systems, but only code refactoring. Not critical because it is a task, and nothing is actually broken. But each child issue is most likely Normal (just in one system). per https://www.drupal.org/core/issue-priority |
Unfrozen changes | It is*not* unfrozen only changes. |
Prioritized changes | The main goal of this issue is removing previously deprecated code. |
Disruption | Refactoring usages of the deprecated code in Core is not disruptive for contributed and custom modules/themes because it will NOT require a BC break in contrib. The deprecated functions are not themselves being removed in children of this meta, just usages of them changed to be using what they are supposed to be using. |
This seems to be allowed. But individual children might have greater disruption so should be looked at.
Proposed resolution
We need to replace all calls to ENTITY_TYPE_load() and ENTITY_TYPE_load_multiple() with these new static method calls. An example:
Before:
// Variants for loading a single node entity.
$node = node_load($nid);
$node = entity_load('node', $nid);
// Variants for loading multiple node entities.
$nodes = node_load_multiple($nids);
$nodes = entity_load_multiple($nids);
// Clear the entity cache for a bunch of nodes.
node_load_multiple($nids, TRUE);
After:
use Drupal\node\Entity\Node;
// Load a single node entity.
$node = Node::load($nid);
// Load multiple node entities.
$nodes = Node::loadMultiple($nids);
// Clear the entity cache for a bunch of nodes.
\Drupal::entityManager()->getStorageController('node')->resetCache($nids);
It is a good candidate for the function replace script, see #2208607: Write script to automate replacement of deprecated functions where possible
Remaining tasks
- Replace all instances of aggregator_feed_load(), entity_load('aggregator_feed') and entity_load_multiple('aggregator_feed') with static method calls. #2321501: Replace calls to aggregator_feed_load(), entity_load('aggregator_feed') and entity_load_multiple() with static method calls Assigned to: rosinegrean
- Replace all instances of block_load(), entity_load('block') and entity_load_multiple('block') with static method calls. #2321593: Replace all instances of block_load(), entity_load('block') and entity_load_multiple('block') with static method calls
- #2377113: Replace all instances of entity_load('block_content') and entity_load_multiple('block_content') with static method calls
- Replace all instances of comment_load(), entity_load('comment') and entity_load_multiple('comment') with static method calls. #2321599: Replace all instances of comment_load(), entity_load('comment') and entity_load_multiple('comment') with static method calls
- Replace all instances of contact_category_load(), entity_load('contact_category') and entity_load_multiple('contact_category') with static method calls.
- #2321701: Replace all instances of contact_form_load(), entity_load('contact_form') and entity_load_multiple('contact_form') with static method calls
- Replace all instances of custom_block_load(), entity_load('custom_block') and entity_load_multiple('custom_block') with static method calls.
- Replace all instances of custom_block_type_load(), entity_load('custom_block_type') and entity_load_multiple('custom_block_type') with static method calls.
- Replace all instances of file_load(), file_load_multiple(), entity_load('file') and entity_load_multiple('file') with static method calls. #2321969: Replace all instances of file_load(), file_load_multiple(), entity_load('file') and entity_load_multiple('file') with static method calls
- #2377115: Replace all instances of entity_load('field_config') and entity_load_multiple('field_config') with static method calls
- #2377117: Replace all instances of entity_load('field_storage_config') and entity_load_multiple('field_storage_config') with static method calls
- Replace all instances of image_style_load(), entity_load('image_style') and entity_load_multiple('image_style') with static method calls. #2321901: Replace all instances of entity_load('image_style') and entity_load_multiple('image_style') with static method calls
- Replace all instances of menu_link_load(), menu_link_load_multiple(), entity_load('menu_link') and entity_load_multiple('menu_link') with static method calls. #2321913: Replace all instances of entity_load('menu_link_content') and entity_load_multiple('menu_link_content') with static method calls
- Replace all instances of menu_load(), entity_load('menu') and entity_load_multiple('menu') with static method calls. #2321919: Replace all instances of menu_load(), entity_load('menu') and entity_load_multiple('menu') with static method calls
- Replace all instances of node_load(), node_load_multiple(), entity_load('node') and entity_load_multiple('node') with static method calls. #2322509: Replace all instances of node_load(), node_load_multiple(), entity_load('node') and entity_load_multiple('node') with static method calls
- Replace all instances of node_type_load(), node_type_get_types(), entity_load('node_type') and entity_load_multiple('node_type') with static method calls. #2322639: Replace all instances of node_type_load(), node_type_get_types(), entity_load('node_type') and entity_load_multiple('node_type') with static method calls
- Replace all instances of responsive_image_mapping_load(), entity_load('responsive_image_mapping') and entity_load_multiple('responsive_image_mapping') with static method calls. #2322037: Replace all instances of responsive_image_mapping_load(), entity_load('responsive_image_mapping') and entity_load_multiple('responsive_image_mapping') with static method calls
- Replace all instances of shortcut_set_load(), entity_load('shortcut_set') and entity_load_multiple('shortcut_set') with static method calls. #2322063: Replace all instances of shortcut_set_load(), entity_load('shortcut_set') and entity_load_multiple('shortcut_set') with static method calls
- Replace all instances of taxonomy_term_load(), taxonomy_term_load_multiple(), entity_load('taxonomy_term') and entity_load_multiple('taxonomy_term') with static method calls. #2322067: Replace all instances of taxonomy_term_load(), entity_load('taxonomy_term') and entity_load_multiple('taxonomy_term') with static method calls
- Replace all instances of taxonomy_vocabulary_load(), taxonomy_vocabulary_load_multiple(), entity_load('taxonomy_vocabulary') and entity_load_multiple('taxonomy_vocabulary') with static method calls. #2322105: Replace all instances of taxonomy_vocabulary_load(), taxonomy_vocabulary_load_multiple(), entity_load('taxonomy_vocabulary') and entity_load_multiple('taxonomy_vocabulary') with static method calls
- Replace all instances of user_load(), user_load_multiple(), entity_load('user') and entity_load_multiple('user') with static method calls. #2322195: Replace all instances of user_load(), user_load_multiple(), entity_load('user') and entity_load_multiple('user') with static method calls
- Replace all instances of user_role_load(), entity_load('user_role') and entity_load_multiple('user_role') with static method calls. #2322233: Replace all instances of user_role_load(), entity_load('user_role') and entity_load_multiple('user_role') with static method calls
User interface changes
No.
API changes
No.
Comment | File | Size | Author |
---|---|---|---|
#15 | drupal_core-replace_entity_load_multiple-2225347-15.patch | 25.67 KB | vincenzodb |
Comments
Comment #1
ianthomas_ukComment #2
pfrenssenComment #3
pfrenssenComment #4
ianthomas_ukComment #5
m1r1k CreditAttribution: m1r1k commentedComment #6
rpayanmComment #7
rpayanmI have some doubts and I need you to explain them
1.
Before:
After:
And for this?
Before
After:
2. For Comment Entity, I have:
Before:
To change should be:
But there is a method:
So my question is: Which is correct?
Greetings.
Comment #8
pfrenssenComment #9
pfrenssen@rpayanm, you can use
Comment::loadMultiple()
to load multiple items. This was wrong in the summary, I have fixed it now, thanks for mentioning it!I have unassigned you from this issue. Assigning an issue to yourself means "I'm working on this, please don't touch!". But since this is a meta issue the actual work need to be done in the child issues, so it does not make much sense to have this assigned to someone. If you are working on one of the child issues, feel free to assign them to yourself!
Comment #10
rpayanmHello:
Thank you very much for the reply.
I am a new contributor, sorry :)
Then the first question which is the answer?
Greetings.
Comment #11
BerdirThat is a bit more complicated now, you need to do something like this as a separate call, first:
\Drupal::entityManager()->getStorage('comment')->resetCache();
If you just want to delete a specific comment from the cache, you can also do this:
\Drupal::entityManager()->getStorage('comment')->resetCache(array($cid));
But in tests, it usually doesn't matter. And those calls should all be in tests.
We explicitly didn't add this back to the new load methods, because it's something that is only needed for tests and people shouldn't call that just in case or so in real code, as it would be bad for performance.
Comment #12
mradcliffeCrell caught that we should be turning these into services.
We should do this in a follow-up issue since several of the child issues have been fixed.
Comment #13
vincenzodb CreditAttribution: vincenzodb commentedComment #14
vincenzodb CreditAttribution: vincenzodb commentedComment #15
vincenzodb CreditAttribution: vincenzodb commentedAdded replacement for entity_load_multiple() calls.
Comment #16
vincenzodb CreditAttribution: vincenzodb commentedComment #17
vincenzodb CreditAttribution: vincenzodb commentedI'm working on entity_load() calls
Comment #19
YesCT CreditAttribution: YesCT commented@vincenzodb This is a meta issue, so I was not expecting a patch here.
To work on one of the types that already has an issue, find a child issue (in the sidebar).
To work on a type that doesn't have a child yet...
Pick one of the child issues in the sidebar, and create a similar issue for each type, (with dreditor clone button and then change the values, or just create a new issue and fill in all the values).
For the list in the summary, you could see if there is already an issue and add the issue to that list item. (Note issues can be linked in the summary and comments with [#NNNNN] where NNNN is the issue number.
Once all the children are created and fixed, then we will close this meta.
Comment #20
YesCT CreditAttribution: YesCT commentedupdated the issue summary for allowed in beta, and seems to be allowed because removing/refactoring usages of deprecated code. (but not removing the deprecated functions themselves, so no BC break)
Comment #21
YesCT CreditAttribution: YesCT commentedunassigning since work should be done in child issues (also see #9)
Comment #22
YesCT CreditAttribution: YesCT commentedupdating the list in the summary matching with child issues. next need to search if there are more child issues existing, but just were not linked to this parent.
Comment #23
YesCT CreditAttribution: YesCT commentedmade some of the children for what was in that patch. Not sure if should have a child for each of the types though, so will wait a bit for feedback before making them all.
Comment #24
YesCT CreditAttribution: YesCT commentedIdentifying this as a meta with children that have a potential good issue for Sprint Weekend. See discussion at #2407325: preparing for a sprint, would be good to have a list of candidate issues. Adding sprint queue tag.
If any child is worked on during the sprint, please add the tag SprintWeekend2015 to the child. Add a comment on the child when you start work saying what you are about to do, so we can coordinate.
Comment #25
DevElCuy CreditAttribution: DevElCuy commentedComment #26
DevElCuy CreditAttribution: DevElCuy commentedRemoved SprintWeekend2015Queue by mistake.
Comment #27
xjmSo there's been lots of great work on this, and it looks like most of the child issues are complete.
However, these changes I think aren't quite right in terms of the prioritized changes list, which specifies:
For #2322195: Replace all instances of user_load(), user_load_multiple(), entity_load('user') and entity_load_multiple('user') with static method calls, the code is marked for removal by 9.0, so we can't remove it, which makes removing usages of it not a prioritized change. And for #2321901: Replace all instances of entity_load('image_style') and entity_load_multiple('image_style') with static method calls and similar,
entity_load()
andentity_load_multiple()
aren't deprecated at all.However, we could decide that since the work is mostly done here (and this issue is considered major after all), it's better to finish the last few and just retain the deprecated wrappers. I might recommend that (as well as verifying that all of these get deprecated for 9.0).
However, this does not extend to other deprecations that are not part of this issue. We should only be doing conversions to remove calls to things deprecated for 8.0.0 in general.
Comment #28
xjmAlso the NW status on a meta was confusing me. :)
Comment #29
alexpotti think if we have a function that is deprecated for 9.0.0 it is good to remove usages. And I don't think that is it that disruptive to do this during the beta.
Wrt to entity_load and entity_load_multiple I think we should deprecate them for 9.0.0 as well since we should be removing them in that release as we plan to remove .module files.
Comment #30
andypostFiled another issue #2465751: Remove config_test_load() and replace with entity_load
Comment #31
andypostThe issue #2465751: Remove config_test_load() and replace with entity_load shows how fragile the static approach could be.
config_test_entity_type_alter()
addsconfig_test_no_status
entity type without class and that leads toAmbiguousEntityClassException
from\Drupal\Core\Entity\EntityManager::getEntityTypeFromClass()
Comment #32
bojanz CreditAttribution: bojanz commentedComment #33
Mile23Is this meta done or what? :-)
#2465751: Remove config_test_load() and replace with entity_load is the only child remaining, but it replaces a function not marked as deprecated. However, it illustrates best practices and removes
ENTITY_load()
. Also, the work is done and RTBC.Comment #34
JeroenT#2465751: Remove config_test_load() and replace with entity_load Is also in, so all the work is done here.