Separate FieldableEntityInterface out of ContentEntityInterface

We would need to make it FieldableEntityInterface extends EntityInterface, because we can not just type hint on FieldableInterface in the relevant methods on the entity storage interface for example?

+1, and also +1 on FieldableEntityInterface rather than FieldableInterface ;-)

+++ b/core/lib/Drupal/Core/Entity/FieldableEntityInterface.php
@@ -0,0 +1,125 @@
+ * When implementing this interface which extends Traversable, make sure to list
+ * IteratorAggregate or Iterator before this interface in the implements clause.

Wasn't aware of that requirement - but is it still valid ? Coz it's not what ContentEntityInterface does now :-)

Otherwise, looks good.

Makes sense. Thanks !

Apart from initTranslation which has a @todo to remove, this is going to be an empty interface soon

+++ b/core/lib/Drupal/Core/Entity/Controller/EntityViewController.php
@@ -70,7 +70,7 @@ public function view(EntityInterface $_entity, $view_mode = 'full', $langcode =
       $label_field = $_entity->getEntityType()->getKey('label');

I agree with splitting the interface out, but I'm really not sure about changing all these type hints. Are you 100% sure that all the places relying on ContentEntityInterface at the moment don't require Revisionable/Translatable compared to just Fieldable?

1. +++ b/core/lib/Drupal/Core/Entity/ContentEntityInterface.php
   @@ -7,28 +7,14 @@
    /**
     * Defines a common interface for all content entity objects.
     *
   ...
     * @ingroup entity_api
     */
   ...
   +interface ContentEntityInterface extends FieldableEntityInterface, RevisionableInterface, TranslatableInterface {
   

   There seems to be basically nothing left here. Can't we add some description, that for example explains when you use this interface? Content, which should be stored in the database.

2. +++ b/core/lib/Drupal/Core/Entity/ContentEntityInterface.php
   --- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
   +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
   
   +++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
   @@ -167,10 +167,10 @@ public function finalizePurge(FieldStorageDefinitionInterface $storage_definitio
   -  protected function invokeTranslationHooks(ContentEntityInterface $entity) {
   +  protected function invokeTranslationHooks(FieldableEntityInterface $entity) {
   

   Is there a possibility to rename the class or at least extract the fieldableEntity specific concepts out of it?

+++ b/core/modules/entity_reference/src/Plugin/Field/FieldWidget/AutocompleteWidgetBase.php
@@ -219,7 +219,7 @@ protected function getSelectionHandlerSetting($setting_name) {
     $target_type_info = \Drupal::entityManager()->getDefinition($target_type);
-    return $target_type_info->isSubclassOf('\Drupal\Core\Entity\ContentEntityInterface');
+    return $target_type_info->isSubclassOf('\Drupal\Core\Entity\FieldableEntityInterface');
   }

Are you sure about this change? This method is used to identify whether an entity ID is numeric or not. Afaik this is unrelated to the concept of fieldability. Yes, this is a genera assumption of internal details, but the content entity interface seems to mach a bit better ...

+++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
@@ -189,10 +189,10 @@ protected function invokeTranslationHooks(ContentEntityInterface $entity) {
    * @param string $method
    *   The method name.
-   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   * @param \Drupal\Core\Entity\FieldableEntityInterface $entity
    *   The entity object.
    */
-  protected function invokeFieldMethod($method, ContentEntityInterface $entity) {
+  protected function invokeFieldMethod($method, FieldableEntityInterface $entity) {

This change is wrong, as it is about content entity storage and relies on TranslatableInterface.

Looking at the remaining uses of ContentEntityInterface:
- StringFormatterTest could use FieldableEntityInterface I guess but no biggie.
- EntityReferenceItem::schema() should go with FieldableEntityInterface I think?
- entity reference autocomplete widget has a pretty weird isContentEntity check, but I think changing that is out of scope.
- ContentEntityNormalizer could maybe just be about fieldable, not sure. Ok with doing that elsewhere too.
- CommentLinkBuilderInterface has a wrong docblock
- entity.api.php has a suboptimal reference, which assumes that there is nothing else than config or content entities, but not too problematic.

Rerolled Patch #33

1) Changed mock Test in EntityManagerTest.php
2) Changed entity.api.php file dependency from ContentEntityInterface to FieldableEntityInterface also make Drupal\Component\Utility\String dependency intact.

Still looks good to me. API Changes need to be signed off, but otherwise good to go.

protected function getOptions(ContentEntityInterface $entity) {

In OptionsWidgetBase looks like it should be typehinted on FieldableEntityInterface

  /**
   * Gets the entity that field belongs to.
   *
   * @return \Drupal\Core\Entity\ContentEntityInterface
   *   The entity object.
   */
  public function getEntity();

In FieldItemInterface looks like it should be typehinted on FieldableEntityInterface

    if ($entity_type->isSubclassOf('\Drupal\Core\Entity\ContentEntityInterface')) {
      $this->setLastInstalledFieldStorageDefinitions($entity_type_id, $this->getFieldStorageDefinitions($entity_type_id));
    }

In EntityManager looks like it should be typehinted on FieldableEntityInterface

        if (!in_array('Drupal\Core\Entity\ContentEntityInterface', class_implements($entity_type_class))) {
          $this->propertyDefinitions = array();
        }

In EntityDataDefinition looks like it should be typehinted on FieldableEntityInterface

Back to RTBC

Committed ba2b869 and pushed to 8.0.x. Thanks!