Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Sending some patches to this issue #2490966: [Meta] Replace deprecated usage of entity_create with a direct call to the entity type class some core developers argued that in test classes we should use Dependency Injection instead of calling static methods. This helps decoupling and makes tests easier.
Some developers do not agree with it, saying that the entity maintainers already made the decision and codified it in the @deprecated section of the doxygen of entity_create and entity_load. Moreover, maybe we should not decouple everything. It makes code more difficult to read and to debug.
In addition, the methods EntityClass::create() are not bullet proof and they do not work for 100% of the cases. See the error got by the testbot while using them: https://www.drupal.org/pift-ci-job/124330 This error was fixed by using the long chaining way in this patch: https://www.drupal.org/node/2641518#comment-10715660 . This is hardly relevant as it's just a bug but it's an excellent excuse to make Drupal 8 even harder to understand, develop and debug.
We had some discussion on IRC #drupal-contribute, but we could not reach a consensus. Another concern was whether overriding an entity class with setClass would break Node::load(). It would not. Entity API, while sometimes perhaps clunky and certainly a bit performance challenged is a reasonably well working and not too fragile unlike other parts of D8.
For consistency, we should pick a way we will work in core.
Proposed resolution
Choose a standard way to create and load entities.
Decide if the same way will be used both in procedural code and also in test classes. This is also a straw man, there is barely any procedural code working with entities in core and test classes are the place everyone copies the code out from. So if we were to override the already made decision solely for tests it would spread to everywhere.
Decide how to proceed with that meta issue and others, while we do not make the final decision for this standard here.
Remaining tasks
Get some people to get off the theory high horse. The decision was made long ago and it's the simple good one:
https://api.drupal.org/api/drupal/core!includes!entity.inc/function/enti...
Use The method overriding Entity::create() for the entity type, e.g. \Drupal\node\Entity\Node::create() if the entity type is known. If the entity type is variable, use the entity storage's create() method to construct a new entity:
Comments
Comment #2
jhodgdonThis is not a docs issue, nor a task. Changing issue factors accordingly.
Comment #3
chx commentedThis decision is made clear and loud already in the entity_load @deprecated documentation
Same for entity_create
dawehner raised a practical concern whether these work properly when someone uses setClass -- yes they do! That's why getEntityTypeFromClass calls getOriginalClass.
Theoretical concerns making everything thousand times more complicated like other parts of Drupal 8 are not relevant here.
Comment #4
Crell commentedTestability is very relevant. The use of static class calls harms testability. This is hardly a new statement, nor even a controversial one.
Comment #5
chx commentedEdit: deleted.
Comment #6
chx commentedComment #7
chx commentedComment #8
chx commentedComment #9
dawehnerIMHO there is just no reason for procedural code. If you have a hook you can simply call out to an object (service even, if needed).
Comment #10
bojanz commented1) If you must have procedural code, use the static methods. That was why entity_load & friends point to them in the first place, for procedural usage.
2) If you have a class, inject the entity type manager and get the storage.
Most/all code already does #1 and #2. The question is just about whether to take shortcuts in tests or not (#1 vs #2).
Entity API is not unit testable anyway, so tests can get away with #1, but #2 is a good habit to have in general.
Comment #11
Crell commentedIf there were a +1 button, I would just push it on #9 and #10 several times and be done with it. :-)
Comment #12
mac_weber commented@chx why not change the documentation adding instructions for test classes as pointed by #9 and #10?
Even if it spreads everywhere, as you said, "there is barely any procedural code working with entities in core". Then, it would spread to contrib.
A simple way to avoid such spreading is to add a line of comment before the implementations in test files. Yet I think contrib developers not just copy the code, but also read the documentation - which would also have that note about the difference in test files and procedural code.
Comment #26
quietone commentedThere has been no discussion here for 8 years. There is a general sense that this is a working as designed. Therefore, I am restoring that status. If that is wrong, reopen the issue, by setting the status to 'Active', and add a comment.
Comment #27
quietone commentedChanging to latest version when this was closed.