Remove entity info from the Entity classes

Yep, having the entity-info lying around there was already criticized multiple times. I don't like it either so let's try to improve that.

+++ b/core/modules/entity/entity.class.inc
@@ -186,26 +164,36 @@ class Entity implements EntityInterface {
   public function id() {
-    return isset($this->{$this->idKey}) ? $this->{$this->idKey} : NULL;
+    $id_key = $this->idKey();
+    return isset($this->$id_key) ? $this->$id_key : NULL;
   }

What about removing the idKey() method/property at all and just go with the id() method everywhere in the class + do the lookup directly in id()? That should be faster and cleaner.

Then, we can implement the id-key lookup just directly in id(), whereas entity type classes could override it and just hardcode the id-key there. Although it's a bit of information duplication, I'd say the class would be even cleaner and it would be a bit faster too.

Thus we'd have:

class Comment extends Entity {

  public function id() {
    return $this->cid;
  }
}

Analogously the bundle. Thoughts?

Just a note that "how does a [field api widget|image effect|input filter...] access the $info that lies in its hook_info() entry" is also a question in the in-progress Plugin system. Not sure whether entity types and controller classes will actually leverage the Plugin system at some point, but that $info stuff is a common pattern.

I'm currently embedding a copy of that $info in the FieldTypeHandler, WidgetHandler... classes in my "OO Field API" sandbox, but I'm not too happy with that either.

Spoke to Berdir about this a bit in irc.

Since we specify the class in the info hook, it seems good to me if we can stop the class itself calling back to entity_get_info() as much as possible - i.e. more or less what fago suggests in #3.

Yep, I'd like the solution of #3 most.

@performance:
Thanks for the benchmark! We might not call $entity->id() 1000000 times per page load, but still getting the id() is very foundational and a difference of 1 : 7,5 is rather huge. Also this is about $entity->bundle() too which is checked quite often; e.g. 1000 calls would already make a difference of ~1.5ms.

- Change default implementation of bundle() to return NULL, provide overrides as necessary.

Let's have bundle() default to the entity type as this is the default for no-bundles as defined by the interface. (That default should probably change too, but that's another issue again ;)

+++ b/core/modules/entity/entity.class.inc
@@ -226,7 +195,7 @@ class Entity implements EntityInterface {
+    return $this->entityType();

Seems like this could use the entityType variable directly, instead of the method ?!

The failing tests seem to be caused by a similar unset in TaxonomyVocabularyUnitTest::testUninstallReinstall()

This fixes #17 and fixes the remaining test, at least locally.
Interdiff to #15 attached.

Marking needs review.

+++ b/core/modules/taxonomy/taxonomy.entity.inc
@@ -75,6 +75,22 @@ class TaxonomyTerm extends Entity {
   public $vocabulary_machine_name;
+
+
+
+  /**

Too many empty lines. Fixed.

+++ b/core/modules/taxonomy/taxonomy.entity.inc
@@ -263,6 +279,15 @@ class TaxonomyVocabulary extends Entity {
   public $weight = 0;
+
+
+
+  /**

Again. Fixed.

+++ b/core/modules/taxonomy/taxonomy.module
@@ -1532,7 +1532,7 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
       $term = entity_create('taxonomy_term', $item);
-      unset($term->tid);
+      $term->enforceIsNew();

enforceIsNew() does not unset the term id, but is for keeping the set term id. However, 'autocreate' is no valid term id. Thus, fixed that so 'autocreate' is never passed to the entity system.

Updated patch attached. Should be good now.

Also I've added the patch to the entity-conversion sandbox over at http://drupal.org/sandbox/fago/1497344 under the entity-info branch. Please use this repository for further follow-ups and contact me if you need access (I've granted access to you already).

#20 looks good to me.

Excellent little clean-up. Committed to 8.x. Great job. :-)

This patch broke createDuplicate() implementations as we forgot to convert them. See #1547300: createDuplicate() is broken for most entity types and needs tests

Updated issue summary.