I'm not sure if this makes sense, but coming into the D8 entity API as a user this was very confusing.
I was converting Spark to Drupal 8, and got this error:
An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: http://localhost/spark-git/8xx/core/install.php?langcode=en&profile=spar... StatusText: OK ResponseText: Recoverable fatal error: Argument 1 passed to taxonomy_vocabulary_save() must be an instance of Drupal\taxonomy\Vocabulary, instance of stdClass given, called in /Users/abyron/Sites/spark-git/8xx/profiles/spark/spark.install on line 269 and defined in taxonomy_vocabulary_save() (line 442 of /Users/abyron/Sites/spark-git/8xx/core/modules/taxonomy/taxonomy.module).
"Ah-ha!" I thought to myself, sipping some newly acquired Cherry Coke from a recent trip to Bellingham. "This is because of that new fancy-pants OO Entity API. Well then! Let's dig in."
Here's what my old code was in Drupal 7:
$vocabulary = (object) array(
'name' => st('Tags'),
'description' => $description,
'machine_name' => 'tags',
'help' => $help,
);
taxonomy_vocabulary_save($vocabulary);
Here's what I expected the new code to be in Drupal 8:
use Drupal\taxonomy\Vocabulary;
...
$vocabulary = new Vocabulary();
$vocabulary->name = st('Tags');
$vocabulary->description = $description;
$vocabulary->machine_name = 'tags';
$vocabulary->help = $help;
$vocabulary->save();
Here's what the code actually is in standard.install:
$vocabulary = entity_create('taxonomy_vocabulary', array(
'name' => st('Tags'),
'description' => $description,
'machine_name' => 'tags',
'langcode' => language_default()->langcode,
'help' => $help,
));
taxonomy_vocabulary_save($vocabulary);
...Wat? :(
Also, I have no idea why we are duplicating tags and issue components, but doing the same here since that seems to be what other people do.
Comments
Comment #1
webchickAlso, this.
Comment #2
sunWhat we could possibly do is to add a
The entity type would still have to be specified though.
I actually did a survey on this topic very recently: https://sun.webform.com/form/2651
The results suggest that there's a large majority who doesn't use the capability of overriding entity types/classes, but that there's also a non-zero, significant share of developers who needed to do that for their projects, which pretty much means that we cannot get rid of the additional entity type abstraction.
Comment #3
tstoecklerAlso, we can do cool stuff like:
See DatabaseStorageController::create()
Comment #4
attiks CreditAttribution: attiks commentedTwo remarks:
Are comments like
/* @var $breakpoint Breakpoint */
allowed, this at least solves problem 1.Comment #5
tstoecklerDo you mean in-line, i.e. right before $breakpoint is being defined? We don't use the multiline "/** */" syntax in-line, but
// \Drupal\breakpoint\Breakpoint $breakpoint
would be allowed. I don't know if IDEs support that, though.Ideally, each entity type would define a corresponding interface, so sub-classes would type-hint
BreakpointInterface extends EntityInterface
. That is not currently being done for any entity, so to allow overridability, we need to type-hint EntityInterface.Comment #6
attiks CreditAttribution: attiks commentedNetbeans 7:
/* @var $breakpoint \Drupal\breakpoint\Breakpoint */
works/** @var $breakpoint \Drupal\breakpoint\Breakpoint */
doesn't work// @var $breakpoint \Drupal\breakpoint\Breakpoint
doesn't workSo it only works using "/* */".
Comment #7
tstoecklerWell, I personally wouldn't mind an exception to our coding standards in case it helps some people. We should test a couple more IDEs maybe.
Comment #8
andypostI think better to use actual class names, but we still have a lot of wrappers and EntityNG which inherits from Entity
Using direct entity-class-name will help IDE and prevent from troubles with namespace and use like we have in #1805776: Fatal error: Class 'Drupal\Core\Template\ArrayIterator' not found in core/lib/Drupal/Core/Template/Attribute.php
Also I'd like to point to #1796760: Change order of parameters in Entity constructors
Currently core has near 90 text occurrences of entity_create() so it's trivial patch
Comment #9
fagoWe support swapping out the class, so
$v = new Vocbulary();
won't cut it.The survey of #2 is interesting, though it does not really apply as the entity class in D7 is not used by many modules. But in D8, some stuff like entity uri and label can only be controlled and swapped out via the class, so it will be more important.
We could look into doing away with the need for that though, and support going with fixed class to improve DX. Still, we'd probably loose quite some points of being able to "hack" into the system with a module if the entity classes are fixed.
Comment #10
webchickHow about
$v = new Entity('vocabulary');
? In other words, switch the type vs. values array, make values optional. Seems like I could then if I'm crazy_override_taxonomy.module, do:$v->entity_type = 'new_vocabulary'; $v->other stuff ... $v->save();
Anything that gets us out from using an array to instantiate class is good for me. ;)
Comment #11
andypost@webchick
$v->other stuff
could be a problem when properties are protected|private so better use$v->set('property', $value)
probably with EntityNG this magic should be removed...@fago please comment on recommended/compatible setters!
Comment #12
fagoGenerally, I could see it working as following:
Thus, we could just use the bundle property value as second argument, if a bundle is used. We'll need the value for the bundle before any bundle specific fields can be set, so best we already enforce having a bundle value upon creation.
$v = new Entity('vocabulary');
won't work for the same reasons: no possibility to customize the used class, but moverover it would disallow custom per entity-type classes likeVocabulary
Comment #13
BerdirI think we can safely close this, we need an API to create them, #2096899: Add $EntityType::create() to simplify creating new entities will help with the DX.