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

webchick’s picture

Also, this.

sun’s picture

What we could possibly do is to add a

public static function Entity::createFrom($entity_type, $values);

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

Did you ever override the entity class definition for an entity type?
I have no idea what you're talking about. 11
I did not know that this was possible. 7
No. What is that good for anyway? 16
Once. Strange project. 7
Twice, even. 3
Yes. Always. Is it possible without? 3

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.

tstoeckler’s picture

Also, we can do cool stuff like:

  // Assign a new UUID if there is none yet.
  if ($this->uuidKey && !isset($entity->{$this->uuidKey})) {
    $uuid = new Uuid();
    $entity->{$this->uuidKey} = $uuid->generate();
  }

See DatabaseStorageController::create()

attiks’s picture

Two remarks:

  1. Most IDE's aren't able to detect the type of the object, so no code completion.
  2. All overridden functions can't use the real class for the parameter, but have to use EntityInterface

Are comments like /* @var $breakpoint Breakpoint */ allowed, this at least solves problem 1.

tstoeckler’s picture

Are comments like /* @var $breakpoint Breakpoint */ allowed, this at least solves problem 1.

Do 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.

All overridden functions can't use the real class for the parameter, but have to use EntityInterface

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.

attiks’s picture

Netbeans 7:
/* @var $breakpoint \Drupal\breakpoint\Breakpoint */ works

/** @var $breakpoint \Drupal\breakpoint\Breakpoint */ doesn't work
// @var $breakpoint \Drupal\breakpoint\Breakpoint doesn't work

So it only works using "/* */".

tstoeckler’s picture

Well, 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.

andypost’s picture

I 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

fago’s picture

We 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.

webchick’s picture

How 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. ;)

andypost’s picture

@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!

fago’s picture

Generally, I could see it working as following:

$term = entity_create('taxonomy_term', 'vocab-name');
$term->name->value= 'Tags';
$term->description->value = $description;
$term->save();

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 like Vocabulary

Berdir’s picture

Status: Active » Closed (works as designed)

I 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.